Installing an outdated version should show a warning, not an error #27268

Closed
wants to merge 1 commit into from

4 participants

@iainbeeston

Problem

I'd like to use the brew bundle command, but it has an annoying bug.

If you have a Brewfile like this:

install redis
install mysql

and you already have redis-2.8.1 installed, but the latest version is redis-2.8.7, then running brew bundle will fail like so:

Error: redis-2.8.1 already installed
To install this version, first `brew unlink redis'
Error: Command failed: L4:brew install redis

There are two problems with this:

  1. The error message is unhelpful (there is no explanation of why redis being installed is an error - surely that should be a good thing?)
  2. Any commands after the error are not executed

If you do already have redis-2.8.7 installed, the output is this:

Warning: redis-2.8.7 already installed

But at least mysql gets installed.

Solution

Digging into the code, the bundle command exits if any commands in the Brewfile fail. I believe that if the package is already installed, then install should not be failing at all (maybe outputting a warning like it does when the latest version is installed, but not failing).

To make it behave like this, I have added a new error class, to be used when an outdated version is installed. This is treated the same as when the latest version is installed, but shows a different (and more helpful) warning message.

With this patch my example above produces the following output:

Warning: redis-2.8.1 installed but redis-2.8.7 is the latest
To install the latest version, first `brew unlink redis'

and the mysql formula is installed successfully.

@iainbeeston iainbeeston commented on an outdated diff Mar 6, 2014
Library/Homebrew/exceptions.rb
@@ -94,6 +94,8 @@ class CannotInstallFormulaError < RuntimeError; end
class FormulaAlreadyInstalledError < RuntimeError; end
+class OutdatedVersionInstalledError < CannotInstallFormulaError; end

I'd rather this extended from RuntimeError but I've made it extend CannotInstallFormulaError for backwards compatibility

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@MikeMcQuaid
Homebrew member

I'm 👎 on changing working Homebrew internals for Brewfile. Someonee should instead fix Brewfile so it can handle the "install or upgrade" case and perhaps not just be a list of commands that are blindly executed.

@iainbeeston
@MikeMcQuaid
Homebrew member

People who want to script Homebrew should check if the formula is installed and use brew upgrade if it is.

@iainbeeston
@MikeMcQuaid
Homebrew member

Yeh, perhaps. The problem with stuff like this is we can't break the existing functionality. With that in mind PRs are probably better for discussing.

@iainbeeston
@MikeMcQuaid
Homebrew member

The error message should reference brew upgrade rather than brew unlink but sure.

@chdiza

but what about people who want to script homebrew?

Such people (and I include myself here) should use shell scripts or ruby scripts to do it. I never understood the point of Brewfiles, for exactly the reason that you can't put any conditional execution in there. It does nothing for the user except eliminate the need to write brew in the file, which is so small a gain that it's dwarfed by other considerations. I just make shell scripts that call brew under the conditions I want. Example:

if [ $HOSTTYPE == "powerpc" ]
  then brew install -d curl-ca-bundle
fi

if [[ `gzip --version` =~ "1.5" ]]
  then
  brew tap homebrew/dupes && brew install -d gzip
fi

I've also made ruby files that do similar things, except that you can make them into external commands and therefore gain access to some of the internals of homebrew.

@MikeMcQuaid
Homebrew member

@chdiza I wrote an implementation of Brewfiles a while ago that was more like Gemfiles; an actual DSL rather than just a list of commands. It's a question of simple implementation vs simple API. I'm concerned the current Brewfile syntax isn't really going to cope well with the more advanced use-cases.

@iainbeeston
@chdiza

I hope I didn't sound like I wanted the Brewfile syntax to be changed. Indeed, I was suggesting the opposite: that users who need more flexibility than Brewfiles provide can get it elsewhere without needing to complicate the existing machinery.

@iainbeeston iainbeeston Tweaked the error message when trying to install a formula but an
outdated version is already installed

Made it explain what the problem is (i.e. you already have it but it's
an older version, so brew can't install) and what to do about it (i.e.
run upgrade)
e3041d1
@iainbeeston

@mikemcquaid I've updated this PR to only change the error message now. Please let me know if there's anything else you'd like in order to get this merged.

@MikeMcQuaid MikeMcQuaid commented on the diff Mar 7, 2014
Library/Homebrew/formula_installer.rb
@@ -103,8 +103,8 @@ def install
if f.linked_keg.directory?
# some other version is already installed *and* linked
raise CannotInstallFormulaError, <<-EOS.undent
- #{f}-#{f.linked_keg.realpath.basename} already installed
- To install this version, first `brew unlink #{f}'
+ #{f}-#{f.linked_keg.realpath.basename} installed but #{f}-#{f.pkg_version} is the latest
+ To install the latest version, run `brew upgrade #{f}'
@MikeMcQuaid
Homebrew member

Looking at this again: should this not be/is this brew install specific or will it happen with brew link too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@MikeMcQuaid
Homebrew member

@jacknagel @adamv @mistydemeo Need some more input here, thanks.

@jacknagel

I have to think about it for a bit.

@iainbeeston

Sorry guys, any update on this?

@jacknagel jacknagel self-assigned this May 9, 2014
@jacknagel

@mikemcquaid are the recent outdated changes enough to close this?

@jacknagel jacknagel removed their assignment Aug 26, 2014
@MikeMcQuaid
Homebrew member

Yes, we'll pass on this I'm afraid. Outside brew bundle you should use e.g. brew outdated git || brew upgrade git.

@xu-cheng xu-cheng locked and limited conversation to collaborators Feb 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.