Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: use actions/checkout@(v1, v2) to v3 #2522

Merged
merged 1 commit into from
Aug 31, 2022
Merged

chore: use actions/checkout@(v1, v2) to v3 #2522

merged 1 commit into from
Aug 31, 2022

Conversation

ericswanson-dfinity
Copy link
Member

@ericswanson-dfinity ericswanson-dfinity commented Aug 31, 2022

Upgrading from actions/checkout@v1 or actions/checkout@v2 to @v3.

I had an interesting CI failure today: https://github.com/dfinity/sdk/runs/8102506163?check_suite_focus=true

When the e2e build_dfx (ubuntu) step ran, it checked out one merge commit, and built the dfx binary:

HEAD is now at 57088df8 Merge 00bb8ac0c5298534ef68ca49a5dd3f03f5cb4105 into 21048c4b76896316b72b05288a1231267bdc47d3

Then, another PR was merged to master. The merged PR contained a new test, and a new dfx command. The binary built above did not contain the new command that the new test required, because it was built from a commit when that command did not exist.

Next, the dfx/info tests ran, using the binary built from the earlier step, but checked out the repo at a different (newer) merge commit:

HEAD is now at 4a74aeb5 Merge 00bb8ac0c5298534ef68ca49a5dd3f03f5cb4105 into 3f0c9e9b6aba8067748750538cadfd8479852e30

The result was a test failure.

It seems strange that the default checkout behavior would cause different jobs from the same workflow for the same pull request event to check out different merge commits. This issue describes the problem exactly, but is closed without a resolution. I've seen suggestions to use the pull request HEAD commit instead of MERGE commit, but it's unclear how this would work for workflows that also run on push.

While it seems clear that the correct behavior would be to use a consistent merge commit, using the base commit from the pull request event rather than the head of the base branch at the instant of any given checkout, I have no reason to believe that actions/checkout@v3 fixes this issue. At the same time there is no good reason to use some v1, some v2, and some v3.

@ericswanson-dfinity ericswanson-dfinity requested a review from a team as a code owner August 31, 2022 00:40
@ericswanson-dfinity ericswanson-dfinity changed the title chore: use actions/checkout v3 where v1 or v2 were used chore: use actions/checkout@(v1, v2) to v3 Aug 31, 2022
@lwshang lwshang merged commit de5c66c into master Aug 31, 2022
@lwshang lwshang deleted the git-checkout-v3 branch August 31, 2022 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants