bpo-40548: Fix "Check for source changes (pull_request)" GH Action job by vstinner · Pull Request #21806 · python/cpython

vstinner · 2020-08-10T11:25:29Z

On Git 2.28, "git diff master..." (3 dots) no longer works when
"fetch --depth=1" is used, whereas it works on Git 2.26.

Replace "..." (3 dots) with ".." (2 dots) in the "git diff" command
computing the list of modified files between the base branch and the
PR branch.

https://bugs.python.org/issue40548

On Git 2.28, "git diff master..." (3 dots) no longer works when "fetch --depth=1" is used, whereas it works on Git 2.26. Replace "..." (3 dots) with ".." (2 dots) in the "git diff" command computing the list of modified files between the base branch and the PR branch.

vstinner · 2020-08-10T11:26:22Z

cc @brettcannon @Mariatta @FFY00 @zooba @pablogsal

vstinner · 2020-08-10T14:42:49Z

The macOS failure is unrelated and can be ignored: https://bugs.python.org/issue40275#msg375122

vstinner · 2020-08-10T14:53:51Z

This change can be associated to https://bugs.python.org/issue40548 which introduced the "Check for source changes" job.

shihai1991 · 2020-08-10T16:16:48Z

Oh, you are so quick. I try to fix it a minute ago: shihai1991#30 it) Lol~

shihai1991 · 2020-08-10T16:20:33Z

+ # into the PR branch anyway.
+ #
+ # https://github.com/python/core-workflow/issues/373
+ git diff --name-only origin/$GITHUB_BASE_REF.. | grep -qvE '(\.rst$|^Doc|^Misc)' && echo '::set-output name=run_tests::true' || true


I test it in last weekend, it can work. and removing the git fetch origin $GITHUB_BASE_REF --depth=1 can fix it too.
I guess depth=1 is not enough deep to find the merge base.

vstinner · 2020-08-10T16:23:41Z

I guess depth=1 is not enough deep to find the merge base.

Correct. But depth=1 is part of https://bugs.python.org/issue40548 design to make the "Check for source changes" job fast. For example, on my latest change, it only took 18 seconds overall!

I don't think that "..." (3 dots) to find the last common commit is needed for this job. ".." (2 dots) should be enough.

shihai1991

Thanks, It's can work for me. so LGTM.

vstinner · 2020-08-10T16:40:55Z

I tested manully: with this change, doc-only PR still skip build jobs.

I created #21812 to test this PR. It's a "documentation-only" PR. Build GitHub Action jobs have been skipped as expected, and "Docs / Docs" is running as expected.

vstinner · 2020-08-10T16:49:49Z

@shihai1991 @brettcannon: Thanks for the review! I merged my PR. So far, it works as expected. If something goes wrong, we can change the test again.

Python 3.9 is also affected, example: #21809 Check for source changes: "fatal: ... no merge base". I backport the change to 3.8 and 3.9.

miss-islington · 2020-08-10T16:50:01Z

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

miss-islington · 2020-08-10T16:50:01Z

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

bedevere-bot · 2020-08-10T16:50:18Z

GH-21813 is a backport of this pull request to the 3.8 branch.

pythonGH-21806) On Git 2.28, "git diff master..." (3 dots) no longer works when "fetch --depth=1" is used, whereas it works on Git 2.26. Replace "..." (3 dots) with ".." (2 dots) in the "git diff" command computing the list of modified files between the base branch and the PR branch. (cherry picked from commit eaa5517) Co-authored-by: Victor Stinner <vstinner@python.org>

bedevere-bot · 2020-08-10T16:50:29Z

GH-21814 is a backport of this pull request to the 3.9 branch.

pythonGH-21806) On Git 2.28, "git diff master..." (3 dots) no longer works when "fetch --depth=1" is used, whereas it works on Git 2.26. Replace "..." (3 dots) with ".." (2 dots) in the "git diff" command computing the list of modified files between the base branch and the PR branch. (cherry picked from commit eaa5517) Co-authored-by: Victor Stinner <vstinner@python.org>

GH-21806) On Git 2.28, "git diff master..." (3 dots) no longer works when "fetch --depth=1" is used, whereas it works on Git 2.26. Replace "..." (3 dots) with ".." (2 dots) in the "git diff" command computing the list of modified files between the base branch and the PR branch. (cherry picked from commit eaa5517) Co-authored-by: Victor Stinner <vstinner@python.org>

FFY00 · 2020-08-10T19:12:39Z

@vstinner sorry for the delay, I did not have access to my computer over the weekend.

I implemented this in another project and was running into an issue, this was my solution:

https://github.com/FFY00/arch-python-repo/blob/9cf94335c441638a5a7fbed5b58e4852fbdd5792/.github/workflows/build.yml#L39

 - name: Check for source changesid: checkenv:
PREVIOUS: ${{ github.event.before }}CURRENT: ${{ github.sha }}run: | git fetch origin $PREVIOUS git diff --name-only $PREVIOUS $CURRENT | grep -q ... || true

Turns out github actions makes the previous object SHA available for us to use. We can use this to explicitly fetch it and to run the diff. It should be more reliable than the current approach. What do you think?

vstinner · 2020-08-10T21:14:27Z

Turns out github actions makes the previous object SHA available for us to use. We can use this to explicitly fetch it and to run the diff. It should be more reliable than the current approach. What do you think?

I tried git diff origin/$GITHUB_BASE_REF..$GITHUB_SHA: it avoids the "git checkout", but the checkout is part of actions/checkout@v2, we cannot avoid it. With the checkout, origin/$GITHUB_BASE_REF..$GITHUB_SHA is the same than origin/$GITHUB_BASE_REF..HEAD which is the same than origin/$GITHUB_BASE_REF...

From what I understood, in practice, git diff $PREVIOUS $CURRENT is the same than git diff origin/$GITHUB_BASE_REF...

FFY00 · 2020-08-10T22:06:32Z

No, GITHUB_BASE_REF (also github.base_ref) is different from github.event.before.

github.base_ref:

The base_ref or target branch of the pull request in a workflow run. This property is only available when the event that triggers a workflow run is a pull_request.

github.event.before is the SHA of the top commit before the event.

It is documented in the webhooks documentation, see https://docs.github.com/en/actions/reference/events-that-trigger-workflows for context.

github.event.before:

The SHA of the most recent commit on ref before the push.

Github is doing a bad job with the documentation here, it took a lot of digging to uncover this. But it does work and is documented.

pythonGH-21806) On Git 2.28, "git diff master..." (3 dots) no longer works when "fetch --depth=1" is used, whereas it works on Git 2.26. Replace "..." (3 dots) with ".." (2 dots) in the "git diff" command computing the list of modified files between the base branch and the PR branch.

miss-islington · 2022-05-05T16:18:39Z

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

pythonGH-21806) On Git 2.28, "git diff master..." (3 dots) no longer works when "fetch --depth=1" is used, whereas it works on Git 2.26. Replace "..." (3 dots) with ".." (2 dots) in the "git diff" command computing the list of modified files between the base branch and the PR branch. (cherry picked from commit eaa5517) Co-authored-by: Victor Stinner <vstinner@python.org>

bedevere-bot · 2022-05-05T16:18:49Z

GH-92342 is a backport of this pull request to the 3.7 branch.

GH-21806) (GH-92342) On Git 2.28, "git diff master..." (3 dots) no longer works when "fetch --depth=1" is used, whereas it works on Git 2.26. Replace "..." (3 dots) with ".." (2 dots) in the "git diff" command computing the list of modified files between the base branch and the PR branch. (cherry picked from commit eaa5517) Co-authored-by: Victor Stinner <vstinner@python.org>

vstinner added skip issue skip news labels Aug 10, 2020

the-knights-who-say-ni added the CLA signed label Aug 10, 2020

bedevere-bot added the awaiting core review label Aug 10, 2020

vstinner mentioned this pull request Aug 10, 2020
GitHub Actions issues not triggering python/core-workflow#373
Closed

Complete the comment
e21df38

vstinner mentioned this pull request Aug 10, 2020
Debug https://github.com/python/core-workflow/issues/373 #21805
Closed

vstinner changed the title ~~Fix "Check for source changes (pull_request)" GH Action job~~bpo-40548: Fix "Check for source changes (pull_request)" GH Action jobAug 10, 2020

vstinner removed the skip issue label Aug 10, 2020

vstinner mentioned this pull request Aug 10, 2020
bpo-40275: Fix failed test cases by using test helpers #21811
Merged

shihai1991 reviewed Aug 10, 2020
View reviewed changes

vstinner mentioned this pull request Aug 10, 2020
bpo-1635741: port winapi to multi-stage init #21371
Merged

shihai1991 approved these changes Aug 10, 2020
View reviewed changes

brettcannon approved these changes Aug 10, 2020
View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Aug 10, 2020

vstinner merged commit eaa5517 into python:masterAug 10, 2020

bedevere-bot removed the awaiting merge label Aug 10, 2020

vstinner deleted the fix_workflow_diff branch August 10, 2020 16:37

vstinner mentioned this pull request Aug 10, 2020
bpo-40548: [WIP] Documentation only PR #21812
Closed

vstinner mentioned this pull request Aug 10, 2020
Travis CI doesn't report its status or doesn't run on Python pull requests python/core-workflow#371
Closed

vstinner added the needs backport to 3.8 label Aug 10, 2020

vstinner added the needs backport to 3.9 label Aug 10, 2020

bedevere-bot removed the needs backport to 3.8 label Aug 10, 2020

bedevere-bot removed the needs backport to 3.9 label Aug 10, 2020

erlend-aasland mentioned this pull request May 5, 2022
[3.7] gh-80254: Disallow recursive usage of cursors in sqlite3 converters #92334
Merged

JelleZijlstra added the needs backport to 3.7 label May 5, 2022

bedevere-bot removed the needs backport to 3.7 label May 5, 2022

AA-Turner mentioned this pull request Feb 3, 2025
Convert change detection to a Python script #129627
Merged

Uh oh!

Uh oh!

Conversation

vstinner commented Aug 10, 2020• edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner commented Aug 10, 2020

Uh oh!

vstinner commented Aug 10, 2020

Uh oh!

vstinner commented Aug 10, 2020

Uh oh!

shihai1991 commented Aug 10, 2020

Uh oh!

shihai1991Aug 10, 2020

Choose a reason for hiding this comment

Uh oh!

vstinner commented Aug 10, 2020

Uh oh!

shihai1991 left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner commented Aug 10, 2020

Uh oh!

vstinner commented Aug 10, 2020

Uh oh!

miss-islington commented Aug 10, 2020

Uh oh!

miss-islington commented Aug 10, 2020

Uh oh!

bedevere-bot commented Aug 10, 2020

Uh oh!

bedevere-bot commented Aug 10, 2020

Uh oh!

FFY00 commented Aug 10, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner commented Aug 10, 2020

Uh oh!

FFY00 commented Aug 10, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miss-islington commented May 5, 2022

Uh oh!

bedevere-bot commented May 5, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

vstinner commented Aug 10, 2020•
edited by bedevere-bot
Loading

FFY00 commented Aug 10, 2020•
edited
Loading

FFY00 commented Aug 10, 2020•
edited
Loading