Freeze installed formulae. #18515

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
6 participants
Contributor

urdh commented Mar 16, 2013

See mxcl/homebrew#18386.

  • Pinning/unpinning formulae
  • Pinned formulae don't upgrade
  • Forcing upgrade of pinned formulae by named argument to upgrade
  • Documentation of the pinning feature
  • Unpin formulae on uninstall
  • Move pin symlink to new version on upgrade (or: symlink to `opt/´)
Contributor

urdh commented Mar 16, 2013

Would keeping the code inside Formula instead of delegating to a separate class be a better approach?

Contributor

jacknagel commented Mar 16, 2013

installed_version is not broken, it's installed_prefix that is weird, but it can't be changed because so much code relies on the existing behavior of installed?.

Contributor

jacknagel commented Mar 16, 2013

If there is a "frozen" symlink directly inside the rack, every part of Homebrew will think that the version "frozen" is installed; which is bad. It needs to be placed in some other location.

Contributor

jacknagel commented Mar 16, 2013

Would keeping the code inside Formula instead of delegating to a separate class be a better approach?

No, we definitely want to keep this code outside of Formula, which is far too big as it is. Probably just add a "freezer" (or something) method that memoizes its return value.

Contributor

urdh commented Mar 17, 2013

Is there a standard directory in which Homebrew stores miscellaneous information, or is "YAML file in the Cellar" an acceptable solution?

Contributor

jacknagel commented Mar 17, 2013

I would think we we would still want it to be a symlinked directory rather than serialized data, but outside of the cellar hierarchy. We have Library/LinkedKegs for example.

We should also think a bit about the terminology here, for one thing, we shouldn't use "freeze" as a method name because that masks Object#freeze.

Owner

MikeMcQuaid commented Mar 18, 2013

'Hold' or 'pin' are other names I've seen used for this process in other package managers.

Contributor

urdh commented Mar 18, 2013

Made some changes:

  • Terminology: pin/unpin instead of freeze/thaw.
  • Pinning information is stored in HOMEBREW_LIBRARY/PinnedKegs instead of the cellar.
  • FormulaPin takes a Formula instead of a formula name, so there's no need to create new Formula objects inside.
Contributor

sheerun commented Mar 20, 2013

I think lock / unlock is the easiest to remember.

Contributor

adamv commented Mar 20, 2013

I like pin/unpin over lock/unlock.

Owner

MikeMcQuaid commented Mar 20, 2013

+1 for pin

@MikeMcQuaid MikeMcQuaid commented on the diff Mar 22, 2013

Library/Homebrew/cmd/pin.rb
@@ -0,0 +1,16 @@
+require 'formula'
+
+module Homebrew extend self
+ def pin
+ if Process.uid.zero? and not File.stat(HOMEBREW_BREW_FILE).uid.zero?
+ abort "Cowardly refusing to `sudo pin'"
+ end
+ raise FormulaUnspecifiedError if ARGV.named.empty?
+ ARGV.formulae.each do |fmla|
@MikeMcQuaid

MikeMcQuaid Mar 22, 2013

Owner

I'd just use formula instead of fmla. No big deal though.

@MikeMcQuaid MikeMcQuaid commented on the diff Mar 22, 2013

Library/Homebrew/cmd/upgrade.rb
outdated = Homebrew.outdated_brews
else
+ upgrade_pinned = true
@MikeMcQuaid

MikeMcQuaid Mar 22, 2013

Owner

This behaviour seems reasonable to me; don't upgrade unless you specify the package by name, right? @samueljohn @mxcl @mistydemeo @adamv @Sharpie @jacknagel Thoughts?

@samueljohn

samueljohn Mar 22, 2013

Contributor

I like that brew upgrade skips pinned and explicit brew upgrade x upgrades x, even if pinned.

Owner

MikeMcQuaid commented Mar 22, 2013

Patch looks great to me. Probably one of the most thorough pull-requests I've seen in a while (including my own). Well done. @jacknagel If/when you're good with this I am too.

Contributor

samueljohn commented Mar 22, 2013

Have you tested this with formulae from taps?
Why does brew list --pinned return possibly multiple versions for a keg? Can I pin more than one or did I misread the code?

I'd love to have an API to return the pinned kegs, so I could use that in an update to the coloring of brew list. I could highlight pinned kegs.

Contributor

urdh commented Mar 22, 2013

No, only one version can be pinned. The code from brew list --pinned is just a dumb copy of brew list --versions with if formula.pinned? appended. Only showing the actual pinned version (or no version at all) is probably better; I should fix that.

Not sure what you mean by API. Formula#pinned? will help with filtering out pinned formulae.

Contributor

samueljohn commented Mar 22, 2013

Overall really good work!

Alright, then please remove the code where you versions*' ' - if it is not necessary for pinned. At least I was confused.

For my mxcl#14465 (comment) work, I could (later) extend it to highlight pinned stuff, too. But for that to work, I would need list_pinnend that returns a list and does not yet puts out the stuff. Basically it would be enough if, instead of puts, it returns the generated list.

Contributor

urdh commented Mar 22, 2013

I've updated the commit with a new list_pinned. I made it print all pinned formulae, but it should be easy to adapt to #14465 by simply removing the last each block of the method.

I tested it a bit with taps, and it seems to work with tapped formulae as well.

Contributor

jacknagel commented Mar 22, 2013

I think it looks pretty good, there isn't anything too intrusive. At first I wondered if it should do anything with kegs that are not backed by a formula (i.e. custom installs), but by definition they can't be upgraded so that shouldn't matter.

Lets wait a couple of days and make sure we've considered all of the other parts of Homebrew that might interact with it. (For starters, how about brew cleanup?)

urdh added some commits Mar 11, 2013

@urdh urdh Pin installed formulae. 6cfd08d
@urdh urdh Added `pin` et. al. to manpage.
* Added `brew pin` to `brew.1`
* Added `brew unpin` to `brew.1`
* Added `brew list --pinned` to `brew.1`
* Added information about frozen formulae to `brew upgrade` in `brew.1`
0663142
@urdh urdh Added `pin` et.al. to completion scripts. 3f8682c
Contributor

urdh commented Mar 26, 2013

Made some minor changes, rebasing on master and fixing a bug in the update.rb changes. Tested with cleanup and it seems to work as expected (i.e. doesn't remove still installed pinned formulae, does remove versions older than the newest one installed). Are there more command that could break?

Contributor

jacknagel commented Mar 26, 2013

If I have version 1 of something installed and pinned, then I unlink it and install version 2, with 1 still pinned, and run brew cleanup, what happens?

Contributor

urdh commented Mar 26, 2013

Version 1 will be removed. Formulae are not pinned at specific versions, they are pinned at the most recent version installed, and omitted when upgrading.

It will leave a broken symlink in PinnedKegs, however. That might not be ideal, but since the symlink does nothing except signal by name which formulae are pinned, it has no real effect. It could be solved by doing unpin, pin (repin?) after forcibly upgrading a pinned formula.

Come to think of it, this goes for uninstalled pinned formulae as well, i.e. if you brew rm a pinned formula, it will leave broken symlinks in PinnedKegs. So remove.rb needs a fix as well. I'll have a look at it.

Contributor

samueljohn commented Mar 26, 2013

@urdh without much thinking (of me): what if the symlink for PinnedKeks points to the opt/<formula> instead? That one does not change by going from version 1 to 2.

@urdh urdh Fixes to the pinning mechanism
* Unpin formulae when uninstalling them
* Unpin and re-pin formulae when upgrading (avoids stale symlink)
5a3b12f
Contributor

urdh commented Mar 27, 2013

Upgrading a formula now unpins and re-pins the formula so that the symlink in PinnedKegs points to the correct keg. Uninstalling a formula will unpin it. I'll squash the latest commit if the changes look reasonable.

@samueljohn that's also a good idea, but it discards information (what version was the formula pinned at). Granted, that information is useless now given how these changes work, so maybe it doesn't matter? What's the general consensus on that matter?

Owner

MikeMcQuaid commented Mar 30, 2013

Looks good to me. @jacknagel ?

Contributor

jacknagel commented Mar 30, 2013

Let's merge it and reserve the right to change the semantics a bit (whether it should pin versions, etc.) depending on how it works out.

Owner

MikeMcQuaid commented Mar 30, 2013

Pushed. Thanks and well done @urdh!

urdh deleted the unknown repository branch Mar 31, 2013

urdh referenced this pull request Mar 31, 2013

Closed

Fixes to `brew-pin`. #18858

@Sharpie Sharpie pushed a commit to Sharpie/homebrew that referenced this pull request Apr 7, 2013

@urdh @MikeMcQuaid urdh + MikeMcQuaid brew-pin: prevent selected formulae from upgrade.
* Added `pin` et. al. to manpage.
* Added `brew pin` to `brew.1` * Added `brew unpin` to `brew.1`
* Added `brew list --pinned` to `brew.1`
* Added information about frozen formulae to `brew upgrade` in `brew.1`
* Added `pin` et.al. to completion scripts.
* Unpin formulae when uninstalling them
* Unpin and re-pin formulae when upgrading (avoids stale symlink)

References #18386.
Closes #18515.

Signed-off-by: Mike McQuaid <mike@mikemcquaid.com>
1e76317

@nesv nesv added a commit to nesv/homebrew that referenced this pull request Apr 12, 2013

@urdh @nesv urdh + nesv brew-pin: prevent selected formulae from upgrade.
* Added `pin` et. al. to manpage.
* Added `brew pin` to `brew.1` * Added `brew unpin` to `brew.1`
* Added `brew list --pinned` to `brew.1`
* Added information about frozen formulae to `brew upgrade` in `brew.1`
* Added `pin` et.al. to completion scripts.
* Unpin formulae when uninstalling them
* Unpin and re-pin formulae when upgrading (avoids stale symlink)

References #18386.
Closes #18515.

Signed-off-by: Mike McQuaid <mike@mikemcquaid.com>
77c6378

xu-cheng locked and limited conversation to collaborators Feb 16, 2016

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