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.

Sign up for GitHub

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

Jump to bottom

fix(repair broken cached-checkout-install step) #1454

Merged
merged 8 commits into from
Nov 21, 2024
Merged

fix(repair broken cached-checkout-install step) #1454

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a2fa270
conditionally run npm ci if a cache miss occurs
Spencer6497 Nov 20, 2024
d5d62a7
tweak cache step to actually cache node_modules folder
Spencer6497 Nov 20, 2024
45080bc
Merge branch 'main' into fix-gha-cached-install
Spencer6497 Nov 20, 2024
051bb79
Merge branch 'main' into fix-gha-cached-install
Spencer6497 Nov 20, 2024
3a5aaba
Merge branch 'main' into fix-gha-cached-install
Spencer6497 Nov 20, 2024
b31771f
add node version to cache key to avoid node upgrade issues
Spencer6497 Nov 20, 2024
f2352b7
Merge branch 'main' into fix-gha-cached-install
aaschlote Nov 20, 2024
4c65b70
Merge branch 'main' into fix-gha-cached-install
Spencer6497 Nov 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions .github/actions/cached-checkout-install/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,24 @@ runs:
with:
node-version: "20.x"
cache: "npm"
- name: Grab node version
run: echo "NODE_VERSION=$(node --version)" >> $GITHUB_ENV
shell: bash
- name: Cache node modules
id: cache-npm
uses: actions/cache@v4
env:
cache-name: cache-node-modules
with:
path: ~/.npm
key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }}
path: ./node_modules
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi https://github.com/actions/cache/blob/main/examples.md#node---npm

Note It is not recommended to cache node_modules, as it can break across Node versions and won't work with npm ci

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw that; two thoughts:

  1. to avoid this breaking across Node versions, we could hash the node version and add it to the key
  2. npm ci only runs on a cache miss, so node_modules won't exist in that case

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the explanation. I checked the build time for all previous pull requests and didn't see any significant improvement. Am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a slight speedup for pull requests, but PRs only trigger the first 3 steps of the CI flow; every other step also uses cached-checkout-install, so full deployments should be faster by a few minutes, maybe 3 or so

key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ env.NODE_VERSION }}-${{ hashFiles('**/package-lock.json') }}
restore-keys: |
${{ runner.os }}-build-${{ env.cache-name }}-${{ env.NODE_VERSION }}-
${{ runner.os }}-build-${{ env.cache-name }}-
${{ runner.os }}-build-
${{ runner.os }}-
- if: ${{ steps.cache-npm.outputs.cache-hit != 'true' }}
name: List the state of node modules
name: Perform a clean install of node dependencies
continue-on-error: true
run: npm list
shell: bash
- run: npm ci
run: npm ci
shell: bash
Loading