node: 22.9.1 (updated install instructions) #194041
Conversation
updated the install mechanisms to respect the will of the Node.js project and remove behaviours that are unexpected towards end-users
github-actions
bot
added
long build
github-actions
bot
added
the
automerge-skip
| # We track major/minor from upstream Node releases. | ||
| # We will accept *important* npm patch releases when necessary. | ||
| resource "npm" do | ||
| url "https://registry.npmjs.org/npm/-/npm-10.8.3.tgz" | ||
| sha256 "b7dc7eb48d7479b93668e913c7ad686ab2aa71c705d4a56b5323d1bffdba2972" | ||
| end |
There was a problem hiding this comment.
This is the same version as what's included with Node 22.9.0: nodejs/node@61047dd130
There was a problem hiding this comment.
So that's what # We track major/minor from upstream Node releases. refers to?
Is there any technical reason of why then omitting the default way Node will attempt to bundle NPM?
There was a problem hiding this comment.
If there any reasons at all, whnich IMO are valid if given to OS or compatibility reasons -- can a warn/info be added?
Like "Homebrew will manually bundle the targeted npm version for this Node.js version -- read more at XXX.XYZ"
Giving knowledge to the end-user is always fundamental IMO.
There was a problem hiding this comment.
So that's what
# We track major/minor from upstream Node releases.refers to?
Yes. It means that the major and minor version should be the same, and ideally the patch version is the same unless there are very strong reasons not for it to be (see line just below that).
Is there any technical reason of why then omitting the default way Node will attempt to bundle NPM?
Not really sure -- it's been done this way since Homebrew/legacy-homebrew#28075, which is well before I was involved in Homebrew.
But, given that it's been done this way for nearly a decade (or more?), and it seems to be working well since then: we'd better have very strong reasons to change it.
If there any reasons at all, whnich IMO are valid if given to OS or compatibility reasons -- can a warn/info be added?
Like "Homebrew will manually bundle the targeted npm version for this Node.js version -- read more at XXX.XYZ"
We can add it to caveats whenever the version doesn't match from the one upstream, but I don't know of any instance when it didn't.
|
Thanks, @carlocab for the prompty review :) -- apologies if I'm breaking any contributor guideline here, tried to glance over it; And there are a lot of unknowns from my side, so let me know if there's anything I can do here to adhere to Homebrew's contribution guidelines 🙏 |
| bash_completion.install bootstrap/"lib/utils/completion.sh" => "npm" | ||
| end | ||
|
|
||
| def post_install |
There was a problem hiding this comment.
Note that removing this will likely mean that user-installed Node modules go into the 🗑️ when they brew upgrade node.
There was a problem hiding this comment.
I had no idea, and this was not intentional. What chunks of the post_install cover that?
| # We track major/minor from upstream Node releases. | ||
| # We will accept *important* npm patch releases when necessary. | ||
| resource "npm" do | ||
| url "https://registry.npmjs.org/npm/-/npm-10.8.3.tgz" | ||
| sha256 "b7dc7eb48d7479b93668e913c7ad686ab2aa71c705d4a56b5323d1bffdba2972" | ||
| end |
There was a problem hiding this comment.
So that's what
# We track major/minor from upstream Node releases.refers to?
Yes. It means that the major and minor version should be the same, and ideally the patch version is the same unless there are very strong reasons not for it to be (see line just below that).
Is there any technical reason of why then omitting the default way Node will attempt to bundle NPM?
Not really sure -- it's been done this way since Homebrew/legacy-homebrew#28075, which is well before I was involved in Homebrew.
But, given that it's been done this way for nearly a decade (or more?), and it seems to be working well since then: we'd better have very strong reasons to change it.
If there any reasons at all, whnich IMO are valid if given to OS or compatibility reasons -- can a warn/info be added?
Like "Homebrew will manually bundle the targeted npm version for this Node.js version -- read more at XXX.XYZ"
We can add it to caveats whenever the version doesn't match from the one upstream, but I don't know of any instance when it didn't.
I'm definitely fine keeping it (not that my purview is any indication or that Homebrew needs my approval) -- but I would really like if Homebrew attempted to stick with the official method of bundling npm. This is just to remove extra steps and things that could (or could not) conflict with Node in the future. If an issue existed behind this, I would actually be interested in finding a solution within |
|
Thanks for contributing to Homebrew! 🎉 It looks like you're having trouble with a CI failure. See our contribution guide for help. You may be most interested in the section on dealing with CI failures. You can find the CI logs in the Checks tab of your pull request. |
|
I'll close this for the time being, as I believe the core contributors are discussing a path forward and I believe they are better suited for making this change if necessary 🙏 |
HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where<formula>is the name of the formula you're submitting?brew test <formula>, where<formula>is the name of the formula you're submitting?brew audit --strict <formula>(after doingHOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it passbrew audit --new <formula>?Note: I haven't actually tested if builds pass yet, but I am going to do so another time, I just wanted to first weather-check what Homebrew maintainers feel about this change.
Relates to: #193982 (comment)