GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Convert change detection to a Python script by AA-Turner · Pull Request #129627 · python/cpython

Conversation

AA-Turner commented Feb 3, 2025

Copy link

Copy Markdown

Member

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cmaloney and picnixz reacted with thumbs up emoji

All reactions

👍 2 reactions


          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

AA-Turner commented Feb 3, 2025

Copy link

Copy Markdown

Member Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

webknjaz commented Feb 4, 2025

Copy link

Copy Markdown

Member

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

AA-Turner reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hugovk reviewed

Feb 4, 2025

View reviewed changes

hugovk left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Mostly nits here:

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Tools/build/compute-changes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

.github/workflows/reusable-change-detection.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Tools/build/compute-changes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Tools/build/compute-changes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Tools/build/compute-changes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Tools/build/compute-changes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Tools/build/compute-changes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Tools/build/compute-changes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

AA-Turner and others added 3 commits

February 4, 2025 10:55


          Hugo's review

4c0a7a4

Co-authored-by: Hugo van Kemenade <[email protected]>


          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`

webknjaz reviewed

Feb 5, 2025

View reviewed changes

Comment thread

.github/workflows/reusable-change-detection.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.

webknjaz reviewed

Feb 5, 2025

View reviewed changes

Comment thread

.github/workflows/reusable-change-detection.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.

webknjaz reviewed

Feb 5, 2025

View reviewed changes

Comment thread

.github/workflows/reusable-change-detection.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.

webknjaz reviewed

Feb 5, 2025

View reviewed changes

Comment thread

Tools/build/compute-changes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

webknjaz reviewed

Feb 5, 2025

View reviewed changes

Comment thread

Tools/build/compute-changes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

webknjaz reviewed

Feb 5, 2025

View reviewed changes

Comment thread

.github/workflows/reusable-change-detection.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.

webknjaz reviewed

Feb 5, 2025

View reviewed changes

Comment thread

.github/workflows/reusable-change-detection.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.


          no # bool

d65e5be

Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>

AA-Turner requested a review from webknjaz

February 5, 2025 12:52

webknjaz commented Feb 5, 2025

Copy link

Copy Markdown

Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

webknjaz reviewed

Feb 5, 2025

View reviewed changes

Comment thread

.github/workflows/reusable-change-detection.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.

AA-Turner added 2 commits

February 5, 2025 14:43


          Add step name

b40113e


          empty

1ee4e0e

webknjaz reviewed

Feb 5, 2025

View reviewed changes

Comment thread

.github/workflows/reusable-change-detection.yml

		- name: Compute hash for config cache key
		id: config-hash
		run: \|
		echo "hash=${{ hashFiles('configure', 'configure.ac', '.github/workflows/build.yml') }}" >> "$GITHUB_OUTPUT"

webknjaz Feb 5, 2025

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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…

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

AA-Turner Feb 5, 2025

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

webknjaz Feb 5, 2025

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"reusable-choose-workflows"?

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

webknjaz Feb 5, 2025

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🤷‍♂️

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

AA-Turner Feb 5, 2025

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

webknjaz Feb 5, 2025

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AA-Turner reacted with thumbs up emoji

All reactions

👍 1 reaction

webknjaz reviewed

Feb 5, 2025

View reviewed changes

Comment thread

Tools/build/compute-changes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

webknjaz reviewed

Feb 5, 2025

View reviewed changes

Comment thread

Tools/build/compute-changes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

webknjaz reviewed

Feb 5, 2025

View reviewed changes

Comment thread

Tools/build/compute-changes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

webknjaz reviewed

Feb 5, 2025

View reviewed changes

Comment thread

Tools/build/compute-changes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

AA-Turner merged commit 7d9a22f into python:main

Feb 5, 2025

bedevere-app Bot removed the awaiting core review label

Feb 5, 2025

AA-Turner deleted the change-detection-py branch

February 5, 2025 17:19

cmaloney commented Feb 5, 2025

Copy link

Copy Markdown

Contributor

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AA-Turner commented Feb 5, 2025

Copy link

Copy Markdown

Member Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

sobolevn mentioned this pull request

Feb 5, 2025

New compute-changes.py should be checked with mypy #129707

Closed

cmaloney commented Feb 5, 2025

Copy link

Copy Markdown

Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

webknjaz commented Feb 6, 2025

Copy link

Copy Markdown

Member

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

cmaloney reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hugovk commented Feb 6, 2025

Copy link

Copy Markdown

Member

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

cmaloney reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Feb 6, 2025

gh-127833: Fix grammar snippet formatting for help() output #129692

Closed

gh-128446: Skip Windows CI for Unix build system changes #128450

Merged

srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request

Feb 7, 2025


          Convert change detection to a Python script (python#129627)

20f5f97

Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>

cmaloney pushed a commit to cmaloney/cpython that referenced this pull request

Feb 8, 2025


          Convert change detection to a Python script (python#129627)

df013d8

Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>

hugovk added needs backport to 3.12

only security fixes

needs backport to 3.13 only security fixes labels

Feb 20, 2025

miss-islington-app Bot commented Feb 20, 2025

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington-app Bot commented Feb 20, 2025

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington-app Bot commented Feb 20, 2025

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington-app Bot assigned AA-Turner

Feb 20, 2025

miss-islington-app Bot commented Feb 20, 2025

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hugovk added a commit to hugovk/cpython that referenced this pull request

Feb 20, 2025


          [3.13] Convert change detection to a Python script (pythonGH-129627)

5c354ca

(cherry picked from commit 7d9a22f)

Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>

hugovk added a commit to hugovk/cpython that referenced this pull request

Feb 20, 2025


          [3.13] Convert change detection to a Python script (pythonGH-129627)

45bf3d4

(cherry picked from commit 7d9a22f)

Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>

bedevere-app Bot commented Feb 20, 2025

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot removed the needs backport to 3.13 only security fixes label

Feb 20, 2025

hugovk added a commit to hugovk/cpython that referenced this pull request

Feb 20, 2025


          [3.12] Convert change detection to a Python script (pythonGH-129627)

0ce2b94

(cherry picked from commit 7d9a22f)

Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>

bedevere-app Bot commented Feb 20, 2025

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot removed the needs backport to 3.12 only security fixes label

Feb 20, 2025

webknjaz commented Feb 20, 2025

Copy link

Copy Markdown

Member

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hugovk added a commit that referenced this pull request

Feb 21, 2025


          [3.12] Convert change detection to a Python script (GH-129627) (#130370)

91e5e24

Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>

AA-Turner added a commit that referenced this pull request

Feb 22, 2025


          [3.13] Convert change detection to a Python script (GH-129627) (#130367)

019918a

Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>

Reviewers

hugovk

hugovk left review comments

tomasr8

tomasr8 left review comments

webknjaz

webknjaz approved these changes

ezio-melotti

Awaiting requested review from ezio-melotti

ezio-melotti is a code owner

1st1

Awaiting requested review from 1st1

Uh oh!

There was an error while loading. Please reload this page.

Assignees

AA-Turner

Labels

skip issue skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.