actions / cache

Cache dependencies and build outputs in GitHub Actions
MIT License
4.49k stars 1.2k forks source link

`!cache-hit` is `false` when `cache-hit` is `false` #1262

Open andreasabel opened 11 months ago

andreasabel commented 11 months ago

This is the workflow I caught red-handed: https://github.com/andreasabel/hasktorch/actions/runs/6536754694/job/17749144021

  1. Version: actions/cache@v3 SHA:704facf57e6136b1bc63b828d79edcd491f0ee84 https://github.com/andreasabel/hasktorch/actions/runs/6536754694/job/17749144021
  2. Cache restored from restore-key https://github.com/andreasabel/hasktorch/actions/runs/6536754694/job/17749144021#step:7:4
    Run actions/cache/restore@v3
    with:
      path: ~/.cabal/store
      key: macOS-cabal-3.6.2.0-ghc-9.2.8-plan-813f23bfbcb4c800c8178d743c3df6e501ec38580a651c180dcd99b0abc9319d
      restore-keys: macOS-cabal-3.6.2.0-ghc-9.2.8-
    ...
    Cache restored from key: macOS-cabal-3.6.2.0-ghc-9.2.8-813f23bfbcb4c800c8178d743c3df6e501ec38580a651c180dcd99b0abc9319d

    Note that this key is not the primary key, which has an additional -plan before the SHA.

  3. The step "Cache dependencies" (if: !steps.cache.outputs.cache-hit) is skipped", it's code is https://github.com/andreasabel/hasktorch/actions/runs/6536754694/workflow
    - name: Cache dependencies
      if:   ${{ !steps.cache.outputs.cache-hit }}
      uses: actions/cache/save@v3
      with:
        path: ~/.cabal/store
        key: ${{ steps.cache.outputs.cache-primary-key }}

According to the docs, outputs.cache-hit should not be true if the cache was restored with a fallback key. This is stressed here: https://github.com/actions/cache/blob/704facf57e6136b1bc63b828d79edcd491f0ee84/README.md?plain=1#L63-L65 And again for the restore action: https://github.com/actions/cache/blob/704facf57e6136b1bc63b828d79edcd491f0ee84/restore/README.md?plain=1#L15-L22

andreasabel commented 11 months ago

Update: Debug printing shows that actually, it is not that cache-hit is true: https://github.com/andreasabel/hasktorch/actions/runs/6537791407/job/17757232711#step:7:8229

##[debug]Set output cache-primary-key = macOS-cabal-3.6.2.0-ghc-9.2.8-plan-813f23bfbcb4c800c8178d743c3df6e501ec38580a651c180dcd99b0abc9319d
##[debug]Set output cache-matched-key = macOS-cabal-3.6.2.0-ghc-9.2.8-813f23bfbcb4c800c8178d743c3df6e501ec38580a651c180dcd99b0abc9319d
##[debug]Set output cache-hit = false

However, ! cache-hit is nevertheless false: https://github.com/andreasabel/hasktorch/actions/runs/6537791407/job/17757232711#step:9:2

##[debug]Evaluating: (success() && !steps.cache.outputs.cache-hit)
##[debug]Evaluating And:
##[debug]..Evaluating success:
##[debug]..=> true
##[debug]..Evaluating Not:
##[debug]....Evaluating Index:
##[debug]......Evaluating Index:
##[debug]........Evaluating Index:
##[debug]..........Evaluating steps:
##[debug]..........=> Object
##[debug]..........Evaluating String:
##[debug]..........=> 'cache'
##[debug]........=> Object
##[debug]........Evaluating String:
##[debug]........=> 'outputs'
##[debug]......=> Object
##[debug]......Evaluating String:
##[debug]......=> 'cache-hit'
##[debug]....=> 'false'
##[debug]..=> false
##[debug]=> false
##[debug]Expanded: (true && !'false')
##[debug]Result: false

So cache-hit isn't a proper boolean, it is a String, and negation isn't boolean negation, but something abstruse. 🤣 When have type systems be invented? 1970s? Earlier than that? And when is now? 21th century? I actually doubt it... 😢

andreasabel commented 11 months ago

This must be a regression, because I have used the pattern !steps.cache.outputs.cache-hit in the past and it worked as expected. Here is evidence where it works: https://github.com/andreasabel/bnfc3/actions/runs/6539942580/job/17758962129#step:3:25

Run andreasabel/fix-whitespace-action@v1
Run export EXEDIR="$HOME/.local/fix-whitespace/bin"
Run actions/cache@v3
Cache not found for input keys: Linux-fix-whitespace-0.1
Run # Download binary
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0

100 16.0M  100 16.0M    0     0  35.1M      0 --:--:-- --:--:-- --:--:-- 35.1M
curl exit code: 0
curl http code: 200
-rw-r--r-- 1 runner docker ... /home/runner/.local/fix-whitespace/bin/fix-whitespace

What happens here is that a cache is not found, and thus the following download step is executed. This download step is guarded by !steps.CACHE_STEP_ID.outputs.cache-hit, as can be seen here: https://github.com/andreasabel/fix-whitespace-action/blob/e60f45943b1dfbd36fbc0412a4783945de29102b/action.yml#L47-L55

I am totally confused now...

jenseng commented 11 months ago

I'm not sure how this would have worked before, unless cache-hit was an empty string. Unfortunately your workflow run where it worked correctly now gives a 404, so we can't see the details any more.

Action outputs are always strings, see the docs here. So this is a limitation of GitHub Actions generally, and isn't specific to this one. That said, I do agree that the docs for this action should be clarified around this 👍

andreasabel commented 11 months ago

Yes, I looked at the source code and there wasn't a recent regression. A clarification proposal of the docs has been submitted here:

github-actions[bot] commented 4 months ago

This issue is stale because it has been open for 200 days with no activity. Leave a comment to avoid closing this issue in 5 days.

andreasabel commented 4 months ago

I'd be happy to get my PR merged.