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

Enable Corepack #480

Closed
shellscape opened this issue May 2, 2022 · 20 comments
Closed

Enable Corepack #480

shellscape opened this issue May 2, 2022 · 20 comments
Labels
feature request New feature or request to improve the current logic

Comments

@shellscape
Copy link

Description:

Related to this issue and comment #182 (comment), I'd like to see support for enabling corepack without having to have a separate run step to do so.

Justification:

This would reduce overall runtime of actions, as no separate steps are required to enable corepack nor to install package managers, which is a net benefit to users in both regards.

Are you willing to submit a PR?

Surely

@shellscape shellscape added feature request New feature or request to improve the current logic needs triage labels May 2, 2022
@e-korolevskii
Copy link
Contributor

Hi @shellscape,
We will investigate the possibility of adding this feature.

@e-korolevskii
Copy link
Contributor

Hi @shellscape,
Unfortunately, corepack is experimental now, so we would not implement that at least while its not become stable.
That is why I will close this issue now. If you have any questions or concerns, please feel free to leave a comment on this thread or open another issue.

@shellscape
Copy link
Author

Hm, I don't see anything codified in this repo that states experimental features aren't welcome. Seems that a experimental property would be a very good fit here. I think this was closed prematurely.

@styfle
Copy link

styfle commented May 4, 2022

A PR was already started in #482

Also, we should get clarity around the path from experimental to stable nodejs/corepack#104

@shellscape
Copy link
Author

@styfle ah I missed that, thanks for pointing it out. I'll continue discussion there.

@dmitry-shibanov
Copy link
Contributor

Hello @shellscape. I think the main reason to not enable corepack by default because some customers can use setup-node with cache enable in conjunction with pnpm/action-setup before setup-node. When corepack is enabled it will override preinstalled pnpm (from pnpm/action-setup).
For example users can specify use the latest version of pnpm (7th), but corepack can override it to 6th. This also applies to yarn.

@Ethan-Arrowood
Copy link

corepack will not override it to 6 unless that is what is set for packageManager

@dmitry-shibanov
Copy link
Contributor

Hello @Ethan-Arrowood. Thank you for your response. I've tried to use this snippet and it overrides pnpm version.

steps:
  - name: Checkout repository
    uses: actions/checkout@v3
  - uses: pnpm/action-setup@v2
    with:
      version: 7
  - run: pnpm --version
  - uses: actions/setup-node@v3
    with:
      node-version: 16.9.0
  - run: |
      corepack enable
      pnpm --version

@Ethan-Arrowood
Copy link

Is packageManager in your package.json set to pnpm@7.0.0?

@dmitry-shibanov
Copy link
Contributor

Is packageManager in your package.json set to pnpm@7.0.0?

No it does not. With packageManager it works as expected. But not all users have packageManager field, that is why it can break existing behaviour some users if corepack is enabled by default with caching.

@viceice
Copy link

viceice commented May 5, 2022

Hello @Ethan-Arrowood. Thank you for your response. I've tried to use this snippet and it overrides pnpm version.

steps:
  - name: Checkout repository
    uses: actions/checkout@v3
  - uses: pnpm/action-setup@v2
    with:
      version: 7
  - run: pnpm --version
  - uses: actions/setup-node@v3
    with:
      node-version: 16.9.0
  - run: |
      corepack enable
      pnpm --version

it makes no sense to setup pnpm before node. 🤔

@Ethan-Arrowood
Copy link

No it does not. With packageManager it works as expected. But not all users have packageManager field, that is why it can break existing behaviour some users if corepack is enabled by default with caching.

Ahh I see; then I can add a check for the "packageManager" property and only execute corepack when it exists. That should reconcile this issue I believe

@Ethan-Arrowood
Copy link

it makes no sense to setup pnpm before node. 🤔

This is necessary step in order to get caching to work. This action executes pnpm to get the path of the cache during the node set up action; if pnpm or yarn are not installed then the operation will fail. One could argue this action should start to support pnpm automatically like I believe it does with yarn

@JanVoracek
Copy link

What about enabling corepack explicitly using an input?

- uses: actions/setup-node@v3
  with:
    node-version: 16
    corepack-enable: true

@dmitry-shibanov
Copy link
Contributor

Hello @JanVoracek. Thank you for your suggestion. I think in this variant it won't reduce yaml file size because you also need only one line to enable it after the setup-node action.

@shellscape
Copy link
Author

For me it's less about line reduction and more about consolidation. Remember: different users value different metrics, it's a folly to assume users will value the same as one's self.

wincent added a commit to wincent/masochist that referenced this issue Aug 30, 2023
Haven't tried it yet, but I'd expect that we'd need this.

See:

- actions/setup-node#480
deining pushed a commit to deining/setup-node that referenced this issue Nov 9, 2023
Bumps [prettier](https://github.com/prettier/prettier) from 2.8.3 to 2.8.4.
- [Release notes](https://github.com/prettier/prettier/releases)
- [Changelog](https://github.com/prettier/prettier/blob/main/CHANGELOG.md)
- [Commits](prettier/prettier@2.8.3...2.8.4)

---
updated-dependencies:
- dependency-name: prettier
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@naXa777
Copy link

naXa777 commented Nov 21, 2023

is this change released?

Now for users who used a separate step to enable corepack, the github action is broken. Hence the declared goal of this change request is not met: "which is a net benefit to users in both regards".

sample yaml:

jobs:
  test:
    steps:
      - name: Checkout code
        uses: actions/checkout@v3

      - name: Setup Node.js
        uses: actions/setup-node@v3
        with:
          node-version: '18'
          cache: 'yarn'

      - name: Enable corepack
        run: |
          corepack enable
          corepack prepare yarn@3.5.1 --activate
          yarn set version 3.5.1

The step "Setup Node.js" fails with error:

error This project's package.json defines "packageManager": "yarn@3.3.1". However the current global version of Yarn is 1.22.21.

Presence of the "packageManager" field indicates that the project is meant to be used with Corepack, a tool included by default with all official Node.js distributions starting from 16.9 and 14.[19](https://github.com/Found-ee/react-ui/actions/runs/6940271054/job/18878934903#step:3:21).
Corepack must currently be enabled by running corepack enable in your terminal. For more information, check out https://yarnpkg.com/corepack.
Error: error This project's package.json defines "packageManager": "yarn@3.3.1". However the current global version of Yarn is 1.22.[21](https://github.com/Found-ee/react-ui/actions/runs/6940271054/job/18878934903#step:3:23).

Presence of the "packageManager" field indicates that the project is meant to be used with Corepack, a tool included by default with all official Node.js distributions starting from 16.9 and 14.19.
Corepack must currently be enabled by running corepack enable in your terminal. For more information, check out https://yarnpkg.com/corepack.

The step "Enable corepack" that follows is not executed, cause the action is already failed.

@darkbasic
Copy link

darkbasic commented Nov 21, 2023

Yeah I've noticed that as well. Initially it started happening randomly and re-triggering the CI fixed it, but now it happens consistently. The solution is to enable corepack before setup-node. I hope that one day setup-node will officially support corepack and such hacks won't be necessary anymore.

@BeroBurny
Copy link

like we had cache: 'yarn' in config

      - name: Set up Node.js
        uses: actions/setup-node@v3
        with:
          node-version: "18"
          cache: 'yarn'

after removing it solved the problem 🤷‍♂️
but found it after applied @darkbasic solution

gilbsgilbs added a commit to gilbsgilbs/babel-plugin-i18next-extract that referenced this issue Dec 30, 2023
CI was failing, probably because of a change in the setup-node action:
https://github.com/gilbsgilbs/babel-plugin-i18next-extract/actions/runs/7363053375/job/20042120587?pr=264

Enabling corepack before starting the action should fix the issue. See
actions/setup-node#480 (comment)
gilbsgilbs added a commit to gilbsgilbs/babel-plugin-i18next-extract that referenced this issue Dec 30, 2023
@dylants
Copy link

dylants commented Jan 29, 2024

Explicitly putting together what others have said, this worked for me:

name: CI

on:
  push:
    branches: [main]
  pull_request:
    branches: [main]

jobs:
  build:
    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v4
      - name: Enable Corepack
        run: corepack enable
      - name: Use Node.js 20.x
        uses: actions/setup-node@v4
        with:
          node-version: "20.x"
          cache: 'yarn'
      - name: Install dependencies
        run: yarn install --immutable
      - name: Run lint
        run: yarn lint
      - name: Run tests
        run: yarn test
      - name: Run build
        run: yarn build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request to improve the current logic
Projects
None yet
Development

No branches or pull requests