Skip to content

Commit eafa235

Browse files
samestepfacebook-github-bot
authored andcommitted
Clarify and document commit choice for CI jobs (pytorch#54967)
Summary: PRs pytorch#53652 and pytorch#54693 attempted to increase the consistency of our choice of commit (head vs merge) for CI on PRs, and have so far been unsuccessful. This PR takes a less ambitious approach to the problem by clarifying the choice in one specific way (see the following paragraph) and documenting it in `CONTRIBUTING.md`. In addition to documentation, this PR also removes the current behavior of our GHA jobs that checkout the PR tip instead of the merge commit. At first glance, this behavior seems to increase consistency (by eliminating the special-case for `ghstack` PRs), but in reality, it actually just means that for non-`ghstack` PRs, the question "Which commit is used in CI?" has *two* answers instead of just one; see the description of pytorch#53652 for more details. Once merged, this PR will unblock other PRs that add modify our GHA workflows in breaking ways, such as pytorch#54737. Pull Request resolved: pytorch#54967 Test Plan: None. Reviewed By: walterddr, seemethere Differential Revision: D27435913 Pulled By: samestep fbshipit-source-id: 405fb419cf015cf88107d5eb2498cfb5bcb7ce33
1 parent 18e61d1 commit eafa235

7 files changed

+57
-19
lines changed

.github/workflows/build_linux_conda.yml

-2
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ jobs:
1717
steps:
1818
- name: Clone pytorch/pytorch
1919
uses: actions/checkout@v2
20-
with:
21-
ref: ${{ github.event.pull_request.head.sha }}
2220
- name: Generating build matrix
2321
id: set-matrix
2422
run: |

.github/workflows/build_linux_libtorch.yml

-2
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ jobs:
1717
steps:
1818
- name: Clone pytorch/pytorch
1919
uses: actions/checkout@v2
20-
with:
21-
ref: ${{ github.event.pull_request.head.sha }}
2220
- name: Generating build matrix
2321
id: set-matrix
2422
run: |

.github/workflows/build_linux_wheels.yml

-2
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ jobs:
1717
steps:
1818
- name: Clone pytorch/pytorch
1919
uses: actions/checkout@v2
20-
with:
21-
ref: ${{ github.event.pull_request.head.sha }}
2220
- name: Generating build matrix
2321
id: set-matrix
2422
run: |

.github/workflows/clang_format.yml

-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ jobs:
1515
- name: Fetch PyTorch
1616
uses: actions/checkout@v2
1717
with:
18-
ref: ${{ github.event.pull_request.head.sha }}
1918
fetch-depth: 0 # deep clone, to allow us to use git merge-base
2019
- name: Run clang-format
2120
run: |

.github/workflows/lint.yml

-9
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ jobs:
1717
architecture: x64
1818
- name: Checkout PyTorch
1919
uses: actions/checkout@v2
20-
with:
21-
ref: ${{ github.event.pull_request.head.sha }}
2220
- name: Ensure consistent CircleCI YAML config
2321
run: |
2422
pip install -r requirements.txt
@@ -66,8 +64,6 @@ jobs:
6664
architecture: x64
6765
- name: Fetch PyTorch
6866
uses: actions/checkout@v2
69-
with:
70-
ref: ${{ github.event.pull_request.head.sha }}
7167
- name: Get HEAD commit SHA
7268
run: echo ::set-output name=commit-sha::$(git rev-parse HEAD)
7369
id: get-commit-sha
@@ -102,7 +98,6 @@ jobs:
10298
- name: Checkout PyTorch
10399
uses: actions/checkout@v2
104100
with:
105-
ref: ${{ github.event.pull_request.head.sha }}
106101
fetch-depth: 0 # deep clone, to allow us to use git merge-base
107102
- name: Get HEAD commit SHA
108103
run: echo ::set-output name=commit-sha::$(git rev-parse HEAD)
@@ -203,8 +198,6 @@ jobs:
203198
architecture: x64
204199
- name: Fetch PyTorch
205200
uses: actions/checkout@v2
206-
with:
207-
ref: ${{ github.event.pull_request.head.sha }}
208201
- name: Run cmakelint
209202
run: |
210203
set -eux
@@ -224,8 +217,6 @@ jobs:
224217
architecture: x64
225218
- name: Fetch PyTorch
226219
uses: actions/checkout@v2
227-
with:
228-
ref: ${{ github.event.pull_request.head.sha }}
229220
- name: Install dependencies
230221
run: |
231222
set -eux

.github/workflows/test_tools.yml

-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ jobs:
1818
- name: Checkout PyTorch
1919
uses: actions/checkout@v2
2020
with:
21-
ref: ${{ github.event.pull_request.head.sha }}
2221
fetch-depth: 0 # deep clone, to allow us to use git log
2322
- name: Install dependencies
2423
# mypy and boto3 versions copied from

CONTRIBUTING.md

+57-2
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
- [Why no leak detection?](#why-no-leak-detection)
4040
- [Caffe2 notes](#caffe2-notes)
4141
- [CI failure tips](#ci-failure-tips)
42+
- [Which commit is used in CI?](#which-commit-is-used-in-ci)
4243

4344
## Contributing to PyTorch
4445

@@ -1137,8 +1138,9 @@ Once you submit a PR or push a new commit to a branch that is in
11371138
an active PR, CI jobs will be run automatically. Some of these may
11381139
fail and you will need to find out why, by looking at the logs.
11391140

1140-
Fairly often, a CI failure might be unrelated to your changes. In this
1141-
case, you can usually ignore the failure.
1141+
Fairly often, a CI failure might be unrelated to your changes. In this case, you
1142+
can usually ignore the failure. See [the following
1143+
subsection](#which-commit-is-used-in-ci) for more details.
11421144

11431145
Some failures might be related to specific hardware or environment
11441146
configurations. In this case, if the job is run by CircleCI, you can
@@ -1169,3 +1171,56 @@ following steps:
11691171
For certain Windows failures, it may be useful to have a full [Remote
11701172
Desktop](https://docs.microsoft.com/en-us/windows-server/remote/remote-desktop-services/clients/remote-desktop-clients) connection. See detailed instructions [here](https://github.com/pytorch/pytorch/wiki/Debugging-Windows-with-Remote-Desktop-or-CDB-(CLI-windbg)-on-CircleCI)
11711173
for how to set that up after rerunning the job.
1174+
1175+
### Which commit is used in CI?
1176+
1177+
For CI run on `master`, this repository is checked out for a given `master`
1178+
commit, and CI is run on that commit (there isn't really any other choice). For
1179+
PRs, however, it's a bit more complicated. Consider this commit graph, where
1180+
`master` is at commit `A`, and the branch for PR #42 (just a placeholder) is at
1181+
commit `B`:
1182+
1183+
```
1184+
o---o---B (refs/pull/42/head)
1185+
/ \
1186+
/ C (refs/pull/42/merge)
1187+
/ /
1188+
---o---o---o---A (refs/heads/master)
1189+
```
1190+
1191+
There are two possible choices for which commit to use:
1192+
1193+
1. Checkout commit `B`, the head of the PR (manually committed by the PR
1194+
author).
1195+
2. Checkout commit `C`, the hypothetical result of what would happen if the PR
1196+
were merged into `master` (automatically generated by GitHub).
1197+
1198+
This choice depends on several factors; here is the decision tree as of
1199+
2021-03-30:
1200+
1201+
- For CI jobs on CircleCI:
1202+
- If the name of the job (or one of its ancestors in the workflow DAG)
1203+
contains "xla" or "gcc5", choice **2** is used. This includes the following
1204+
jobs:
1205+
- pytorch_linux_xenial_py3_6_gcc5_4_build
1206+
- pytorch_cpp_doc_build
1207+
- pytorch_doc_test
1208+
- pytorch_linux_backward_compatibility_check_test
1209+
- pytorch_linux_xenial_py3_6_gcc5_4_jit_legacy_test
1210+
- pytorch_linux_xenial_py3_6_gcc5_4_test
1211+
- pytorch_python_doc_build
1212+
- pytorch_xla_linux_bionic_py3_6_clang9_build
1213+
- pytorch_xla_linux_bionic_py3_6_clang9_test
1214+
- Otherwise, choice **1** is used.
1215+
- For CI jobs on GitHub Actions:
1216+
- If the PR was created using [`ghstack`](https://github.com/ezyang/ghstack),
1217+
choice **1** is used.
1218+
- Otherwise, choice **2** is used.
1219+
1220+
This is important to be aware of, because if you see a CI failure on your PR and
1221+
choice **2** is being used for that CI job, it is possible that the failure is
1222+
nondeterministically caused by a commit that does not exist in the ancestry of
1223+
your PR branch. If you happen to have write access to this repo, you can choose
1224+
to use `ghstack` to eliminate this nondeterminism for GitHub Actions jobs on
1225+
your PRs, but it will still be present for the select CircleCI jobs listed
1226+
above.

0 commit comments

Comments
 (0)