GeezerVison

where is the cache file stored? is it using per-repo or per-org storage?
wondering about that, thinking how it will behave in forks.

I'm not familiar with config.cache - are the configure and configure.ac files the only relevant inputs?
what's the impact on total CI runtime when there's a cache hit? cache miss?

I also tried thinking about cross-branch and cross-PR interactions, but since the key uses the hash of the input files then if the hashes are identical across branches and PRs then it should be ok to reuse the cached file.

one concern is implicit inputs that are not captured by the current key:

• the "job name" is a proxy to flags that affect the result, but it is possible that flags change and the job name stays the same
• the versions of the github actions used may change and implicitly affect the result, but afaik that is not captured in the cache key

where is the cache file stored? is it using per-repo or per-org storage? wondering about that, thinking how it will behave in forks.

https://github.com/actions/cache does it per-org, so it won't be shared with forks. Forks will have their own copies.

I'm not familiar with config.cache - are the configure and configure.ac files the only relevant inputs?

Let's ask @erlend-aasland :)

what's the impact on total CI runtime when there's a cache hit? cache miss?

I ran this PR twice: once with no cache, and second time with cache present.

• See the "Post Restore config.cache" steps at the end for uploading a cache in the first run: they take either 0 or 1 seconds.

• See the "Restore config.cache" steps at the start for restoring a cache in the second run: they take either 0 or 1 seconds.

I also tried thinking about cross-branch and cross-PR interactions, but since the key uses the hash of the input files then if the hashes are identical across branches and PRs then it should be ok to reuse the cached file.

one concern is implicit inputs that are not captured by the current key:

• the "job name" is a proxy to flags that affect the result, but it is possible that flags change and the job name stays the same

Could you give an example?

• the versions of the github actions used may change and implicitly affect the result, but afaik that is not captured in the cache key

I think the action will invalidate the cache itself if they change something, but I couldn't find anything to confirm. I also don't think the action's version is exposed, anyway.

Could you give an example?

• check_generated_files: --config-cache --with-pydebug --enable-shared
• build_macos: --config-cache --with-pydebug --prefix=/opt/python-dev --with-openssl="$(brew --prefix [email protected])"
• build_ubuntu, build_ubuntu_ssltests, test_hypothesis: --config-cache --with-pydebug --with-openssl=$OPENSSL_DIR
• build_asan: --config-cache --with-address-sanitizer --without-pymalloc

Also, environment variables:

• build_macos:
  • HOMEBREW_NO_ANALYTICS: 1
  • HOMEBREW_NO_AUTO_UPDATE: 1
  • HOMEBREW_NO_INSTALL_CLEANUP: 1
  • PYTHONSTRICTEXTENSIONBUILD: 1
• build_ubuntu, test_hypothesis:
  • OPENSSL_VER: 1.1.1t
  • PYTHONSTRICTEXTENSIONBUILD: 1
• build_ubuntu_ssltests:
  • OPENSSL_VER: ${{ matrix.openssl_ver }}
  • MULTISSL_DIR: ${{ github.workspace }}/multissl
  • OPENSSL_DIR: ${{ github.workspace }}/multissl/openssl/${{ matrix.openssl_ver }}
  • LD_LIBRARY_PATH: ${{ github.workspace }}/multissl/openssl/${{ matrix.openssl_ver }}/lib
• build_asan:
  • OPENSSL_VER: 1.1.1t
  • PYTHONSTRICTEXTENSIONBUILD: 1
  • ASAN_OPTIONS: detect_leaks=0:allocator_may_return_null=1:handle_segv=0

@arhadthedev provided examples :)

the result can also be affected by the exact OS version (I don't know how granular runner.os is), which toolchain is used (gcc/clang), toolchain versions, and host architecture. any changes in any of these should invalidate the cache.

regarding the implementation - hashFiles can operate on multiple files, so use hashFiles(file1, file2) instead.
also for a given commit, the hash of these files is constant - would it be possible to extract the hash computation to perform it once and share between jobs?

@arhadthedev provided examples :)

👍

the result can also be affected by the exact OS version (I don't know how granular runner.os is), which toolchain is used (gcc/clang), toolchain versions, and host architecture. any changes in any of these should invalidate the cache.

runner.os: "The operating system of the runner executing the job. Possible values are Linux, Windows, or macOS."

https://docs.github.com/en/actions/learn-github-actions/contexts#runner-context

I added the workflow file to hashFiles, so it will invalidate and make a new cache when things defined in the file change, for example the env vars and CLI flags.

There's a runner.arch: "The architecture of the runner executing the job. Possible values are X86, X64, ARM, or ARM64" but we're using defaults and don't expect these will change?

regarding the implementation - hashFiles can operate on multiple files, so use hashFiles(file1, file2) instead.

Combined.

also for a given commit, the hash of these files is constant - would it be possible to extract the hash computation to perform it once and share between jobs?

Possibly but I'm not sure if it's worth the complexity: we can just have hashFiles computed each time.

LG!

I added the workflow file to hashFiles, so it will invalidate and make a new cache when things defined in the file change, for example the env vars and CLI flags.

this will be over-conservative, invalidating for any change in the workflow file, even if it doesn't affect the result (which is probably most changes). probably a fair trade off (preferring correctness over CI latency).

Possibly but I'm not sure if it's worth the complexity: we can just have hashFiles computed each time.

agreed. I'd suggest at least refactoring the list of files to be a shared constant.

I guess we want this CI speedup in the other branches as well.

I'd suggest at least refactoring the list of files to be a shared constant.

Do you know how to do this? I tried a few things, but didn't get it working.

Do you know how to do this? I tried a few things, but didn't get it working.

I think workflow-level environment variable can work for this

https://docs.github.com/en/actions/learn-github-actions/variables

I tried that like:

env:
  CONFIG_CACHE_HASH_FILES: ${{ hashFiles('configure', 'configure.ac', '.github/workflows/build.yml') }}

and:

          key: ${{ github.job }}-${{ runner.os }}-${{ env.CONFIG_CACHE_HASH_FILES }}

But it said "The workflow is not valid ", "Unrecognized function: 'hashFiles'"

I also tried:

env:
  CONFIG_CACHE_HASH_FILES: ('configure', 'configure.ac', '.github/workflows/build.yml')

and:

          key: ${{ github.job }}-${{ runner.os }}-${{ hashFiles(env.CONFIG_CACHE_HASH_FILES) }}

But it couldn't read the env var:

Cache not found for input keys: check_generated_files-Linux-

Let me try again with format():

https://github.com/orgs/community/discussions/25761#discussioncomment-4626908

Hmm, no luck, because it formats into strings, and we need a sort of tuple of strings.

We might be able to do something like:

env:
  CONFIG_CACHE_HASH_FILE1: 'configure'
  CONFIG_CACHE_HASH_FILE2: 'configure.ac'
  CONFIG_CACHE_HASH_FILE3: '.github/workflows/build.yml'

And then format it something like this:

          key: ${{ github.job }}-${{ runner.os }}-${{ hashFiles(format('{0}', env.CONFIG_CACHE_HASH_FILE1))-${{ hashFiles(format('{0}', env.CONFIG_CACHE_HASH_FILE2))-${{ format('{0}', env.CONFIG_CACHE_HASH_FILE3)) }}

But it feels like a backward refactor...

But it feels like a backward refactor...

yeah, I agree, this doesn't improve anything. let's leave it as explicit lists of files, I might give it another try separately after this merges.

Backport through all security branches?

Sorry @hugovk, I had trouble checking out the 3.12 backport branch.
Please retry by removing and re-adding the "needs backport to 3.12" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 1080c4386dd3beb9ee808acdf3c3f01835f73860 3.12

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

GH-104968 is a backport of this pull request to the 3.11 branch.

Thanks @hugovk for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

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

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

Thanks @hugovk for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

Sorry, @hugovk, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 1080c4386dd3beb9ee808acdf3c3f01835f73860 3.7

Sorry @hugovk, I had trouble checking out the 3.9 backport branch.
Please retry by removing and re-adding the "needs backport to 3.9" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 1080c4386dd3beb9ee808acdf3c3f01835f73860 3.9

Sorry, @hugovk, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 1080c4386dd3beb9ee808acdf3c3f01835f73860 3.8

Sorry @hugovk, I had trouble checking out the 3.10 backport branch.
Please retry by removing and re-adding the "needs backport to 3.10" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 1080c4386dd3beb9ee808acdf3c3f01835f73860 3.10

Actually, maybe we should skip the security branches?

They've already diverged quite a bit, and don't get built (or backported) as often, so there's less benefit.

Actually, maybe we should skip the security branches?

They've already diverged quite a bit, and don't get built (or backported) as often, so there's less benefit.

Yeah, makes sense.

I might give it another try separately after this merges.

I think it worked out nicely in gh-105008