search

Homebrew / homebrew-bundle

📦 Bundler for non-Ruby dependencies from Homebrew, Homebrew Cask and the Mac App Store.
MIT License
5.28k stars 283 forks source link

`brew bundle cleanup`: exit nonzero if action would be taken (or flag to opt into nonzero exit code) #1418

Closed ian-h-chamberlain closed 1 month ago

ian-h-chamberlain commented 1 month ago

brew bundle cleanup without --force is a convenient way to figure out if I installed some package but forgot to put it in my Brewfile — but it would be nice if it also exited with a nonzero status if any action would be taken. This would make it easy to use e.g. brew bundle cleanup as a pre-commit hook or other scripting workflows.

For now a workaround is to check if there was any output, but that feels a bit less programmatic and subject to change if brew bundle cleanup ever had any output added for the "happy path". For backward compatibility, it might make sense to make this opt-in behavior, perhaps with a --check flag.

Alternatively, I suppose the brew bundle check command could be extended to work with --cleanup and also have the behavior I'm describing, since the semantics are already pretty similar, and this would mirror the behavior of brew bundle install --cleanup. From what I can tell, --cleanup has no effect on check in the current implementation.

Edit: one other possibility would be some combination of flags to brew bundle install flag that would fail the command if some packages need to be removed. I'm not sure there's much of a use case for this compared to the other suggestion options though

Would you be open to a PR implementing one of these options or something equivalent? I think it would be handy for devs looking to make sure their Brewfile and environment are always in sync. Thanks!

MikeMcQuaid commented 1 month ago

but it would be nice if it also exited with a nonzero status if any action would be taken

This is a good idea!

For backward compatibility, it might make sense to make this opt-in behavior, perhaps with a --check flag.

Don't think this needs to have backwards compatibility changes in this case (without --force).

Would you be open to a PR implementing one of these options or something equivalent?

Yes, please! Would love to review a PR of the type I suggested above.

ian-h-chamberlain commented 1 month ago

Great, I'll see if I can open a PR soon. One question that came to mind as I glanced at the current implementation was this: https://github.com/Homebrew/homebrew-bundle/blob/master/lib/bundle/commands/cleanup.rb#L77-L79

Do you think it makes sense for cleanup.empty? to be excluded from the criteria for a nonzero exit code? The scripting use case I was originally thinking of would probably not really care if any brew cleanup needed to be performed, but would care if any packages, casks. etc. would be uninstalled as part of cleanup.

Maybe a different exit code (like bitflags or an enum) could be used too, but I'd probably lean towards simpler to start and expand if there turns out to be a use case that cares whether brew cleanup would have done anything...