v4.0 save always not working

I have setup the save always to true such that the cache step should always fail even if one of the tasks fail. See screenshots below:
Save always set to true in step:

Then further in the job a step fails but the post cache step is not run unlike the post repositry setup:

Related to
#1242

For context this is how I did save cache in v3:

I think we can also optimise like this to not attempt to save cache when there was a cache hit

ci: Fix JS caching (#270)

I am also facing this issue. With save-always: true, the Post Cache is still skipped if the previous build step has failed:

We're observing the same issue. It just doesn't seem to work as documented.

The "save-always" flag in actions/cache is broken

#2 change from `save-always` flag to `if: always()` see actions/cache…

# This is a combination of 31 commits.

@DanMawdsleyBA

👋 Thank you for the bug report and apologies for the lack of a timely response.

As I shared in #1325 (comment), I do not believe this is a feature we can support in this action today due to limitations in what's possible in post-if expressions.

As @DanMawdsleyBA shared above, if you want to override the post-if behavior of the default cache action you can utilize the separate actions/cache/save and actions/cache/restore. This sort of granularity is what these actions were originally intended for - #1035.

That might look something like:

name: Always Caching Primes

on: push

jobs:
  build:
    runs-on: ubuntu-latest

    steps:
    - uses: actions/checkout@v4

    - name: Restore cached Primes
      id: cache-primes-restore
      uses: actions/cache/restore@v4
      with:
        path: |
          path/to/dependencies
          some/other/dependencies
        key: ${{ runner.os }}-primes

    # Intermediate workflow steps

    - name: Always Save Primes 
      id: cache-primes-save
      if: always() && steps.cache-primes-restore.outputs.cache-hit != 'true'
      uses: actions/cache/save@v4
      with:
        path: |
          path/to/dependencies
          some/other/dependencies
        key: ${{ steps.cache-primes-restore.outputs.cache-primary-key }}

Given that save-always never worked, I believe it would make the most sense to deprecate that action input and remove it in a future version release. It's not something we can confidently support long term with the current features of post-if - any workarounds we implement today are likely to break in the future if they're relying on undocumented behavior. #1242 should not have been approved or merged without sufficient testing from us to validate that it was working as intended.

My current plan is to:

Deprecate save-always in the current major version to indicate to users that it's not working as intended
Document in the README how to support this scenario with actions/cache/save
Remove save-always in the next major version

I realize this won't likely won't be a popular course of action, but I believe it's best for the long term maintainability of this action. I'd appreciate any and all feedback here, especially around the propose workaround if scenarios where it might not work.

@joshmgross

Document in the README how to support this scenario with actions/cache/save

@joshmgross , is there more that should be added to specified here?

https://github.com/actions/cache/blob/main/save/README.md#always-save-cache

Document in the README how to support this scenario with actions/cache/save

@joshmgross , is there more that should be added to specified here?

https://github.com/actions/cache/blob/main/save/README.md#always-save-cache

I was thinking it'd make sense to include the cache-hit output in that if conditional. The dedicated save cache action won't have access to the intra action state used by the restore step to indicate if the save is necessary.

cache/src/saveImpl.ts

Lines 46 to 48 in 0c45773

    
           // If matched restore key is same as primary key, then do not save cache 
        
           // NO-OP in case of SaveOnly action 
        
           const restoredKey = stateProvider.getCacheState();

Use separate cache step to save via if always

Save benchmark cache even the job is failed (#5694)

Ensure cache is always saved

Deprecate `save-always` input

Bug actions/cache#1315 <-- save-always not working.

Here is the current documentation with the workaround.

And it does NOT recommend using save-always

https://github.com/actions/cache/blob/main/caching-strategies.md#saving-cache-even-if-the-build-fails

smoke tests: leverage caching (#4665)

Deprecate `save-always` input (#1452)

save-always has been deprecated as it's not something we can support at this time. #1452

See #1315 (comment) for more details.

CI: migrate cache save always

fix: vulnerabilities (#2)

Use separate cache step to save via if always

Merge pull request actions#1747 from actions/yacaovsnc/update-depende…

Fix native addon cache ordering and force baseline x64 ISA

Restore Windows x64 release packaging, fix oh-my-pi CI provisioning g…

Navigation Menu

Repository navigation

v4.0 save always not working #1315

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	// If matched restore key is same as primary key, then do not save cache
	// NO-OP in case of SaveOnly action
	const restoredKey = stateProvider.getCacheState();

v4.0 save always not working #1315

Description

Activity

DanMawdsleyBA commented on Jan 30, 2024

SukkaW commented on Feb 8, 2024

sascha-wolf commented on Feb 27, 2024

joshmgross commented on Jun 12, 2024

ruffsl commented on Jun 18, 2024

joshmgross commented on Jun 18, 2024

fulldecent commented on Sep 9, 2024

joshmgross commented on Oct 7, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions