migrator: allow new cellar to exist already by ilovezfs · Pull Request #2359 · Homebrew/brew

ilovezfs · 2017-03-17T16:41:59Z

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?

Lets us migrate a formula to a name that may have previously been used.

If gnupg 1.x is installed as "gnupg" and gnupg 2.x is installed as
"gnupg2," it's currently not possible to rename gnupg2 -> gnupg, since
the 1.4 keg will already be installed in the gnupg cellar, so in order
to reclaim the name "gnupg" to be used for 2.1, either 1.x must be
manually uninstalled, or the new cellar needs to be allowed to exist
already.

The other choice here would be to continue using the name "gnupg2" for
the 2.1 series, and not reclaim the name "gnupg" from 1.x, which,
incidentally, would open up the possibility of a gnupg -> gnupg@1.4
rename.

MikeMcQuaid · 2017-03-18T13:41:08Z

CC @vladshablinsky for review and thoughts here.

@ilovezfs in cases like this I'd have thought the best thing would be to create a new gnupg@1 formula and upgrade the existing gnupg formula to be 2.X. Thoughts?

ilovezfs · 2017-03-18T13:50:16Z

That's what we're doing.

MikeMcQuaid · 2017-03-18T13:52:35Z

Ok, I reread and I think I understand now, thanks.

vladshablinsky · 2017-03-19T00:04:36Z

@ilovezfs do we really need that gnupg2 -> gnupg rename? I think I need some time to revise how all that things with versions work but something doesn't feel right for now. I think we can eventually end up with a rename that will probably break things, but I'm not sure. Will answer as soon as I can.

Also, I hope we'll allow any types of renames and deletions sooner or later so we're able to rename anything to anything if that't name is free at specific moment of time. So probably we need to be careful with complicating renames logic. Now when versioned formulae have arrived it got really more complicated than it used to be.

ilovezfs · 2017-03-19T11:03:29Z

@ilovezfs do we really need that gnupg2 -> gnupg rename?

If we want to upgrade all gnupg 1.0.x and 2.0.x users to 2.1.x, then yes.

Note that we also have been sitting on Homebrew/homebrew-core#8334 and not shipping it for three months for the same reason.

MikeMcQuaid · 2017-03-19T15:57:48Z

do we really need that gnupg2 -> gnupg rename?

I think if there are any known problems we should fix them but if there's none specifically known then it's fine to 🚢 this.

Note that we also have been sitting on Homebrew/homebrew-core#8334 and not shipping it for three months for the same reason.

Not quite right; it's blocked on #1770.

ilovezfs · 2017-03-19T15:59:18Z

Not quite right; it's blocked on #1770.

So does 1770 affect the gpg upgrades too then? Not sure I see a difference between the situations really.

ilovezfs · 2017-03-19T16:02:37Z

Not quite right; it's blocked on #1770.

Also, let me re-phrase then. Regardless of the disposition of #1770, which I did see existed, this PR also seems to be a requirement for the qt5 -> qt rename to proceed. Whether that had been realized heretofore or not isn't especially relevant.

MikeMcQuaid · 2017-03-19T16:09:10Z

So does 1770 affect the gpg upgrades too then?

I have no idea. I've still not been able to debug that problem yet.

ilovezfs · 2017-03-20T09:02:20Z

@vladshablinsky #1770 (comment)

vladshablinsky · 2017-03-20T23:36:46Z

@ilovezfs #1772 (comment)

vladshablinsky

The only concern I have is what if somehow we got a different formula installed under newname cellar? I think it can be possible if we allow multiple renames.

ilovezfs · 2017-03-20T23:47:06Z

@vladshablinsky I think it's link_newname's decision which of the kegs to link.

vladshablinsky · 2017-03-20T23:59:51Z

I think it's link_newname's decision which of the kegs to link.

@ilovezfs Also, let's suppose we have a non-core tap formula installed under newname and oldname -> newname core rename. What happens if we migrate oldname to newname allowing newname to exist while performing migration? Well, we'll have two different formulae sharing same name under the same Cellar. I can remember that when formula renames were implemented we only supposed that only one formula can be installed into one cellar subdirectory. So we at least need an extra check that installed formula is from the same tap as the formula we're trying to migrate. Thoughts?

Check existing newname to be from the same tap as oldname renamed to newname

ilovezfs · 2017-03-21T01:19:40Z

The tap migrations from homebrew/versions to homebrew/core all had a rename to the @ naming scheme, so I'm not sure we can require the source and destination taps to be the same.

vladshablinsky · 2017-03-21T01:51:36Z

The tap migrations from homebrew/versions to homebrew/core all had a rename to the @ naming scheme, so I'm not sure we can require the source and destination taps to be the same.

I might have confused you, what I meant were formula renames and migrations of renamed formula.

Let's migrate oldname2 from homebrew/version to homebrew/core, then we have
oldname2 installed and its tap is homebrew/core due to migration being performed.
Now we want to rename it to oldname@2 while Cellar/oldname@2 already exists. No problem, but if we want to perform a migration for this rename we need to be sure Cellar/oldname@2's kegs are homebrew/core's and not from some other taps that also provides oldname@2.
Am I wrong?

ilovezfs · 2017-03-21T02:07:28Z

Yes I think that makes sense. Even though there never was a formula named gcc48 in homebrew/core, I think a lookup of the tap of a not-yet-renamed-to-gcc@4.8 gcc48 keg originally installed from homebrew/versions would return homebrew/core.

ilovezfs · 2017-03-22T02:35:12Z

@vladshablinsky Yes, you are right. It looks like the from_same_taps? test doesn't consider the tap of a pre-existing new-name cellar. I'm wondering if we should tolerate that situation and defer the migration until the conflicting cellar goes away, or print a manual tap migration error of some kind, or something else.

MikeMcQuaid · 2017-03-22T09:23:04Z

defer the migration until the conflicting cellar goes away

When would that happen?

vladshablinsky · 2017-03-22T11:55:27Z

defer the migration until the conflicting cellar goes away

When would that happen?

Probably when user eventually uninstalls it.

it looks like the from_same_taps? test doesn't consider the tap of a pre-existing new-name cellar.

@ilovezfs even though what you say is true, please notice that this is not the case we're discussing.

from_same_taps? checks if oldname installed is from the same tap as Formula for oldname however the case we're discussing is if Formula for newname (for which we migrate its old Cellar) is from the same tap as newname installed.

UPD: probably you meant same things I said, though.

MikeMcQuaid · 2017-03-22T21:17:21Z

Probably when user eventually uninstalls it.

I'm wondering if we should uninstall it for the user in a case like this (or at least prompt them to do so before migration?).

vladshablinsky · 2017-03-23T10:52:04Z

I'm wondering if we should uninstall it for the user in a case like this (or at least prompt them to do so before migration?).

IMO, migration must be impossible unless --force given. With force we should either uninstall it and perform migration or merge several formulae's kegs under Cellar/newname.

👍 for prompting.

Lets us migrate a formula to a name that may have previously been used. If gnupg 1.x is installed as "gnupg" and gnupg 2.x is installed as "gnupg2," it's currently not possible to rename gnupg2 -> gnupg, since the 1.4 keg will already be installed in the "gnupg" Cellar, so in order to reclaim the name "gnupg" to be used for 2.1, either 1.x must be manually uninstalled, or the new cellar needs to be allowed to exist already.

ilovezfs · 2017-03-23T11:11:28Z

PR refreshed.

ilovezfs mentioned this pull request Mar 17, 2017

gnupg 2.1.19, gnupg@2.0 (new formula), gnupg@1.4 (new formula) Homebrew/homebrew-core#11083

Closed

4 tasks

ilovezfs force-pushed the migrator-allow-new-cellar-to-exist-already branch from a4eb7c5 to 9f9a2de Compare March 17, 2017 17:40

MikeMcQuaid requested a review from vladshablinsky March 18, 2017 13:40

ilovezfs mentioned this pull request Mar 20, 2017

brew upgrade misbehaves when a formula's dependency is renamed #1770

Closed

3 tasks

vladshablinsky mentioned this pull request Mar 20, 2017

upgrade: unlink kegs of renamed formulae #1772

Closed

5 tasks

vladshablinsky previously approved these changes Mar 20, 2017

View reviewed changes

vladshablinsky reviewed Mar 20, 2017

View reviewed changes

ilovezfs force-pushed the migrator-allow-new-cellar-to-exist-already branch from 9f9a2de to 845d083 Compare March 23, 2017 11:10

MikeMcQuaid approved these changes Mar 23, 2017

View reviewed changes

ilovezfs merged commit 54abadb into Homebrew:master Mar 23, 2017

Homebrew locked and limited conversation to collaborators May 3, 2018

Uh oh!

Conversation

ilovezfs commented Mar 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MikeMcQuaid commented Mar 18, 2017

Uh oh!

ilovezfs commented Mar 18, 2017

Uh oh!

MikeMcQuaid commented Mar 18, 2017

Uh oh!

vladshablinsky commented Mar 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ilovezfs commented Mar 19, 2017

Uh oh!

MikeMcQuaid commented Mar 19, 2017

Uh oh!

ilovezfs commented Mar 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ilovezfs commented Mar 19, 2017

Uh oh!

MikeMcQuaid commented Mar 19, 2017

Uh oh!

ilovezfs commented Mar 20, 2017

Uh oh!

vladshablinsky commented Mar 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vladshablinsky left a comment

Choose a reason for hiding this comment

Uh oh!

ilovezfs commented Mar 20, 2017

Uh oh!

vladshablinsky commented Mar 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ilovezfs commented Mar 21, 2017

Uh oh!

vladshablinsky commented Mar 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ilovezfs commented Mar 21, 2017

Uh oh!

ilovezfs commented Mar 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MikeMcQuaid commented Mar 22, 2017

Uh oh!

vladshablinsky commented Mar 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MikeMcQuaid commented Mar 22, 2017

Uh oh!

vladshablinsky commented Mar 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ilovezfs commented Mar 23, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ilovezfs commented Mar 17, 2017 •

edited

Loading

vladshablinsky commented Mar 19, 2017 •

edited

Loading

ilovezfs commented Mar 19, 2017 •

edited

Loading

vladshablinsky commented Mar 20, 2017 •

edited

Loading

vladshablinsky commented Mar 20, 2017 •

edited

Loading

vladshablinsky commented Mar 21, 2017 •

edited

Loading

ilovezfs commented Mar 22, 2017 •

edited

Loading

vladshablinsky commented Mar 22, 2017 •

edited

Loading

vladshablinsky commented Mar 23, 2017 •

edited

Loading