Installing the Homebrew dependencies takes between 10 to 20 seconds of CI time. We can consider caching those deps (cc.@hugovk, as our CI/GitHub/DevOps oracle).

ApparentlyTools/build/check_extension_modules.py is too fragile. We should harden it and ensure that it dumps enough debug info on failure, so we won't end up with a week of red CI crosses again.

I intend to merge this as soon as the CI turns green. If there are objections, we can always revert or create a follow-up PR.

@erlend-aasland What's difference between#101991?

@erlend-aasland What's difference between#101991?

Oh, sorry, I missed you had a similar PR!

@erlend-aasland What's difference between#101991?

AFAICS, my PR is slightly cleaner in that it still uses multiple build steps (first set environment variables, then run configure, etc.); it makes debugging the CI easier. Also, I did not touch any of the tests.

Status check is done, and it's a success ✅.

Thanks@erlend-aasland for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

Thanks for the review, Dong-hee!

it makes debugging the CI easier. Also, I did not touch any of the tests.

It looks like the GHA performance issue is resolved.
During the weekend it was not.

AFAICS, my PR is slightly cleaner in that it still uses multiple build steps (first set environment variables, then run configure

see:#101991 (comment)

Re: caching Homebrew:

https://stackoverflow.com/a/65056232/724176 shows how to do it, but warns:

You should note that this cache is unlikely to speed up your workflow. Installing Homebrew bottles (the default) will usually have similar performance to downloading from GitHub/Azure's own storage cache since the bottle files are served on bintray's CDN.

https://stackoverflow.com/a/63681598/724176 has 3 options, which aren't great: 1 is caching everything; 2 means identifying and copying binaries; 3 is Docker.

Looking at thelog:

==> Running `brew cleanup pkg-config`...Disable this behaviour by setting HOMEBREW_NO_INSTALL_CLEANUP.

We can set that if it makes things quicker, we don't need to do any cleanup, the whole CI run is disposed of after the run. And the first SO answer above setsHOMEBREW_NO_AUTO_UPDATE, that may save time too.

Here's a run ofmain with this PR,brew install takes 25s:

• https://github.com/hugovk/cpython/actions/runs/4223815148/jobs/7333994448

WithHOMEBREW_NO_AUTO_UPDATE it drops to 15s:

• https://github.com/hugovk/cpython/actions/runs/4223831861/jobs/7334030713

And withHOMEBREW_NO_INSTALL_CLEANUP it's 16s, basically the same:

• https://github.com/hugovk/cpython/actions/runs/4223836429/jobs/7334040607

So let's add them.

     needs: check_source     if: needs.check_source.outputs.run_tests == 'true'     env:+      HOMEBREW_NO_AUTO_UPDATE: 1+      HOMEBREW_NO_INSTALL_CLEANUP: 1       PYTHONSTRICTEXTENSIONBUILD: 1     steps:     - uses: actions/checkout@v3

Edit: Well, these are all within the expected range anyway:

Installing the Homebrew dependencies takes between 10 to 20 seconds of CI time.

But these env vars shouldn'tadd time :)

@hugovk, yeah adding theHOMEBREW_* variables makes sense.@corona10, can you update your PR to include those?

And they're documented athttps://docs.brew.sh/Manpage#environment

Sorry that I'm a bit late to this discussion but I have to point out that there appears to be a bit of a misunderstanding here of whatconfigure does and, thus, this PR is not doing what its title asserts :)

configure goes to great lengths to ensure that the necessary environment specified at the timeconfigure is run is preserved in the generatedMakefile such that whenevermake is run the necessary environment variable values are created in themake environment. That means that, once you have runconfigure, you can go back later, in a different terminal session, say, and runmake without having to define the essential environment variables (likeCFLAGS andLDFLAGS) as they were set atconfigure time.

However, theMakefile does allow for augmenting those environment variables if they are defined in the environment when runningmake. So, if those environment variables have been defined externally when runningmake, you may get different results of a build. That's fine if that's what you want to do. But, by default, you should not have to define those variables for a normal build after runningconfigure. That's the way things have worked for a very long time.

Note that the devguide recipe for 3.10+ deliberately doesnot export any environment variables in the shell environment when runningconfigue andmake; by placing the environment variables definitions on theconfigure command line, they are local to theconfigure process and its children (and preserved by it in theMakefile). With the github workflow as merged here, theCFLAGS andLDFLAGS values are effectively unnecessarily being exported into each subsequent build step by usingGITHUB_ENV. This isn't just a stylistic change; it bypasses a small but important test of the build process that those variables are being properly preserved and recreated bymake steps so that we are confident that the Makefile stands alone and is not dependent on externally defining any configuration-related variables.

@ned-deily, this is true (I noticed my error post merge, see my comments on Discord). Dong-hee had a fixup PR ready at some point, but he reverted that specific change. I definitely agree that we should provide the environment variables only whenconfigure is run!

Thanks@erlend-aasland for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

Thanks@erlend-aasland for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

Sorry,@erlend-aasland, I could not cleanly backport this to3.9 due to a conflict.
Please backport usingcherry_picker on command line.
cherry_picker 27136310414965a3ea7f835e416cf74b91cefb48 3.9

Sorry@erlend-aasland, I had trouble checking out the3.8 backport branch.
Please retry by removing and re-adding the "needs backport to 3.8" label.
Alternatively, you can backport usingcherry_picker on the command line.
cherry_picker 27136310414965a3ea7f835e416cf74b91cefb48 3.8

I'll create backports for 3.9 and 3.8 with the fixup in#102131 applied.

GH-102186 is a backport of this pull request to the3.9 branch.

GH-102187 is a backport of this pull request to the3.8 branch.