Don't uninstall stable keg with --HEAD

[cnnrmnn]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew typecheck with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

At the moment, installing the head version of a formula with brew install --HEAD uninstalls the stable version of the formula if it is installed. This is obviously undesirable if you want to have the head and stable versions of a formula installed simultaneously.

This behavior seemed odd when I first discovered it, but it's actually the product of brew install uninstalling any kegs that have older versions than the one being installed. The stable keg is obviously older than the head keg, so it's uninstalled.

This PR changes brew install so that it doesn't do any clean-up if --HEAD is given.

[Rylan12]

I may be backtracking a bit on what I said in #11397 but I wonder if what needs to be fixed is that installing a stable version doesn't remove the HEAD version.

I think that's more consistent with what we do elsewhere in Homebrew (i.e. installing a formula removes the other versions unless HOMEBREW_NO_INSTALL_CLEANUP is set).

Is there a specific reason you have in mind for this?

[cnnrmnn]

Is there a specific reason you have in mind for this?

It would make it easier to have head and stable versions installed simultaneously. I think that this is a more common use case than just keeping older kegs when a new keg is installed (think people who keep "stable" and "nightly" builds installed simultaneously). I like the idea of being able to have head and stable kegs installed without keeping every old keg as would happen if HOMEBREW_NO_INSTALL_CLEANUP were set. I don't think having this as a default does any harm or is particularly confusing.

I think that's more consistent with what we do elsewhere in Homebrew (i.e. installing a formula removes the other versions unless HOMEBREW_NO_INSTALL_CLEANUP is set).

That is true, but I think that the inconsistency could be warranted. Also, installing a formula only removes older versions unless HOMEBREW_NO_INSTALL_CLEANUP is set.

I may be backtracking a bit on what I said in #11397 but I wonder if what needs to be fixed is that installing a stable version doesn't remove the HEAD version.

This is a fair option as well. The use case I'm describing may be rare enough where it makes sense to require users to set HOMEBREW_NO_INSTALL_CLEANUP.

[cnnrmnn]

I'm happy to make changes if you'd like to go in the other direction.

[Rylan12]

Yeah, I see you're point. I guess I'm not sure what the best thing is. Let's get some feedback from other maintainers.

[carlocab]

I'd find this useful. I also view HOMEBREW_NO_INSTALL_CLEANUP as doing something different: setting it means you don't want brew cleanup to remove outdated formula installs. Having HEAD installed doesn't make stable outdated. (cf. brew outdated --fetch-HEAD foo when the stable version of foo is installed)

On the other hand, we've removed support for switching between arbitrary versions of formulae on purpose, and perhaps some of the reasons we did that apply here.

[Rylan12]

In that case, I'm fine with this change.

To me, switching between arbitrary version does feel different from switching between a head and stable version, so I think this change is fine.

I think, though, that the change should probably be made to the Formula#eligible_kegs_for_cleanup method rather than just conditionally running Cleanup::install_formula_clean!. The latter also does some other cleanup that I think we still want to occur (e.g. Cleanup#cleanup_lockfiles)

[cnnrmnn]

I think, though, that the change should probably be made to the Formula#eligible_kegs_for_cleanup method rather than just conditionally running Cleanup::install_formula_clean!. The latter also does some other cleanup that I think we still want to occur (e.g. Cleanup#cleanup_lockfiles)

Good point, my mistake. I've pushed a fix.

Formula#eligible_kegs_for_cleanup now returns all installed kegs except the newest head keg (compared only with other head kegs) and the newest non-head keg (compared only with other non-head kegs).

[Rylan12]

Looks good to me, thanks @cnnrmnn!

[Rylan12]

Hmm, how do you re-link the stable version once you've installed a HEAD version?

If I check out this PR and install a stable version, then install the head version, then try to link the stable version with brew link foo, it links the HEAD version.

[carlocab]

Just to be sure:

With this PR, if I have the stable version of foo installed, I should be able to install the HEAD version with brew unlink foo && brew install --HEAD foo, correct?

What about if I have HEAD installed, and want the stable one? Would brew unlink foo && brew install foo also work?

[Rylan12]

Okay, I think the issue I was having was that this PR was created before #11397 was merged. Here's what I did to test (after rebasing the PR). I used htop because it has a head block and doesn't take too long to build:

$ brew install htop # install stable version

$ ls $(brew --cellar htop)
3.0.5

$ brew unlink htop
Unlinking /usr/local/Cellar/htop/3.0.5... 5 symlinks removed.

$ brew install --HEAD htop # install HEAD version

$ ls $(brew --cellar htop)
3.0.5        HEAD-c752c54

$ brew unlink htop
Unlinking /usr/local/Cellar/htop/HEAD-c752c54... 5 symlinks removed.

$ brew link htop
Linking /usr/local/Cellar/htop/3.0.5... 5 symlinks created.

$ brew unlink htop
Unlinking /usr/local/Cellar/htop/3.0.5... 5 symlinks removed.

$ brew link --HEAD htop
Linking /usr/local/Cellar/htop/HEAD-c752c54... 5 symlinks created.

$ brew uninstall --force htop # uninstall everything

$ brew install --HEAD htop # try installing HEAD first this time

$ ls $(brew --cellar htop)
HEAD-c752c54

$ brew unlink htop
Unlinking /usr/local/Cellar/htop/HEAD-c752c54... 5 symlinks removed.

$ brew install htop # links stable version

$ ls $(brew --cellar htop)
3.0.5        HEAD-c752c54

$ brew unlink htop
Unlinking /usr/local/Cellar/htop/3.0.5... 5 symlinks removed.

$ brew link htop
Linking /usr/local/Cellar/htop/3.0.5... 5 symlinks created.

$ brew unlink htop
Unlinking /usr/local/Cellar/htop/3.0.5... 5 symlinks removed.

$ brew link --HEAD htop
Linking /usr/local/Cellar/htop/HEAD-c752c54... 5 symlinks created.

Everything there looks good to me. Does that help, @carlocab? Is there something else to test?

[Rylan12]

@cnnrmnn can you rebase this PR onto the latest master branch? I don't expect there will be any issues but because it goes alongside #11397 I'd like to make sure they both get through CI together

[cnnrmnn]

Did I do that incorrectly? I had to force push it. I ran git rebase master on no-uninstall-stable. Would it have been better to just merge master into no-uninstall-stable?

[carlocab]

You did fine. git rebase master (or git rebase origin/master) is definitely what you want here. git merge will create a merge commit, and we don't want one of those until a PR is merged.

[Rylan12]

Did I do that incorrectly? I had to force push it. I ran git rebase master on no-uninstall-stable. Would it have been better to just merge master into no-uninstall-stable?

You're good! You do need to force push after a rebase because you're modifying the history of the commits on the PR branch rather than just adding new ones. Here's a quick article about rebase in case you're curious: https://www.atlassian.com/git/tutorials/rewriting-history/git-rebase or https://www.atlassian.com/git/tutorials/merging-vs-rebasing

[carlocab]

I think this is a regression:

❯ brew upgrade --fetch-HEAD neovim
...
==> Summary
🍺  /usr/local/Cellar/neovim/HEAD-3cd688f_2: 1,542 files, 21.2MB, built in 2 minutes 36 seconds
Warning: Skipping (old) /usr/local/Cellar/neovim/HEAD-3cd688f_2 due to it being linked
❯ brew info neovim
neovim: stable 0.4.4 (bottled), HEAD
Ambitious Vim-fork focused on extensibility and agility
https://neovim.io/
/usr/local/Cellar/neovim/HEAD-3cd688f_2 (1,542 files, 21.2MB) *
  Built from source on 2021-05-31 at 14:06:26
/usr/local/Cellar/neovim/HEAD-43956de_2 (1,542 files, 21.2MB)
  Built from source on 2021-05-29 at 13:24:08

It seems if I have an older HEAD version installed, it will no longer be unlinked automatically when I do brew upgrade.

[Rylan12]

Is there an easy way to determine which HEAD keg is the "latest" one? I wonder if that's where the problem is coming from. Haven't looked into it. Are there easy reproduction steps for what you're seeing, @carlocab, or do I need to simulate an old HEAD keg to see this behavior?

[carlocab]

Is there an easy way to determine which HEAD keg is the "latest" one?

Not that I know of... I know brew outdated --fetch-HEAD always knows which one.

Are there easy reproduction steps for what you're seeing, @carlocab, or do I need to simulate an old HEAD keg to see this behavior?

Simulating an old HEAD might be the only way here. It might also be enough to rename the directory with the version/commit hash to something that isn't the latest commit on the default branch.

[Rylan12]

I think I've got it. PR opened: #11475

[cnnrmnn]

Thanks for the fix, @Rylan12! Sorry I couldn't get to it myself; I've been traveling.