Add `brew link --HEAD` #11397

cnnrmnn · 2021-05-17T16:53:40Z

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?

Currently, if the head and stable versions of a formula are both installed, it isn't possible to link the head version of the formula. This PR adds the --HEAD flag to link the head version of a formula.

I had to revise the link command to use to_formulae_and_casks(only: :formula, method: :kegs) rather than to_kegs (which calls to_formulae_and_casks(only: :formula, method: :keg) to get both the head and stable kegs. I may have inadvertently created an issue if multiple non-head kegs can be installed. My modifications make the link command attempt to link any non-head kegs unless --HEAD is given.

I would appreciate it if a maintainer weighed in on this.

cnnrmnn · 2021-05-17T17:20:05Z

typecheck is currently failing because to_formulae_and_casks doesn't return an array of kegs like to_kegs does. Going to wait to fix this until someone weighs in on issue I outlined in previous message.

Library/Homebrew/cmd/link.rb

MikeMcQuaid · 2021-05-18T10:53:58Z

Nice work again so far @cnnrmnn!

I may have inadvertently created an issue if multiple non-head kegs can be installed

Yes, you can have multiple non-head kegs installed 😭. Let me know if you need a hand with how to progress.

cnnrmnn · 2021-05-18T11:48:17Z

I may have inadvertently created an issue if multiple non-head kegs can be installed

Yes, you can have multiple non-head kegs installed 😭. Let me know if you need a hand with how to progress.

Do you know why to_kegs is named to_kegs despite using method: :keg when calling to_formulae_and_casks? I'm considering defining a similar method that uses method: :kegs to correct the current type issue.

MikeMcQuaid · 2021-05-18T12:01:54Z

Do you know why to_kegs is named to_kegs despite using method: :keg when calling to_formulae_and_casks? I'm considering defining a similar method that uses method: :kegs to correct the current type issue.

My guess is just it's pluralised like to_formulae_and_casks is. I agree the naming is a little confusing in this case and would be fine with it being changed.

Co-authored-by: Mike McQuaid <mike@mikemcquaid.com>

cnnrmnn · 2021-05-18T14:04:15Z

My guess is just it's pluralised like to_formulae_and_casks is. I agree the naming is a little confusing in this case and would be fine with it being changed.

It was called to_kegs because it converted each of the named arguments to kegs (same reason as to_formulae_and_casks, as you suggested).

Library/Homebrew/cmd/link.rb

cnnrmnn · 2021-05-18T15:12:25Z

The code doesn't work in its current state. Main issue is that brew link <formula> (without --HEAD) will link the head version of the formula if

It's the only installed version of the formula. This may or may not be behavior we want.
If it is the most recently installed version of the formula and is thus linked in HOMEBREW_PREFIX/opt and HOMEBREW_LINKED_KEGS. This is obviously undesirable since it leaves no way to link a non-head version of the formula if the head version of the formula is the most recently installed version of the formula.

Why do both HOMEBREW_PREFIX/opt and HOMEBREW_LINKED_KEGS both exist? They seem to mirror each other.

MikeMcQuaid · 2021-05-18T15:22:32Z

It's the only installed version of the formula. This may or may not be behavior we want.

I think this is desirable 👍🏻

2. If it is the most recently installed version of the formula and is thus linked in HOMEBREW_PREFIX/opt and HOMEBREW_LINKED_KEGS. This is obviously undesirable since it leaves no way to link a non-head version of the formula if the head version of the formula is the most recently installed version of the formula.

I think this would be OK, actually, we could output a message staging that brew unlink is required first.

Why do both HOMEBREW_PREFIX/opt and HOMEBREW_LINKED_KEGS both exist? They seem to mirror each other.

opt links should always be present for any installed formula. HOMEBREW_LINKED_KEGS indicates a formula has been brew linked (either by default on installation or through brew link --force afterwards).

cnnrmnn · 2021-05-18T15:35:46Z

Another strange behavior I noticed is that installing the head version of a formula uninstalls any non-head versions of that formula that are installed. However, installing a non-head version of a formula does not uninstall the head version of the formula if it's installed.

Also, if there are multiple non-head versions of a formula installed, there's no way to control which one gets linked by brew link.

cnnrmnn · 2021-05-18T15:46:03Z

I think this would be OK, actually, we could output a message staging that brew unlink is required first.

brew unlink wouldn't solve the problem. If the head version of a formula is the most recently installed version, it is linked in HOMEBREW_PREFIX/opt and HOMEBREW_LINKED_KEGS. Calling brew unlink unlinks it in HOMEBREW_LINKED_KEGS, but not in HOMEBREW_PREFIX/opt. to_keg (called when using brew link without --HEAD), uses the link in HOMEBREW_PREFIX/opt to get the keg, so calling brew link and brew unlink repeatedly links and unlinks the head version of the formula.

Example (formula is initially unlinked):

cnnrmnn · 2021-05-18T15:53:12Z

I think the main question here is if the link in HOMEBREW_PREFIX can't be used to determine which version of the formula should be linked by brew link, what should we use?

Also, if there are multiple non-head versions of a formula installed, there's no way to control which one gets linked by brew link.

I think the answer should ideally address this.

Perhaps a version flag should be added to brew link so that user can optionally choose the version to be linked. Default could be linking highest installed non-head version of the formula or the head version of the formula if no non-head versions are installed.

Library/Homebrew/cli/named_args.rb

Library/Homebrew/test/cli/named_args_spec.rb

Library/Homebrew/dev-cmd/linkage.rb

Library/Homebrew/cmd/list.rb

Rylan12 · 2021-05-19T16:29:55Z

Library/Homebrew/cli/named_args.rb

@@ -98,8 +98,8 @@ def load_formula_or_cask(name, only: nil, method: nil)
              Formulary.factory(name, *spec, force_bottle: @force_bottle, flags: @flags)
            when :resolve
              resolve_formula(name)
-            when :keg
-              resolve_keg(name)
+            when :default_kegs


@MikeMcQuaid is this going to be a backward-incompatible change? It looks like Homebrew::CLI::NamedArgs is in the private API but Homebrew::CLI::Args#named is a public method that returns a NamedArgs array. I wonder, then, if the @api private label here is incorrect.

In fact, I realized that I'm using args.named.to_formulae in a few taps, not realizing it wasn't a public method.

I think this is. Ideally we'd allow but deprecate (on the next minor version e.g. add a commented out # odeprecated) the existing naming.

@MikeMcQuaid should the @api level be changed?

Rylan12 · 2021-05-19T16:38:35Z

Another strange behavior I noticed is that installing the head version of a formula uninstalls any non-head versions of that formula that are installed. However, installing a non-head version of a formula does not uninstall the head version of the formula if it's installed.

I don't know if there's a reason for this but I can't think of one off the top of my head. It seems to me that what a user would expect when installing is that whatever version they've most recently installed is the one that's linked. I don't see why installing a stable version shouldn't wipe the HEAD version but installing HEAD does wipe the stable. If anything, I would think it should be the opposite because a new stable version might now match the HEAD version.

Perhaps a version flag should be added to brew link so that user can optionally choose the version to be linked. Default could be linking highest installed non-head version of the formula or the head version of the formula if no non-head versions are installed.

In what situation are there two versions installed, other than a HEAD and a stable version? I don't know of a way to cause this to happen when it isn't a mistake, but I guess I could be missing something.

cnnrmnn · 2021-05-19T17:33:04Z

In what situation are there two versions installed, other than a HEAD and a stable version? I don't know of a way to cause this to happen when it isn't a mistake, but I guess I could be missing something.

I can't think of any off of the top of my head, but @MikeMcQuaid mentioned earlier in this PR that it is possible to have multiple non-head kegs installed:

Yes, you can have multiple non-head kegs installed 😭. Let me know if you need a hand with how to progress.

Rylan12 · 2021-05-19T19:56:20Z

I guess my thinking is that even if it's possible for there to be multiple we don't need to support doing anything with the non-(head-or-latest-version) keg.

So I think just autodetecting the latest version is the way to go; no need to add any new flags. Same goes for brew list in my opinion

Let's hear from Mike and see if he has a different opinion.

Library/Homebrew/cli/named_args.rb

Library/Homebrew/test/cli/named_args_spec.rb

Co-authored-by: Rylan Polster <rslpolster@gmail.com>

cnnrmnn · 2021-05-20T17:05:58Z

My recent changes make brew link <formula> (no --HEAD) link the non-head keg with the most recent version or the head keg if there are no installed non-head kegs.

Library/Homebrew/cli/named_args.rb

Library/Homebrew/test/cli/named_args_spec.rb

Co-authored-by: Mike McQuaid <mike@mikemcquaid.com>

cnnrmnn · 2021-05-21T16:01:28Z

syntax is currently failing because style doesn't like duplicate branch bodies with :default_kegs and to-be-deprecated :keg.

Library/Homebrew/cmd/unlink.rb

Library/Homebrew/cli/named_args.rb

Library/Homebrew/test/cli/named_args_spec.rb

Library/Homebrew/cmd/link.rb

Rylan12

Just one suggestion and then I think this will be good

Library/Homebrew/cmd/link.rb

Rylan12

Looks good to me!

Thanks again for the great work, @cnnrmnn!

Connor Mann added 2 commits May 17, 2021 11:28

Add brew link --HEAD

8de9baa

Update usage message

212b648

MikeMcQuaid reviewed May 18, 2021

View reviewed changes

Library/Homebrew/cmd/link.rb Outdated Show resolved Hide resolved

cnnrmnn and others added 3 commits May 18, 2021 09:35

Update Library/Homebrew/cmd/link.rb

f92300f

Co-authored-by: Mike McQuaid <mike@mikemcquaid.com>

Rename to_kegs to to_keg

d46881c

Update tests for to_keg

d754c0f

Add to_kegs

15dcc6f

MikeMcQuaid reviewed May 18, 2021

View reviewed changes

Library/Homebrew/cmd/link.rb Outdated Show resolved Hide resolved

Switch to using to_keg and to_kegs

f23d4d4

Rylan12 reviewed May 18, 2021

View reviewed changes

Library/Homebrew/cli/named_args.rb Outdated Show resolved Hide resolved

Connor Mann added 3 commits May 19, 2021 09:34

Rename to_keg to to_default_kegs

fe9a9bb

Rename method :keg to :default_kegs

137914d

Rename resolve_keg to resolve_default_keg

49aa057

Rylan12 reviewed May 19, 2021

View reviewed changes

Rename missed to_keg

2bfdbc6

Connor Mann added 2 commits May 19, 2021 13:58

Fix style

3637924

Add to_kegs test

08bbb95

Deprecate method: :keg

26a3218

Rylan12 reviewed May 20, 2021

View reviewed changes

cnnrmnn and others added 6 commits May 20, 2021 11:30

Update Library/Homebrew/cli/named_args.rb

b1ce7ef

Co-authored-by: Rylan Polster <rslpolster@gmail.com>

Update to_default_kegs tests

2938b27

Update to_kegs tests

cad6654

Update Library/Homebrew/test/cli/named_args_spec.rb

0938500

Co-authored-by: Rylan Polster <rslpolster@gmail.com>

Add to_latest_kegs

fa9a525

Change brew link to use to_latest_kegs

b72f5ef

Connor Mann added 3 commits May 20, 2021 15:10

Rename memoized variable

6f005ca

Warn if no HEAD keg installed w/ --HEAD

e977f35

Start catching all nil kegs

074face

MikeMcQuaid reviewed May 21, 2021

View reviewed changes

Library/Homebrew/cli/named_args.rb Show resolved Hide resolved

Library/Homebrew/cli/named_args.rb Outdated Show resolved Hide resolved

Library/Homebrew/test/cli/named_args_spec.rb Show resolved Hide resolved

Connor Mann and others added 3 commits May 21, 2021 11:52

Fix tests

c18c931

Update Library/Homebrew/cli/named_args.rb

3b02439

Co-authored-by: Mike McQuaid <mike@mikemcquaid.com>

Update Library/Homebrew/cli/named_args.rb

73ef297

Co-authored-by: Mike McQuaid <mike@mikemcquaid.com>

Rylan12 reviewed May 23, 2021

View reviewed changes

Library/Homebrew/cmd/unlink.rb Show resolved Hide resolved

Library/Homebrew/cli/named_args.rb Outdated Show resolved Hide resolved

Library/Homebrew/test/cli/named_args_spec.rb Show resolved Hide resolved

Library/Homebrew/cmd/link.rb Outdated Show resolved Hide resolved

Connor Mann added 4 commits May 23, 2021 16:09

Add tests for to_latest_kegs

b81a522

Fix style

0a16142

Fix to_kegs test

d50b323

Change kegs map to array

ef4c401

Rylan12 reviewed May 25, 2021

View reviewed changes

Library/Homebrew/cmd/link.rb Outdated Show resolved Hide resolved

Library/Homebrew/cmd/link.rb Outdated Show resolved Hide resolved

Clean up and fix typecheck issue

074b2d6

Rylan12 approved these changes May 26, 2021

View reviewed changes

Rylan12 merged commit d0f5a08 into Homebrew:master May 26, 2021

cnnrmnn deleted the add-link-head branch May 26, 2021 15:48

Rylan12 mentioned this pull request May 26, 2021

Don't uninstall stable keg with --HEAD #11438

Merged

7 tasks

github-actions bot added the outdated PR was locked due to age label Jun 26, 2021

github-actions bot locked as resolved and limited conversation to collaborators Jun 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `brew link --HEAD` #11397

Add `brew link --HEAD` #11397

cnnrmnn commented May 17, 2021 •

edited

cnnrmnn commented May 17, 2021

MikeMcQuaid commented May 18, 2021

cnnrmnn commented May 18, 2021

MikeMcQuaid commented May 18, 2021

cnnrmnn commented May 18, 2021

cnnrmnn commented May 18, 2021 •

edited

MikeMcQuaid commented May 18, 2021

cnnrmnn commented May 18, 2021

cnnrmnn commented May 18, 2021 •

edited

cnnrmnn commented May 18, 2021 •

edited

Rylan12 May 19, 2021

MikeMcQuaid May 20, 2021

Rylan12 May 20, 2021

Rylan12 commented May 19, 2021

cnnrmnn commented May 19, 2021 •

edited

Rylan12 commented May 19, 2021

cnnrmnn commented May 20, 2021

cnnrmnn commented May 21, 2021

Rylan12 left a comment

Rylan12 left a comment

Add brew link --HEAD #11397

Add brew link --HEAD #11397

Conversation

cnnrmnn commented May 17, 2021 • edited

cnnrmnn commented May 17, 2021

MikeMcQuaid commented May 18, 2021

cnnrmnn commented May 18, 2021

MikeMcQuaid commented May 18, 2021

cnnrmnn commented May 18, 2021

cnnrmnn commented May 18, 2021 • edited

MikeMcQuaid commented May 18, 2021

cnnrmnn commented May 18, 2021

cnnrmnn commented May 18, 2021 • edited

cnnrmnn commented May 18, 2021 • edited

Rylan12 May 19, 2021

Choose a reason for hiding this comment

MikeMcQuaid May 20, 2021

Choose a reason for hiding this comment

Rylan12 May 20, 2021

Choose a reason for hiding this comment

Rylan12 commented May 19, 2021

cnnrmnn commented May 19, 2021 • edited

Rylan12 commented May 19, 2021

cnnrmnn commented May 20, 2021

cnnrmnn commented May 21, 2021

Rylan12 left a comment

Choose a reason for hiding this comment

Rylan12 left a comment

Choose a reason for hiding this comment

Add `brew link --HEAD` #11397

Add `brew link --HEAD` #11397

cnnrmnn commented May 17, 2021 •

edited

cnnrmnn commented May 18, 2021 •

edited

cnnrmnn commented May 18, 2021 •

edited

cnnrmnn commented May 18, 2021 •

edited

cnnrmnn commented May 19, 2021 •

edited