Convert change detection to a Python script #129627
Conversation
|
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! |
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`
|
@AA-Turner were the CI jobs cancelled by accident? It's not immediately obvious what happened in there… |
| else | ||
| echo "Branch too old for CIFuzz tests; or no C files were changed" | ||
| echo "run-cifuzz=false" >> "$GITHUB_OUTPUT" | ||
| fi | ||
| - name: Compute hash for config cache key | ||
| id: config-hash | ||
| run: | | ||
| echo "hash=${{ hashFiles('configure', 'configure.ac', '.github/workflows/build.yml') }}" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
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…
There was a problem hiding this comment.
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
There was a problem hiding this comment.
"reusable-choose-workflows"?
"reusable-build-settings"? "reusable-settings"? "reusable-workflow-run-context"?
There was a problem hiding this comment.
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 🤷♂️
There was a problem hiding this comment.
If we change the key, all current caches will be invalidated?
There was a problem hiding this comment.
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 |
|
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 |
|
That's weird. |
|
Yeah, we often have to "Update branch" after making CI checks stricter, for example, recently when making Blurb validation stricter. |
hugovk
added
needs backport to 3.12
|
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 |
|
Sorry, @AA-Turner, I could not cleanly backport this to |
|
GH-130367 is a backport of this pull request to the 3.13 branch. |
|
GH-130370 is a backport of this pull request to the 3.12 branch. |
I meant that this is supposed to be happening automatically. Just restarting the jobs should be enough. |
The change detection workflow is becoming increasingly complex.
We have grown from a fairly simple
grepcommand 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:
CODEOWNERSupdate #128754reusable-change-detection.ymlonworkflow_dispatch#122966regexpinbuild.ymlto not trigger the jobs on*.mdand*.inifiles. #120435There are further proposed changes, such as skipping Windows tests on changes to the Unix build configuration:
configure/Makefilechanges #128446Even having recently improved readability of the central
grepcommand (#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)