brew icon indicating copy to clipboard operation
brew copied to clipboard

Keep formulas unlinked when reinstalling/upgrading [WIP]

Open alexreg opened this issue 3 years ago • 18 comments

@carlocab recently indicated that it would be desirable to have upgrades of unlinked formulas stay unlinked after the upgrade. I believe this makes sense for reinstalls too. This PR is an attempt to implement that behaviour.

I'd be happy to write a test if someone could confirm this (very straightforward) approach is sensible.

  • [x] Have you followed the guidelines in our Contributing document?
  • [x] Have you checked to ensure there aren't other open Pull Requests for the same change?
  • [x] 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.
  • [x] Have you successfully run brew style with your changes locally?
  • [x] Have you successfully run brew typecheck with your changes locally?
  • [x] Have you successfully run brew tests with your changes locally?

alexreg avatar Aug 22 '22 00:08 alexreg

For context: the idea was for brew reinstall and/or brew upgrade to avoid undoing a previous user-requested brew unlink.

Discussion of that here: https://github.com/Homebrew/discussions/discussions/3555#discussioncomment-3439381

carlocab avatar Aug 22 '22 13:08 carlocab

For context: the idea was for brew reinstall and/or brew upgrade to avoid undoing a previous user-requested brew unlink.

Discussion of that here: Homebrew/discussions#3555 (comment)

Will reply in discussion 👍🏻

MikeMcQuaid avatar Aug 22 '22 13:08 MikeMcQuaid

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

github-actions[bot] avatar Sep 13 '22 00:09 github-actions[bot]

Any update on which approach you wish to take, @MikeMcQuaid? Or whether this PR or part of it is desirable in any case?

alexreg avatar Sep 13 '22 18:09 alexreg

Any update on which approach you wish to take, @MikeMcQuaid? Or whether this PR or part of it is desirable in any case?

I don't think this PR is desirable, I'm afraid. I think we need to instead fix the reasons people want individual non-keg-only formulae to stay unlinked rather than allow it.

MikeMcQuaid avatar Sep 14 '22 11:09 MikeMcQuaid

@MikeMcQuaid Fair enough. Now, what's the process for adopting a solution like the one you suggested?

alexreg avatar Sep 14 '22 23:09 alexreg

I think we need to instead fix the reasons people want individual non-keg-only formulae to stay unlinked rather than allow it.

One common reason is conflicts.

One example we get multiple issues/PRs/complaints about is terraform/tfenv/terragrunt. Many users want to use terragrunt with tfenv. That should work, except that terragrunt declares depends_on "terraform", and tfenv declares conflicts_with "terraform" (and vice-versa), so one cannot just do

brew install tfenv terragrunt

even though they work perfectly well together outside of brew.

carlocab avatar Sep 15 '22 06:09 carlocab

@MikeMcQuaid Fair enough. Now, what's the process for adopting a solution like the one you suggested?

@alexreg Someone opening some pull requests, I guess, or proposing a solution in an issue for discussion before beginning implementation.

One example we get multiple issues/PRs/complaints about is terraform/tfenv/terragrunt. Many users want to use terragrunt with tfenv. That should work, except that terragrunt declares depends_on "terraform", and tfenv declares conflicts_with "terraform" (and vice-versa), so one cannot just do

brew install tfenv terragrunt

even though they work perfectly well together outside of brew.

This feels like something solved by either:

  • make things keg-only so they don't conflict and brew link is sticky
  • remove binaries/symlinks so they don't conflict
  • use the gnubin like approach to require a manual path addition to avoid conflicts

MikeMcQuaid avatar Sep 15 '22 08:09 MikeMcQuaid

@MikeMcQuaid I'm tempted to say we should fix any formulas that depend on other formulas actually being linked... but that could end up being a mammoth task potentially. Mind you, without that guarantee, I don't see how (even with your DSL approach) you could avoid the situation where installing a new formula causes another formula to unlink, and thus a breakage somewhere in the dependency graph.

alexreg avatar Sep 15 '22 22:09 alexreg

I'm tempted to say we should fix any formulas that depend on other formulas actually being linked... but that could end up being a mammoth task potentially.

Agreed and agreed.

you could avoid the situation where installing a new formula causes another formula to unlink, and thus a breakage somewhere in the dependency graph.

A potential breakage for a small number of clashing formula seems preferable to doing this globally without testing/verification.

Even then, though, we have several other, better ways of handling conflicts as I mentioned in my previous comment. These are generally best handles in the formula or cask level.

MikeMcQuaid avatar Sep 16 '22 07:09 MikeMcQuaid

This feels like something solved by either:

  • make things keg-only so they don't conflict and brew link is sticky

  • remove binaries/symlinks so they don't conflict

  • use the gnubin like approach to require a manual path addition to avoid conflicts

Alternatively, we could also try out removing the terraform dependency from terragrunt. The build doesn't need it, and the test we run doesn't even require terraform.

I also like the first option above, though -- it's not too far away from making brew unlink sticky.

carlocab avatar Sep 16 '22 08:09 carlocab

I also like the first option above, though -- it's not too far away from making brew unlink sticky.

Note: the "brew link is sticky" bit is already true.

Alternatively, we could also try out removing the terraform dependency from terragrunt. The build doesn't need it, and the test we run doesn't even require terraform.

👍🏻

MikeMcQuaid avatar Sep 16 '22 08:09 MikeMcQuaid

Note: the "brew link is sticky" bit is already true.

I thought it would be; that's good to know.

Alternatively, we could also try out removing the terraform dependency from terragrunt. The build doesn't need it, and the test we run doesn't even require terraform.

👍🏻

Homebrew/homebrew-core#110879

carlocab avatar Sep 16 '22 09:09 carlocab

@MikeMcQuaid That's fair enough.

Here's a new proposal, that I think is slightly more conservative and less surprising. Imagine a workflow like the following.

  1. User installs formula X. It and all its dependencies get linked.
  2. User tries to install formula Y that conflicts with X or some dependency thereof. User gets an error (current behaviour), but also a notice that he can specify option '--unlinked` to install it without linking.
  3. User tries to install formula Y again, but this time specifies the option --unlinked, so it succeeds. The installation of that formula is marked as "explicitly unlinked" and sticky in that respect.
  4. User tries to install formula Z, which depends on formula Y (directly or indirectly). This fails because it is assumed that formula Z requires Y to be linked (even though hopefully this isn't true), and Y is "explicitly unlinked".
  5. User tries to install formula Z, this time with some sort of "force" option that ignores that Y is "explicitly unlinked". User gets a warning about a possible issue with Y not being linked, but installation and linkage of Z succeeds.
  6. User uninstalls formula X.
  7. User tries to install formula W, which depends on formula Y. Homebrew recognises that Y no longer conflicts with any installed formula, and proceeds by unmarking it as "explicitly unlinked" and linking it (as I believe would happen currently with any unlinked dependency of the formula being installed). The installation of Z is then carried out successfully. Note that it was not necessary to specify any "force" option this time, when installing a dependent of Y.

As far as I can tell, this does not change the current semantics unless an additional option it specified. It nevertheless allows conflicting formula to be installed side by side, at the expense of one or two new options to install and perhaps link. A little extra metadata needs to be kept around. Moving forwards, there would need to be a CI check for PRs that a new or manually updated formula does not require any of its dependencies to be linked.

An even safer approach would be to implement the above but make the warning a hard error in the case when Z requires Y to be linked, and in parallel make the warning disappear when Z does not require Y to be linked. For existing formula, I imagine that a one-off run could generate the metadata for such linkage requirements and mark those formulas accordingly. This could either be done on a per-dependency level or a coarse-grained level of "this formula does (not) require all dependencies to be linked".

alexreg avatar Sep 20 '22 01:09 alexreg

3. User tries to install formula Y again, but this time specifies the option --unlinked, so it succeeds. The installation of that formula is marked as "explicitly unlinked" and sticky in that respect.

I don't think we should make unlinked formula sticky, full stop.

5. User tries to install formula Z, this time with some sort of "force" option that ignores that Y is "explicitly unlinked". User gets a warning about a possible issue with Y not being linked, but installation and linkage of Z succeeds.

This feels very messy.


As above: this is an issue that should be resolved in formulae/casks so they do not conflict/overwrite each other.

MikeMcQuaid avatar Sep 20 '22 17:09 MikeMcQuaid

@MikeMcQuaid I can't say I agree about it being messy. Seems fairly straightforward. Anyway, if you prefer to resolve it within formulas/casks, how do you handle two formulas/casks that are direct alternatives for each other, e.g. mono-mdk and mono-mdk-for-visual-studio? Some users may need one or the other to be linked (it may vary) but both installed, for the sake of dependents.

alexreg avatar Sep 20 '22 18:09 alexreg

Some users may need one or the other to be linked (it may vary) but both installed, for the sake of dependents.

Why would you need two casks to be installed for dependents?

For formulae: we should fix the dependency tree if you need both installed.

For casks: I am suggesting we should fix the cask or formula to handle these link failures more gracefully.

MikeMcQuaid avatar Sep 21 '22 09:09 MikeMcQuaid

@MikeMcQuaid Because I need mono-mdk-for-visual-studio as a user for Visual Studio for Mac, but I also want to use a cask that depends on a mono-mdk`.

alexreg avatar Sep 21 '22 13:09 alexreg

Passing on this, sorry.

MikeMcQuaid avatar Sep 27 '22 10:09 MikeMcQuaid

@MikeMcQuaid No worries. Did you have an idea how to deal with the situation of my previous comment? Since I don't think your proposal handles that one.

alexreg avatar Sep 27 '22 14:09 alexreg

@alexreg They should not both link the same files in the same places.

MikeMcQuaid avatar Sep 27 '22 14:09 MikeMcQuaid

@MikeMcQuaid The two casks are simply different version of the same software (Mono MDK). One guarantees compatibility with Visual Studio for Mac. So it doesn't make sense for them to be installed in different locations, and indeed that would cause all sorts of problems anyway. This is basically why I wanted a custom dependency for Mono, despite the issue of build determinism. At least unofficial support.

alexreg avatar Sep 27 '22 18:09 alexreg

@alexreg Again: installing them both at once should not try to both link the same files in the same places.

Arguably, they should not even be allowed to be both installed at once. Formulae in a similar situation would be marked as conflicting.

If we do want them both installed at once: one of them should not install the symlinks.

None of this requires changes in Homebrew/brew.

MikeMcQuaid avatar Sep 28 '22 13:09 MikeMcQuaid

@MikeMcQuaid It may be possible to install the two casks in different places, though given they're the literally same software, just different versions, I would reiterate that it's awfully odd.

They can't be installed at the same time now, but the issue is the above-described one: some formulas depend on mono-mdk, but many people need mono-mdk-for-visual-studio installed and accessible.

alexreg avatar Sep 28 '22 15:09 alexreg

@alexreg These casks are:

  1. already marked as conflicting
  2. literally the same thing: they have the same version and URL

MikeMcQuaid avatar Sep 30 '22 13:09 MikeMcQuaid

Indeed, and that's precisely the problem when it comes to dependencies. Oh well, I'll have to figure out my own solution here, I suppose.

alexreg avatar Sep 30 '22 14:09 alexreg