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.

Sign up for GitHub

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

Jump to bottom

docs: updates the node formula example to use the node installed by Homebrew #17539

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+13 −1
Open

docs: updates the node formula example to use the node installed by Homebrew #17539

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion docs/Node-for-Formula-Authors.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,19 @@ class Foo < Formula

def install
system "npm", "install", *Language::Node.std_npm_install_args(libexec)
bin.install_symlink Dir["#{libexec}/bin/*"]

# "link" the executable `foo`, ensuring that the version of node installed by Homebrew is used
# (the created `foo` will set the ENV before exec'ing your executable)
env = { PATH: "#{HOMEBREW_PREFIX/"bin"}:$PATH" }
(bin/"foo").write_env_script "#{libexec}/bin/foo", env

# Uncomment if you simply want to symlink the executable – note that this means the first
# `node` on the PATH will be used (not necessarily the one Homebrew installed)
# bin.install_symlink Dir["#{libexec}/bin/*"]
Comment on lines +106 to +113
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which/how many formulae do this each of these ways? CC @homebrew/core for thoughts.

Copy link
Author

@ctaintor ctaintor Jun 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally don't know when you'd ever want it this way – it's just how it was documented previously. At the very least, if the formula has a depends_on to node then I'd expect it to always use the write_env_script to force the usage of the Homebrew-installed node.

yarn will use the first node on the path but I'd imagine this is an outlier (and it is also not calling bin.install_symlink – but it is not trying to always use homebrew's node)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the 173 formulae with require "language/node":

5 use write_env_script, although only 3 use it in the exact way suggested here:

  • protoc-gen-grpc-web
    • This doesn't actually use system "npm", "install", *Language::Node.std_npm_install_args(libexec) so maybe it shouldn't count
  • mongosh
  • code-server
  • bcoin
  • emscripten
    • This is another case where the formula is much more complex than the sample here os it probably shouldn't be included

On the other hand, there are 155 formulae that use bin.install_symlink and 145 of those use bin.install_symlink Dir[libexec/"foo"]. So, the original way this is written seems to be by far the most prevalent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Rylan12!

On the other hand, there are 155 formulae that use bin.install_symlink and 145 of those use bin.install_symlink Dir[libexec/"foo"]. So, the original way this is written seems to be by far the most prevalent.

Given the prevalence here: I think this may be worth opening a Homebrew/core issue to discuss if we want to migrate the approach here. I don't think it makes sense to have the documentation recommend the opposite of what most formulae are doing.

Copy link
Author

@ctaintor ctaintor Jul 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened it here – Homebrew/homebrew-core#176257 – apologies if that was not the way to do it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's perfect, thanks!


# Uncomment if you want to write the completion scripts for bash, fish, and zsh (assuming
# your executable has a command "completion" which returns a completion script)
# generate_completions_from_executable(libexec/"bin/foo", "completion", base_name: 'foo')
Comment on lines +114 to +117
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How many formulae does this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure but personally when I was writing a formula I didn't know that this was an option (adding completions) and I also did not know that the generate_completions_from_executable function exists. I kept these comments here to help people know that adding completions is easy.

(In my case, my CLI already supported completions – so it was as simple as knowing that it's possible and easy to have Homebrew set it up)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like 387 formulae use generate_completions_from_executable in total.

However, out of the 173 formulae that have require "language/node", only 10 use generate_completions_from_executable.

Of those 10, only cdktf uses the generate_completions_from_executable(libexec/"bin/foo") scheme described here, the other 9 use generate_completions_from_executable(bin/"foo").

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of those 10, only cdktf uses the generate_completions_from_executable(libexec/"bin/foo") scheme described here, the other 9 use generate_completions_from_executable(bin/"foo")

Note that when you use write_env_script then you must use libexec here (using bin will result in a runtime error)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this is used by <10% of Node formulae: I'm not sure it's worth documenting specifically here.

end

test do
Expand Down