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

added a upgrade-all command #10040

Closed
wants to merge 4 commits into from
Closed

Conversation

JensTimmerman
Copy link

fixes #4551

@JensTimmerman
Copy link
Author

I'ma a new contributor to pip, so any hints on what kinds of tests to add for this command?

@JensTimmerman
Copy link
Author

JensTimmerman commented Jun 4, 2021

I see this will immediately require a change after https://github.com/pypa/pip/pull/10036/files is merged.

The problem here seems to be that I needed to copy paste the package creation list from inside the run function.

The best way is probably to abstract out the process of creating the package list based on the options into a different function and reuse that function in both run functions.
Shall I do this in a separate pr?

AUTHORS.txt Outdated Show resolved Hide resolved
Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Thanks for this PR!

I don't think this is the approach we should take for implementing this command. Instead of trying to mimic the carefully crafted command line invocation and adding a dependency between two pip commands, we should use lower level components of pip to implement this instead.

Specifically, we'd need to determine which packages were explicitly installed by the user (i.e. aren't marked with PEP 376's REQUIRED file), make the dependency resolver ignore already installed packages in the environment and add the ability to uninstall dependencies that are no longer required in the environment.

This would be a bit more complex compared to the current implementation, but I think it'd be worth it for the correctness -- we'd want to backtrack in case of incompatible dependencies and only upgrade to the most recent version that's compatible with the entire package set.

@JensTimmerman
Copy link
Author

JensTimmerman commented Jun 6, 2021

@pradyunsg from all comments and feedback I have read it seems there isn't really a full consensus yet on what this command should do?

Some people said it should mimic pip3 list --format=freeze | grep -v '^\-e' | cut -d = -f 1 | xargs -n1 pip3 install -U
others want pip3 list --outdated --format=freeze | grep -v '^\-e' | cut -d = -f 1 | xargs -n1 pip3 install -U

You want to determine what has been explicitly installed,

I know people who would like pip to update all packages in their local site folder
others would like pip to upgrade all python packges on the system (putting updates for system packages in their local site folder)

Others want all packages that have security updates to be updated, irrespective if this breaks anything, since they consider a safe system more important than a working system.

Personally I don't use venvs, and I like to have all packages in my local enviroment to be the latest version, yes sometimes something breaks, but then I submit a pr to the broken package to get it fixed...

So to make it easyer for me to implement:

Could you please specify a set of example package states and how you would like the upgrade-all command to interact with them and what the final state should be? I'll create a few test cases based on that and implement what you request :)

The reason I used this approach is because it leverages everything pip list can do, and everything pip install can do, making it very flexible to the users to select whatever option they like.

@uranusjr
Copy link
Member

uranusjr commented Jun 7, 2021

One design issue about the command in general: How should extras be handled? Currently extras requested at installation time are not recorded, so say you have

a    depends on nothing
a[z] depends on b>=1,<2
c    depends on b>=1

pip install a[z] c would give you a==1 b==1 c==1, but then pip upgrade-all would produce a==1 b==2 c==1, which is broken.

I don’t think it’s worthwhile to “fix” this “extra not recorded” problem in general; the issue has been discussed for a long while and no viable solutions have surfaced. But at least upgrade-all should provide a mechanism for the user to say something like “upgrade everything to latest possible but also respect a[z]”.

@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Jun 11, 2021
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Jun 12, 2021
@github-actions github-actions bot added the needs rebase or merge PR has conflicts with current master label Aug 15, 2021
@pradyunsg
Copy link
Member

Closing this out for now, since this likely isn't going to be the appraoch we'd take here and there's still the need to build concensus for this in #4551.

@pradyunsg pradyunsg closed this Sep 18, 2021
@JensTimmerman
Copy link
Author

@pradyunsg Thank you for the clarity, I've made a new pr with a different approach in #10491

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs rebase or merge PR has conflicts with current master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an upgrade-all command
5 participants