postinstall only works with --verbose

[rrotter]

Add a simple postinstall command to Brewfile:
brew "tig", postinstall: "/bin/ls -la"

Try to run the postinstall:

% brew rm tig
% brew bundle
Installing tig
Error: No such file or directory - /bin/ls -la

otoh, brew bundle -v works as expected.

[colindean]

According to my read of

homebrew-bundle/lib/bundle/brew_installer.rb

Lines 139 to 145 in ac88e94

	def postinstall_change_state!(verbose:)
	return true if @postinstall.blank?
	return true unless changed?
	puts "Running postinstall for #{@name}." if verbose
	Bundle.system(@postinstall, verbose:)
	end

You cannot pass args to the postinstall as currently implemented and are only notified of its execution (and its output) when verbose is passed.

But the docs indicate otherwise:

homebrew-bundle/README.md

Lines 45 to 46 in ac88e94

	brew "postgresql@16",
	postinstall: "${HOMEBREW_PREFIX}/opt/postgresql@16/bin/postgres -D ${HOMEBREW_PREFIX}/var/postgresql@16"

[colindean]

I'm able to reproduce this failure in the tests with this, but I'm not sure of the best way to address it.

diff --git a/spec/bundle_utils_spec.rb b/spec/bundle_utils_spec.rb
index c2d5195..fddc18e 100644
--- a/spec/bundle_utils_spec.rb
+++ b/spec/bundle_utils_spec.rb
@@ -29,6 +29,24 @@
     end
   end

+  context "when passed a cmd with options" do
+    before(:example) do
+      @tmp = Tempfile.new('foo')
+    end
+    it "executes with verbose" do
+      described_class.system "/bin/bash -c touch #{@tmp.path}", verbose: true
+      expect(File.exist?(@tmp)).to be(true)
+    end
+    it "executes without verbose" do
+      described_class.system "/bin/bash -c touch #{@tmp.path}", verbose: false
+      expect(File.exist?(@tmp)).to be(true)
+    end
+    after(:example) do
+      @tmp.unlink
+    end
+
+  end
+
   context "when checking for homebrew/cask", :needs_macos do
     it "finds it when present" do
       allow(File).to receive(:directory?).with("#{HOMEBREW_PREFIX}/Caskroom").and_return(true)

The Kernel.system invoked via the super when verbose is true handles both the case when the first param is a command line vs an exe_path with *args, according to the docs, while the implementation thereafter in Bundle.system relies on having two args: the exe path and args.

I think there are a couple of options:

1. Allow the DSL's postinstall to accept a string or a two-element array, the first is the exe path as a string and the second is an arg array when calling Bundle.system.
2. Parse the DSL's postinstall, splitting on whitespace, the passing the first element as a the exe path and the second as the arg array into Bundle.system.

[Bo98]

ah that explains why I was confused whether this worked: #1555 (comment)

Maybe worth making it report an error when the postinstall fails.

[rrotter]

I think there are a couple of options:

1. Allow the DSL's postinstall to accept a string or a two-element array, the first is the exe path as a string and the second is an arg array when calling Bundle.system.
2. Parse the DSL's postinstall, splitting on whitespace, the passing the first element as a the exe path and the second as the arg array into Bundle.system.

Could also replace IO.popen with Open3.

[Bo98]

IO.popen does work fine as shell mode too - it would just need to take cmd rather than [cmd, *args]. Could do so automatically or perhaps a safer approach is a more explicit opt-in (e.g. shell: true) to ensure only postinstall is run as a shell.

[MikeMcQuaid]

Thanks for the report @rrotter! Opened #1595 to fix.

The Kernel.system invoked via the super when verbose is true handles both the case when the first param is a command line vs an exe_path with *args, according to the docs, while the implementation thereafter in Bundle.system relies on having two args: the exe path and args.

We're now just always using Kernel.system, as we do elsewhere in a few cases in Homebrew/bundle.

Maybe worth making it report an error when the postinstall fails.

We already do, hence the error in the original issue body.