Skip to content

Commit

Permalink
Merge pull request #17867 from branchvincent/std_npm_args
Browse files Browse the repository at this point in the history
  • Loading branch information
MikeMcQuaid committed Aug 5, 2024
2 parents 78132b1 + 96c9ea1 commit 1e56012
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 27 deletions.
6 changes: 1 addition & 5 deletions Library/Homebrew/formula_creator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,6 @@ def template
# Documentation: https://docs.brew.sh/Formula-Cookbook
# https://rubydoc.brew.sh/Formula
# PLEASE REMOVE ALL GENERATED COMMENTS BEFORE SUBMITTING YOUR PULL REQUEST!
<% if @mode == :node %>
require "language/node"
<% end %>
class #{Formulary.class_s(name)} < Formula
<% if @mode == :python %>
include Language::Python::Virtualenv
Expand Down Expand Up @@ -179,7 +175,7 @@ def install
system "meson", "compile", "-C", "build", "--verbose"
system "meson", "install", "-C", "build"
<% elsif @mode == :node %>
system "npm", "install", *Language::Node.std_npm_install_args(libexec)
system "npm", "install", *std_npm_args
bin.install_symlink Dir["\#{libexec}/bin/*"]
<% elsif @mode == :perl %>
ENV.prepend_create_path "PERL5LIB", libexec/"lib/perl5"
Expand Down
37 changes: 37 additions & 0 deletions Library/Homebrew/rubocops/lines.rb
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,43 @@ def audit_formula(formula_nodes)
end
end

# This cop makes sure that formulae use `std_npm_args` instead of older
# `local_npm_install_args` and `std_npm_install_args`.
class StdNpmArgs < FormulaCop
extend AutoCorrector

sig { override.params(formula_nodes: FormulaNodes).void }
def audit_formula(formula_nodes)
return if (body_node = formula_nodes.body_node).nil?

find_method_with_args(body_node, :local_npm_install_args) do
problem "Use 'std_npm_args' instead of '#{@offensive_node.method_name}'." do |corrector|
corrector.replace(@offensive_node.source_range, "std_npm_args(prefix: false)")
end
end

find_method_with_args(body_node, :std_npm_install_args) do |method|
problem "Use 'std_npm_args' instead of '#{@offensive_node.method_name}'." do |corrector|
if (param = parameters(method).first.source) == "libexec"
corrector.replace(@offensive_node.source_range, "std_npm_args")
else
corrector.replace(@offensive_node.source_range, "std_npm_args(prefix: #{param})")
end
end
end

find_every_method_call_by_name(body_node, :system).each do |method|
first_param, second_param = parameters(method)
next if !node_equals?(first_param, "npm") ||
!node_equals?(second_param, "install") ||
method.source.match(/(std_npm_args|local_npm_install_args|std_npm_install_args)/)

offending_node(method)
problem "Use `std_npm_args` for npm install"
end
end
end

# This cop makes sure that formulae depend on `openssl` instead of `quictls`.
class QuicTLSCheck < FormulaCop
extend AutoCorrector
Expand Down
87 changes: 87 additions & 0 deletions Library/Homebrew/test/rubocops/text/std_npm_args_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# frozen_string_literal: true

require "rubocops/lines"

RSpec.describe RuboCop::Cop::FormulaAudit::StdNpmArgs do
subject(:cop) { described_class.new }

context "when auditing node formulae" do
it "reports an offense when `npm install` is called without std_npm_args arguments" do
expect_offense(<<~RUBY)
class Foo < Formula
def install
system "npm", "install"
^^^^^^^^^^^^^^^^^^^^^^^ FormulaAudit/StdNpmArgs: Use `std_npm_args` for npm install
end
end
RUBY
end

it "reports and corrects an offense when using local_npm_install_args" do
expect_offense(<<~RUBY)
class Foo < Formula
def install
system "npm", "install", *Language::Node.local_npm_install_args, "--production"
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FormulaAudit/StdNpmArgs: Use 'std_npm_args' instead of 'local_npm_install_args'.
end
end
RUBY

expect_correction(<<~RUBY)
class Foo < Formula
def install
system "npm", "install", *std_npm_args(prefix: false), "--production"
end
end
RUBY
end

it "reports and corrects an offense when using std_npm_install_args with libexec" do
expect_offense(<<~RUBY)
class Foo < Formula
def install
system "npm", "install", *Language::Node.std_npm_install_args(libexec), "--production"
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FormulaAudit/StdNpmArgs: Use 'std_npm_args' instead of 'std_npm_install_args'.
end
end
RUBY

expect_correction(<<~RUBY)
class Foo < Formula
def install
system "npm", "install", *std_npm_args, "--production"
end
end
RUBY
end

it "reports and corrects an offense when using std_npm_install_args without libexec" do
expect_offense(<<~RUBY)
class Foo < Formula
def install
system "npm", "install", *Language::Node.std_npm_install_args(buildpath), "--production"
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FormulaAudit/StdNpmArgs: Use 'std_npm_args' instead of 'std_npm_install_args'.
end
end
RUBY

expect_correction(<<~RUBY)
class Foo < Formula
def install
system "npm", "install", *std_npm_args(prefix: buildpath), "--production"
end
end
RUBY
end

it "does not report an offense when using std_npm_args" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
def install
system "npm", "install", *std_npm_args
end
end
RUBY
end
end
end
34 changes: 12 additions & 22 deletions docs/Node-for-Formula-Authors.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@ This document explains how to successfully use Node and npm in a Node module bas

## Running `npm install`

Homebrew provides two helper methods in a `Language::Node` module: `std_npm_install_args` and `local_npm_install_args`. They both set up the correct environment for npm and return arguments for `npm install` for their specific use cases. Your formula should use these instead of invoking `npm install` explicitly. The syntax for a standard Node module installation is:
Homebrew provides a helper method `std_npm_args` to set up the correct environment for npm and return arguments for `npm install`. Your formula should use this when invoking `npm install`. The syntax for a standard Node module installation is:

```ruby
system "npm", "install", *Language::Node.std_npm_install_args(libexec)
system "npm", "install", *std_npm_args
```

where `libexec` is the destination prefix (usually the `libexec` variable).

## Download URL

If the Node module is also available on the npm registry, we prefer npm-hosted release tarballs over GitHub (or elsewhere) hosted source tarballs. The advantages of these tarballs are that they don't include the files from the `.npmignore` (such as tests) resulting in a smaller download size and that any possible transpilation step is already done (e.g. no need to compile CoffeeScript files as a build step).
Expand All @@ -30,7 +28,7 @@ Node modules which are compatible with the latest Node version should declare a
depends_on "node"
```

If your formula requires being executed with an older Node version you should use one of its versioned formulae (e.g. `node@12`).
If your formula requires being executed with an older Node version you should use one of its versioned formulae (e.g. `node@20`).

### Special requirements for native addons

Expand All @@ -48,23 +46,17 @@ Node modules should be installed to `libexec`. This prevents the Node modules fr

In the following we distinguish between two types of Node modules installed using formulae:

* formulae for standard Node modules compatible with npm's global module format which should use [`std_npm_install_args`](#installing-global-style-modules-with-std_npm_install_args-to-libexec) (like [`apollo-cli`](https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/a/apollo-cli.rb) or [`webpack`](https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/w/webpack.rb))
* formulae where the `npm install` call is not the only required install step (e.g. need to also compile non-JavaScript sources) which have to use [`local_npm_install_args`](#installing-module-dependencies-locally-with-local_npm_install_args) (like [`emscripten`](https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/e/emscripten.rb) or [`grunt-cli`](https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/g/grunt-cli.rb))
- formulae for standard Node modules compatible with npm's global module format which should use [`std_npm_args`](#installing-global-style-modules-with-std_npm_args-to-libexec) (like [`angular-cli`](https://github.com/Homebrew/homebrew-core/blob/master/Formula/a/angular-cli.rb) or [`webpack`](https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/w/webpack.rb))
- formulae where the `npm install` call is not the only required install step (e.g. need to also compile non-JavaScript sources) which have to use [`std_npm_args`](#installing-module-dependencies-locally-with-std_npm_args) (like [`emscripten`](https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/e/emscripten.rb) or [`grunt-cli`](https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/g/grunt-cli.rb))

What both methods have in common is that they are setting the correct environment for using npm inside Homebrew and are returning the arguments for invoking `npm install` for their specific use cases. This includes fixing an important edge case with the npm cache (caused by Homebrew's redirection of `HOME` during the build and test process) by using our own custom `npm_cache` inside `HOMEBREW_CACHE`, which would otherwise result in very long build times and high disk space usage.

To use them you have to require the Node language module at the beginning of your formula file with:

```ruby
require "language/node"
```

### Installing global style modules with `std_npm_install_args` to `libexec`
### Installing global style modules with `std_npm_args` to `libexec`

In your formula's `install` method, simply `cd` to the top level of your Node module if necessary and then use `system` to invoke `npm install` with `Language::Node.std_npm_install_args` like:
In your formula's `install` method, simply `cd` to the top level of your Node module if necessary and then use `system` to invoke `npm install` with `std_npm_args` like:

```ruby
system "npm", "install", *Language::Node.std_npm_install_args(libexec)
system "npm", "install", *std_npm_args
```

This will install your Node module in npm's global module style with a custom prefix to `libexec`. All your modules' executables will be automatically resolved by npm into `libexec/bin` for you, which are not symlinked into Homebrew's prefix. To make sure these are installed, we need to symlink all executables to `bin` with:
Expand All @@ -73,12 +65,12 @@ This will install your Node module in npm's global module style with a custom pr
bin.install_symlink Dir["#{libexec}/bin/*"]
```

### Installing module dependencies locally with `local_npm_install_args`
### Installing module dependencies locally with `std_npm_args`

In your formula's `install` method, do any installation steps which need to be done before the `npm install` step and then `cd` to the top level of the included Node module. Then, use `system` to invoke `npm install` with `Language::Node.local_npm_install_args` like:
In your formula's `install` method, do any installation steps which need to be done before the `npm install` step and then `cd` to the top level of the included Node module. Then, use `system` to invoke `npm install` with `std_npm_args(prefix: false)` like:

```ruby
system "npm", "install", *Language::Node.local_npm_install_args
system "npm", "install", *std_npm_args(prefix: false)
```

This will install all of your Node modules' dependencies to your local build path. You can now continue with your build steps and handle the installation into the Homebrew prefix on your own, following the [general Homebrew formula instructions](Formula-Cookbook.md).
Expand All @@ -88,8 +80,6 @@ This will install all of your Node modules' dependencies to your local build pat
Installing a standard Node module based formula would look like this:

```ruby
require "language/node"

class Foo < Formula
desc "An example formula"
homepage "https://example.com"
Expand All @@ -101,7 +91,7 @@ class Foo < Formula
# depends_on "python" => :build

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

Expand Down

0 comments on commit 1e56012

Please sign in to comment.