Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make brew install $ALIAS follow alias changes #567

Closed
MikeMcQuaid opened this issue Jul 21, 2016 · 25 comments
Closed

Make brew install $ALIAS follow alias changes #567

MikeMcQuaid opened this issue Jul 21, 2016 · 25 comments
Assignees
Labels
features New features
Milestone

Comments

@MikeMcQuaid
Copy link
Member

If brew install boost is an alias to brew install boost123 then if the boost alias changes from boost123 to boost124 then brew upgrade should upgrade between these versions.

This will allow us to use the Homebrew versions-style formula naming (or an alternative like @UniqMartin has suggested e.g. boost+1.2.3) combined with these aliases to allow us to have formula pinned to versions if they need to be and have users (and some formulae) follow the latest version of e.g. boost if they don't break on newer versions.

When this issue is done we'll have a larger discussion about a new approach to versioning in Homebrew.

@MikeMcQuaid MikeMcQuaid added the help wanted We want help addressing this label Jul 21, 2016
@MikeMcQuaid MikeMcQuaid self-assigned this Jul 21, 2016
@MikeMcQuaid MikeMcQuaid added help wanted We want help addressing this and removed help wanted We want help addressing this labels Jul 21, 2016
@MikeMcQuaid MikeMcQuaid removed their assignment Jul 21, 2016
@scpeters
Copy link
Member

Sounds good to me.

@MikeMcQuaid MikeMcQuaid modified the milestone: 1.0.0 Aug 11, 2016
@alyssais
Copy link
Contributor

Would this be a reasonable way to do this?

  1. Persist the alias used to install a formula in the INSTALL_RECEIPT
  2. When upgrading, if an alias is in the INSTALL_RECEIPT and the target of the alias has changed, install the new formulae the alias refers to (and store the alias in its INSTALL_RECEIPT too).

@MikeMcQuaid
Copy link
Member Author

@penman Yep, that's the plan! Please feel free to beat me to a PR though 😀

alyssais added a commit to alyssais/brew that referenced this issue Sep 3, 2016
@alyssais
Copy link
Contributor

alyssais commented Sep 3, 2016

If somebody had run brew install boost and got boost+1.2.3, then boost 1.2.4 was released they ran brew upgrade boost, they'd get boost+1.2.4 installed.

But what if they run brew upgrade boost+1.2.3? Seems to me that that should ignore that it was originally installed with just boost and install the latest version of the boost+1.2.3 formula, even if boost has been re-aliased to boost+1.2.4. If that's the case, should that re-write the INSTALL_RECEIPT so that future brew upgrades also stick to boost+1.2.3, or leave it alone?

@MikeMcQuaid
Copy link
Member Author

If somebody had run brew install boost and got boost+1.2.3, then boost 1.2.4 was released they ran brew upgrade boost, they'd get boost+1.2.4 installed.

Yep 👍

But what if they run brew upgrade boost+1.2.3? Seems to me that that should ignore that it was originally installed with just boost and install the latest version of the boost+1.2.3 formula, even if boost has been re-aliased to boost+1.2.4. If that's the case, should that re-write the INSTALL_RECEIPT so that future brew upgrades also stick to boost+1.2.3, or leave it alone?

I think brew upgrade boost+1.2.3 (although we're actually using boost@1.2.3 now) should probably be a no-op unless there's been a revision upgrade and we just treat it like a separate formula. I think this behaviour will occur as-is, though 👍

@alyssais
Copy link
Contributor

alyssais commented Sep 5, 2016

although we're actually using boost@1.2.3 now

👍🎉👍🎉👍🎉

(This was my preference but I didn't want to get into bikeshedding :P)

@alyssais
Copy link
Contributor

alyssais commented Sep 5, 2016

So it seems to me them at there are three different ways an aliased formula could be updated:

  • brew upgrade boost — should follow boost alias and install the new version
  • brew upgrade boost@1.2.3 — should not follow boost alias (even if it was installed with brew install boost), just check for changes in the boost@1.2.3 formula.
  • brew upgrade — should follow the alias iff the alias was originally used to install the formula

I'll check these off as I test them and get them working correctly.

@alyssais
Copy link
Contributor

alyssais commented Sep 5, 2016

Something that's come up as I've been trying to implement this is that there's no good way to find formula installed with a previous value for an alias after it's changed.

The only way I've found to check is to loop through Formula.installed and check the alias_path for all of them. I have a lot of formula installed and can't say I've noticed a performance hit when trying it this way, but it seems like a lot more work than necessary.

Any ideas?

@MikeMcQuaid
Copy link
Member Author

Looks good to me, @penman. Using Formula.installed seems fine for now; it doesn't matter too much if brew upgrade is a little sluggish as we scan all the installed formulae for their e.g. versions anyway.

@alyssais
Copy link
Contributor

alyssais commented Sep 5, 2016

This would affect brew upgrade boost (ie a upgrading a single formula) in addition to brew upgrade (all), but I think it'll be okay.

@MikeMcQuaid MikeMcQuaid added the features New features label Sep 11, 2016
@MikeMcQuaid
Copy link
Member Author

@penman How's this looking?

@alyssais
Copy link
Contributor

@MikeMcQuaid My computer and I haven't been getting on well this week, but I've made some good progress today.

@alyssais
Copy link
Contributor

alyssais commented Sep 14, 2016

If boost@1.2.3 was installed with brew install boost, then boost is changed to point to boost@1.2.4, what should another brew install boost do? Install boost@1.2.4 or error saying boost is already installed?

@MikeMcQuaid
Copy link
Member Author

I think either option is probably fine but saying boost is already installed and pointing them to brew upgrade feels nicer.

@alyssais
Copy link
Contributor

And in that situation, would brew outdated boost@1.2.3 report outdatedness or not?

@MikeMcQuaid
Copy link
Member Author

I think brew outdated boost would but not boost@1.2.3. I think basically if you specify something by its full version it should never "jump" to a newer version but if you specify just the alias (e.g. boost) then it should act like it does currently.

Worth nothing is that it won't actually be boost@1.2.3 but boost@1.2 and boost@1.3 which can both have upgrades e.g. from 1.2.1 to 1.2.2 and 1.3.1 to 1.3.2. Make sense?

@alyssais
Copy link
Contributor

Yeah, makes sense. Thanks.

@alyssais
Copy link
Contributor

Just about ready to put up a PR. One more question: how should brew outdated's JSON output be changed to handle alias changes? Here's what it's currently showing without me touching it when hello@2 is installed with brew install hello, and hello is then changed to target hello@3:

[
  {
    "name": "hello@2",
    "installed_versions": [
      "2.10"
    ],
    "current_version": "2.10"
  }
]

I guess we'll need a new JSON version, like brew outdated --json=v2, but should there be any changes to the v1 version as well? With this new logic for what's outdated, its current output doesn't really make sense.

@MikeMcQuaid
Copy link
Member Author

I think that makes sense for brew outdated hello@2's output but for brew outdated hello would ideally output:

[
  {
    "name": "hello",
    "installed_versions": [
      "2.10"
    ],
    "current_version": "2.10"
  }
]

or something, I think?

@alyssais
Copy link
Contributor

I was just worried that doing that could break a JSON consumer that didn't know how to handle Homebrew aliases. Is that something we need to worry about?

@alyssais
Copy link
Contributor

The ideal JSON (that would require a version bump), would look like this, I think:

[
  {
    "name": "hello",
    "installed": [
      {
        "formula": "hello@2",
        "version": "2.10"
      }
    ],
    "current": {
      "formula": "hello@3",
      "version": "3.0"
    }
  }
]

@MikeMcQuaid
Copy link
Member Author

Or alternatively we could just leave the JSON output as-is for now and consider iteration on it if the previous version causes problems. I think my format would be fine if we assume it's being used to decide whether to run e.g. brew upgrade on a given formula. Thoughts?

@alyssais
Copy link
Contributor

Okay, I'll just leave it for now and get a PR up, then we can take it from there.

@alyssais
Copy link
Contributor

This is done now, right?

@MikeMcQuaid
Copy link
Member Author

Yes 🎉! Well done @penman!

@ghost ghost assigned MikeMcQuaid Sep 20, 2016
@ghost ghost removed the help wanted We want help addressing this label Sep 20, 2016
@Homebrew Homebrew locked and limited conversation to collaborators May 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
features New features
Projects
None yet
Development

No branches or pull requests

3 participants