Skip upgrade of pinned dependency if it's outdated

[vladshablinsky]

• 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 tests with your changes locally?

---

Description:

A fix for #831 was introduced in #846 but the behaviour of install process should have been reconsidered anyway. This PR implements the ideas being discussed in #846.

Idea: Don't upgrade dependency of formula if this dependency is pinned while installing the formula.

Let's suppose A depends on B and then do the following:
      * install B with version 0.1.
      * pin B with version 0.1.
      * bump new version of B, e.g. 1.0
      * install A

    B shouldn't get upgraded while we installing A if it's pinned and
    installed with the proper options. This commit implements such
    behaviour.

TODO?

I'm not sure about two things here:

1. Do I need to add a test for the change. And if we need a test, then where should I put it. Possible variants are test_integration_cmds, test_formula_installer or test_dependency?
2. Do we need to repin formula B if it's pinned and installation of A that depends on B requires different options from ones being used in pinned keg of B, so we eventually upgrade/reinstall B, but don't repin it.

CC @MikeMcQuaid @ilovezfs

[MikeMcQuaid]

Do I need to add a test for the change. And if we need a test, then where should I put it. Possible variants are test_integration_cmds, test_formula_installer or test_dependency?

Ideally yes. Add it to test_formula_installer or test_dependency, whichever you think is best 👍

Do we need to repin formula B if it's pinned and installation of A that depends on B requires different options from ones being used in pinned keg of B, so we eventually upgrade/reinstall B, but don't repin it.

I think if it's pinned we should probably just refuse to do anything with it (reinstall or upgrade) and require the user to manually uninstall/unpin/upgrade to resolve it.

[MikeMcQuaid]

This is a bit different to what I think we want. Instead of considering this satisfied it should be considered unsatisfied, refuse to upgrade and print out a message telling the user to unpin.

[vladshablinsky]

Ah, I see, will change it this way, then and ask for the comments later.

[vladshablinsky]

@MikeMcQuaid how does the current implementation look?

[vladshablinsky]

TODO: remove.

[MikeMcQuaid]

I wonder if it's worth making this an onoe and using next so it'll print a warning for all of them rather than just the first?

[MikeMcQuaid]

A few small comments but I like the approach 👍

[vladshablinsky]

Changed the logic a bit, as we don't need to fail if dependency is statisfied? even if it's pinned.

[ilovezfs]

We don't? Since most things go through opt now I think we do indeed need to fail …

[vladshablinsky]

@ilovezfs if a pinned dependency is satisfied doesn't it mean that if we unpin it then it won't be reinstalled anyway?

https://github.com/Homebrew/brew/blob/master/Library/Homebrew/formula_installer.rb#L385-L386

[MikeMcQuaid]

Thanks again @vladshablinsky!