Change all node formulas to use Homebrew-installed node rather than the first node on the path

[ctaintor]

This is in response to the discussion on updating the documentation for Node for Formula Authors. @MikeMcQuaid suggested opening a discussion here.

To summarize - most node formulas in homebrew-core add a binary by symlinking, likely because this is how the current documentation suggests it be done. However, using a symlink will mean that the first node binary on your PATH will be used and this may not be the one which was installed by the formula via depends_on "node". This is problematic if the binary installed by the formula requires newer versions of node or if the architecture of the Homebrew-node differs from that which is installed on the PATH (e.g. arm64 vs x86, which is what made me realize this problem).

To fix this, it is suggested that all formulas which include require "language/node" and have a depends_on "node" be changed, removing something like bin.install_symlink Dir["#{libexec}/bin/*"] and instead doing something like:

env = { PATH: "#{HOMEBREW_PREFIX/"bin"}:$PATH" }
(bin/"foo").write_env_script "#{libexec}/bin/foo", env

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)?

Consistently run a CLI which was installed by a homebrew node formula.

What happened (include all command output)?

In my case, a user was getting an error which was related to npm not having installed an x86 version of a dependency. The reason they were getting this error is because they had an x86 version of node on their PATH while the Homebrew formula had installed and used an arm64 version of node, thus meaning that npm had only installed the arm64 version of a dependency.

What did you expect to happen?

I expected that the formula would use the Homebrew-installed node and not fail due to something the user had installed elsewhere.

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

* install any of the homebrew node formulas
* add a binary named `node` to be in your PATH before Homebrew's path
* note that when you call the homebrew formula's CLI, it will try to execute the binary you added, not homebrew's node

[MikeMcQuaid]

@Homebrew/core thoughts here? Seems fairly sensible to me but I may be missing something.

[Rylan12]

env = { PATH: "#{HOMEBREW_PREFIX/"bin"}:$PATH" }
(bin/"foo").write_env_script "#{libexec}/bin/foo", env

If this pattern is used enough, it feels like we should have a simpler way to specify it (I don't think this pattern is necessarily unique to node formulae...). Maybe have a function that returns the env hash?:

(bin/"foo").write_env_script "#{libexec}/bin/foo", homebrew_bin_env

(not a fan of homebrew_bin_env as the name)

[cho-m]

If this pattern is used enough, it feels like we should have a simpler way to specify it (I don't think this pattern is necessarily unique to node formulae...).

I don't think that pattern is commonly used. Couldn't find any usage for node formulae.

More common are prepending opt_bin, e.g.

homebrew-core/Formula/b/bcoin.rb

Line 36 in 0fdd17a

(bin/"bcoin").write_env_script libexec/"bin/bcoin", PATH: "#{node.opt_bin}:$PATH"

homebrew-core/Formula/m/mongosh.rb

Line 24 in 0fdd17a

(bin/"mongosh").write_env_script libexec/"bin/mongosh", PATH: "#{Formula["node"].opt_bin}:$PATH"

Or shebang modification

homebrew-core/Formula/a/apify-cli.rb

Line 32 in 0fdd17a

rewrite_shebang detected_node_shebang, libexec/"lib/node_modules/apify-cli/src/bin/run"

homebrew-core/Formula/c/cloudflare-wrangler2.rb

Line 28 in 741721d

rewrite_shebang detected_node_shebang, *Dir["#{libexec}/lib/node_modules/**/*"]

homebrew-core/Formula/l/lando-cli.rb

Lines 31 to 32 in 0fdd17a

	# We have to replace the shebang in the main executable from "/usr/bin/env node"
	rewrite_shebang detected_node_shebang, libexec/"lib/node_modules/@lando/cli/bin/lando"

---

I guess the original approach didn't require manually checking executable names as everything in libexec/bin/ would be linked. If new executables are added in a future release, then they would automatically be available.

EDIT: Also, brew create --node ... would usually result in a functioning formula without major modifications.

Though, something similar to Java formulae logic could be used instead that generates env scripts for everything in libexec/bin/.

    bin.install Dir["#{libexec}/bin/*"]
    bin.env_script_all_files libexec/"bin", Language::Java.overridable_java_home_env

---

EDIT: On general topic, I agree that it does make sense to use a specific dependency. This is in alignment with how we handle other languages, e.g.

• Python - default shebang of virtualenvs will resolve to dependency
• Ruby - default shebang of gem install uses path to dependency
• Perl - brew does shebang rewrites during keg cleaning
• Java - we use Language::Java.java_home_env or Language::Java.overridable_java_home_env (though latter does cause similar issues for some users)

[MikeMcQuaid]

Or shebang modification

We automatically rewrite shebangs for some stuff (Python, Perl, ?), right? Doing this for NodeJS (and perhaps Ruby, etc.) too would make more sense to me than requiring DSL changes in hundreds of formulae.

• Perl - brew does shebang rewrites during keg cleaning

Yeh, this.

• Python - default shebang of virtualenvs will resolve to dependency
• Ruby - default shebang of gem install uses path to dependency

Ok, yeh, so it's handled by upstream in these cases. Feels like if npm doesn't do this for us: we should do it (and the Perl approach makes most sense to me).

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[SenseiMarv]

Hi @MikeMcQuaid, unfortunately this change broke setups that relied on version managers (like https://github.com/nvm-sh/nvm). I installed pnpm through Brew, which has a dependency on Node, so that was also installed through Brew. Different projects in my company rely on different versions of Node, all of which are defined by .nvmrc files. This way, nvm automatically ensures that the correct node version is used for each project. This no longer works after this change, as the nvm version, which is also the one set in the PATH, is now overridden by Brew's version.
The overriding behaviour was also confusing to me, as it's not obvious why pnpm would use a different Node version than what the shell reports:

Is there a way to disable this so that I'm not forced to stop using Brew to manage my pnpm installation or have to use --build-from-source?

There are also others who have been confused by this behaviour and thought it was pnpm's fault. See: pnpm/pnpm#8384

[jstcki]

I second the comment by @SenseiMarv. Suddenly pnpm is broken on systems where node versions are managed with fnm too.

[ctaintor]

I think the fix should be that the shebang rewrite only happens if node is a required dependency. For the examples above (and also for yarn formula), node is not required - but is necessary to build and/or test.

Or have a way for the formula to opt out of the rewrite - but I'd expect that to only be the case in situations where node is not a required dependency

[carlocab]

Or have a way for the formula to opt out of the rewrite - but I'd expect that to only be the case in situations where node is not a required dependency

This seems reasonable to me, and can already be handled in specific formulae. See pnpm.rb for an example. (If that isn't working for some reason, that may be a brew bug.)

[jstcki]

@ctaintor I don't understand the intricacies of what is rewritten and when. I only can say that this change broke every setup where node versions are managed through nvm/fnm or similar, so I believe it should be reverted asap. yarn, pnpm, you name it.

[MikeMcQuaid]

I think the fix should be that the shebang rewrite only happens if node is a required dependency. For the examples above (and also for yarn formula), node is not required - but is necessary to build and/or test.

I agree: this is the correct fix here.

This seems reasonable to me, and can already be handled in specific formulae. See pnpm.rb for an example. (If that isn't working for some reason, that may be a brew bug.)

@carlocab I'm a bit 😢 that that skip_clean is present without a comment. I'd love to see that removed unless needed.

so I believe it should be reverted asap

We're going to fix forward instead of reverting.

[Bo98]

@carlocab I'm a bit 😢 that that skip_clean is present without a comment. I'd love to see that removed unless needed

Yeah this isn't good. Cleaner is not supposed to have false positives so if we're going to start adding skip_clean everywhere then I don't think the cleaner is the correct place for this logic to be. My approval on the PR was conditional on that.

I note that pnpm does not have a runtime dependency on Node, so that's perhaps where the bug is - it definitely should not be adjusting the shebang in that scenario.

[MikeMcQuaid]

We're going to fix forward instead of reverting.

Have opened Homebrew/brew#17993 for this.

[carlocab]

@carlocab I'm a bit 😢 that that skip_clean is present without a comment.

Same here.

I'd love to see that removed unless needed.

#180632