Merge pull request #21351 from AbdelrahmanHafez/fix/bundle-cleanup

fix(bundle): prevent autoremove from removing Brewfile packages during cleanup

Author: MikeMcQuaid

Library/Homebrew/bundle.rb

@@ -173,6 +173,32 @@ def reset!
         @formula_versions_from_env = T.let(nil, T.nilable(T::Hash[String, String]))
         @upgrade_formulae = T.let(nil, T.nilable(T::Array[String]))
       end
+
+      # Marks Brewfile formulae as installed_on_request to prevent autoremove
+      # from removing them when their dependents are uninstalled.
+      sig { params(entries: T::Array[Dsl::Entry]).void }
+      def mark_as_installed_on_request!(entries)
+        return if entries.empty?
+
+        require "tab"
+
+        installed_formulae = Formula.installed_formula_names
+        return if installed_formulae.empty?
+
+        entries.each do |entry|
+          next if entry.type != :brew
+
+          name = entry.name
+          next if installed_formulae.exclude?(name)
+
+          tab = Tab.for_name(name)
+          next if tab.tabfile.blank? || !tab.tabfile.exist?
+          next if tab.installed_on_request
+
+          tab.installed_on_request = true
+          tab.write
+        end
+      end
     end
   end
 end

Library/Homebrew/bundle/commands/cleanup.rb

@@ -44,6 +44,12 @@ def self.run(global: false, file: nil, force: false, zap: false, dsl: nil,
             end
 
             if formulae.any?
+              # Mark Brewfile formulae as installed_on_request to prevent autoremove
+              # from removing them when their dependents are uninstalled
+              require "bundle/brewfile"
+              @dsl ||= Brewfile.read(global:, file:)
+              Homebrew::Bundle.mark_as_installed_on_request!(@dsl.entries)
+
               Kernel.system HOMEBREW_BREW_FILE, "uninstall", "--formula", "--force", *formulae
               puts "Uninstalled #{formulae.size} formula#{"e" if formulae.size != 1}"
             end

Library/Homebrew/bundle/commands/install.rb

@@ -22,10 +22,16 @@ module Install
         def self.run(global: false, file: nil, no_lock: false, no_upgrade: false, verbose: false, force: false,
                      quiet: false)
           @dsl = Brewfile.read(global:, file:)
-          Homebrew::Bundle::Installer.install!(
+          result = Homebrew::Bundle::Installer.install!(
             @dsl.entries,
             global:, file:, no_lock:, no_upgrade:, verbose:, force:, quiet:,
-          ) || exit(1)
+          )
+
+          # Mark Brewfile formulae as installed_on_request to prevent autoremove
+          # from removing them when their dependents are uninstalled
+          Homebrew::Bundle.mark_as_installed_on_request!(@dsl.entries)
+
+          result || exit(1)
         end
 
         sig { returns(T.nilable(Dsl)) }

Library/Homebrew/test/bundle/bundle_spec.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "bundle"
+require "bundle/dsl"
 
 RSpec.describe Homebrew::Bundle do
   context "when the system call succeeds" do
@@ -46,4 +47,63 @@
       expect(described_class.mas_installed?).to be(true)
     end
   end
+
+  describe ".mark_as_installed_on_request!", :no_api do
+    subject(:mark_installed!) { described_class.mark_as_installed_on_request!(entries) }
+
+    let(:entries) { dsl.entries }
+    let(:dsl) { Homebrew::Bundle::Dsl.new(Pathname.new("/fake/Brewfile")) }
+    let(:tabfile) { Pathname.new("/fake/INSTALL_RECEIPT.json") }
+
+    before do
+      allow(DevelopmentTools).to receive_messages(needs_libc_formula?: false, needs_compiler_formula?: false)
+      allow_any_instance_of(Pathname).to receive(:read).and_return(brewfile_content)
+      allow(tabfile).to receive_messages(blank?: false, exist?: true)
+    end
+
+    context "when formula is installed but not marked as installed_on_request" do
+      let(:brewfile_content) { "brew 'myformula'" }
+      let(:tab) { instance_double(Tab, installed_on_request: false, tabfile:) }
+
+      before do
+        allow(Formula).to receive(:installed_formula_names).and_return(["myformula"])
+        allow(Tab).to receive(:for_name).with("myformula").and_return(tab)
+      end
+
+      it "sets installed_on_request=true and writes" do
+        expect(tab).to receive(:installed_on_request=).with(true)
+        expect(tab).to receive(:write)
+        mark_installed!
+      end
+    end
+
+    context "when formula is not installed" do
+      let(:brewfile_content) { "brew 'notinstalled'" }
+
+      before do
+        allow(Formula).to receive(:installed_formula_names).and_return([])
+      end
+
+      it "skips the formula" do
+        expect(Tab).not_to receive(:for_name)
+        mark_installed!
+      end
+    end
+
+    context "when formula is already marked as installed_on_request" do
+      let(:brewfile_content) { "brew 'alreadymarked'" }
+      let(:tab) { instance_double(Tab, installed_on_request: true, tabfile:) }
+
+      before do
+        allow(Formula).to receive(:installed_formula_names).and_return(["alreadymarked"])
+        allow(Tab).to receive(:for_name).with("alreadymarked").and_return(tab)
+      end
+
+      it "skips writing" do
+        expect(tab).not_to receive(:installed_on_request=)
+        expect(tab).not_to receive(:write)
+        mark_installed!
+      end
+    end
+  end
 end

Library/Homebrew/test/bundle/commands/cleanup_spec.rb

@@ -211,6 +211,8 @@
                                                  taps_to_untap:                  [],
                                                  vscode_extensions_to_uninstall: [],
                                                  flatpaks_to_uninstall:          [])
+      allow(Homebrew::Bundle).to receive(:mark_as_installed_on_request!)
+      allow_any_instance_of(Pathname).to receive(:read).and_return("")
     end
 
     it "uninstalls formulae" do
@@ -357,4 +359,25 @@
       described_class.system_output_no_stderr("true")
     end
   end
+
+  context "when running with force" do
+    before do
+      described_class.reset!
+      allow(described_class).to receive_messages(
+        casks_to_uninstall:             [],
+        formulae_to_uninstall:          %w[some_formula],
+        taps_to_untap:                  [],
+        vscode_extensions_to_uninstall: [],
+        flatpaks_to_uninstall:          [],
+      )
+      allow(Kernel).to receive(:system)
+      allow(described_class).to receive(:system_output_no_stderr).and_return("")
+      allow_any_instance_of(Pathname).to receive(:read).and_return("")
+    end
+
+    it "marks Brewfile formulae as installed_on_request before uninstalling" do
+      expect(Homebrew::Bundle).to receive(:mark_as_installed_on_request!)
+      described_class.run(force: true)
+    end
+  end
 end

Library/Homebrew/test/bundle/commands/install_spec.rb

@@ -92,5 +92,18 @@
 
       expect { described_class.run }.to raise_error(SystemExit)
     end
+
+    it "marks Brewfile formulae as installed_on_request after installing" do
+      allow(Homebrew::Bundle::TapInstaller).to receive(:preinstall!).and_return(false)
+      allow(Homebrew::Bundle::VscodeExtensionInstaller).to receive(:preinstall!).and_return(false)
+      allow(Homebrew::Bundle::FlatpakInstaller).to receive(:preinstall!).and_return(false)
+      allow(Homebrew::Bundle::FormulaInstaller).to receive_messages(preinstall!: true, install!: true)
+      allow(Homebrew::Bundle::CaskInstaller).to receive_messages(preinstall!: true, install!: true)
+      allow(Homebrew::Bundle::MacAppStoreInstaller).to receive_messages(preinstall!: true, install!: true)
+      allow_any_instance_of(Pathname).to receive(:read).and_return("brew 'test_formula'")
+
+      expect(Homebrew::Bundle).to receive(:mark_as_installed_on_request!)
+      described_class.run
+    end
   end
 end