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

Implement a "pip upgrade" command #3194

Closed
wants to merge 13 commits into
base: develop
from

Conversation

Projects
None yet
9 participants
@jkseppan

jkseppan commented Oct 18, 2015

Upgrade those requirements given directly but dependencies only as needed to satisfy dependency relations. Inspired by Nathaniel Smith's post at http://thread.gmane.org/gmane.comp.python.scientific.user/36377.

See also: #59

Review on Reviewable

jkseppan added some commits Oct 18, 2015

Implement a "pip upgrade" command
Upgrade those requirements given directly but dependencies only as
needed to satisfy dependency relations. Inspired by Nathaniel Smith's
post at http://thread.gmane.org/gmane.comp.python.scientific.user/36377

See also: #59
@njsmith

This comment has been minimized.

Member

njsmith commented Oct 19, 2015

You might want to deprecate the -U / --upgrade option to install as well?

@dstufft

This comment has been minimized.

Member

dstufft commented Oct 19, 2015

We'll want to deprecate -U / --upgrade yes, and I think we'll also want to add a --recursive option to pip upgrade to get the old behavior back.

@jkseppan

This comment has been minimized.

jkseppan commented Oct 19, 2015

It seems that --upgrade does two things: it causes all dependencies to be upgraded, and it requests overwriting existing packages when used with --target. Should the latter be enabled by some different option?

@njsmith

This comment has been minimized.

Member

njsmith commented Oct 19, 2015

@dstufft: I'm dubious about this --recursive option. The operations of "please give me the latest version of these specific packages" and "please give me the latest version of everything" -- those are sensible concepts with clear use cases. ("Oo I just read about some shiny new feature I want" and "let's get up to date", respectively.) The operation of "please give me the latest version of this specific package, plus all of its transitive dependencies, but not anything else" OTOH makes no sense to me, and isn't included in any other package system I'm aware of. What's the use case? Why does this make more sense than, say, "please give me the latest version of all packages whose name starts with 'l'"?

And of course -U will still be there during the deprecation period if anyone does turn out to desperately want this for some reason, so waiting for a use case to appear is low risk.

@qwcode

This comment has been minimized.

Contributor

qwcode commented Oct 19, 2015

I'm dubious about this --recursive option
What's the use case?

let's say I'm using the pyramid framework, and I want the latest version, and also all the bugfixes for all the dependencies.

upgrading pyramid alone will update dependencies when there's a change to the lower bound (or upper bound) constraints, but it won't naturally bring in minor/patch releases otherwise.

P.S.: just want to state clearly that pip install -U does honor upper bound constraints for dependencies if they exists (at least as best as it can given the limitations it has related to #988, see next P.S.), so it's not a completely aggressive upgrade to the latest version.

P.S.: the lack of proper resolution (#988) does make recursive upgrades more dangerous than they should be, but it's not the fault of the "recursion" itself, but rather that pip does not properly consider the constraints of the existing environment, and also does not properly compile constraints as it discovers them in the resolution process.

@rgommers

This comment has been minimized.

rgommers commented Oct 19, 2015

Thanks for working on this @jkseppan!

There's some text in docs/user_guide.rst about pip install --upgrade and the plan to implement pip upgrade that needs adapting.

@njsmith

This comment has been minimized.

Member

njsmith commented Oct 19, 2015

let's say I'm using the pyramid framework, and I want the latest version, and also all the bugfixes for all the dependencies.

In what circumstance would I specifically want to get all bug fixes in whatever packages happen to be in pyramid's dependency chain, but avoid getting bug fixes in other packages? To be clear, I'm assuming we will eventually have an upgrade --all, and I'm asking for use cases where someone would look at --recursive and --all and choose --recursive.

(AFAICT the only reason -U works the way it does is that in the old easy_install view of the world, upgrade --all doesn't make sense -- both because there's no resolver, and because before virtualenv won, the model was that you'd have multiple versions of the same package in a single global environment, so what does "all" mean? So -U was designed as a poor man's --all.)

@qwcode

This comment has been minimized.

Contributor

qwcode commented Oct 19, 2015

use cases where someone would look at --recursive and --all and choose --recursive.

why would someone look at --all and <one package> and just choose the single package? because right or wrong, they're focused on updates in one library (and ignoring the rest of the environment).. and if we're willing to give the user that ability, then it seems very natural to me to allow them to upgrade all the dependencies that support that one library.

@njsmith

This comment has been minimized.

Member

njsmith commented Oct 20, 2015

why would someone look at --all and <one package> and just choose the single package?

Ahh, maybe this is the confusion, because this case was so obvious to me that I didn't think it needed explaining :-). (Which either means that I have some relevant insight, or some special misconception ;-).)

The use case for pip upgrade <some package> is very concrete and clear -- you use it when there is a specific feature or bug fix that you have in mind and want to get. E.g., right now I am waiting eagerly for scipy 0.16.1 to be released, because 0.16.0 has a nasty bug that breaks part of the functionality of a library I maintain. Or it's pretty common that I'll be writing some code using, say, pandas, check the online docs to find a nice clean way to do what I want, and then discover that it doesn't work because my local version is too old. When this happens, what I want is a way to quickly say "give me <bugfix X> or <feature Y>", and I want to do this with minimal disturbance to the rest of environment, because I'm in the middle of code writing mode, not validate new releases and hunt down weird little compatibility bugs mode. This is exactly what a non-recursive (but still dependency-satisfying) upgrade command does.

I mean, if you want to support --recursive then I guess I don't see the harm either. I just feel like if I were in your position maintaining something like pip, then I'd be eager to avoid adding new features with no more concrete use case than "well, it's possible" :-).

@qwcode

This comment has been minimized.

Contributor

qwcode commented Oct 20, 2015

because this case was so obvious to me that I didn't think it needed explaining

I understand the case of wanting a specific fix : )

But I also think people think in terms of "let me get the latest fixes" generally for some library (and it's dependencies).

if we already had a safe "upgrade-all", then I'd be more inclined to forget about it.

I guess this is a case, where I'll go along with the majority opinion, because I can't claim to know the mind of "most people" that I'm trying to represent here.

@njsmith

This comment has been minimized.

Member

njsmith commented Oct 20, 2015

I guess my suggestion would be to deprecate -U/--upgrade without
immediately providing a replacement UI like --recursive, BUT to plan to
leave -U/--upgrade available until upgrade-all was implemented and it was
clear that there were no howls of protest from users asking for
--recursive? I'm a big fan of deprecation as a way to force users to speak
up when nothing else works :-)

On Mon, Oct 19, 2015 at 5:51 PM, Marcus Smith notifications@github.com
wrote:

because this case was so obvious to me that I didn't think it needed
explaining

I understand the case of wanting a specific fix : )

But I also think people think in terms of "let me get the latest fixes"
generally for some library (and it's dependencies).

if we already had a safe "upgrade-all", then I'd be more inclined to
forget about it.

I guess this is a case, where I'll go along with the majority opinion,
because I can't claim to know the mind of "most people" that I'm trying to
represent here.


Reply to this email directly or view it on GitHub
#3194 (comment).

Nathaniel J. Smith -- http://vorpus.org

@dstufft

This comment has been minimized.

Member

dstufft commented Oct 20, 2015

As an tangent. I'm a fan of upgrade -all instead of upgrade-all

Sent from my iPhone

On Oct 19, 2015, at 9:00 PM, Nathaniel J. Smith notifications@github.com wrote:

I guess my suggestion would be to deprecate -U/--upgrade without
immediately providing a replacement UI like --recursive, BUT to plan to
leave -U/--upgrade available until upgrade-all was implemented and it was
clear that there were no howls of protest from users asking for
--recursive? I'm a big fan of deprecation as a way to force users to speak
up when nothing else works :-)

On Mon, Oct 19, 2015 at 5:51 PM, Marcus Smith notifications@github.com
wrote:

because this case was so obvious to me that I didn't think it needed
explaining

I understand the case of wanting a specific fix : )

But I also think people think in terms of "let me get the latest fixes"
generally for some library (and it's dependencies).

if we already had a safe "upgrade-all", then I'd be more inclined to
forget about it.

I guess this is a case, where I'll go along with the majority opinion,
because I can't claim to know the mind of "most people" that I'm trying to
represent here.


Reply to this email directly or view it on GitHub
#3194 (comment).

Nathaniel J. Smith -- http://vorpus.org

Reply to this email directly or view it on GitHub.

name = 'install'
summary = 'Install packages.'
upgrade_option = True
upgrade_direct = False

This comment has been minimized.

@qwcode

qwcode Oct 20, 2015

Contributor

this may be bikeshedding, but this "upgrade_direct" phrase is pretty meaningless to me.

I'd prefer "upgrade_recursive", which will be True for the InstallCommand, and False for the UpgradeCommand

@@ -255,7 +241,8 @@ def run(self, options, args):
build_dir=build_dir,
src_dir=options.src_dir,
download_dir=options.download_dir,
upgrade=options.upgrade,
upgrade=self.upgrade_option and options.upgrade,
upgrade_direct=self.upgrade_direct,
as_egg=options.as_egg,

This comment has been minimized.

@qwcode

qwcode Oct 20, 2015

Contributor

again, maybe bikeshedding, here, but I want the RequirementSet properties to be meaningful for the long term

what we're doing here is keeping the general word "upgrade" to refer to the old style "recursive upgrade", and then tacking on a new property "upgrade_direct" for the new non-recursive type.

maybe that allows for a smaller PR diff, but I care more about readability.

to me the most readable properties for the Set would be:

upgrade: whether it's an upgrade of any kind
upgrade_recursive: is it recursive or not

@jsocol

This comment has been minimized.

jsocol commented Oct 22, 2015

Just 2¢, but the install -U behavior is helpful when a library or framework isn't fully self-contained. pip upgrade django will update all dependencies, because they're vendored, but pip upgrade flask would not pick up necessary updates to werkzeug, jinja, etc. That, to me, is the argument for maintaining the behavior via --recursive: I may never interact with a library like werkzeug directly, but if I want to get the latest Flask (the pip upgrade <one package> use case), I want to make sure it picks up what it needs.

@qwcode

This comment has been minimized.

Contributor

qwcode commented Oct 22, 2015

pip upgrade flask would not pick up necessary updates

it will, if flask has updated the lower bound for one of it's dependencies.

for example, currently, it requires click>=2.0. If that had changed to click>=3.0, then pip upgrade flask would pick that up, and you'd end up with the latest version of click, where using -U would force the upgrade of click regardless of a lower bound change

@jsocol

This comment has been minimized.

jsocol commented Oct 23, 2015

Ah, ok, but it won't get available but not required updates, unless I manually specify all the dependencies?

@dstufft

This comment has been minimized.

Member

dstufft commented Oct 23, 2015

That is correct. The default of pip upgrade would be "do the minimal amount of work needed to upgrade the named packages without violating version constraints". In the future, the --all flag would be "upgrade everything in this environment to the latest version without violating constraints". The thing that's up for discussion if we need or not is something more akin to what pip install --upgrade does now which is "upgrade the named things and all of their dependencies without violating constraints".

In any case though, the end goal should be that we never violate version constraints (at least without some sort of flag to force it).

@jsocol

This comment has been minimized.

jsocol commented Oct 23, 2015

The thing that's up for discussion if we need or not is something more akin to what pip install --upgrade does now which is "upgrade the named things and all of their dependencies without violating constraints".

I still think for cases like frameworks this is valuable, and I still think the Flask example stands: pip install -U flask will get the latest (constraint-satisfying) versions of Werkzeug, click, et al. Code that I may never interact with, but would like to stay up to date on, when updating Flask. (While Flask may work with older versions, and there may be reasons for people not to upgrade, e.g. if they are interacting with Werkzeug, in this example, that's probably not the default.) For some libraries and frameworks, "get this up to date" does mean "and its dependencies that I don't use directly." Maintaining this behavior via upgrade --recursive supports that.

@qwcode

This comment has been minimized.

Contributor

qwcode commented Oct 23, 2015

@jsocol understood. your voice on this is exactly what I was trying to represent in a comment earlier #3194 (comment)

@jsocol

This comment has been minimized.

jsocol commented Oct 23, 2015

@qwcode I thought that's what part of your comment was, just providing a +1 and concrete example! 👍

@pfmoore

This comment has been minimized.

Member

pfmoore commented Oct 23, 2015

I typically don't have complex enough dependencies for it to matter much to me one way or another, but I also see the benefit of this behaviour. (It's what I'd intend when thinking something like "upgrade the flask-related stuff for my project").

@jkseppan

This comment has been minimized.

jkseppan commented Oct 24, 2015

Let me see if I understand the points presented in the discussion correctly. People have use cases for:

  • upgrading a single package and any necessary dependencies without upgrading everything listed as a dependency, e.g. pip upgrade matplotlib should not usually install the newest numpy (assuming matplotlib declares numpy as a dependency in the future)
  • upgrading a single package and all its recursive dependencies, e.g. pip upgrade --recursive flask should upgrade Werkzeug and Jinja even if Flask's declared lower bounds have not increased
  • upgrading everything in a virtualenv with pip upgrade --all, using some future dependency resolution algorithm (#988)

There has been a suggestion to deprecate pip install --upgrade and replace it with pip upgrade --recursive. That way all upgrades would be done with the new pip upgrade command. A better dependency resolution algorithm would probably be useful for the recursive use case (what if some hypothetical future versions of Werkzeug and Jinja declare conflicting dependencies?) but since pip install --upgrade is useful as it is now, it should be fine to make pip upgrade --recursive do the same thing before we have that resolution algorithm.

Here's one thing that is unclear to me: pip install --upgrade not only does recursive upgrades but also causes pip install --target=/some/dir to overwrite existing versions. Should that become a separate option such as --overwrite?

jkseppan added some commits Oct 24, 2015

A more meaningful name
upgrade_recursive should be more obvious than upgrade_direct
Improve RequirementSet API
Use upgrade and upgrade_recursive instead of upgrade (recursive)
and upgrade_direct (only this package).
@qwcode

This comment has been minimized.

Contributor

qwcode commented Oct 24, 2015

upgrading a single package and all its recursive dependencies, e.g. pip
upgrade --recursive flask should upgrade Werkzeug and Jinja even if
Flask's declared lower bounds have not increased

but still honor upper bounds (which it does currently)

  • upgrading everything in a virtualenv with pip upgrade --all, using
    some future dependency resolution algorithm (#988
    #988)

the #988 fix(es) will affect everything, not just --all. any pip
installation, upgrade or not, uses the overly simplistic "first found,
wins" algorithm, and ignores constraints already present in the given
installation.

since pip install --upgrade is useful as it is now, it should be fine to
make pip upgrade --recursive do the same thing before we have that
resolution algorithm.

IMO, yes

Here's one thing that is unclear to me: pip install --upgrade not only
does recursive upgrades but also causes pip install --target=/some/dir to
overwrite existing versions. Should that become a separate option such as
--overwrite?

I'll respond later... I need to look closer at the state of --target
right now...


Reply to this email directly or view it on GitHub
#3194 (comment).

@qwcode

This comment has been minimized.

Contributor

qwcode commented Oct 24, 2015

also causes pip install --target=/some/dir to overwrite existing versions.
Should that become a separate option such as --overwrite?

I don't think so. The overwrite is how the upgrade is actually performed currently for --target installs.
The --target stuff is historically pretty buggy, and maybe needs a rehaul altogether, but you shouldn't worry about that here IMO.

summary = 'Install packages.'
upgrade_command = False
upgrade_option = True
upgrade_is_recursive = True

This comment has been minimized.

@qwcode

qwcode Oct 24, 2015

Contributor

you've got an inconsistency here. InstallCommand has upgrade_is_recursive, whereas UpgradeCommand has upgrade_recursive.

I'd prefer the shorter upgrade_recursive

@jkseppan

This comment has been minimized.

jkseppan commented Oct 29, 2015

I think I would find --install-missing misleading, if the only thing it does is alter whether the directly named packages are installed if they are missing. If pip upgrade has an optional --install-missing flag, then I'd assume it doesn't install newly added dependencies by default, or it might be intended to fix broken dependencies.

I think I would actually prefer pip upgrade --install, which also has an amusing symmetry with the old pip install --upgrade.

@dstufft

This comment has been minimized.

Member

dstufft commented Oct 29, 2015

I'm trying to think why we wouldn't just let pip upgrade install things too. Is there any reason that wouldn't work?

Sent from my iPhone

On Oct 29, 2015, at 3:58 PM, Jouni K. Seppänen notifications@github.com wrote:

I think I would find --install-missing misleading, if the only thing it does is alter whether the directly named packages are installed if they are missing. If pip upgrade has an optional --install-missing flag, then I'd assume it doesn't install newly added dependencies by default, or it might be intended to fix broken dependencies.

I think I would actually prefer pip upgrade --install, which also has an amusing symmetry with the old pip install --upgrade.


Reply to this email directly or view it on GitHub.

@jkseppan

This comment has been minimized.

jkseppan commented Oct 29, 2015

I think #3194 (comment) is the objection: it could be surprising if pip upgrade numpy installs numpy even if no version was already installed.

@dstufft

This comment has been minimized.

Member

dstufft commented Oct 29, 2015

hmm, I'm not sure if it's really that surprising, and I really hate the idea of an --install-missing flag because it feels like a little turd that people are going to just always use (or not use, get an error then use it in anger).

@jkseppan

This comment has been minimized.

jkseppan commented Oct 29, 2015

I think I tend to agree. At least if I type pip upgrade foo from the command line, I expect the latest version of foo to get installed. I wonder if there are uses from scripts that should fail if the package is not yet installed? I can't really think of a convincing use case.

@dstufft

This comment has been minimized.

Member

dstufft commented Oct 29, 2015

I can't think of a convincing use case either, if one emerges we can cross that bridge when we come to it. I'd much rather have the "user typing at the console" case require as few flags as possible and the "script invokes a thing" case require flags, when they need different behavior.

@njsmith

This comment has been minimized.

Member

njsmith commented Oct 29, 2015

Yeah, the only reason not to have upgrade install by default is the
potential for user confusion due to the misleading name. Any time there's a
discrepancy between what a command obviously does, and what it actually
does, it creates problems, because no one reads the docs for commands whose
semantics seem to be obvious. Agreed that --install-missing is also ugly,
though :-(.
.
It's too bad that we can't rename the current 'install' to be 'require' or
similar, and then make 'install' be the install-or-upgrade command...

@xavfernandez

This comment has been minimized.

Contributor

xavfernandez commented Oct 29, 2015

Well one reason is that pip install and pip upgrade would in the end be identical and only differ in the way they deal with managing dependencies upgrade... maybe not sufficient to need a new command ? Adding a --upgrade-only-if-needed option to pip install could give us the same benefit as a new command ?

@njsmith

This comment has been minimized.

Member

njsmith commented Oct 29, 2015

Well, here's a question: how bad would it be to transition 'pip install' so
that its default was to upgrade all packages that are named on the command
line without specified version restrictions? I've been assuming this was
off the table due to compatibility concerns, but maybe it's worth
checking...
On Oct 29, 2015 2:21 PM, "Xavier Fernandez" notifications@github.com
wrote:

Well one reason is that pip install and pip upgrade would in the end be
identical and only differ in the way they deal with managing dependencies
upgrade... maybe not sufficient to need a new command ? Adding a
--upgrade-only-if-needed option to pip install could give us the same
benefit as a new command ?


Reply to this email directly or view it on GitHub
#3194 (comment).

@rgommers

This comment has been minimized.

rgommers commented Oct 29, 2015

Adding a --upgrade-only-if-needed option to pip install could ...

This has been discussed in gh-59 and elsewhere I think. The "only if needed behavior" needs to be the default behavior for the command that does upgrading, and the pip devs don't want to change the default of pip install because of backwards compat. So --upgrade-only-if-needed doesn't solve anything.

@dstufft

This comment has been minimized.

Member

dstufft commented Oct 29, 2015

So here's another possible option:

We don't add pip upgrade, instead we add a new flag to pip install called something like --upgrade-strategy. Then we could have a number of strategies available for deciding what to upgrade like minimal or recursive. The invocations would then be:

  • pip install --upgrade thing: Upgrades "thing" with the default strategy.
  • pip install --upgrade --upgrade-strategy recursive thing: Upgrades "thing" with the recursive strategy.
  • pip install --upgrade --upgrade-strategy minimal thing: Upgrades "thing" with the minimal strategy.
  • pip install --upgrade :all: (or * or some sigil to mean everything): Upgrades everything to the latest versions.

The exact spelling of these could be different, like we could do --upgrade-minimal and --upgrade-recursive and then just --upgrade for the default, but the basic idea is there.

The "default" (only) option right now is the recursive upgrade, this is unacceptable to many projects and I understand why that's a problem. I think that if we don't find the idea of a new command palatable and we'd prefer to continue to just have the install command, then we're going to have to default to a safer option for --upgrade and move it to default to minimal. What exactly that process would look like I'm not sure (do we have a deprecation period? A warning? idk) but I think it'd be doable.

We wouldn't need to implement the :all: support right now, since that's unlikely to ever be "safe" without a real resolver, but I do think that however we go forward, we should have some idea about what that is going to look like and how it will work.

I will admit that I find the idea of having pip install and pip upgrade which only slightly differ isn't super appealing to me, but neither is the idea of having a bunch of --upgrade-foo or --upgrade-strategy commands or the idea of having pip upgrade fail just because the item wasn't installed already.

Maybe the answer is to just change the default behavior of pip install also upgrade the named packages (the ones explicitly mentioned) and default to a minimal upgrade strategy and then just add a --recursive flag (e.g., take the pip upgrade from this PR and name it pip install).

Anyways, I think it's really just about deciding which option is the least worst for us because the current situation isn't particularly great and I don't think there's a perfect option for us here.

@dstufft

This comment has been minimized.

Member

dstufft commented Oct 29, 2015

I guess the tl;dr is that I totally think that we need to fix the "default" behavior, we just need to settle on the specific implementation of fixing it:

  1. Keep the UX be pip install --upgrade and change the default.
  2. Move the UX to pip upgrade with the "right" default and add a --recursive flag.
  3. Move the UX to pip install, remove --upgrade, and add a --recursive flag.

@pypa/pip-developers Thoughts ^^?

@njsmith

This comment has been minimized.

Member

njsmith commented Oct 29, 2015

Maybe the answer is to just change the default behavior of pip install also upgrade the named packages (the ones explicitly mentioned) and default to a minimal upgrade strategy and then just add a --recursive flag (e.g., take the pip upgrade from this PR and name it pip install).

I think this is 100% the best solution if we can get there. It's not confusing -- in fact it's arguably even simpler to explain than the current behavior ("pip install foo means you want to install the latest version of foo, and it doesn't matter whether foo was already installed", as compared to now where there's some state dependence). And it's how all the other package systems I'm familiar with work. Maybe we should just go for it?

(We'd also want to add some option to restore the current behavior for the rare case where you really do want to say "I need some version of foo but I don't care what", like pip require foo or pip install --no-eager foo or whatever, but that's a detail.)

@dstufft

This comment has been minimized.

Member

dstufft commented Oct 30, 2015

Moving the discussion about what the exact UI should be over to #59 instead of having dueling comment threads.

@dstufft

This comment has been minimized.

Member

dstufft commented May 18, 2016

Accidentally closed this, reopening. Sorry!

@dstufft dstufft reopened this May 18, 2016

@BrownTruck

This comment has been minimized.

Contributor

BrownTruck commented May 19, 2016

Hello!

As part of an effort to ease the contribution process and adopt a more standard workflow pip has switched to doing development on the master branch. However, this Pull Request was made against the develop branch so it will need to be resubmitted against master. Unfortunately, this pull request does not cleanly merge against the current master branch.

If you do nothing, this Pull Request will be automatically closed by @BrownTruck since it cannot be merged.

If this pull request is still valid, please rebase it against master (or merge master into it) and resubmit it against the master branch, closing and referencing the original Pull Request.

If you choose to rebase/merge and resubmit this Pull Request, here is an example message that you can copy and paste:

Upgrade those requirements given directly but dependencies only as needed to satisfy dependency relations. Inspired by Nathaniel Smith's post at http://thread.gmane.org/gmane.comp.python.scientific.user/36377.

See also: #59

---

*This was migrated from pypa/pip#3194 to reparent it to the ``master`` branch. Please see original pull request for any previous discussion.*
@BrownTruck

This comment has been minimized.

Contributor

BrownTruck commented May 26, 2016

This Pull Request was closed because it cannot be automatically reparented to the master branch and it appears to have bit rotted.

Please feel free to re-open it or re-submit it if it is still valid and you have rebased it onto master or merged master into it.

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