From 1771bf1bbb4e52bb4e24c779adfae004402a4989 Mon Sep 17 00:00:00 2001 From: Branch Vincent Date: Thu, 25 Jul 2024 21:00:10 -0700 Subject: [PATCH 1/2] rubocops/lines: audit `std_npm_args` usage --- Library/Homebrew/rubocops/lines.rb | 37 ++++++++ .../test/rubocops/text/std_npm_args_spec.rb | 87 +++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 Library/Homebrew/test/rubocops/text/std_npm_args_spec.rb diff --git a/Library/Homebrew/rubocops/lines.rb b/Library/Homebrew/rubocops/lines.rb index 7932991e3a75a..2c1ef8103c605 100644 --- a/Library/Homebrew/rubocops/lines.rb +++ b/Library/Homebrew/rubocops/lines.rb @@ -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 diff --git a/Library/Homebrew/test/rubocops/text/std_npm_args_spec.rb b/Library/Homebrew/test/rubocops/text/std_npm_args_spec.rb new file mode 100644 index 0000000000000..c0bb9e97edaf5 --- /dev/null +++ b/Library/Homebrew/test/rubocops/text/std_npm_args_spec.rb @@ -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 From 96c9ea14857b9382411c8a078499696ceafaa87d Mon Sep 17 00:00:00 2001 From: Branch Vincent Date: Thu, 25 Jul 2024 22:24:09 -0700 Subject: [PATCH 2/2] docs: update with `std_npm_args` --- Library/Homebrew/formula_creator.rb | 6 +---- docs/Node-for-Formula-Authors.md | 34 ++++++++++------------------- 2 files changed, 13 insertions(+), 27 deletions(-) diff --git a/Library/Homebrew/formula_creator.rb b/Library/Homebrew/formula_creator.rb index 8f362a82f9776..c9fd8e01841aa 100644 --- a/Library/Homebrew/formula_creator.rb +++ b/Library/Homebrew/formula_creator.rb @@ -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 @@ -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" diff --git a/docs/Node-for-Formula-Authors.md b/docs/Node-for-Formula-Authors.md index 9a709135d9a75..99ad8b1e2d764 100644 --- a/docs/Node-for-Formula-Authors.md +++ b/docs/Node-for-Formula-Authors.md @@ -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). @@ -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 @@ -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: @@ -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). @@ -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" @@ -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