From d1297c09743baf11dfbedd47d2db154d4eecac37 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Sat, 12 Aug 2023 12:36:29 -0400 Subject: [PATCH 1/5] node: add shebang rewriting Formulae that depend on `node` sometimes contain files that use a shebang like `#!/usr/bin/env node` and this can lead to issues when the `node` in a user's environment isn't brewed `node`. For example, some node modules are compiled when the formula is built but if the user's `node` is a different major version than brew's `node`, the differing `NODE_MODULE_VERSION` can produce an error when certain parts of the application are used. The formula may build and test fine and the issue may only become apparent when more of the application is exercised. This adds a `Language::Node::Shebang` module (borrowing from the existing Perl and Python examples), which allows us to use `rewrite_shebang detected_node_shebang, ...` in formulae to address this type of issue. --- Library/Homebrew/language/node.rb | 30 ++++++++ Library/Homebrew/language/node.rbi | 9 +++ .../test/language/node/shebang_spec.rb | 71 +++++++++++++++++++ 3 files changed, 110 insertions(+) create mode 100644 Library/Homebrew/language/node.rbi create mode 100644 Library/Homebrew/test/language/node/shebang_spec.rb diff --git a/Library/Homebrew/language/node.rb b/Library/Homebrew/language/node.rb index b5bb25389b..5974e3bc25 100644 --- a/Library/Homebrew/language/node.rb +++ b/Library/Homebrew/language/node.rb @@ -85,5 +85,35 @@ module Language --#{npm_cache_config} ] end + + # Mixin module for {Formula} adding shebang rewrite features. + module Shebang + module_function + + # A regex to match potential shebang permutations. + NODE_SHEBANG_REGEX = %r{^#! ?/usr/bin/(?:env )?node( |$)}.freeze + + # The length of the longest shebang matching `SHEBANG_REGEX`. + NODE_SHEBANG_MAX_LENGTH = "#! /usr/bin/env node ".length + + # @private + sig { params(node_path: T.any(String, Pathname)).returns(Utils::Shebang::RewriteInfo) } + def node_shebang_rewrite_info(node_path) + Utils::Shebang::RewriteInfo.new( + NODE_SHEBANG_REGEX, + NODE_SHEBANG_MAX_LENGTH, + "#{node_path}\\1", + ) + end + + sig { params(formula: T.untyped).returns(Utils::Shebang::RewriteInfo) } + def detected_node_shebang(formula = self) + node_deps = formula.deps.map(&:name).grep(/^node(@.+)?$/) + raise ShebangDetectionError.new("Node", "formula does not depend on Node") if node_deps.empty? + raise ShebangDetectionError.new("Node", "formula has multiple Node dependencies") if node_deps.length > 1 + + node_shebang_rewrite_info(Formula[node_deps.first].opt_bin/"node") + end + end end end diff --git a/Library/Homebrew/language/node.rbi b/Library/Homebrew/language/node.rbi new file mode 100644 index 0000000000..15faf4a5c6 --- /dev/null +++ b/Library/Homebrew/language/node.rbi @@ -0,0 +1,9 @@ +# typed: strict + +module Language + module Node + module Shebang + include Kernel + end + end +end diff --git a/Library/Homebrew/test/language/node/shebang_spec.rb b/Library/Homebrew/test/language/node/shebang_spec.rb new file mode 100644 index 0000000000..e691f66976 --- /dev/null +++ b/Library/Homebrew/test/language/node/shebang_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require "language/node" +require "utils/shebang" + +describe Language::Node::Shebang do + let(:file) { Tempfile.new("node-shebang") } + let(:f) do + f = {} + + f[:node18] = formula "node@18" do + url "https://brew.sh/node-18.0.0.tgz" + end + + f[:versioned_node_dep] = formula "foo" do + url "https://brew.sh/foo-1.0.tgz" + + depends_on "node@18" + end + + f[:no_deps] = formula "foo" do + url "https://brew.sh/foo-1.0.tgz" + end + + f[:multiple_deps] = formula "foo" do + url "https://brew.sh/foo-1.0.tgz" + + depends_on "node" + depends_on "node@18" + end + + f + end + + before do + file.write <<~EOS + #!/usr/bin/env node + a + b + c + EOS + file.flush + end + + after { file.unlink } + + describe "#detected_node_shebang" do + it "can be used to replace Node shebangs" do + allow(Formulary).to receive(:factory) + allow(Formulary).to receive(:factory).with(f[:node18].name).and_return(f[:node18]) + Utils::Shebang.rewrite_shebang described_class.detected_node_shebang(f[:versioned_node_dep]), file.path + + expect(File.read(file)).to eq <<~EOS + #!#{HOMEBREW_PREFIX/"opt/node@18/bin/node"} + a + b + c + EOS + end + + it "errors if formula doesn't depend on node" do + expect { Utils::Shebang.rewrite_shebang described_class.detected_node_shebang(f[:no_deps]), file.path } + .to raise_error(ShebangDetectionError, "Cannot detect Node shebang: formula does not depend on Node.") + end + + it "errors if formula depends on more than one node" do + expect { Utils::Shebang.rewrite_shebang described_class.detected_node_shebang(f[:multiple_deps]), file.path } + .to raise_error(ShebangDetectionError, "Cannot detect Node shebang: formula has multiple Node dependencies.") + end + end +end From 370e61e5046659299f9702d84cdd0c919726386b Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Mon, 14 Aug 2023 20:22:47 -0400 Subject: [PATCH 2/5] perl: refactor to align Shebang modules This primarily reworks `Language::Perl::Shebang` to use constants for the shebang regex and max length (like the previous Node commit) and to extract the `RewriteInfo` call into a separate method (like Python and Node). Besides that, this also adds type signatures to the methods. --- Library/Homebrew/language/perl.rb | 37 ++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/Library/Homebrew/language/perl.rb b/Library/Homebrew/language/perl.rb index c9b1775cef..45829a7e8c 100644 --- a/Library/Homebrew/language/perl.rb +++ b/Library/Homebrew/language/perl.rb @@ -10,24 +10,35 @@ module Language module Shebang module_function - def detected_perl_shebang(formula = self) - perl_deps = formula.declared_deps.select { |dep| dep.name == "perl" } - perl_path = if perl_deps.present? - if perl_deps.any? { |dep| !dep.uses_from_macos? || !dep.use_macos_install? } - Formula["perl"].opt_bin/"perl" - else - "/usr/bin/perl#{MacOS.preferred_perl_version}" - end - else - raise ShebangDetectionError.new("Perl", "formula does not depend on Perl") - end + # A regex to match potential shebang permutations. + PERL_SHEBANG_REGEX = %r{^#! ?/usr/bin/(?:env )?perl( |$)}.freeze + # The length of the longest shebang matching `SHEBANG_REGEX`. + PERL_SHEBANG_MAX_LENGTH = "#! /usr/bin/env perl ".length + + # @private + sig { params(perl_path: T.any(String, Pathname)).returns(Utils::Shebang::RewriteInfo) } + def perl_shebang_rewrite_info(perl_path) Utils::Shebang::RewriteInfo.new( - %r{^#! ?/usr/bin/(?:env )?perl( |$)}, - 21, # the length of "#! /usr/bin/env perl " + PERL_SHEBANG_REGEX, + PERL_SHEBANG_MAX_LENGTH, "#{perl_path}\\1", ) end + + sig { params(formula: T.untyped).returns(Utils::Shebang::RewriteInfo) } + def detected_perl_shebang(formula = self) + perl_deps = formula.declared_deps.select { |dep| dep.name == "perl" } + raise ShebangDetectionError.new("Perl", "formula does not depend on Perl") if perl_deps.empty? + + perl_path = if perl_deps.any? { |dep| !dep.uses_from_macos? || !dep.use_macos_install? } + Formula["perl"].opt_bin/"perl" + else + "/usr/bin/perl#{MacOS.preferred_perl_version}" + end + + perl_shebang_rewrite_info(perl_path) + end end end end From 0b3e3b72707ba6c44f414ed7063bb3db9c98f556 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Mon, 14 Aug 2023 20:24:34 -0400 Subject: [PATCH 3/5] python: refactor to align Shebang modules This reworks `Language::Python::Shebang` to use constants for the shebang regex and max length (like the previous Node commit). Besides that, this also adds type signatures to the existing methods. --- Library/Homebrew/language/python.rb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/language/python.rb b/Library/Homebrew/language/python.rb index 32ea425122..1f6c76c187 100644 --- a/Library/Homebrew/language/python.rb +++ b/Library/Homebrew/language/python.rb @@ -94,21 +94,28 @@ module Language module Shebang module_function + # A regex to match potential shebang permutations. + PYTHON_SHEBANG_REGEX = %r{^#! ?/usr/bin/(?:env )?python(?:[23](?:\.\d{1,2})?)?( |$)}.freeze + + # The length of the longest shebang matching `SHEBANG_REGEX`. + PYTHON_SHEBANG_MAX_LENGTH = "#! /usr/bin/env pythonx.yyy ".length + # @private + sig { params(python_path: T.any(String, Pathname)).returns(Utils::Shebang::RewriteInfo) } def python_shebang_rewrite_info(python_path) Utils::Shebang::RewriteInfo.new( - %r{^#! ?/usr/bin/(?:env )?python(?:[23](?:\.\d{1,2})?)?( |$)}, - 28, # the length of "#! /usr/bin/env pythonx.yyy " + PYTHON_SHEBANG_REGEX, + PYTHON_SHEBANG_MAX_LENGTH, "#{python_path}\\1", ) end + sig { params(formula: T.untyped, use_python_from_path: T::Boolean).returns(Utils::Shebang::RewriteInfo) } def detected_python_shebang(formula = self, use_python_from_path: false) python_path = if use_python_from_path "/usr/bin/env python3" else python_deps = formula.deps.map(&:name).grep(/^python(@.*)?$/) - raise ShebangDetectionError.new("Python", "formula does not depend on Python") if python_deps.empty? if python_deps.length > 1 raise ShebangDetectionError.new("Python", "formula has multiple Python dependencies") From 87935f8d0f3bf7e9a20257b2b7cd2de52f24ac7d Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Mon, 14 Aug 2023 20:26:09 -0400 Subject: [PATCH 4/5] perl: expand and refactor tests This brings coverage of `Language::Perl::Shebang` to 100%. --- .../test/language/perl/shebang_spec.rb | 47 +++++++++++++++---- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/Library/Homebrew/test/language/perl/shebang_spec.rb b/Library/Homebrew/test/language/perl/shebang_spec.rb index a6dee15226..06ee219825 100644 --- a/Library/Homebrew/test/language/perl/shebang_spec.rb +++ b/Library/Homebrew/test/language/perl/shebang_spec.rb @@ -5,17 +5,30 @@ require "utils/shebang" describe Language::Perl::Shebang do let(:file) { Tempfile.new("perl-shebang") } - let(:perl_f) do - formula "perl" do + let(:f) do + f = {} + + f[:perl] = formula "perl" do url "https://brew.sh/perl-1.0.tgz" end - end - let(:f) do - formula "foo" do + + f[:depends_on] = formula "foo" do + url "https://brew.sh/foo-1.0.tgz" + + depends_on "perl" + end + + f[:uses_from_macos] = formula "foo" do url "https://brew.sh/foo-1.0.tgz" uses_from_macos "perl" end + + f[:no_deps] = formula "foo" do + url "https://brew.sh/foo-1.0.tgz" + end + + f end before do @@ -31,10 +44,23 @@ describe Language::Perl::Shebang do after { file.unlink } describe "#detected_perl_shebang" do - it "can be used to replace Perl shebangs" do + it "can be used to replace Perl shebangs when depends_on \"perl\" is used" do allow(Formulary).to receive(:factory) - allow(Formulary).to receive(:factory).with(perl_f.name).and_return(perl_f) - Utils::Shebang.rewrite_shebang described_class.detected_perl_shebang(f), file.path + allow(Formulary).to receive(:factory).with(f[:perl].name).and_return(f[:perl]) + Utils::Shebang.rewrite_shebang described_class.detected_perl_shebang(f[:depends_on]), file.path + + expect(File.read(file)).to eq <<~EOS + #!#{HOMEBREW_PREFIX}/opt/perl/bin/perl + a + b + c + EOS + end + + it "can be used to replace Perl shebangs when uses_from_macos \"perl\" is used" do + allow(Formulary).to receive(:factory) + allow(Formulary).to receive(:factory).with(f[:perl].name).and_return(f[:perl]) + Utils::Shebang.rewrite_shebang described_class.detected_perl_shebang(f[:uses_from_macos]), file.path expected_shebang = if OS.mac? "/usr/bin/perl#{MacOS.preferred_perl_version}" @@ -49,5 +75,10 @@ describe Language::Perl::Shebang do c EOS end + + it "errors if formula doesn't depend on perl" do + expect { Utils::Shebang.rewrite_shebang described_class.detected_perl_shebang(f[:no_deps]), file.path } + .to raise_error(ShebangDetectionError, "Cannot detect Perl shebang: formula does not depend on Perl.") + end end end From 4314daa2cf6409de1fa6506a063ed79e22fad51f Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Mon, 14 Aug 2023 20:27:16 -0400 Subject: [PATCH 5/5] python: expand and refactor tests This brings coverage of `Language::Python::Shebang` to 100%. --- .../test/language/python/shebang_spec.rb | 51 ++++++++++++++++--- 1 file changed, 43 insertions(+), 8 deletions(-) diff --git a/Library/Homebrew/test/language/python/shebang_spec.rb b/Library/Homebrew/test/language/python/shebang_spec.rb index 33a553a4d7..4e430cc093 100644 --- a/Library/Homebrew/test/language/python/shebang_spec.rb +++ b/Library/Homebrew/test/language/python/shebang_spec.rb @@ -5,17 +5,31 @@ require "utils/shebang" describe Language::Python::Shebang do let(:file) { Tempfile.new("python-shebang") } - let(:python_f) do - formula "python@3.11" do + let(:f) do + f = {} + + f[:python311] = formula "python@3.11" do url "https://brew.sh/python-1.0.tgz" end - end - let(:f) do - formula "foo" do + + f[:versioned_python_dep] = formula "foo" do url "https://brew.sh/foo-1.0.tgz" depends_on "python@3.11" end + + f[:no_deps] = formula "foo" do + url "https://brew.sh/foo-1.0.tgz" + end + + f[:multiple_deps] = formula "foo" do + url "https://brew.sh/foo-1.0.tgz" + + depends_on "python" + depends_on "python@3.11" + end + + f end before do @@ -33,9 +47,9 @@ describe Language::Python::Shebang do describe "#detected_python_shebang" do it "can be used to replace Python shebangs" do allow(Formulary).to receive(:factory) - allow(Formulary).to receive(:factory).with(python_f.name).and_return(python_f) + allow(Formulary).to receive(:factory).with(f[:python311].name).and_return(f[:python311]) Utils::Shebang.rewrite_shebang( - described_class.detected_python_shebang(f, use_python_from_path: false), file.path + described_class.detected_python_shebang(f[:versioned_python_dep], use_python_from_path: false), file.path ) expect(File.read(file)).to eq <<~EOS @@ -48,7 +62,7 @@ describe Language::Python::Shebang do it "can be pointed to a `python3` in PATH" do Utils::Shebang.rewrite_shebang( - described_class.detected_python_shebang(f, use_python_from_path: true), file.path + described_class.detected_python_shebang(f[:versioned_python_dep], use_python_from_path: true), file.path ) expect(File.read(file)).to eq <<~EOS @@ -58,5 +72,26 @@ describe Language::Python::Shebang do c EOS end + + it "errors if formula doesn't depend on python" do + expect do + Utils::Shebang.rewrite_shebang( + described_class.detected_python_shebang(f[:no_deps], use_python_from_path: false), + file.path, + ) + end.to raise_error(ShebangDetectionError, "Cannot detect Python shebang: formula does not depend on Python.") + end + + it "errors if formula depends on more than one python" do + expect do + Utils::Shebang.rewrite_shebang( + described_class.detected_python_shebang(f[:multiple_deps], use_python_from_path: false), + file.path, + ) + end.to raise_error( + ShebangDetectionError, + "Cannot detect Python shebang: formula has multiple Python dependencies.", + ) + end end end