Skip to content

GH-133410: Use commit hashes for change detection #133416

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

Merged
merged 2 commits into from
May 5, 2025

Conversation

AA-Turner
Copy link
Member

@AA-Turner AA-Turner commented May 5, 2025

As seen recently, if the pull-request branch on a fork repository has the same name as the target branch on the upstream repository, the branches shadow each other when set up by @actions/checkout, meaning that change detection passes with no changes having occured.

This changes to instead perform the git diff with the commit SHA of the head commit. This is slightly tricky due to what GitHub exposes, but in short, we always compare the tip of the PR branch (fork) against the default branch (upstream).

A

cc @JelleZijlstra

@AA-Turner AA-Turner added skip news infra CI, GitHub Actions, buildbots, Dependabot, etc. labels May 5, 2025
@AA-Turner AA-Turner requested a review from 1st1 as a code owner May 5, 2025 00:22
@ambv ambv merged commit d530e74 into python:main May 5, 2025
42 checks passed
@hugovk hugovk added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels May 5, 2025
@miss-islington-app
Copy link

Thanks @AA-Turner for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Thanks @AA-Turner for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 5, 2025
)

(cherry picked from commit d530e74)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 5, 2025
)

(cherry picked from commit d530e74)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@bedevere-app
Copy link

bedevere-app bot commented May 5, 2025

GH-133426 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label May 5, 2025
@bedevere-app
Copy link

bedevere-app bot commented May 5, 2025

GH-133427 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label May 5, 2025
@hugovk
Copy link
Member

hugovk commented May 5, 2025

Also needs backporting to 3.12 so the CI isn't skipped there.

@hugovk
Copy link
Member

hugovk commented May 5, 2025

And let's update CODEOWNERS so @1st1 isn't mistakenly pinged for reviews on .github/workflows/reusable-context.yml due to:

# Core
**/*context* @1st1

JelleZijlstra pushed a commit that referenced this pull request May 5, 2025
…133427)

GH-133410: Use commit hashes for change detection (gh-133416)
(cherry picked from commit d530e74)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Yhg1s pushed a commit that referenced this pull request May 5, 2025
…133426)

GH-133410: Use commit hashes for change detection (gh-133416)
(cherry picked from commit d530e74)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@emmatyping
Copy link
Member

I believe this broke change detection in personal/forked branches prefixed with e.g. 3.14-. For example https://github.com/emmatyping/cpython/actions/runs/14848418162/job/41687428491

@hugovk
Copy link
Member

hugovk commented May 6, 2025

Oh yes, let's fix that :)

@hugovk
Copy link
Member

hugovk commented May 8, 2025

hugovk added a commit to hugovk/cpython that referenced this pull request May 8, 2025
@hugovk
Copy link
Member

hugovk commented May 8, 2025

Let's continue in the issue, #133410.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra CI, GitHub Actions, buildbots, Dependabot, etc. skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants