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 pinning more robust #3748

Merged
merged 1 commit into from Feb 5, 2018

Conversation

Projects
None yet
3 participants
@MikeMcQuaid
Copy link
Member

MikeMcQuaid commented Jan 29, 2018

Don’t autoremove pins on uninstall or upgrade and note this in the manpage.

@@ -46,6 +46,15 @@ def uninstall
rm_pin rack
else
kegs.each do |keg|
begin
if Formulary.from_rack(rack).pinned?
onoe "#{f.full_name} is pinned. You must unpin it to uninstall (without --force)."

This comment has been minimized.

@ilovezfs

ilovezfs Jan 29, 2018

Contributor

The f is undefined here so it ends up in rescue nil

@ilovezfs

This comment has been minimized.

Copy link
Contributor

ilovezfs commented Jan 29, 2018

I'm having trouble reproducing the original issue without first restoring the default_formula stuff. It looks like Homebrew/homebrew-core@a04030a is kicking in. Are you still able to trigger it?

This is still probably a good idea, though, since there are other path ways to what is likely unintended removal of pinned formulae (e.g. brew bundle --force cleanup)

@MikeMcQuaid MikeMcQuaid force-pushed the MikeMcQuaid:pin-more-robust branch from e3be516 to 413afe2 Jan 29, 2018

@MikeMcQuaid MikeMcQuaid force-pushed the MikeMcQuaid:pin-more-robust branch from 413afe2 to 52b4714 Feb 5, 2018

Make pinning more robust
Don’t autoremove pins on uninstall or upgrade and note this in the
manpage.

@MikeMcQuaid MikeMcQuaid force-pushed the MikeMcQuaid:pin-more-robust branch from 52b4714 to 3a2e6b8 Feb 5, 2018

@MikeMcQuaid MikeMcQuaid merged commit af8f8f1 into Homebrew:master Feb 5, 2018

1 of 3 checks passed

codecov/patch 40% of diff hit (target 70.31%)
Details
codecov/project 70.29% (-0.03%) compared to 62f85cf
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@MikeMcQuaid MikeMcQuaid deleted the MikeMcQuaid:pin-more-robust branch Feb 5, 2018

@chdiza

This comment has been minimized.

Copy link
Contributor

chdiza commented Feb 6, 2018

Maybe I'm missing something, but I don't see where, in the man page, it newly describes "Don’t autoremove pins on uninstall". I only see that it will prevent an upgrade.

@ilovezfs

This comment has been minimized.

Copy link
Contributor

ilovezfs commented Feb 6, 2018

Every single behavior of every single command isn't documented in the man page. The messaging is clear so I'm not sure what that would add.

@chdiza

This comment has been minimized.

Copy link
Contributor

chdiza commented Feb 6, 2018

Yeah, but the commit message says it was added, even though it isn't.

@ilovezfs

This comment has been minimized.

Copy link
Contributor

ilovezfs commented Feb 6, 2018

The references to the possibility of upgrade blowing through pins were removed, which is what that was referring to.

@chdiza

This comment has been minimized.

Copy link
Contributor

chdiza commented Feb 6, 2018

which is what that was referring to.

That's only part of what the commit message referred to.

on uninstall

That part of the commit message is not mentioned in the manpage.

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.