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

corepack installed with node@20 but not with node@22 #193982

Closed
4 tasks done
karlhorky opened this issue Oct 12, 2024 · 28 comments
Closed
4 tasks done

corepack installed with node@20 but not with node@22 #193982

karlhorky opened this issue Oct 12, 2024 · 28 comments
Labels
bug Reproducible Homebrew/homebrew-core bug

Comments

@karlhorky
Copy link
Contributor

karlhorky commented Oct 12, 2024

brew gist-logs <formula> link OR brew config AND brew doctor output

$ brew config
HOMEBREW_VERSION: 4.4.0-127-g3291ad4
ORIGIN: https://github.com/Homebrew/brew
HEAD: 3291ad4fc78f33f8e9bb7ce37745d2a0e697f5fb
Last commit: 14 hours ago
Core tap HEAD: 4465032b218818d8f645e17e2cf45435964bd854
Core tap last commit: 4 months ago
Core tap JSON: 12 Oct 14:02 UTC
Core cask tap HEAD: ff7c82f562e07a6613213e0daad7d36bc9af9137
Core cask tap last commit: 4 months ago
Core cask tap JSON: 12 Oct 14:02 UTC
HOMEBREW_PREFIX: /opt/homebrew
HOMEBREW_CASK_OPTS: []
HOMEBREW_MAKE_JOBS: 8
HOMEBREW_SORBET_RUNTIME: set
Homebrew Ruby: 3.3.5 => /opt/homebrew/Library/Homebrew/vendor/portable-ruby/3.3.5/bin/ruby
CPU: octa-core 64-bit arm_firestorm_icestorm
Clang: 16.0.0 build 1600
Git: 2.39.2 => /opt/homebrew/bin/git
Curl: 8.7.1 => /usr/bin/curl
macOS: 15.0.1-arm64
CLT: 16.0.0.0.1.1724870825
Xcode: 16.0
Rosetta 2: false
$ brew doctor
Your system is ready to brew.

Verification

  • My brew doctor output says Your system is ready to brew. and am still able to reproduce my issue.
  • I ran brew update and am still able to reproduce my issue.
  • I have resolved all warnings from brew doctor and that did not fix my problem.
  • I searched for recent similar issues at https://github.com/Homebrew/homebrew-core/issues?q=is%3Aissue and found no duplicates.

What were you trying to do (and why)?

I was trying to use Homebrew to install Node.js v22 and use Corepack, in the same way that I installed Node.js v20 and used Corepack

What happened (include all command output)?

Following the official Node.js download page, I used Homebrew to install node@22, and then tried to enable the corepack binary as in the Node.js docs and it is not available:

$ brew install node@22

...

$ corepack enable
zsh: command not found: corepack

This is divergent behavior from node@20 (and other versions), where installation yields a working, installed Corepack out of the box:

$ brew install node@20

...

$ brew link node@20

...

$ corepack enable
$ # Completed successfully

What did you expect to happen?

node@22 should install the Corepack binary like node@20 does (Corepack is expected to be a part of Node.js)

Step-by-step reproduction instructions (by running brew commands)

Already included above
@karlhorky karlhorky added the bug Reproducible Homebrew/homebrew-core bug label Oct 12, 2024
@karlhorky
Copy link
Contributor Author

Further Background

After doing a bit of research in the Homebrew repository, I can see the following:

  1. This seems to be caused by the --without-corepack flag in the node.rb Formula
    1. --without-corepack
  2. Homebrew team member @cho-m recommends usage of Homebrew to install Corepack separately with brew install corepack (which would mean the version number is managed by Homebrew). An exception is made for old versions such as node@20, node@18, etc, where the --without-corepack is not present
    1. corepack not installed with formula node #151439 (comment)
    2. This means that the Homebrew behavior will be different for different versions:
      1. node@22: corepack not installed
      2. node@20 and older: corepack installed
  3. The --without-corepack flag was first added in a PR for Node.js 17.3.0, involved Homebrew members @chenrui333 @carlocab @SMillerDev @bevanjkay
    1. node 17.3.0 #91540
    2. The original reason was "Tests are failing at the moment because of a symlink collision with corepack.", so seems like a Homebrew CI problem

I originally reported this over in the Node.js repository:

And received a response that this should be reported here.

@ovflowd
Copy link

ovflowd commented Oct 12, 2024

Hey Homebrew maintainers (@carlocab @SMillerDev @bevanjkay @chenrui333 @cho-m), I'm Claudio, one of the core collaborators of Node.js, I'm chiming in here as I've never noticed that Homebrew intentionally removed core features from Node.js (https://github.com/Homebrew/homebrew-core/blob/master/Formula/n/node.rb#L69-L70) on its formulae.

This is not a sanctioned way that we (Node.js) approve of people distributing Node.js. This might cause many expectation issues and hinder how we expect community package managers to ship Node.

So, I'd like to ask Homebrew maintainers to remove those flags formally. Node.js users expect all installation methods bundled via Node.js version managers, OS package managers, or community package managers to install Node similarly.

We cannot endorse Homebrew as a way of installing Node.js if Homebrew intentionally changes (without the user's explicit consent and knowledge) what gets shipped when they install Node. (Note that this is not a threat, although it might sound; We just want to ensure that the official ways we recommend on our website all have the same behaviour)

Even worse, Homebrew is deliberately changing which npm version gets installed with the Node.js formula, which should always contain the npm version that comes with the respective Node.js binary. Otherwise, it is incorrect to advertise to the end-user that a specific version of Node.js is being installed when certain components are being modified. This also violates our LICENSE, as you are distributing Node.js in a way that sounds and looks like it is binary but changes the result. On a separate note, I can understand why these args are being passed, but anything else is definitely, let's say "not cool".

I'd certainly appreciate it if this behavior is undone.

Edit: My side assumed that different versions are shipped, which is incorrect. However, Homebrew is still discouraged from manually bundling npmin an extraneous way.

Edit 2: It is important to mention that my statements are not a representation of Node.js as a whole but as someone who is deeply involved with its infrastructure and development.

@ovflowd
Copy link

ovflowd commented Oct 12, 2024

I've made a PR here: #194041 but no idea yet if it passes, if the changes are correct; I'll test on my MacBook later :)

@carlocab
Copy link
Member

Even worse, Homebrew is deliberately changing which npm version gets installed with the Node.js formula

That's not actually accurate -- we always make sure that the npm version matches what's shipped with Node:

# We track major/minor from upstream Node releases.
# We will accept *important* npm patch releases when necessary.

If there's been a mismatch historically then that was an oversight.

@ovflowd
Copy link

ovflowd commented Oct 12, 2024

That's not actually accurate -- we always make sure that the npm version matches what's shipped with Node:

Appreciate for chiming in. I raised a question within my PR OOC to better understand Homebrew's deliberate decision here; Apologies for my lack of context here 🙇

@Bo98
Copy link
Member

Bo98 commented Oct 12, 2024

This also violates our LICENSE, as you are distributing Node.js in a way that sounds and looks like it is binary but changes the result.

This is quite an alarming sentence to deliver to send to downstreams and implies NodeJS has a very strong stance against https://opensource.org/osd.


Focussing on the pull request in isolation: I'm ok with making changes. The corepack decision came at a time where corepack was still experimental several years ago (and the corepack build flags didn't actually exist yet). I recognise things have changed - it's simply been that nobody has raised the question since, perhaps in part because the % of users that install corepack is very low.

Node 23 is in a couple days and is an excellent opportunity to make any big changes along with a fresh node@22.

npm was actually done in response to upstream reported issues: npm/npm#3794. The default npm build within Node didn't work properly under Homebrew so we needed to build it separately for it to work. It's possible things have improved since - I'll need to figure out what the test case for that is as 10 years is a long time. Note that we have for years guaranteed that we will always match upstream versions (unless there's some critical bugfix but that hasn't been the case for many years). I do however agree that using the bundled version is nicer if it works better nowadays.

But I'm overall OK for removing --without-corepack (but only if we delete the separate corepack formula too and merge it in).

@cho-m
Copy link
Member

cho-m commented Oct 12, 2024

Node.js users expect all installation methods bundled via Node.js version managers, OS package managers, or community package managers to install Node the same way.

Based on quick check through Repology, this statement doesn't seem to hold up:

Footnotes

  1. https://salsa.debian.org/js-team/nodejs/-/blob/master-20.x/debian/rules#L26-27

  2. https://src.fedoraproject.org/rpms/nodejs22/blob/rawhide/f/nodejs22.spec#_591

  3. https://gitlab.alpinelinux.org/alpine/aports/-/blob/master/community/nodejs-current/APKBUILD#L133

  4. https://gitlab.archlinux.org/archlinux/packaging/packages/nodejs/-/blob/main/PKGBUILD?ref_type=heads#L53

  5. https://github.com/macports/macports-ports/blob/master/devel/nodejs22/Portfile#L102

  6. https://ports.macports.org/port/nodejs22/details/

@carlocab
Copy link
Member

carlocab commented Oct 12, 2024

This also violates our LICENSE, as you are distributing Node.js in a way that sounds and looks like it is binary but changes the result.

This is quite an alarming sentence to deliver to send to downstreams and implies NodeJS has a very strong stance against opensource.org/osd.

Yes, agreed. I'll also note that a number of packages mentioned in upstream documentation do some pretty similar things as we do:

Are any/all of these in violation of Node's license?

Edit: Got scooped by @cho-m. Didn't see the previous comment till I submitted mine 😄

@carlocab
Copy link
Member

But I'm overall OK for removing --without-corepack (but only if we delete the separate corepack formula too and merge it in).

Might be tricky for some dependents to both depend on node but conflict with node:

conflicts_with "yarn", because: "both install `yarn` and `yarnpkg` binaries"
conflicts_with "pnpm", because: "both install `pnpm` and `pnpx` binaries"

@ovflowd
Copy link

ovflowd commented Oct 12, 2024

This is quite an alarming sentence to deliver to send to downstreams and implies NodeJS has a very strong stance against opensource.org/osd.

Apologies, this is indeed an unwarranted statement from my side. As per https://github.com/nodejs/node/blob/main/LICENSE, I don't think any license is actually being breached.

The corepack decision came at a time where corepack was still experimental several years ago (and the corepack build flags didn't actually exist yet). I recognise things have changed - it's simply been that nobody has raised the question since, perhaps in part because the % of users that install corepack is very low.

I understand that, but there are active conversations within the Node team about whether corepack should be removed or not; removing it from Homebrew install paths at this time is not recommended.

Based on quick check through Repology, this statement doesn't seem to hold up:

I doubt that the we are aware that this was the case. We don't have control over what package managers decide to do or not with Node, hence I'm reaching out here. I appreciate you for compiling this list, I'm actually shocked that this is the case, and I'm not sure if the Node.js release team is aware of this -- I'll reach them out and check-in.

@ovflowd
Copy link

ovflowd commented Oct 12, 2024

npm was actually done in response to upstream reported issues: npm/npm#3794. The default npm build within Node didn't work properly under Homebrew so we needed to build it separately for it to work.

I imagined something like this was the case 🤔

I'll need to figure out what the test case for that is as 10 years is a long time.

I don't want to waste your volunteering time unnecessarily. I would appreciate it if you could check that, but only if that's a reasonable ask from my side.

@ovflowd
Copy link

ovflowd commented Oct 12, 2024

Node 23 is in a couple days and is an excellent opportunity to make any big changes along with a fresh node@22.

Node 23 got delayed a bit, I can check back what is the updated timeline, the irony is that we're facing issues specifically with macOS (nodejs/Release#1034)

@Bo98
Copy link
Member

Bo98 commented Oct 12, 2024

Might be tricky for some dependents to both depend on node but conflict with node:

I think the corepack build in Node skips shipping yarn etc binaries so should be OK I think. Should of course double check before shipping to ensure only the corepack binary is installed.

@ovflowd
Copy link

ovflowd commented Oct 12, 2024

Apologies, this is indeed an unwarranted statement from my side. As per nodejs/node@main/LICENSE, I don't think any license is actually being breached.

Apologies again for the unnecessary and inaccurate statement (regarding LICENSE violations) -- to clarify, my intent here was to refer as the official binaries of Node.js and untethered modifications on them; But this is not applicable (nor actually part of the LICENSE of Node) here as Node is built from source by Homebrew.

@ovflowd
Copy link

ovflowd commented Oct 12, 2024

Node 23 is in a couple days and is an excellent opportunity to make any big changes along with a fresh node@22.

Node 23 got delayed a bit, I can check back what is the updated timeline, the irony is that we're facing issues specifically with macOS (nodejs/Release#1034)

I linked the wrong issue, nodejs/node#55181

@aduh95
Copy link

aduh95 commented Oct 12, 2024

This is not a sanctioned way that we (Node.js) approve of people distributing Node.js.

AFAIK this is not correct, we (Node.js) maintain the ./configure flags that enable and disable certain features, precisely so folks can build a version of Node.js that fit their use case. I also don't think we need to approve distributing Node.js, it's an open-source software.
The part on user expectations still holds though, but I think we can only complain as Homebrew users, not as Node.js maintainers.

I doubt that the we are aware that this was the case. We don't have control over what package managers decide to do or not with Node, hence I'm reaching out here. I appreciate you for compiling this list, I'm actually shocked that this is the case, and I'm not sure if the Node.js release team is aware of this -- I'll reach them out and check-in.

We are certainly aware, AFAICT it's always been a quite transparent process.

@ovflowd
Copy link

ovflowd commented Oct 12, 2024

The part on user expectations still holds though, but I think we can only complain as Homebrew users, not as Node.js maintainers.

Indeed, concur here.

AFAIK this is not correct, we (Node.js) maintain the ./configure flags that enable and disable certain features, precisely so folks can build a version of Node.js that fit their use case. I also don't think we need to approve distributing Node.js, it's an open-source software.

I partially agree here, but I cannot speak as for Node as a whole, and your statements (as a TSC member) will supersede mine.

(Referring to the part of community package managers changing what is bundled or not with default "nodejs" distributions -- I would understand if this was a bottle of brew install node --without-corepack (and same for Debian, and others) -- but that is just my expectation)

We are certainly aware, AFAICT it's always been a quite transparent process.

Then this is possibly a me issue. Was there any documentation regarding which OS managers/package managers are deviating from the default "ship" (?)

Suppose this is a well-known knowledge (which I unfortunately was unaware of). In that case, we do need to update the Node.js website to reflect that these package managers diverge from other installation methods. Which, in my opinion, is still a breakage of consistency.

@ovflowd
Copy link

ovflowd commented Oct 12, 2024

Since @aduh95 has chimed in, I'll defer his opinions as what would be a relay of Node.js will here; If he believes this is completely fine for Homebrew to do, then I believe Homebrew will not need to remove the --without-corepack flag, and I'll proceed with proposing changes internally on our website so that these installation methods have explicit mentions that they might not come with these features.

Having that said, it is hard for me (as someone contributing to Node's infra and website) to keep track of what other managers are doing or not and I could only assume and wish that they'd distribute Node as close as possible from pristine.

@aduh95
Copy link

aduh95 commented Oct 12, 2024

My opinion (and does not necessarily represent the TSC's, or any other group I'm a part of, and is obviously not legal advice) is that Homebrew is in no "obligation" to follow the same conventions as the upstream project when it comes to distribution.
As I said above, IMO it's certainly fine for Homebrew users to request changes, and IMO it's perfectly fine to have Homebrew maintainers being the ultimate judges.

As a user, I agree that aligning with Node.js defaults seems to be the least likely to cause confusion.

I'm certainly not qualified to decide Node.js will, and everyone is welcome to disagree with my position.

@ovflowd
Copy link

ovflowd commented Oct 12, 2024

My opinion (and does not necessarily represent the TSC's, or any other group I'm a part of, and is obviously not legal advice) is that Homebrew is in no "obligation" to follow the same conventions as the upstream project when it comes to distribution.

As I said above, IMO it's certainly fine for Homebrew users to request changes, and IMO it's perfectly fine to have Homebrew maintainers being the ultimate judges.

As a user, I agree that aligning with Node.js defaults seems to be the least likely to cause confusion.

I'm certainly not qualified to decide Node.js will, and everyone is welcome to disagree with my position.

I appreciate your words here, couldn't have spoken better, in full agreement here.

@branchvincent branchvincent mentioned this issue Oct 16, 2024
1 task
@p-linnane
Copy link
Member

Hello all. I'm a member of Homebrew's Project Leadership Committee. You've also heard from @Bo98 who is on our TSC. We try to ship binaries as close to upstream as possible, as long as it makes sense for us to do so. As other maintainers have mentioned, we are operating in the same way we have for a decade or longer. We're happy to make formula changes if it helps our users, but we will not make sudden changes that may disrupt large amounts of users. We're happy to discuss with Node's maintainers if there is upstream concern, otherwise we will continue to package and ship as we always have.

@ovflowd Claims of OSS license violations are something we take very seriously. Please ensure your facts are in order before opening an issue and making claims are incorrect and alarmist.

@aduh95 Thank you for chiming in from the Node side. We will discuss internally on how we should handle corepack for Node 23.

@ovflowd
Copy link

ovflowd commented Oct 21, 2024

We're happy to make formula changes if it helps our users, but we will not make sudden changes that may disrupt large amounts of users. We're happy to discuss with Node's maintainers if there is upstream concern, otherwise we will continue to package and ship as we always have.

There are concerns that we would like to have it shipped as close upstream as possible, which is not precisely the case right now. But at the same time, you decide to decide what to ship.

@ovflowd Claims of OSS license violations are something we take very seriously. Please ensure your facts are in order before opening an issue and making claims are incorrect and alarmist.

I already apologised for my unnecessary escalation here; it was wrong, excessive, and mostly done as an initially rushed and concerned act from my side. I was shocked that Homebrew was doing something differently, failing to realise this was the status quo. Still, it was factually incorrect and wrong; apologies for that. I have a terrible habit of jumping, and sometimes, I forget to think for 2 seconds about whether I should write something or not.

@chenrui333
Copy link
Member

Now we have corepack bundled with node formula.

@chenrui333
Copy link
Member

should be good now, closing the issue.

@ovflowd
Copy link

ovflowd commented Oct 28, 2024

🎉

@privatenumber
Copy link

privatenumber commented Oct 31, 2024

I'm a bit confused by this change.

As I understand, there are plans for corepack to be separated from the Node release in favor of a standalone installation (https://github.com/nodejs/package-maintenance/pull/606/files). When that happens, will the corepack formula be reintroduced?

I've been installing corepack via Homebrew in anticipation of this change, but now it's been removed. For context, I preferred using Homebrew over npm for installing corepack because I switch between multiple Node/npm versions with nvm.

I wonder if it could've been possible to keep the corepack formula as-is for standalone installation?

@p-linnane
Copy link
Member

We're opting for consistency among our formulae right now. When Node makes whatever changes they plan to make, we will adapt accordingly. They may involve bringing the corepack formula back, but we will address that when necessary.

@privatenumber
Copy link

I agree that including corepack with node for consistency makes sense.

But Corepack also supports independent installation, so being consistent with that option seems important as well: https://github.com/nodejs/corepack#manual-installs

Could we consider reintroducing the standalone corepack formula as an option for these users? This seems like a flexible solution that upholds consistency while accommodating broader use cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Reproducible Homebrew/homebrew-core bug
Projects
None yet
Development

No branches or pull requests

9 participants