Skip to content

Conversation

aryairani
Copy link
Contributor

Reverts #5798 temporarily; need to change the cache key consistently in a couple of places if we're changing it, related to #5800. I can add more context here, but want to get trunk passing quickly first.

@aryairani aryairani requested a review from a team as a code owner July 11, 2025 08:59
@aryairani
Copy link
Contributor Author

cc @sellout

@aryairani
Copy link
Contributor Author

My bad for not anticipating this during review.

@aryairani aryairani merged commit 9f48a80 into trunk Jul 11, 2025
45 of 47 checks passed
@aryairani aryairani deleted the revert-5798-check-integration-diffs branch July 11, 2025 09:48
@sellout
Copy link
Contributor

sellout commented Jul 11, 2025

Ugh, sorry about this.

This kind of thing comes up a lot because GitHub workflows only partially implement YAML. In particular, they’re missing support for anchors and aliases. But it looks like that might be changing this year.

I’ve done things like generating workflows from Nix, or using some templating script (which are still necessary workarounds if you want to share stuff between workflows or other files, like with our pinned tool versions).

If these cache keys are workflow-local, I think we should just revisit this across workflows once actions/runner#1182 is fixed. Then we can prefix the key with something like &binary-cache-key when caching, and replace the repeated key with *binary-cache-key when restoring. E.g.,

- name: cache ucm binaries
  id: cache-ucm-binaries
  uses: actions/cache@d4323d4df104b026a6aa633fdb11d772146be0bf # v4
  with:
    path: ${{env.ucm_local_bin}}
    key: &binary-cache-key ucm-${{ matrix.os }}-${{ hashFiles('**/ci.yaml', '**/stack.yaml', '**/package.yaml', '**/*.hs', '**/unison-cli-integration/integrationtests/IntegrationTests/*')}}
## [...]
- name: restore binaries
  uses: actions/cache/restore@d4323d4df104b026a6aa633fdb11d772146be0bf # v4
  if: steps.cache-transcript-test-results.outputs.cache-hit != 'true'
  with:
    path: ${{env.ucm_local_bin}}
    key: *binary-cache-key
    fail-on-cache-miss: true

And if GitHub ends up supporting merge keys, it can be a bit nicer, as we can anchor the entire with value, then use the merge key (<<) to allow us to still add fail-on-cache-miss to the map.

- name: cache ucm binaries
  id: cache-ucm-binaries
  uses: actions/cache@d4323d4df104b026a6aa633fdb11d772146be0bf # v4
  with: &binary-cache
    path: ${{env.ucm_local_bin}}
    key: ucm-${{ matrix.os }}-${{ hashFiles('**/ci.yaml', '**/stack.yaml', '**/package.yaml', '**/*.hs', '**/unison-cli-integration/integrationtests/IntegrationTests/*')}}
## [...]
- name: restore binaries
  uses: actions/cache/restore@d4323d4df104b026a6aa633fdb11d772146be0bf # v4
  if: steps.cache-transcript-test-results.outputs.cache-hit != 'true'
  with:
    << : *binary-cache
    fail-on-cache-miss: true

But man, all of this stuff just makes me miss Dhall (or at least Nix) for configuration.

@sellout
Copy link
Contributor

sellout commented Aug 13, 2025

This kind of thing comes up a lot because GitHub workflows only partially implement YAML. In particular, they’re missing support for anchors and aliases.

I don’t think it applies in this case any more, but GitHub now supports YAML anchors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants