Convert change detection to a Python script by AA-Turner · Pull Request #129627 · python/cpython · GitHub

The change detection workflow is becoming increasingly complex.

We have grown from a fairly simple grep command to skip documentation files (#19983) to a large and complex shell script (embedded within a YAML document), indeed one sufficiently complex to merit a dedicated workflow file (#122336)

A potted history of significant revisions is thus:

There are further proposed changes, such as skipping Windows tests on changes to the Unix build configuration:

Even having recently improved readability of the central grep command (#128754), this workflow remains difficult to correctly modify (#128450).

This PR converts the core logic into a short Python script, Tools/build/compute-changes.py, which determines which workflows to run. I imagine this will make it easier to introduce future conditional workflows, which we should probably adopt more of to reduce time and resources spent waiting for CI.

I have reused the "changed files" logic introduced in #108065, meaning we can also combine the duplicative MSI and Docs changes steps, which reduces the overall work done.

I've tested this quite a bit on my fork and detection works well, as does workflow dispatch.

A

cc @webknjaz (sorry for the ping; I can't request-review)

Sample workflow run from this branch: https://github.com/python/cpython/actions/runs/13125212512/job/36620039265?pr=129627

I'll try to review later today. Thanks for the ping!

Thank you for this! I was thinking of something like this given the wrangling of #128450 :)

Mostly nits here:

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>

* Rename `config_hash` to `config-hash` * Rename `run_tests` to `run-tests` * Rename `run-win-msi` to `run-windows-msi` * Rename `run_hypothesis` to `run-hypothesis` * Rename `run_cifuzz` to `run-ci-fuzz`

Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>

@AA-Turner were the CI jobs cancelled by accident? It's not immediately obvious what happened in there…

I wonder if this would belong in the same script… In the past I'd just use sha512 to compute hashes in python: https://github.com/ansible/awx-plugins/blob/0d569b5/.github/workflows/ci-cd.yml#L222C16-L222C52.

If not, I'd question if this reusable workflow should even be called “change detection”. I think I tend to call the computation job “pre-setup” or something, since the changes isn't the only thing being detected…

I'd question if this reusable workflow should even be called “change detection”.

"reusable-choose-workflows"?

I wonder if this would belong in the same script

We could move it to compute-changes.py, sure. It seems it should be possible to replicate the same output as hashFiles

"reusable-choose-workflows"?

"reusable-build-settings"? "reusable-settings"? "reusable-workflow-run-context"?

We could move it to compute-changes.py, sure. It seems it should be possible to replicate the same output as hashFiles

Totally, although, it doesn't really matter if it's the same given the context where it's being used. It just has to be unique and predictable. Hashing is only used because you can't put entire file contents into cache keys 🤷‍♂️

If we change the key, all current caches will be invalidated?

Yes, but they won't be picked up after this PR anyway because they depend on the contents of .github/workflows/build.yml which you're changing here. You're already invalidating the cache.

As a note, this change made it so PRs need to merge in main to get the script otherwise the github workflow tries running a script which doesn't exist and fails. I really like having it in Python, but is a hazard that breaking changes to the script may break outstanding PRs until they merge in main.

Do you have a link to an example run? Not ideal, but it should be fixable by clicking the update branch button.

Update branch button definitely worked for my case and was straightforward to figure out and get working.

#129560 is a PR opened before this change, I made more commits and pushed, that caused this run after compute-changes.py was committed https://github.com/python/cpython/actions/runs/13163983268/job/36739530516, then I did "update branch" and https://github.com/python/cpython/actions/runs/13164025409/job/36739667617 run happened and succeeded

That's weird. actions/checkout usually gets the merged-in changes into jobs..

Yeah, we often have to "Update branch" after making CI checks stricter, for example, recently when making Blurb validation stricter.

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>

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

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

Sorry, @AA-Turner, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 7d9a22f50923309955a2caf7d57013f224071e6e 3.12

Sorry, @AA-Turner, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 7d9a22f50923309955a2caf7d57013f224071e6e 3.13

(cherry picked from commit 7d9a22f) Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>

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

(cherry picked from commit 7d9a22f) Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>

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

Yeah, we often have to "Update branch" after making CI checks stricter, for example, recently when making Blurb validation stricter.

I meant that this is supposed to be happening automatically. Just restarting the jobs should be enough.

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>

Convert change detection to a Python script

819da3b

AA-Turner added skip issue skip news labels Feb 3, 2025

AA-Turner requested review from ezio-melotti and hugovk as code owners February 3, 2025 23:52

bedevere-app Bot added the awaiting core review label Feb 3, 2025

hugovk reviewed Feb 4, 2025

View reviewed changes

AA-Turner and others added 3 commits February 4, 2025 10:55

Hugo's review

4c0a7a4

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>

Simplify

4169b46

Standardise workflow variables:

49f02be

* Rename `config_hash` to `config-hash` * Rename `run_tests` to `run-tests` * Rename `run-win-msi` to `run-windows-msi` * Rename `run_hypothesis` to `run-hypothesis` * Rename `run_cifuzz` to `run-ci-fuzz`