cmd/pin: Update pinned formula messaging

[apainintheneck]

• 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?

---

Resolves #16212

The hope is that this will be clearer and less annoying for users.

A user came to us a couple weeks ago stating that it was confusing that the brew upgrade command printed an error when a pinned formula had a new version available and didn't get upgraded. Not upgrading a package is the expected result of pinning it.

This PR changes that message to a warning from an error. While looking into this we found that there is another message that gets printed when a package dependent doesn't get upgraded because it is pinned and that got turned into a warning from a normal message. Honestly that should be more worrying for the user anyway; it could lead to a program not working correctly in the worst case scenario.

I also added a message to the brew pin command warning about potential unintended behavior if a dependency gets pinned and another package requires a newer version of it.

Lastly, I added a commented out deprecation notice for the brew upgrade --ignore-pinned command since it's now essentially the default.

[MikeMcQuaid]

Looks good so far, thanks! My questions are:

1. does this indicates if a specific formula has not been upgraded because of the brew pin of a dependency?
2. does brew upgrade foo fail/error if foo is pinned?
3. same as 2) but if one of foo's dependencies are pinned.

[apainintheneck]

1. does this indicates if a specific formula has not been upgraded because of the brew pin of a dependency?

3. same as 2) but if one of foo's dependencies are pinned.

The answer to 1 and 3 is essentially the same.

If one of foo's dependencies is pinned and the current version is too old, the install will raise an error in the FormulaInstaller#check_install_sanity method (great method name by the way).

brew/Library/Homebrew/formula_installer.rb

Lines 348 to 352 in ab6a7ed

	return if pinned_unsatisfied_deps.empty?
	raise CannotInstallFormulaError,
	"You must `brew unpin #{pinned_unsatisfied_deps * " "}` as installing " \
	"#{formula.full_name} requires the latest version of pinned dependencies"

This error gets caught, printed and then we skip trying to install that formula. That seems to be the case when installing or upgrading formulae so it's even more complex than I originally thought.

brew/Library/Homebrew/install.rb

Lines 280 to 282 in ab6a7ed

	rescue CannotInstallFormulaError => e
	ofail e.message
	nil

brew/Library/Homebrew/upgrade.rb

Lines 110 to 112 in ab6a7ed

	rescue CannotInstallFormulaError => e
	ofail e
	nil

2. does brew upgrade foo fail/error if foo is pinned?

No, it would print a warning. This used to be an error message (ofail) and is the message that was brought up in the original issue. That means it won't return a non-zero exit code either anymore.

[MikeMcQuaid]

Great, 1 and 3 sound right 👍🏻

No, it would print a warning. This used to be an error message (ofail) and is the message that was brought up in the original issue. That means it won't return a non-zero exit code either anymore.

I feel like brew upgrade shouldn't print an error or have non-zero exit code but brew upgrade foo should because you've specifically requested an upgrade of foo, one is available but you've pinned it. Thoughts?

[apainintheneck]

I feel like brew upgrade shouldn't print an error or have non-zero exit code but brew upgrade foo should because you've specifically requested an upgrade of foo, one is available but you've pinned it. Thoughts?

That sounds reasonable. In that case, should we still deprecate the --ingore-pinned option? This change probably removes the need for that option for most uses cases but not all.

[MikeMcQuaid]

In that case, should we still deprecate the --ignore-pinned option?

I don't feel strongly either way. brew upgrade foo --ignore-pinned when foo is pinned feels like a pretty weird invocation. If there's other cases outside of that: I'm happy to leave it. Could always leave it for now and see if we end up iterating post-merge based on more feedback?

[apainintheneck]

I don't feel strongly either way. brew upgrade foo --ignore-pinned when foo is pinned feels like a pretty weird invocation.

Me thoughts exactly. I'll leave the deprecation in.

[MikeMcQuaid]

Thanks for all the work here @apainintheneck and sorry for the amount of back and forth needed!

[Moulick]

In that case, should we still deprecate the --ignore-pinned option?

I don't feel strongly either way. brew upgrade foo --ignore-pinned when foo is pinned feels like a pretty weird invocation. If there's other cases outside of that: I'm happy to leave it. Could always leave it for now and see if we end up iterating post-merge based on more feedback?

Does feel weird, the one case that comes to mind is that if foo is pinned and you want to upgrade foo only this time but leave it pinned for the next time, without having to unpin, upgrade, re-pin

[apainintheneck]

Does feel weird, the one case that comes to mind is that if foo is pinned and you want to upgrade foo only this time but leave it pinned for the next time, without having to unpin, upgrade, re-pin

@Moulick I don't believe that was possible before this change either. This strictly changes messaging not how brew install behaves when a formula is pinned.

[Moulick]

Does feel weird, the one case that comes to mind is that if foo is pinned and you want to upgrade foo only this time but leave it pinned for the next time, without having to unpin, upgrade, re-pin

@Moulick I don't believe that was possible before this change either. This strictly changes messaging not how brew install behaves when a formula is pinned.

Sure, I get that this is just a message change, not arguing that fact at all 😄. From a power users' perspective, looking at the command brew upgrade foo --ignore-pinned, logically I'd think it would upgrade foo ignoring the fact that it is pinned or not, even though it does not do that. Anyways, I don't think we really need it anyways, it's more of a convenience that would help if someone wanted to upgrade without having to unpin, but for now a user can unpin, upgrade, re-pin if they so wanted. (Like I do for a couple of dependencies) ¯\_(ツ)_/¯