From efd02b956d66fdada11815824247c10fb7ebb6ba Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 4 Aug 2023 16:20:38 -0700 Subject: [PATCH 1/5] Revert "Revert "Make `inreplace` a purely static method"" This reverts commit 8656caa67ce2dd9ec6484969b183c1fd7805451e. --- Library/Homebrew/cask/migrator.rb | 4 +--- Library/Homebrew/formula.rb | 3 +-- Library/Homebrew/utils/inreplace.rb | 6 ++---- Library/Homebrew/utils/inreplace.rbi | 7 ------- 4 files changed, 4 insertions(+), 16 deletions(-) delete mode 100644 Library/Homebrew/utils/inreplace.rbi diff --git a/Library/Homebrew/cask/migrator.rb b/Library/Homebrew/cask/migrator.rb index d850627706..1c3d9f4f20 100644 --- a/Library/Homebrew/cask/migrator.rb +++ b/Library/Homebrew/cask/migrator.rb @@ -6,8 +6,6 @@ require "utils/inreplace" module Cask class Migrator - extend ::Utils::Inreplace - attr_reader :old_cask, :new_cask sig { params(old_cask: Cask, new_cask: Cask).void } @@ -74,7 +72,7 @@ module Cask def self.replace_caskfile_token(path, old_token, new_token) case path.extname when ".rb" - inreplace path, /\A\s*cask\s+"#{Regexp.escape(old_token)}"/, "cask #{new_token.inspect}" + ::Utils::Inreplace.inreplace path, /\A\s*cask\s+"#{Regexp.escape(old_token)}"/, "cask #{new_token.inspect}" when ".json" json = JSON.parse(path.read) json["token"] = new_token diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 738000ff6f..e97ebd9d52 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -59,7 +59,6 @@ require "extend/api_hashable" # end class Formula include FileUtils - include Utils::Inreplace include Utils::Shebang include Utils::Shell include Context @@ -2563,7 +2562,7 @@ class Formula ).void } def inreplace(paths, before = nil, after = nil, audit_result = true) # rubocop:disable Style/OptionalBooleanParameter - super(paths, before, after, audit_result) + Utils::Inreplace.inreplace(paths, before, after, audit_result: audit_result) rescue Utils::Inreplace::Error => e onoe e.to_s raise BuildError.new(self, "inreplace", Array(paths), {}) diff --git a/Library/Homebrew/utils/inreplace.rb b/Library/Homebrew/utils/inreplace.rb index 5f51f78169..9743c1608a 100644 --- a/Library/Homebrew/utils/inreplace.rb +++ b/Library/Homebrew/utils/inreplace.rb @@ -18,8 +18,6 @@ module Utils end end - module_function - # Sometimes we have to change a bit before we install. Mostly we # prefer a patch, but if you need the {Formula#prefix prefix} of # this formula in the patch you have to resort to `inreplace`, @@ -45,7 +43,7 @@ module Utils audit_result: T::Boolean, ).void } - def inreplace(paths, before = nil, after = nil, audit_result = true) # rubocop:disable Style/OptionalBooleanParameter + def self.inreplace(paths, before = nil, after = nil, audit_result: true) paths = Array(paths) after &&= after.to_s before = before.to_s if before.is_a?(Pathname) @@ -73,7 +71,7 @@ module Utils end # @api private - def inreplace_pairs(path, replacement_pairs, read_only_run: false, silent: false) + def self.inreplace_pairs(path, replacement_pairs, read_only_run: false, silent: false) str = File.binread(path) contents = StringInreplaceExtension.new(str) replacement_pairs.each do |old, new| diff --git a/Library/Homebrew/utils/inreplace.rbi b/Library/Homebrew/utils/inreplace.rbi deleted file mode 100644 index 02542091d2..0000000000 --- a/Library/Homebrew/utils/inreplace.rbi +++ /dev/null @@ -1,7 +0,0 @@ -# typed: strict - -module Utils - module Inreplace - include Kernel - end -end From 864f31e52a2ad6496b058d366cbeae0f5177395e Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 4 Aug 2023 16:18:54 -0700 Subject: [PATCH 2/5] Forward block argument --- Library/Homebrew/formula.rb | 5 +++-- Library/Homebrew/utils/inreplace.rb | 5 ++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index e97ebd9d52..00b55f5a00 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -2559,10 +2559,11 @@ class Formula before: T.nilable(T.any(Pathname, Regexp, String)), after: T.nilable(T.any(Pathname, String, Symbol)), audit_result: T::Boolean, + blk: T.nilable(T.proc.params(s: StringInreplaceExtension).void), ).void } - def inreplace(paths, before = nil, after = nil, audit_result = true) # rubocop:disable Style/OptionalBooleanParameter - Utils::Inreplace.inreplace(paths, before, after, audit_result: audit_result) + def inreplace(paths, before = nil, after = nil, audit_result = true, &blk) # rubocop:disable Style/OptionalBooleanParameter + Utils::Inreplace.inreplace(paths, before, after, audit_result: audit_result, &blk) rescue Utils::Inreplace::Error => e onoe e.to_s raise BuildError.new(self, "inreplace", Array(paths), {}) diff --git a/Library/Homebrew/utils/inreplace.rb b/Library/Homebrew/utils/inreplace.rb index 9743c1608a..1cd7da37e9 100644 --- a/Library/Homebrew/utils/inreplace.rb +++ b/Library/Homebrew/utils/inreplace.rb @@ -41,9 +41,10 @@ module Utils before: T.nilable(T.any(Pathname, Regexp, String)), after: T.nilable(T.any(Pathname, String, Symbol)), audit_result: T::Boolean, + blk: T.nilable(T.proc.params(s: StringInreplaceExtension).void), ).void } - def self.inreplace(paths, before = nil, after = nil, audit_result: true) + def self.inreplace(paths, before = nil, after = nil, audit_result: true, &blk) paths = Array(paths) after &&= after.to_s before = before.to_s if before.is_a?(Pathname) @@ -57,6 +58,8 @@ module Utils s = StringInreplaceExtension.new(str) if before.nil? && after.nil? + raise ArgumentError, "Must supply a block or before/after params" unless blk + yield s else s.gsub!(T.must(before), T.must(after), audit_result) From 880362669b9f7fc94f8c58f005846cb91339d70f Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 4 Aug 2023 16:17:52 -0700 Subject: [PATCH 3/5] Add tests --- Library/Homebrew/test/dev-cmd/test_spec.rb | 36 ++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/Library/Homebrew/test/dev-cmd/test_spec.rb b/Library/Homebrew/test/dev-cmd/test_spec.rb index fccb80a374..9bbb9dfe53 100644 --- a/Library/Homebrew/test/dev-cmd/test_spec.rb +++ b/Library/Homebrew/test/dev-cmd/test_spec.rb @@ -17,4 +17,40 @@ describe "brew test" do .and not_to_output.to_stderr .and be_a_success end + + describe "using inreplace" do + it "replaces text in file", :integration_test do + install_test_formula "testball", <<~RUBY + test do + (testpath/"file.txt").write "1" + inreplace testpath/"file.txt" do |s| + s.gsub! "1", "2" + end + assert_equal "2", (testpath/"file.txt").read + end + RUBY + + expect { brew "test", "--verbose", "testball" } + .to output(/Testing testball/).to_stdout + .and not_to_output.to_stderr + .and be_a_success + end + + it "fails when assertion fails", :integration_test do + install_test_formula "testball", <<~RUBY + test do + (testpath/"file.txt").write "1" + inreplace testpath/"file.txt" do |s| + s.gsub! "1", "2" + end + assert_equal "3", (testpath/"file.txt").read + end + RUBY + + expect { brew "test", "--verbose", "testball" } + .to output(/Testing testball/).to_stdout + .and output(/Expected: "3"\n Actual: "2"/).to_stderr + .and be_a_failure + end + end end From 2b29c498faf9971ca66c0c7fb6bfd1b2a75f01d1 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Mon, 7 Aug 2023 17:26:46 -0700 Subject: [PATCH 4/5] s/blk/block --- Library/Homebrew/formula.rb | 6 +++--- Library/Homebrew/utils/inreplace.rb | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 00b55f5a00..d88d01ec43 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -2559,11 +2559,11 @@ class Formula before: T.nilable(T.any(Pathname, Regexp, String)), after: T.nilable(T.any(Pathname, String, Symbol)), audit_result: T::Boolean, - blk: T.nilable(T.proc.params(s: StringInreplaceExtension).void), + block: T.nilable(T.proc.params(s: StringInreplaceExtension).void), ).void } - def inreplace(paths, before = nil, after = nil, audit_result = true, &blk) # rubocop:disable Style/OptionalBooleanParameter - Utils::Inreplace.inreplace(paths, before, after, audit_result: audit_result, &blk) + def inreplace(paths, before = nil, after = nil, audit_result = true, &block) # rubocop:disable Style/OptionalBooleanParameter + Utils::Inreplace.inreplace(paths, before, after, audit_result: audit_result, &block) rescue Utils::Inreplace::Error => e onoe e.to_s raise BuildError.new(self, "inreplace", Array(paths), {}) diff --git a/Library/Homebrew/utils/inreplace.rb b/Library/Homebrew/utils/inreplace.rb index 1cd7da37e9..76e7cca5e2 100644 --- a/Library/Homebrew/utils/inreplace.rb +++ b/Library/Homebrew/utils/inreplace.rb @@ -41,10 +41,10 @@ module Utils before: T.nilable(T.any(Pathname, Regexp, String)), after: T.nilable(T.any(Pathname, String, Symbol)), audit_result: T::Boolean, - blk: T.nilable(T.proc.params(s: StringInreplaceExtension).void), + block: T.nilable(T.proc.params(s: StringInreplaceExtension).void), ).void } - def self.inreplace(paths, before = nil, after = nil, audit_result: true, &blk) + def self.inreplace(paths, before = nil, after = nil, audit_result: true, &block) paths = Array(paths) after &&= after.to_s before = before.to_s if before.is_a?(Pathname) @@ -58,7 +58,7 @@ module Utils s = StringInreplaceExtension.new(str) if before.nil? && after.nil? - raise ArgumentError, "Must supply a block or before/after params" unless blk + raise ArgumentError, "Must supply a block or before/after params" unless block yield s else From 5d5c22e104b878be743204486e1dbc7388ef613c Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Mon, 7 Aug 2023 17:36:13 -0700 Subject: [PATCH 5/5] Replace integration test with unit test --- Library/Homebrew/formula.rb | 2 +- Library/Homebrew/test/dev-cmd/test_spec.rb | 36 ---------------------- Library/Homebrew/test/formula_spec.rb | 20 ++++++++++++ Library/Homebrew/utils/inreplace.rb | 2 +- 4 files changed, 22 insertions(+), 38 deletions(-) diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index d88d01ec43..99e1d208ad 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -2559,7 +2559,7 @@ class Formula before: T.nilable(T.any(Pathname, Regexp, String)), after: T.nilable(T.any(Pathname, String, Symbol)), audit_result: T::Boolean, - block: T.nilable(T.proc.params(s: StringInreplaceExtension).void), + block: T.nilable(T.proc.params(s: StringInreplaceExtension).void), ).void } def inreplace(paths, before = nil, after = nil, audit_result = true, &block) # rubocop:disable Style/OptionalBooleanParameter diff --git a/Library/Homebrew/test/dev-cmd/test_spec.rb b/Library/Homebrew/test/dev-cmd/test_spec.rb index 9bbb9dfe53..fccb80a374 100644 --- a/Library/Homebrew/test/dev-cmd/test_spec.rb +++ b/Library/Homebrew/test/dev-cmd/test_spec.rb @@ -17,40 +17,4 @@ describe "brew test" do .and not_to_output.to_stderr .and be_a_success end - - describe "using inreplace" do - it "replaces text in file", :integration_test do - install_test_formula "testball", <<~RUBY - test do - (testpath/"file.txt").write "1" - inreplace testpath/"file.txt" do |s| - s.gsub! "1", "2" - end - assert_equal "2", (testpath/"file.txt").read - end - RUBY - - expect { brew "test", "--verbose", "testball" } - .to output(/Testing testball/).to_stdout - .and not_to_output.to_stderr - .and be_a_success - end - - it "fails when assertion fails", :integration_test do - install_test_formula "testball", <<~RUBY - test do - (testpath/"file.txt").write "1" - inreplace testpath/"file.txt" do |s| - s.gsub! "1", "2" - end - assert_equal "3", (testpath/"file.txt").read - end - RUBY - - expect { brew "test", "--verbose", "testball" } - .to output(/Testing testball/).to_stdout - .and output(/Expected: "3"\n Actual: "2"/).to_stderr - .and be_a_failure - end - end end diff --git a/Library/Homebrew/test/formula_spec.rb b/Library/Homebrew/test/formula_spec.rb index 5f4a6765d4..eabdc075b2 100644 --- a/Library/Homebrew/test/formula_spec.rb +++ b/Library/Homebrew/test/formula_spec.rb @@ -464,6 +464,26 @@ describe Formula do expect { f.inreplace([]) }.to raise_error(BuildError) end + + specify "replaces text in file" do + file = Tempfile.new("test") + File.binwrite(file, <<~EOS) + ab + bc + cd + EOS + f = formula do + url "https://brew.sh/test-1.0.tbz" + end + f.inreplace(file.path) do |s| + s.gsub!("bc", "yz") + end + expect(File.binread(file)).to eq <<~EOS + ab + yz + cd + EOS + end end describe "::installed_with_alias_path" do diff --git a/Library/Homebrew/utils/inreplace.rb b/Library/Homebrew/utils/inreplace.rb index 76e7cca5e2..648aebeb93 100644 --- a/Library/Homebrew/utils/inreplace.rb +++ b/Library/Homebrew/utils/inreplace.rb @@ -41,7 +41,7 @@ module Utils before: T.nilable(T.any(Pathname, Regexp, String)), after: T.nilable(T.any(Pathname, String, Symbol)), audit_result: T::Boolean, - block: T.nilable(T.proc.params(s: StringInreplaceExtension).void), + block: T.nilable(T.proc.params(s: StringInreplaceExtension).void), ).void } def self.inreplace(paths, before = nil, after = nil, audit_result: true, &block)