Always checkout github.head_ref #49578

samestep · 2020-12-18T00:26:14Z

This PR was prompted by a surprising divergence between two runs of the same quick-checks job:

https://github.com/pytorch/pytorch/runs/1573529842 (failure in "Ensure no tabs")
- master was 6db5e85
https://github.com/pytorch/pytorch/runs/1573634166 (success)
- master was ea4ccc7

This was caused by actions/checkout@v1 merging into master by default, causing nondeterministic CI behavior regardless of the base of the current PR. All our other current jobs mitigate this issue by adding a "Checkout PR tip" step that checks out ${{ github.event.pull_request.head.sha }} when on a PR, but that step was missing from the quick-checks job. This PR replaces all those additional steps with just a bit of extra configuration on the actions/checkout@v1 step itself, which is shorter and less error-prone:

with:
  ref: ${{ github.head_ref }}

Solution and discussion can be found here: actions/checkout#27 (comment)

facebook-github-bot · 2020-12-18T00:26:26Z

💊 CI failures summary and remediations

As of commit 6ade4d2 (more details on the Dr. CI page):

7/8 failures possibly* introduced in this PR
- 5/7 non-CircleCI failure(s)
1/8 broken upstream at merge base d17dc37 on Dec 17 from 2:42pm to 4:02pm

2 failures not recognized by patterns:

Job	Step	Action
^{pytorch_ios_12_0_0_x86_64_build}	^{Spin up environment}	🔁 rerun
^{pytorch_macos_10_13_py3_build}	^{Spin up environment}	🔁 rerun

🚧 1 fixed upstream failure:

These were probably caused by upstream breakages that were already fixed.

Please rebase on the viable/strict branch (expand for instructions)

If your commit is older than viable/strict, run these commands:

git fetch https://github.com/pytorch/pytorch viable/strict
git rebase FETCH_HEAD

Check out the recency history of this "viable master" tracking branch.

pytorch_windows_vs2019_py36_cuda10.1_test1 on Dec 17 from 2:42pm to 4:02pm (4b85239 - 6db5e85)
- 🔁 rerun

Extra GitHub checks: 5 failed

Failed: GitHub Actions - clang-tidy
Failed: GitHub Actions - flake8-py3
Failed: GitHub Actions - cmakelint
Failed: GitHub Actions - quick-checks
Failed: GitHub Actions - clang-format

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

This comment has been revised 10 times.

facebook-github-bot

@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

samestep · 2020-12-18T00:28:04Z

oh I seem to have broken everything haha

seemethere · 2020-12-18T00:53:20Z

We should probably go ahead with what actions/checkout#27 (comment) suggests which is to upgrade the actions/checkout to v2

walterddr · 2020-12-18T00:56:16Z

.github/workflows/clang_format.yml

          architecture: x64
      - name: Fetch PyTorch
        uses: actions/checkout@v1
-      - name: Checkout PR tip


restore this.

walterddr · 2020-12-18T00:56:24Z

.github/workflows/lint.yml

          architecture: x64
      - name: Fetch PyTorch
        uses: actions/checkout@v1
-      - name: Checkout PR tip


walterddr · 2020-12-18T01:01:54Z

.github/workflows/lint.yml

          architecture: x64
      - name: Checkout PyTorch
        uses: actions/checkout@v1
+        with:


I think the only reason why this is is the quick-chck branch is missing

- name: Checkout PR tip

while all the others already ensures PR tip

walterddr

actually I take it back. your change here adding ref: ${{ github.head_ref }} and the "checkout PR tip" step addresses different thing.

samestep · 2020-12-18T15:32:30Z

actually I take it back. your change here adding ref: ${{ github.head_ref }} and the "checkout PR tip" step addresses different thing.

no, they're supposed to address the same thing; the difference is just that my PR doesn't work, whereas yours does 😛

walterddr · 2020-12-18T16:26:06Z

actually I think you should still address Eli's comment to upgrade to v2.

samestep · 2020-12-18T16:29:38Z

actually I think you should still address Eli's comment to upgrade to v2.

@seemethere could you clarify what you meant in your comment earlier?

We should probably go ahead with what actions/checkout#27 (comment) suggests which is to upgrade the actions/checkout to v2

it was my understanding that the default behavior of the checkout action is still to try to merge into master, even in v2 (which is why the v2 README has a section for how to not checkout a merge commit); what exactly would upgrading to v2 change in this case?

seemethere · 2020-12-18T16:45:13Z

They were saying that there was an issue in the original v1 where if the base reference is updated before the job actually does its checkout then the job will unknowingly use an updated version of the branch instead of the reference that originally spawned the job.

Since we do an explicit checkout of the github.head_ref I guess this wouldn’t be an issue but we should just update to v2 to get the currently supported version anyway.

samestep · 2020-12-18T16:47:20Z

@seemethere ah I see, yeah that makes sense 👍 when we upgrade to v2, should we set the version as simply v2, or should we pin it to a more specific version such as v2.3.4 to reduce nondeterminism? (this is something @walterddr and I were discussing yesterday while debugging this issue)

walterddr · 2020-12-18T16:51:40Z

as of now I think @v2 should be suffice --> it should give us the benefit of picking up latest bug fixes/changes. if and when we encountered issue we can always pin a version then.

However it would've been nice on the log to printout exact checkout version for reference - in case we need to pin a version, we need to know which version worked previously./

Summary: This PR replaces our current "Checkout PR tip" step (which is duplicated across many places) using a [scenario](https://github.com/actions/checkout#checkout-pull-request-head-commit-instead-of-merge-commit) from the `actions/checkout` README. We previously tried something similar in #49578, but using `github.head_ref` didn't work. The reason this PR works is because, for events besides `pull_request`, the value of `github.event.pull_request.head.sha` defaults to the empty string, so it's as if we didn't set the `ref` option for `actions/checkout` at all, so it just uses its default behavior (e.g. for `push` events). Incidentally, this PR also upgrades our use of `actions/checkout` from `v1` to `v2`, which introduces shallow clones by default. A couple of our jobs require deep clones, so we use `fetch-depth: 0` in those cases. Pull Request resolved: #53719 Test Plan: CI. Reviewed By: albanD Differential Revision: D26949121 Pulled By: samestep fbshipit-source-id: e06f8066682ae0557fb5a055a10ea33b6bd320db

Summary: This PR replaces our current "Checkout PR tip" step (which is duplicated across many places) using a [scenario](https://github.com/actions/checkout#checkout-pull-request-head-commit-instead-of-merge-commit) from the `actions/checkout` README. We previously tried something similar in pytorch#49578, but using `github.head_ref` didn't work. The reason this PR works is because, for events besides `pull_request`, the value of `github.event.pull_request.head.sha` defaults to the empty string, so it's as if we didn't set the `ref` option for `actions/checkout` at all, so it just uses its default behavior (e.g. for `push` events). Incidentally, this PR also upgrades our use of `actions/checkout` from `v1` to `v2`, which introduces shallow clones by default. A couple of our jobs require deep clones, so we use `fetch-depth: 0` in those cases. Pull Request resolved: pytorch#53719 Test Plan: CI. Reviewed By: albanD Differential Revision: D26949121 Pulled By: samestep fbshipit-source-id: e06f8066682ae0557fb5a055a10ea33b6bd320db

Always checkout github.head_ref

6ade4d2

samestep requested review from a team, seemethere and walterddr December 18, 2020 00:26

facebook-github-bot added the cla signed label Dec 18, 2020

facebook-github-bot reviewed Dec 18, 2020

View reviewed changes

samestep marked this pull request as draft December 18, 2020 00:29

walterddr suggested changes Dec 18, 2020

View reviewed changes

walterddr reviewed Dec 18, 2020

View reviewed changes

samestep mentioned this pull request Dec 18, 2020

add checkout PR tip step for quick checks #49590

Closed

samestep closed this Dec 18, 2020

samestep mentioned this pull request Jan 22, 2021

.github: Add GitHub Actions workflow to build wheels #50633

Closed

This was referenced Mar 9, 2021

Don't merge to master in CircleCI #53652

Closed

Remove GHA "Checkout PR tip" step #53719

Closed

Always checkout github.head_ref #49578

Always checkout github.head_ref #49578

Uh oh!

Conversation

samestep commented Dec 18, 2020

Uh oh!

facebook-github-bot commented Dec 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

2 failures not recognized by patterns:

🚧 1 fixed upstream failure:

Extra GitHub checks: 5 failed

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

samestep commented Dec 18, 2020

Uh oh!

seemethere commented Dec 18, 2020

Uh oh!

walterddr Dec 18, 2020

Choose a reason for hiding this comment

Uh oh!

walterddr Dec 18, 2020

Choose a reason for hiding this comment

Uh oh!

walterddr Dec 18, 2020

Choose a reason for hiding this comment

Uh oh!

walterddr left a comment

Choose a reason for hiding this comment

Uh oh!

samestep commented Dec 18, 2020

Uh oh!

walterddr commented Dec 18, 2020

Uh oh!

samestep commented Dec 18, 2020

Uh oh!

seemethere commented Dec 18, 2020

Uh oh!

samestep commented Dec 18, 2020

Uh oh!

walterddr commented Dec 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

facebook-github-bot commented Dec 18, 2020 •

edited

Loading

walterddr commented Dec 18, 2020 •

edited

Loading