search

Homebrew / homebrew-core

🍻 Default formulae for the missing package manager for macOS (or Linux)
https://brew.sh
BSD 2-Clause "Simplified" License
13.77k stars 12.45k forks source link

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

Closed karlhorky closed 2 weeks ago

karlhorky commented 1 month ago

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

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 commented 1 month ago

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. https://github.com/Homebrew/homebrew-core/blob/33b4fac1a0ff77debe83de6f692fcc0c9ed7d6cf/Formula/n/node.rb#L70
  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. https://github.com/Homebrew/homebrew-core/issues/151439#issuecomment-1767121649
    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. https://github.com/Homebrew/homebrew-core/pull/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 commented 1 month ago

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 commented 1 month ago

I've made a PR here: https://github.com/Homebrew/homebrew-core/pull/194041 but no idea yet if it passes, if the changes are correct; I'll test on my MacBook later :)

carlocab commented 1 month ago

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:

https://github.com/Homebrew/homebrew-core/blob/4a0d38c9015445ed0121e2c45541db037f1463be/Formula/n/node.rb#L49-L50

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

ovflowd commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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:

carlocab commented 1 month ago

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 commented 1 month ago

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:

https://github.com/Homebrew/homebrew-core/blob/1e6e3ab490841d333c6a2a9e34487e80bf9f20dd/Formula/c/corepack.rb#L20-L21

ovflowd commented 1 month ago

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 commented 1 month ago

npm was actually done in response to upstream reported issues: https://github.com/npm/npm/issues/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 commented 1 month ago

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 (https://github.com/nodejs/Release/issues/1034)

Bo98 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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, https://github.com/nodejs/node/issues/55181

aduh95 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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.

p-linnane commented 3 weeks ago

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 commented 3 weeks ago

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 commented 2 weeks ago

Now we have corepack bundled with node formula.

chenrui333 commented 2 weeks ago

should be good now, closing the issue.

ovflowd commented 2 weeks ago

🎉

privatenumber commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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.

andreineculau commented 1 week ago

following up on @carlocab 's comment about complications for users of yarn/pnpm formulae what would be the preferred resolution there because this change made the node formula incompatible with yarn/pnpm formulae

yarn/pnpm formulae don't depend on the node formula (beyond build and test phases), but they do require node to be installed, which most of us, and especially because of the bundled notice, would install via brew install node but node and yarn/pnpm formulae cannot coexist right now. Ping @zkochan

andreineculau commented 1 week ago

If I may add a personal 2c on this

Following the thread, I do not understand what made the homebrew maintainers go along with this, as the net benefit from a user perspective is simply skipping "brew install corepack" (IF you actually need corepack features), while actually losing the ability to decide how to make yarn/pnpm available - via corepack or via specific homebrew formulae.

p-linnane commented 1 week ago

@andreineculau Thank you for this additional info. We definitely did not intend to cause issues with yarn/pnpm. We are discussing internally and will have a path forward soon.

aduh95 commented 1 week ago

FWIW on nixpkgs, there's a nodejs package and a nodejs-slim package, maybe it's something that would also benefit Homebrew.

ovflowd commented 1 week ago

Following the thread, I do not understand what made the homebrew maintainers go along with this

It was somewhat a request from upstream to "conform" with the expectations Node has for distributing Node. (Note that we defer the decisions of how Homebrew distributes Node to Homebrew, but at least speaking for myself, It is a desirable expectation for the formulae to be as close as possible of upstream)

Corepack might or might not be removed in the future; I like the idea of a node-slim formulae, but not sure how much of a hassle it would be for Homebrew maintainers.

I also can't see how corepack being installed (it is not enabled by default) would cause any issues for the yarn/pnpm formulae's 🤨 -- this was the behaviour until Node.js v20 on Homebrew, so either the issue existed before or is it a new one, or maybe particular to your system? 👀

ovflowd commented 1 week ago

Also my 2cents, I don't think it is productive to argue what Node is supposed to be or not on a thread on Homebrew core, if you believe Node is doing too much, you're more than welcome to provide feedback upstream. (Apologies if I am overreaching here)

Bo98 commented 1 week ago

this change made the node formula incompatible with yarn/pnpm formulae

Can you clarify, with steps to reproduce, what broke here? The original corepack formula already had a dependency on node so the install footprint before and after should be similar (except the yarn/pnpm shims are now opt-in via corepack enable).

The footprint of the original corepack formula was:

/opt/homebrew/
-> bin/
   -> corepack
   -> pnpm
   -> pnpx
   -> yarn
   -> yarnpkg

(everything else was hidden in /opt/homebrew/opt/corepack/libexec as internal details)

plus everything installed by node as it was a required runtime dependency.

Now the footprint is:

/opt/homebrew/
-> bin/
   -> corepack
-> lib/
   -> node_modules/
      -> corepack/
         -> (contents of corepack package)

plus everything installed by node as before. pnpm/pnpx/yarn/yarnpkg can be returned to their original location via corepack enable.

One difference I do see is the original corepack formula was constructed like:

#!/opt/homebrew/opt/node/bin/node
process.env.COREPACK_ENABLE_DOWNLOAD_PROMPT??='0';
require('./lib/corepack.cjs').runMain(process.argv.slice(2));

but the newer one has #!/usr/bin/env node. Is that the issue you're seeing? We can probably fix that if so.

andreineculau commented 1 week ago

Bo, I have been collecting root-cause data since the last comment, and can nuance the "this change made the node formula incompatible with yarn/pnpm formulae":

Overall one can consider these edge/corner cases, but ultimately corepack's shim functionality (with no guards) coupled with poor practices (assumptions about the environment) make the pure availability of corepack a problem in itself.

One could say that this (not removing corepack from the node formula, and pushing for better practices) is the right thing long term, but it could have benefitted from more consideration about implications and gains (homebrew users skipping brew install corepack if needed).

Off-topic: the shebang discovery probably makes sense to enforce running with homebrew's node

Bo98 commented 1 week ago

in other scenarios, pnpm is installed via older/custom formulae which specify conflicts_with corepack

In terms of older formulae: this is something we can never realistically support. If you're mixing and matching packages from different points in time there's a very high chance things will break. Node dependents that don't link to anything native have it a bit easier - it would be significantly worse if you mixed things like a dylib that breaks ABI often.

In terms of custom formulae, conflicts_with in general does suck. The mistake here was probably ever shipping corepack with the shims pre-linked. We actively try to reduce conflicts_with rather than keep them.

We kept the brew install corepack alias as a consideration to break less people - i.e. so CI scripts that do it do not break. How it also affects conflicts_with is unfortunate, though not one I would've anticipated would have affected many. I can't find any public tap on GitHub that does it that's not a homebrew-core fork or got "test" in its name: https://github.com/search?q=%22conflicts_with+%5C%22corepack%5C%22%22+NOT+is%3Afork+&type=code&p=1, but acknowledge there could be some low-volume ones (or private ones) that haven't been indexed.

The other two scenarios make more sense and do sympathise with scripts breaking as we do try to avoid that. I do have one question about them though: how do they handle Linux distros? Homebrew's permission model is more lax but if I did corepack enable on Ubuntu 24.10 I get:

$ sudo apt install node-corepack
$ corepack enable
Internal Error: EACCES: permission denied, symlink '../share/nodejs/corepack/dist/pnpm.js' -> '/usr/bin/pnpm'

It would perhaps make some sense if Homebrew was able to similarly block corepack enable without an explicit --install-directory (or default it to something else you need to add to your PATH) so that it's not destructive by default.

Unfortunately, I'm not sure if a perfect solution exists without potentially some changes upstream but we'll try our best to try cover the majority.