brew upgrade misbehaves when a formula's dependency is renamed

[cartr]

• Ran brew update and retried your prior step?
• Ran brew doctor, fixed as many issues as possible and retried your prior step?
• Confirmed this is problem with Homebrew/brew and not specific formulae? If it's a formulae-specific problem please file this issue at https://github.com/Homebrew/homebrew-core/issues/new

Here's my brew config and brew doctor output.

Summary

If a formula (that is depended on by another installed formula) is renamed, brew update tries to install it on top of the old one and fails during the brew link step.

Steps to Reproduce

Run these commands:

brew tap cartr/rename-bug-demo
brew install dependent

You'll get two new formulae installed: somepackage (which is GNU Hello) and dependent which depends on it. Next, run these to switch to a branch where somepackage has been renamed to somepackage4 and upgrade:

cd /usr/local/Homebrew/Library/Taps/cartr/homebrew-rename-bug-demo
git config --add remote.origin.fetch "+refs/heads/*:refs/remotes/origin/*" && git fetch
git checkout add-four
brew upgrade

Expected Result

The upgrade completes successfully, migrating somepackage to somepackage4.

Actual Result

bash-3.2$ brew upgrade
Warning: You are using a pre-release version of Xcode.
You may encounter build failures or other breakages.
Please create pull-requests instead of filing issues.
==> Upgrading 1 outdated package, with result:
cartr/rename-bug-demo/somepackage4 2.10_1
==> Upgrading cartr/rename-bug-demo/somepackage4
==> Downloading http://ftpmirror.gnu.org/hello/hello-2.10.tar.gz
==> Downloading from http://mirrors.kernel.org/gnu/hello/hello-2.10.tar.gz
######################################################################## 100.0%
==> ./configure --disable-silent-rules --prefix=/usr/local/Cellar/somepackage4/2.10_1
==> make install
Error: The `brew link` step did not complete successfully
The formula built, but is not symlinked into /usr/local
Could not symlink bin/hello
Target /usr/local/bin/hello
is a symlink belonging to somepackage. You can unlink it:
  brew unlink somepackage

To force the link and overwrite all conflicting files:
  brew link --overwrite somepackage4

To list all files that would be deleted:
  brew link --overwrite --dry-run somepackage4

Possible conflicting files are:
/usr/local/bin/hello -> /usr/local/Cellar/somepackage/2.10_1/bin/hello
/usr/local/share/info/hello.info -> /usr/local/Cellar/somepackage/2.10_1/share/info/hello.info
/usr/local/share/man/man1/hello.1 -> /usr/local/Cellar/somepackage/2.10_1/share/man/man1/hello.1
==> Summary
🍺  /usr/local/Cellar/somepackage4/2.10_1: 10 files, 124.4K, built in 41 seconds

somepackage remains installed in its original location. somepackage4 is installed but unlinked.

[alyssais]

The fix for this is probably to expand Formula#old_installed_formulae. I can probably take a look at this.

[MikeMcQuaid]

HOMEBREW_VERSION: 1.0.4

This is definitely an issue. Follow https://github.com/Homebrew/brew#update-bug and let me know if you can reproduce this afterwards.

[cartr]

@MikeMcQuaid After following the instructions you linked, brew config now reports HOMEBREW_VERSION: 1.1.5-127-g538028a73. I am still able to reproduce this issue after updating.

[MikeMcQuaid]

@cartr It's worth noting that brew upgrade does not do renaming and this is done by brew update.

[MikeMcQuaid]

@cartr Did you try to brew update with this form first so your tap's changes were detected and migrated?

[alyssais]

I just ran into this bug myself with gcc@4.8 conflicting with homebrew/versions/gcc48. brew upgrade automatically updated Homebrew beforehand, so it looks like it should have been migrated but wasn't?

[alyssais]

brew config

HOMEBREW_VERSION: 1.1.6-9-g98dadd907
ORIGIN: https://github.com/Homebrew/brew
HEAD: 98dadd907e31bb5f9a16bfb6aa2138b1852c4db5
Last commit: 52 minutes ago
Core tap ORIGIN: https://github.com/Homebrew/homebrew-core
Core tap HEAD: 1fba10615dd0c5cbac427d74a9d7d5023bd3ae27
Core tap last commit: 73 minutes ago
HOMEBREW_PREFIX: /usr/local
HOMEBREW_REPOSITORY: /usr/local/Homebrew
HOMEBREW_CELLAR: /usr/local/Cellar
HOMEBREW_BOTTLE_DOMAIN: https://homebrew.bintray.com
CPU: quad-core 64-bit haswell
Homebrew Ruby: 2.0.0-p648
Clang: 8.0 build 800
Git: 2.11.0 => /usr/local/bin/git
Perl: /usr/bin/perl
Python: /usr/local/bin/python => /usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/bin/python2.7
Ruby: /Users/alyssa/.rubies/ruby-2.3.3/bin/ruby
Java: 1.8.0_112
macOS: 10.12.2-x86_64
Xcode: 8.2.1
CLT: 8.2.0.0.1.1480973914
X11: N/A

brew doctor

Please note that these warnings are just used to help the Homebrew maintainers
with debugging if you file an issue. If everything you use Homebrew for is
working fine: please don't worry and just ignore them. Thanks!
Warning: You have unlinked kegs in your Cellar

Leaving kegs unlinked can lead to build-trouble and cause brews that depend on

those kegs to fail to run properly once built. Run brew link on these:

cloog018

gcc@4.8
Warning: Broken symlinks were found. Remove them with brew prune:

/usr/local/share/man/man8/prlexec.8

[MikeMcQuaid]

@alyssais Can you try to git reset --hard to the relevant homebrew/versions and homebrew/core revisions before this, run brew update or brew upgrade and see if you can reproduce it?

[alyssais]

I can reproduce on 538028a, but not on master, so it was probably fixed somewhere in between, but if brew is auto-updated as part of brew upgrade it looks like it doesn't pick up on the fix for that run.

[MikeMcQuaid]

@alyssais You'll also need to reset homebrew/core and homebrew/versions repos to older versions and export HOMEBREW_NO_AUTO_UPDATE=1; brew reinstall --force-bottle gcc48 when on the old version to simulate being in the past. If you're able to reproduce with a master Homebrew/brew and older homebrew/core and homebrew/versions then let me know what revisions you git reset --hard to and I'll try to work on a fix. Thanks!

[alyssais]

Have gone to great lengths to try to reproduce this, even with the versions I was using when I saw it, but I can't 😞

[alyssais]

Okay, I think I've got this:

brew update
git -C "$(brew --repository)/Library/Taps/homebrew/homebrew-core" reset --hard b366b398
git -C "$(brew --repository)/Library/Taps/homebrew/homebrew-versions" reset --hard 07b7501
HOMEBREW_NO_AUTO_UPDATE=1 brew install gcc48
brew upgrade

[ilovezfs]

@MikeMcQuaid Yup, @alyssais's commands above do reproduce the problem.

[ilovezfs]

==> Renamed Formulae
antlr2 -> antlr@2                                  freetds091 -> freetds@0.91                         hdf4 -> hdf@4                                      phantomjs192 -> phantomjs@1.92
antlr3 -> antlr@3                                  gcc46 -> gcc@4.6                                   influxdb08 -> influxdb@0.8                         phantomjs198 -> phantomjs@1.98
bash-completion2 -> bash-completion@2              gcc47 -> gcc@4.7                                   jboss-as5 -> jboss-as@5                            play14 -> play@1.4
bazel02 -> bazel@0.2                               gcc5 -> gcc@5                                      jetty8 -> jetty@8                                  postgresql94 -> postgresql@9.4
berkeley-db4 -> berkeley-db@4                      giflib5 -> giflib@5                                jpeg6b -> jpeg@6                                   postgresql95 -> postgresql@9.5
bigdata -> blazegraph                              glfw2 -> glfw@2                                    jpeg9 -> jpeg@9                                    protobuf250 -> protobuf@2.5
bison27 -> bison@2.7                               gnupg21 -> gnupg@2.1                               juju2 -> juju                                      protobuf260 -> protobuf@2.6
boost-python159 -> boost-python@1.59               gnuplot4 -> gnuplot@4                              juju@2.0 -> juju                                   rebar3 -> rebar@3
boost155 -> boost@1.55                             go14 -> go@1.4                                     kafka080 -> kafka@0.80                             recipes -> gnome-recipes
boost157 -> boost@1.57                             go15 -> go@1.5                                     kibana41 -> kibana@4.1                             redis26 -> redis@2.6
boost159 -> boost@1.59                             go16 -> go@1.6                                     kibana44 -> kibana@4.4                             redis28 -> redis@2.8
boost160 -> boost@1.60                             gradle214 -> gradle@2.14                           kubernetes-cli13 -> kubernetes-cli@1.3             ruby187 -> ruby@1.8
cassandra21 -> cassandra@2.1                       grails25 -> grails@2.5                             lua51 -> lua@5.1 ✔                                 ruby193 -> ruby@1.9
cassandra22 -> cassandra@2.2                       gsl1 -> gsl@1                                      lua53 -> lua@5.3                                   ruby20 -> ruby@2.0
clang-format38 -> clang-format@3.8                 gst-ffmpeg010 -> gst-ffmpeg@0.10                   mariadb100 -> mariadb@10.0                         ruby21 -> ruby@2.1
docker111 -> docker@1.11                           gst-plugins-bad010 -> gst-plugins-bad@0.10         open-mpi16 -> open-mpi@1.6                         ruby22 -> ruby@2.2
docker171 -> docker@1.71                           gst-plugins-base010 -> gst-plugins-base@0.10       percona-server55 -> percona-server@5.5             ruby23 -> ruby@2.3
eigen32 -> eigen@3.2                               gst-plugins-good010 -> gst-plugins-good@0.10       percona-server56 -> percona-server@5.6             scala210 -> scala@2.10
erlang-r18 -> erlang@18                            gst-plugins-ugly010 -> gst-plugins-ugly@0.10       perl514 -> perl@5.14                               scala211 -> scala@2.11
ffmpeg28 -> ffmpeg@2.8                             gstreamer010 -> gstreamer@0.10                     perl518 -> perl@5.18

Note the gcc48 -> gcc@4.8 isn't present in the update report.

[ilovezfs]

OK so at those initial revisions, homebrew/core/gcc@4.8 and and homebrew/versions/gcc48 both exist, the rename is in the formula_renames file in core, and the migration is not in the tap_migrations file in versions. So my initial guess is that update_report is relying on the content of tap_migrations before the update rather than after.

[ilovezfs]

Nope, adding it to tap_migrations ahead of time does nothing, so quite broken indeed.

[ilovezfs]

OK, if I set core back to a revision before gcc@4.8 exists, then it works.

[ilovezfs]

If gcc48 and gcc@4.8 both exist at the time update runs, then a migration will not be performed. So as we've folded the non-core taps into homebrew/core, every time the import PR into core was merged before the deletion PR in the original tap, anyone who updated in between will not have been migrated. In most cases, the import PR and deletion PR occurred nearly simultaneously, but there were cases where they were separated by hours or days. Also, anyone who did any ill-timed updates via git fetch and git reset, or via git pull, instead of via brew update, is potentially affected regardless of our timing of the merges.

So the poor man's mitigation without changing any code, if you think you may have been impacted, is to run brew list | xargs -tn1 brew migrate and see if anything happens. In terms of code changes, we need to modify update so that it handles any migrations that were never attempted even though the target formulae already exist now.

The worst case scenario is that a user has installed both the source and target. This is entirely possible because if I had gcc48 installed, and it didn't get migrated to gcc@4.8 for me, then I could have chosen to install gcc@4.8 at any time since then. I think we need a doctor check to detect this case, which manifests as both the left side and right side of a formula rename existing at the same time, with both left and right side being directories. When both exist, but the left side is a symlink, it was a normal migration. If the left side is a directory not a symlink, then it's a double installation, or a failed migration, both of which would be problems doctor should report.

If/when #2359 ships, it will also be possible for a double installation to simply be a migration that hasn't been completed yet. Also, if that PR ships, double installations will be subject to migration attempts, possibly trying to move the exact same version on top of itself.

Ideally, we would also have something preventing double installations from ever happening by not allowing them to proceed, but currently there is nothing to prevent them regardless of whether the migration bug reported here gets fixed. Of course, they'll be far less likely once this is fixed, but they will still currently be possible either way.

[MikeMcQuaid]

I think I have a fix for this in #2370.

[vladshablinsky]

Expected Result
The upgrade completes successfully, migrating somepackage to somepackage4.

The expected result I suppose is at least MigrationNeededError here and it's quite strange it doesn't get thrown. Well, it shouldn't according to the code we have, but it's strange we don't have that check. A good thing to have is to perform that migration automatically however. I'm surprised this bug have been unnoticed so long.

[vladshablinsky]

gcc48 and gcc@4.8 both exist at the

What if we create a separate issue for that? This issue has nothing to do with brew upgrade misbehaviour stated in this issue title.

Speaking of the issue, it happens because if there is a formula gcc48 both in the tap and as an oldname, we'll resolve it from tap where it present if it exist and if it doesn't, then we'll try to resolve it from oldnames. https://github.com/Homebrew/brew/blob/master/Library/Homebrew/formulary.rb#L346-L370
Does this make any sense?