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

Fix other workflows to install Corepack as prereq #246

Merged
merged 2 commits into from
Sep 17, 2024
Merged

Fix other workflows to install Corepack as prereq #246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4ec5c9e
Fix other workflows to install Corepack as prereq
mcmire Jun 11, 2024
0c3dcfa
Merge branch 'main' into fix-workflows-for-yarn-v4
mcmire Jul 18, 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
84 changes: 47 additions & 37 deletions .github/workflows/build-lint-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,41 +7,47 @@ jobs:
prepare:
name: Prepare
runs-on: ubuntu-latest
strategy:
matrix:
node-version: [18.x, 20.x]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
node-version: [18.x, 20.x]
node-version: [18.x, 20.x, 22.x]

steps:
- name: Use Node.js
- uses: actions/checkout@v4
- name: Install Corepack via Node
uses: actions/setup-node@v4
with:
node-version: 'lts/*'
node-version-file: '.nvmrc'
- name: Install Yarn
run: corepack enable
- uses: actions/checkout@v4
- name: Use Node.js and install dependencies
- name: Install Node.js ${{ matrix.node-version }} and restore Yarn cache
uses: actions/setup-node@v4
with:
node-version: 'lts/*'
node-version: ${{ matrix.node-version }}
cache: 'yarn'
- name: Install Yarn dependencies
- name: Install dependencies via Yarn
run: yarn --immutable

build:
name: Build
needs: prepare
runs-on: ubuntu-latest
needs:
- prepare
strategy:
matrix:
node-version: [20.x]
steps:
- name: Use Node.js
- uses: actions/checkout@v4
- name: Install Corepack via Node
uses: actions/setup-node@v4
with:
node-version: 'lts/*'
node-version-file: '.nvmrc'
- name: Install Yarn
run: corepack enable
- uses: actions/checkout@v4
- name: Use Node.js
- name: Install Node.js ${{ matrix.node-version }} and restore Yarn cache
uses: actions/setup-node@v4
with:
node-version: 'lts/*'
node-version: ${{ matrix.node-version }}
cache: 'yarn'
- run: yarn --immutable --immutable-cache
- name: Install dependencies via Yarn
run: yarn --immutable --immutable-cache
- run: yarn build
- name: Require clean working directory
shell: bash
Expand All @@ -53,23 +59,26 @@ jobs:

lint:
name: Lint
needs: prepare
runs-on: ubuntu-latest
needs:
- prepare
strategy:
matrix:
node-version: [20.x]
steps:
- name: Use Node.js
- uses: actions/checkout@v4
- name: Install Corepack via Node
uses: actions/setup-node@v4
with:
node-version: 'lts/*'
node-version-file: '.nvmrc'
- name: Install Yarn
run: corepack enable
- uses: actions/checkout@v4
- name: Use Node.js
- name: Install Node.js ${{ matrix.node-version }} and restore Yarn cache
uses: actions/setup-node@v4
with:
node-version: 'lts/*'
node-version: ${{ matrix.node-version }}
cache: 'yarn'
- run: yarn --immutable --immutable-cache
- name: Install dependencies via Yarn
run: yarn --immutable --immutable-cache
- run: yarn lint
- name: Validate RC changelog
if: ${{ startsWith(github.head_ref, 'release/') }}
Expand All @@ -87,26 +96,26 @@ jobs:

test:
name: Test
needs: prepare
runs-on: ubuntu-latest
needs:
- prepare
strategy:
matrix:
node-version: [18.x, 20.x]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
node-version: [18.x, 20.x]
node-version: [18.x, 20.x, 22.x]

steps:
- name: Use Node.js
- uses: actions/checkout@v4
- name: Install Corepack via Node
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}
node-version-file: '.nvmrc'
- name: Install Yarn
run: corepack enable
- uses: actions/checkout@v4
- name: Use Node.js ${{ matrix.node-version }}
- name: Install Node.js ${{ matrix.node-version }} and restore Yarn cache
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}
cache: 'yarn'
- run: yarn --immutable --immutable-cache
- name: Install dependencies via Yarn
run: yarn --immutable --immutable-cache
- run: yarn test
- name: Require clean working directory
shell: bash
Expand All @@ -118,31 +127,32 @@ jobs:

compatibility-test:
name: Compatibility test
needs: prepare
runs-on: ubuntu-latest
needs:
- prepare
strategy:
matrix:
node-version: [18.x, 20.x]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
node-version: [18.x, 20.x]
node-version: [18.x, 20.x, 22.x]

steps:
- name: Use Node.js
- uses: actions/checkout@v4
- name: Install Corepack via Node
uses: actions/setup-node@v4
Copy link
Contributor

@legobeat legobeat Jun 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The checkout step always goes first, this way .nvmrc can be read.

There was a valid reason to do actions/setup-node before checkout because of the action not being corepack-compatible. IIRC it throws an error when the preinstalled yarn version does not match the packageManager one in package.json (which is then working after a corepack enable).

Could potentially cause issues with any dependencies that rely on the specific package manager version in their installation step, if any (since setup-node performs dependency installation in presence of package.json)?

Copy link
Contributor Author

@mcmire mcmire Jun 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I didn't find evidence in the source that it installs dependencies — I think you still have to do that yourself.

It does run yarn to figure out how to cache Yarn files, and Yarn v1 is preinstalled on the GitHub runners, and Yarn v1 throws if packageManager refers to a non-1.x release. So you're right about that. But... setup-node should only try to run yarn if you pass the cache option to setup-node. I left that option off the first setup-node call (as you'd done) and I re-ran setup-node with caching after installing Corepack. I don't see how that could cause issues, but do you still see a case in which this could fail?

As for your final point:

Could potentially cause issues with any dependencies that rely on the specific package manager version in their installation step

Does Yarn consult packageManager for dependencies? I wasn't aware that it did that. I thought it only consulted it for the root package.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does Yarn consult packageManager for dependencies? I wasn't aware that it did that. I thought it only consulted it for the root package.

It does a few somewhat unexpected things, yes.. Among them: yarnpkg/berry#6258

with:
node-version: ${{ matrix.node-version }}
node-version-file: '.nvmrc'
- name: Install Yarn
run: corepack enable
- uses: actions/checkout@v4
- name: Use Node.js ${{ matrix.node-version }}
- name: Install Node.js ${{ matrix.node-version }} and restore Yarn cache
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}
cache: 'yarn'
- run: rm yarn.lock && YARN_ENABLE_IMMUTABLE_INSTALLS=false yarn
- name: Install dependencies via Yarn
run: rm yarn.lock && YARN_ENABLE_IMMUTABLE_INSTALLS=false yarn
- run: yarn test
- name: Restore lockfile
run: git restore yarn.lock
- name: Require clean working directory
shell: bash
run: |
git restore yarn.lock
if ! git diff --exit-code; then
echo "Working tree dirty at end of job"
exit 1
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/create-release-pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ jobs:
# We check out the specified branch, which will be used as the base
# branch for all git operations and the release PR.
ref: ${{ github.event.inputs.base-branch }}
- name: Setup Node.js
- name: Install Node.js
uses: actions/setup-node@v4
with:
node-version-file: '.nvmrc'
- uses: MetaMask/action-create-release-pr@v3
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
release-type: ${{ github.event.inputs.release-type }}
release-version: ${{ github.event.inputs.release-version }}
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
13 changes: 9 additions & 4 deletions .github/workflows/publish-docs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,19 @@ jobs:
- name: Ensure `destination_dir` is not empty
if: ${{ inputs.destination_dir == '' }}
run: exit 1
- name: Checkout the repository
uses: actions/checkout@v4
- name: Use Node.js
- uses: actions/checkout@v4
- name: Install Corepack via Node
uses: actions/setup-node@v4
with:
node-version-file: '.nvmrc'
- name: Install Yarn
run: corepack enable
- name: Restore Yarn cache
uses: actions/setup-node@v4
with:
node-version-file: '.nvmrc'
cache: 'yarn'
- name: Install npm dependencies
- name: Install dependencies via Yarn
run: yarn --immutable
- name: Run build script
run: yarn build:docs
Expand Down
52 changes: 38 additions & 14 deletions .github/workflows/publish-release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,34 +19,48 @@ jobs:
- uses: actions/checkout@v4
with:
ref: ${{ github.sha }}
- name: Setup Node.js
- name: Install Corepack via Node
uses: actions/setup-node@v4
with:
node-version-file: '.nvmrc'
- name: Install Yarn
run: corepack enable
- name: Restore Yarn cache
uses: actions/setup-node@v4
with:
node-version-file: '.nvmrc'
cache: 'yarn'
- uses: MetaMask/action-publish-release@v3
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- name: Install
run: |
yarn install
yarn build
- uses: actions/cache@v3
id: restore-build
with:
path: |
./dist
./node_modules/.yarn-state.yml
key: ${{ github.sha }}
- run: yarn --immutable
- run: yarn build

publish-npm-dry-run:
runs-on: ubuntu-latest
needs: publish-release
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.sha }}
- name: Install Corepack via Node
uses: actions/setup-node@v4
with:
node-version-file: '.nvmrc'
- name: Install Yarn
run: corepack enable
- name: Restore Yarn cache
uses: actions/setup-node@v4
with:
node-version-file: '.nvmrc'
cache: 'yarn'
- uses: actions/cache@v3
id: restore-build
with:
path: |
./dist
Expand All @@ -62,15 +76,25 @@ jobs:
SKIP_PREPACK: true

publish-npm:
environment: npm-publish
runs-on: ubuntu-latest
needs: publish-npm-dry-run
runs-on: ubuntu-latest
environment: npm-publish
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.sha }}
- name: Install Corepack via Node
uses: actions/setup-node@v4
with:
node-version-file: '.nvmrc'
- name: Install Yarn
run: corepack enable
- name: Restore Yarn cache
uses: actions/setup-node@v4
with:
node-version-file: '.nvmrc'
cache: 'yarn'
- uses: actions/cache@v3
id: restore-build
with:
path: |
./dist
Expand All @@ -86,8 +110,8 @@ jobs:
SKIP_PREPACK: true

get-release-version:
runs-on: ubuntu-latest
needs: publish-npm
runs-on: ubuntu-latest
outputs:
RELEASE_VERSION: ${{ steps.get-release-version.outputs.RELEASE_VERSION }}
steps:
Expand All @@ -99,8 +123,8 @@ jobs:
run: ./scripts/get.sh ".version" "RELEASE_VERSION"

publish-release-to-gh-pages:
needs: get-release-version
name: Publish docs to `${{ needs.get-release-version.outputs.RELEASE_VERSION }}` directory of `gh-pages` branch
needs: get-release-version
permissions:
contents: write
uses: ./.github/workflows/publish-docs.yml
Expand All @@ -110,8 +134,8 @@ jobs:
PUBLISH_DOCS_TOKEN: ${{ secrets.PUBLISH_DOCS_TOKEN }}

publish-release-to-latest-gh-pages:
needs: publish-npm
name: Publish docs to `latest` directory of `gh-pages` branch
needs: publish-npm
permissions:
contents: write
uses: ./.github/workflows/publish-docs.yml
Expand Down