diff --git a/Library/Homebrew/language/node.rb b/Library/Homebrew/language/node.rb index 01d41041be..eaadc54fc7 100644 --- a/Library/Homebrew/language/node.rb +++ b/Library/Homebrew/language/node.rb @@ -1,7 +1,7 @@ module Language module Node def self.npm_cache_config - "cache=#{HOMEBREW_CACHE}/npm_cache\n" + "cache=#{HOMEBREW_CACHE}/npm_cache" end def self.pack_for_installation @@ -10,19 +10,18 @@ module Language # fed to `npm install` only symlinks are created linking back to that # directory, consequently breaking that assumption. We require a tarball # because npm install creates a "real" installation when fed a tarball. - output = Utils.popen_read("npm pack").chomp - raise "npm failed to pack #{Dir.pwd}" unless $CHILD_STATUS.exitstatus.zero? - output + pack_cmd = "npm pack --ignore-scripts" + output = Utils.popen_read(pack_cmd) + if !$CHILD_STATUS.exitstatus.zero? || output.lines.empty? + raise "npm failed to pack #{Dir.pwd}" + end + output.lines.last.chomp end def self.setup_npm_environment - npmrc = Pathname.new("#{ENV["HOME"]}/.npmrc") - # only run setup_npm_environment once per formula - return if npmrc.exist? - # explicitly set npm's cache path to HOMEBREW_CACHE/npm_cache to fix - # issues caused by overriding $HOME (long build times, high disk usage) - # https://github.com/Homebrew/brew/pull/37#issuecomment-208840366 - npmrc.write npm_cache_config + # guard that this is only run once + return if @env_set + @env_set = true # explicitly use our npm and node-gyp executables instead of the user # managed ones in HOMEBREW_PREFIX/lib/node_modules which might be broken begin @@ -42,8 +41,10 @@ module Language # npm install args for global style module format installed into libexec %W[ - --verbose + -ddd --global + --build-from-source + --#{npm_cache_config} --prefix=#{libexec} #{Dir.pwd}/#{pack} ] @@ -52,7 +53,11 @@ module Language def self.local_npm_install_args setup_npm_environment # npm install args for local style module format - ["--verbose"] + %W[ + -ddd + --build-from-source + --#{npm_cache_config} + ] end end end diff --git a/Library/Homebrew/test/language/node_spec.rb b/Library/Homebrew/test/language/node_spec.rb index b2114bc4fd..5ddbde9446 100644 --- a/Library/Homebrew/test/language/node_spec.rb +++ b/Library/Homebrew/test/language/node_spec.rb @@ -2,45 +2,56 @@ require "language/node" describe Language::Node do describe "#setup_npm_environment" do - it "does nothing when npmrc exists" do - expect(subject.setup_npm_environment).to be_nil - end - - it "calls prepend_path when node formula exists and npmrc does not exist" do + it "calls prepend_path when node formula exists only during the first call" do node = formula "node" do url "node-test" end stub_formula_loader(node) - allow_any_instance_of(Pathname).to receive(:exist?).and_return(false) expect(ENV).to receive(:prepend_path) - subject.setup_npm_environment + subject.instance_variable_set(:@env_set, false) + expect(subject.setup_npm_environment).to be_nil + + expect(subject.instance_variable_get(:@env_set)).to eq(true) + expect(ENV).not_to receive(:prepend_path) + expect(subject.setup_npm_environment).to be_nil end - it "does not call prepend_path when node formula does not exist but npmrc exists" do - allow_any_instance_of(Pathname).to receive(:exist?).and_return(false) + it "does not call prepend_path when node formula does not exist" do expect(subject.setup_npm_environment).to be_nil end end describe "#std_npm_install_args" do npm_install_arg = "libexec" + npm_pack_cmd = "npm pack --ignore-scripts" it "raises error with non zero exitstatus" do + allow(Utils).to receive(:popen_read).with(npm_pack_cmd).and_return("error msg") + allow_any_instance_of(Process::Status).to receive(:exitstatus).and_return(42) + allow_any_instance_of(nil::NilClass).to receive(:exitstatus).and_return(42) + expect { subject.std_npm_install_args(npm_install_arg) }.to \ + raise_error("npm failed to pack #{Dir.pwd}") + end + + it "raises error with empty npm pack output" do + allow(Utils).to receive(:popen_read).with(npm_pack_cmd).and_return("") + allow_any_instance_of(Process::Status).to receive(:exitstatus).and_return(0) + allow_any_instance_of(nil::NilClass).to receive(:exitstatus).and_return(0) expect { subject.std_npm_install_args(npm_install_arg) }.to \ raise_error("npm failed to pack #{Dir.pwd}") end it "does not raise error with a zero exitstatus" do - allow(Utils).to receive(:popen_read).with("npm pack").and_return("pack") + allow(Utils).to receive(:popen_read).with(npm_pack_cmd).and_return("pack.tgz") allow_any_instance_of(Process::Status).to receive(:exitstatus).and_return(0) allow_any_instance_of(nil::NilClass).to receive(:exitstatus).and_return(0) resp = subject.std_npm_install_args(npm_install_arg) - expect(resp).to include("--prefix=#{npm_install_arg}", "#{Dir.pwd}/pack") + expect(resp).to include("--prefix=#{npm_install_arg}", "#{Dir.pwd}/pack.tgz") end end specify "#local_npm_install_args" do resp = subject.local_npm_install_args - expect(resp).to include("--verbose") + expect(resp).to include("-ddd", "--build-from-source", "--cache=#{HOMEBREW_CACHE}/npm_cache") end end diff --git a/docs/Node-for-Formula-Authors.md b/docs/Node-for-Formula-Authors.md index b740367537..a3d0624f2d 100644 --- a/docs/Node-for-Formula-Authors.md +++ b/docs/Node-for-Formula-Authors.md @@ -30,7 +30,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 must vendor this older Node version as done in the [`kibana` formula](https://github.com/Homebrew/homebrew-core/blob/c6202f91a129e2f994d904f299a308cc6fbd58e5/Formula/kibana.rb). +If your formula requires being executed with an older Node version you should use one of the versioned node formulae (e.g. `node@6`). ### Special requirements for native addons @@ -47,8 +47,8 @@ Also note that such a formula would only be compatible with the same Node major Node modules should be installed to `libexec`. This prevents the Node modules from contaminating the global `node_modules`, which is important so that npm doesn't try to manage Homebrew-installed Node modules. In the following we distinguish between two types of Node modules 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 [`azure-cli`](https://github.com/Homebrew/homebrew-core/blob/d93fe9ba3bcc9071b699c8da4e7d733518d3337e/Formula/azure-cli.rb) or [`autocode`](https://github.com/Homebrew/homebrew-core/blob/1a670a6269e1e07f86683c2d164977c9bd8a3fb6/Formula/autocode.rb)) and -* formulae where the `npm install` step is only one of multiple not exclusively Node related install steps (not compatible with npm's global module format) which have to use [`local_npm_install_args`](#installing-module-dependencies-locally-with-local_npm_install_args) (like [`elixirscript`](https://github.com/Homebrew/homebrew-core/blob/ec1e40d37e81af63122a354f0101c377f6a4e66d/Formula/elixirscript.rb) or [`kibana`](https://github.com/Homebrew/homebrew-core/blob/c6202f91a129e2f994d904f299a308cc6fbd58e5/Formula/kibana.rb)) +* 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 [`azure-cli`](https://github.com/Homebrew/homebrew-core/blob/0f3b27d252b8112c744e0460d871cfe1def6b993/Formula/azure-cli.rb) or [`webpack`](https://github.com/Homebrew/homebrew-core/blob/6282879973d569962e63da7c81ac4623e1a8336b/Formula/webpack.rb)) and +* formulae where the `npm install` call is not the only required install step (e.g. need to compile non-JavaScript sources too) have to use [`local_npm_install_args`](#installing-module-dependencies-locally-with-local_npm_install_args) (like [`elixirscript`](https://github.com/Homebrew/homebrew-core/blob/4bb491b7b246830aed57b97348a17e9401374978/Formula/elixirscript.rb) Both methods have in common 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. @@ -72,6 +72,8 @@ This will install your Node module in npm's global module style with a custom pr bin.install_symlink Dir["#{libexec}/bin/*"] ``` +**Note:** Because of a required workaround for `npm@5` calling `npm pack` we currently don't support installing modules (from non-npm registry tarballs), which require a prepublish step (e.g. for transpiling sources). See [Homebrew/brew#2820](https://github.com/Homebrew/brew/pull/2820) for more information. + ### Installing module dependencies locally with `local_npm_install_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` with `Language::Node.local_npm_install_args` to invoke `npm install` like: