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

Upgrade: add option to upgrade injected packages #563

Merged
merged 21 commits into from
Nov 28, 2020

Conversation

itsayellow
Copy link
Contributor

  • I have added an entry to docs/changelog.md

Summary of changes

Adds --include-injected option to upgrade and upgrade-all, and if used upgrades all injected packages in the venv(s). Without the --include-injected option upgrade and upgrade-all work as in the past.

Closes #79

Test plan

Tested by running

> pipx install pylint==2.5.3                           
  installed package pylint 2.5.3, Python 3.9.0
  These apps are now globally available
    - epylint
    - pylint
    - pyreverse
    - symilar
done! ✨ 🌟 ✨
> pipx inject pylint black==18.9.b0
  injected package black into venv pylint
done! ✨ 🌟 ✨
> pipx upgrade --include-injected pylint
upgraded package pylint from 2.5.3 to 2.6.0 (location: /Users/mclapp/.local/pipx/venvs/pylint)
upgraded package black from 18.9b0 to 20.8b1 (location: /Users/mclapp/.local/pipx/venvs/pylint)
> pipx uninstall pylint
uninstalled pylint! ✨ 🌟 ✨
> pipx install pylint==2.5.3            
  installed package pylint 2.5.3, Python 3.9.0
  These apps are now globally available
    - epylint
    - pylint
    - pyreverse
    - symilar
done! ✨ 🌟 ✨
> pipx inject pylint black==18.9.b0     
  injected package black into venv pylint
done! ✨ 🌟 ✨
> pipx upgrade pylint    
upgraded package pylint from 2.5.3 to 2.6.0 (location: /Users/mclapp/.local/pipx/venvs/pylint)
>

@itsayellow
Copy link
Contributor Author

I used --include-injected because it exists for other pipx commands, but in general I like shorter options. If people want I can use a different option switch.

@tobwen
Copy link

tobwen commented Nov 27, 2020

Sorry to file this bug here, but @itsayellow doesn't have activated the issues section.

The package MapProxy gets installed into /home/tobwen/.local/pipx/venvs/MapProxy.

When trying to run pipx upgrade --include-injected MapProxy, I'm getting this error:

Removed file /home/tobwen/.local/pipx/logs/cmd_2020-11-27_18.19.11.log
Package is not installed. Expected to find /home/tobwen/.local/pipx/venvs/mapproxy, but it does not exist.

Seems like there's an issue with upper- and lower-case?

@itsayellow
Copy link
Contributor Author

itsayellow commented Nov 27, 2020

Sorry to file this bug here, but @itsayellow doesn't have activated the issues section.

The package MapProxy gets installed into /home/tobwen/.local/pipx/venvs/MapProxy.

When trying to run pipx upgrade --include-injected MapProxy, I'm getting this error:

Removed file /home/tobwen/.local/pipx/logs/cmd_2020-11-27_18.19.11.log
Package is not installed. Expected to find /home/tobwen/.local/pipx/venvs/mapproxy, but it does not exist.

Seems like there's an issue with upper- and lower-case?

This is a fine place to comment on a bug with this PR. 😃

You're coming into contact with a change to pipx's behavior. In a previous release pipx changed from internally using venv names that are exactly what the user typed, to using "normalized" names.

You should be able to fix your pipx internal data by using the development (unreleased as of yet) pipx and either executing pipx reinstall MapProxy or pipx reinstall-all.

The development version also will be ok afterwards if you still want to specify your venv to pipx commands as capitalized, e.g. MapProxy.

@tobwen
Copy link

tobwen commented Nov 28, 2020

Thank you for your reply and apologies for accusing you of creating a possible bug. As always, it has been a feature (of pipx) and not a bug :) Good hint, all packages were reinstalled and everything works now.

Thanks for this great PR, it'll make daily work a bit easier.

@itsayellow
Copy link
Contributor Author

itsayellow commented Nov 28, 2020

Thank you for your reply and apologies for accusing you of creating a possible bug. As always, it has been a feature (of pipx) and not a bug :) Good hint, all packages were reinstalled and everything works now.

Thanks for this great PR, it'll make daily work a bit easier.

@tobwen, I won't derail the discussion here too much, but I will point out that thanks to your feedback I just added PR #570 which should help avoid issues like this in the future. Thanks!

Copy link
Contributor

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

@gaborbernat gaborbernat merged commit 4adfb4e into pypa:master Nov 28, 2020
@itsayellow itsayellow deleted the upgrade_injected branch November 28, 2020 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update injected packages.
3 participants