Skip to content

Tools/build/compute-changes.py looks at the wrong branches #133410

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

Closed
JelleZijlstra opened this issue May 4, 2025 · 2 comments
Closed

Tools/build/compute-changes.py looks at the wrong branches #133410

JelleZijlstra opened this issue May 4, 2025 · 2 comments
Assignees
Labels
infra CI, GitHub Actions, buildbots, Dependabot, etc. type-bug An unexpected behavior, bug, or error

Comments

@JelleZijlstra
Copy link
Member

JelleZijlstra commented May 4, 2025

Bug report

Bug description:

Tools/build/compute-changes.py assumes that both of the branches being compared are on the origin origin. This is usually not going to be the case for PR branches. If the PR branch happens to be called main, this can lead to no tests being run, as happened on #131419.

cc @AA-Turner

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Linked PRs

@JelleZijlstra JelleZijlstra added infra CI, GitHub Actions, buildbots, Dependabot, etc. type-bug An unexpected behavior, bug, or error labels May 4, 2025
@AA-Turner AA-Turner self-assigned this May 5, 2025
@hugovk hugovk changed the title Tools/build/compute_changes.py looks at the wrong branches Tools/build/compute-changes.py looks at the wrong branches May 5, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this issue 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 issue May 5, 2025
)

(cherry picked from commit d530e74)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
JelleZijlstra pushed a commit that referenced this issue 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 issue 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>
hugovk added a commit to hugovk/cpython that referenced this issue May 8, 2025
@hugovk
Copy link
Member

hugovk commented May 8, 2025

The PR has broken builds for non-main branches like 3.14 upstream, and also for forks with branches prefixed like 3.14-: #133416 (comment)

It's because the "is this a PR?" check (target_branch and head_ref) isn't working:

def compute_changes() -> None:
target_branch, head_ref = git_refs()
if target_branch and head_ref:
# Getting changed files only makes sense on a pull request
files = get_changed_files(target_branch, head_ref)
outputs = process_changed_files(files)
else:
# Otherwise, just run the tests
outputs = Outputs(run_tests=True, run_windows_tests=True)

And non-main branches (upstream and forks) have both target_branch and head_ref:

target ref: 'main'
head ref: 'ecb77ba1470bbc3e15dd53139f55f34109e7532b'

https://github.com/emmatyping/cpython/actions/runs/14848418162/job/41687428491#step:6:16

target ref: 'main'
head ref: '153496921a05e205d4c45c4a64888ccd43148897'

https://github.com/python/cpython/actions/runs/14907932653/job/41874703388#step:6:91

At least one of them should be false for PRs, but:

  • target_branch comes from ${{ github.base_ref || github.event.repository.default_branch }}

    • github.base_ref is only available for PRs, but there should always a fallback default branch
  • head_ref comes from ${{ github.event.pull_request.head.sha || github.sha }}

    • similarly, github.event.pull_request.head.sha is only for PRs, but there should always be a fallback github.sha that triggers the workflow

Docs: https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/accessing-contextual-information-about-workflow-runs

Let's come up with a better "is this a PR?" check.

@hugovk
Copy link
Member

hugovk commented May 8, 2025

Please see PR #133671.

@sobolevn sobolevn marked this as a duplicate of #134031 May 15, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this issue May 15, 2025
(cherry picked from commit 319acf3)

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue May 15, 2025
(cherry picked from commit 319acf3)

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@hugovk hugovk closed this as completed May 15, 2025
hugovk added a commit that referenced this issue May 15, 2025
…4055)

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
hugovk added a commit that referenced this issue May 15, 2025
…4054)

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue May 15, 2025
(cherry picked from commit 319acf3)

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
hugovk added a commit that referenced this issue May 15, 2025
…4057)

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
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. type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants