Merge pull request #20298 from Homebrew/forbidden-fix

Fix handling of formula install blocks

Author: Bo98

Library/Homebrew/install.rb

@@ -336,24 +336,28 @@ def install_formulae(
             fi.download_queue = download_queue
           end
         end
+
+        valid_formula_installers = formula_installers.dup
+
         begin
           [:prelude_fetch, :prelude, :fetch].each do |step|
-            formula_installers.each do |fi|
+            valid_formula_installers.select! do |fi|
               fi.public_send(step)
+              true
+            rescue CannotInstallFormulaError => e
+              ofail e.message
+              false
             rescue UnsatisfiedRequirements, DownloadError, ChecksumMismatchError => e
               ofail "#{fi.formula}: #{e}"
-              next
+              false
             end
             download_queue&.fetch
-          rescue CannotInstallFormulaError => e
-            ofail e.message
-            next
           end
         ensure
           download_queue&.shutdown
         end
 
-        formula_installers.each do |fi|
+        valid_formula_installers.each do |fi|
           install_formula(fi)
           Cleanup.install_formula_clean!(fi.formula)
         end

Library/Homebrew/test/cmd/install_spec.rb

@@ -95,4 +95,14 @@ def install
 
     expect(HOMEBREW_CELLAR/"testball1/0.1/bin/test").to be_a_file
   end
+
+  it "refuses to install forbidden formulae", :integration_test do
+    setup_test_formula "testball1"
+
+    expect { brew "install", "testball1", { "HOMEBREW_FORBIDDEN_FORMULAE" => "testball1" } }
+      .to not_to_output(%r{#{HOMEBREW_CELLAR}/testball1/0\.1}o).to_stdout
+      .and output(/testball1 was forbidden/).to_stderr
+      .and be_a_failure
+    expect(HOMEBREW_CELLAR/"testball1").not_to exist
+  end
 end

Library/Homebrew/test/cmd/reinstall_spec.rb

@@ -34,4 +34,18 @@
 
     expect(foo_dir).to exist
   end
+
+  it "refuses to reinstall a forbidden formula", :integration_test do
+    install_test_formula "testball"
+    foo_dir = HOMEBREW_CELLAR/"testball/0.1/bin"
+    expect(foo_dir).to exist
+    FileUtils.rm_r(foo_dir)
+
+    expect { brew "reinstall", "testball", { "HOMEBREW_FORBIDDEN_FORMULAE" => "testball" } }
+      .to not_to_output(%r{#{HOMEBREW_CELLAR}/testball/0\.1}o).to_stdout
+      .and output(/testball was forbidden/).to_stderr
+      .and be_a_failure
+
+    expect(foo_dir).not_to exist
+  end
 end

Library/Homebrew/test/cmd/upgrade_spec.rb

@@ -28,4 +28,15 @@
     expect(HOMEBREW_CELLAR/"testball/0.1").to be_a_directory
     expect(HOMEBREW_CELLAR/"testball/0.0.1").not_to exist
   end
+
+  it "refuses to upgrades a forbidden formula", :integration_test do
+    setup_test_formula "testball"
+    (HOMEBREW_CELLAR/"testball/0.0.1/foo").mkpath
+
+    expect { brew "upgrade", "testball", { "HOMEBREW_FORBIDDEN_FORMULAE" => "testball" } }
+      .to not_to_output(%r{#{HOMEBREW_CELLAR}/testball/0\.1}o).to_stdout
+      .and output(/testball was forbidden/).to_stderr
+      .and be_a_failure
+    expect(HOMEBREW_CELLAR/"testball/0.1").not_to exist
+  end
 end

Library/Homebrew/upgrade.rb

@@ -92,18 +92,23 @@ def self.formula_installers(
     end
 
     def self.upgrade_formulae(formula_installers, dry_run: false, verbose: false)
+      valid_formula_installers = formula_installers.dup
+
       unless dry_run
-        formula_installers.each do |fi|
+        valid_formula_installers.select! do |fi|
           fi.prelude
           fi.fetch
+          true
         rescue CannotInstallFormulaError => e
           ofail e
+          false
         rescue UnsatisfiedRequirements, DownloadError => e
           ofail "#{fi.formula.full_name}: #{e}"
+          false
         end
       end
 
-      formula_installers.each do |fi|
+      valid_formula_installers.each do |fi|
         upgrade_formula(fi, dry_run:, verbose:)
         Cleanup.install_formula_clean!(fi.formula, dry_run:)
       end