Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error using default cache: '' option since v3.7.0 #797

Closed
2 of 5 tasks
colinrotherham opened this issue Jul 6, 2023 · 12 comments
Closed
2 of 5 tasks

Error using default cache: '' option since v3.7.0 #797

colinrotherham opened this issue Jul 6, 2023 · 12 comments
Assignees
Labels
bug Something isn't working

Comments

@colinrotherham
Copy link

colinrotherham commented Jul 6, 2023

Description:
We've noticed that the default cache: '' option throws an error since v3.7.0

Post job cleanup.
Error: Cache folder paths are not retrieved for npm with cache-dependency-path = 

It's documented in README.md Usage

# Used to specify a package manager for caching in the default directory. Supported values: npm, yarn, pnpm.
# Package manager should be pre-installed
# Default: ''
cache: ''

Hope this report helps fix the issue for others. Thanks 😊

Action version:
v3.7.0

Platform:

  • Ubuntu
  • macOS
  • Windows

Runner type:

  • Hosted
  • Self-hosted

Tools version:
Node.js v18.16.1 with default npm 9.5.1

Repro steps:
Add a GitHub Action workflow step with the default cache: '' option

Here we toggle inputs.use-cache to skip cache restoration of /home/runner/.npm when node_modules is already cached. This speeds up our GitHub Actions workflow runs on Windows

runs:
  using: composite

  steps:
    - name: Setup Node.js
      uses: actions/setup-node@v3
      id: setup-node

      with:
        cache: ${{ inputs.use-cache != 'false' && 'npm' || '' }}
        node-version-file: .nvmrc

See the full action.yml for more information

Here's our related PR to pin to version v3.6.0 as a fix:

Expected behavior:
The default cache: '' option to continue working

Actual behavior:
An error is thrown:

Post job cleanup.
Error: Cache folder paths are not retrieved for npm with cache-dependency-path = 

See examples of the error output:

@dusan-trickovic
Copy link

Hello, @colinrotherham ! Thank you for submitting the issue, we will take a closer look at it and see what can be done :)

@dmitry-shibanov
Copy link
Contributor

Hello @colinrotherham. I took a look at your logs and it looks like that during the post job you have different values because cache-hit does not exist at post job but the cache-hit is true during set up step.

@dmitry-shibanov dmitry-shibanov self-assigned this Jul 12, 2023
@colinrotherham
Copy link
Author

Thanks for taking a look @dmitry-shibanov

Yeah that's right

We intentionally set cache: '' because we know we'll skip npm ci later on

On Windows it's too slow to restore the npm get cache directory when we don't need it

@dmitry-shibanov
Copy link
Contributor

Thank you for your response @colinrotherham. I think it is not the issue from actions/setup-node. I've tried to specify cache: '' and it works as expected. I think in your case because the condition changes to post jobs. During the set up step the action gets empty string but during post it gets npm because cache-hit becomes null that is why on post step the setup-node gets npm.

@colinrotherham
Copy link
Author

colinrotherham commented Jul 12, 2023

Unless I'm reading the log messages wrong they're for our node_modules cache restore?

key: npm-install-${{ runner.os }}-${{ hashFiles('package-lock.json', '**/package.json') }}

Do you think our cache-hit is true log messages affect actions/setup-node@v3.7.0?

Cache restored successfully
Cache restored from key: npm-install-Linux-XXXXXXX
##[debug]Node Action run completed with exit code 0
##[debug]Save intra-action state CACHE_KEY = npm-install-Linux-XXXXXXX
##[debug]Save intra-action state CACHE_RESULT = npm-install-Linux-XXXXXXX
##[debug]Set output cache-hit = true

Do our cache key names clash?

Composite action inputs

Otherwise thanks for pointing out our inputs.use-cache != 'false' check

During the set up step the action gets empty string but during post it gets npm because cache-hit becomes null that is why on post step the setup-node gets npm.

We hadn't noticed that the input changed from 'false' (set up step) to null (post step)

I presume in actions/setup-node@v3.6.0 this was handled a bit better?

We'll harden our checks for actions/setup-node@v3.7.0 but good to discuss for anyone else facing the issue

Appreciate it

@dmitry-shibanov
Copy link
Contributor

You didn't face this issue before because previously the action was getting cached paths during the save-step through tools commands.Now the action gets cached paths during the setup steps and pass it to save step that is why now it fails for you because in your case the action during the setup step has empty string for cache but during (no paths for cache were saved) but during post step it gets cache as npm. It does not have cached paths that is why it fails.

It was working previously because the action could save the cache because it could get paths and they were not empty because from the initial run of npm i command the global cache directory was also generated.

@colinrotherham
Copy link
Author

colinrotherham commented Jul 12, 2023

Ah that's a great explanation. Hope this helps others that hadn't realised

I'll update our input checks to include null which appears to fall under:

Will come back and close this issue if it all works

Thanks again


Update: All working 👍

colinrotherham added a commit to alphagov/govuk-frontend that referenced this issue Jul 12, 2023
Looks like our `use-cache` input sometimes defaults to null. This hasn’t been an issue before, but it caused problems with `actions/setup-node@3.7.0`

See GitHub issue: actions/setup-node#797
colinrotherham added a commit to alphagov/govuk-frontend that referenced this issue Jul 13, 2023
Looks like our `use-cache` input sometimes defaults to null. This hasn’t been an issue before, but it caused problems with `actions/setup-node@3.7.0`

See GitHub issue: actions/setup-node#797
colinrotherham added a commit to alphagov/govuk-frontend that referenced this issue Jul 13, 2023
Looks like our `use-cache` input sometimes defaults to null. This hasn’t been an issue before, but it caused problems with `actions/setup-node@3.7.0`

See GitHub issue: actions/setup-node#797
@eddiemonge
Copy link

          I'm getting the same error `Error: Cache folder paths are not retrieved for npm with cache-dependency-path = ` but using this setup: (no `cached-dependency-path` and using npm`)
runs:
  using: 'composite'
  steps:
    - uses: actions/checkout@v3
    - uses: actions/setup-node@v3
      with:
        node-version: 18
        registry-url: https://npm.pkg.github.com/
        cache: ${{ hashFiles('**/package-lock.json') && 'npm' || null }}
    - run: npm install --ignore-scripts
      shell: bash
    - run: npm rebuild && npm run prepare --if-present
      shell: bash

I wonder if it has to do with removing the optional part of this check 8170e22#diff-55f15e2366942ad15f71a41ac983f8ce9a9882b28b7fd9082f3a26c799783064L16

Originally posted by @eddiemonge in #801 (comment)

@colinrotherham how did you actually fix it?

@colinrotherham
Copy link
Author

For the cache option you have to default to '' now not null

@eddiemonge
Copy link

That should really have been a breaking change requiring a bump to version 4 then.

@eddiemonge
Copy link

That did not fix it for me.

I'm using a custom reusable action:

name: Setup NodeJS
description: Setups NodeJS and install dependencies

runs:
  using: 'composite'
  steps:
    - uses: actions/checkout@v3
    - uses: actions/setup-node@v3
      with:
        node-version: 18
        cache: ${{ hashFiles('**/package-lock.json') && 'npm' || '' }}
    - run: npm install --ignore-scripts
      shell: bash
    - run: npm rebuild && npm run prepare --if-present
      shell: bash

The consuming action is:

name: Build Node App

runs:
  using: 'composite'
  steps:
    - uses: custom/setup-node
    - run: npm run build

Output is

Post job cleanup.
Error: Cache folder paths are not retrieved for npm with cache-dependency-path = 

@colinrotherham
Copy link
Author

Sorry it didn't help, this was our fix in alphagov/govuk-frontend@b8165f9

colinrotherham added a commit to alphagov/govuk-frontend that referenced this issue Jul 24, 2023
Looks like our `use-cache` input sometimes defaults to null. This hasn’t been an issue before, but it caused problems with `actions/setup-node@3.7.0`

See GitHub issue: actions/setup-node#797
(cherry picked from commit b8165f9)
colinrotherham added a commit to alphagov/govuk-frontend that referenced this issue Jul 24, 2023
Looks like our `use-cache` input sometimes defaults to null. This hasn’t been an issue before, but it caused problems with `actions/setup-node@3.7.0`

See GitHub issue: actions/setup-node#797
(cherry picked from commit b8165f9)
colinrotherham added a commit to alphagov/govuk-frontend that referenced this issue Jul 28, 2023
Looks like our `use-cache` input sometimes defaults to null. This hasn’t been an issue before, but it caused problems with `actions/setup-node@3.7.0`

See GitHub issue: actions/setup-node#797
(cherry picked from commit b8165f9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants
@eddiemonge @colinrotherham @dmitry-shibanov @dusan-trickovic and others