-
Notifications
You must be signed in to change notification settings - Fork 527
Description
Describe the bug
Recently in this PR the --install option was added to the upgrade command.
Unfortunately this doesn't seem to respect the --system-site-packages option.
How to reproduce
When using upgrade --install the --system-site-packages option isn't respected:
❯ pipx upgrade --install --system-site-packages pycowsay
❯ grep system $HOME/.local/share/pipx/venvs/pycowsay/pyvenv.cfg
include-system-site-packages = falseComparison with the install command, where it works correctly:
❯ pipx install --system-site-packages pycowsay
❯ grep system $HOME/.local/share/pipx/venvs/pycowsay/pyvenv.cfg
include-system-site-packages = true
command = /usr/bin/python3 -m venv --without-pip --system-site-packages /home/sadamczyk/.local/share/pipx/venvs/pycowsayExpected behavior
The upgrade --install command should handle the --system-site-packages option just like the install command does.
Investigation
Looking through the code, the --system-site-packages option seems to be passed to the install command in the venv_args variable from the get_venv_args function:
Lines 170 to 174 in 5482fac
| def get_venv_args(parsed_args: Dict[str, str]) -> List[str]: | |
| venv_args: List[str] = [] | |
| if parsed_args.get("system_site_packages"): | |
| venv_args += ["--system-site-packages"] | |
| return venv_args |
Line 182 in 5482fac
| venv_args = get_venv_args(vars(args)) |
But venv_args is only passed to the run, install and install-all commands
Lines 244 to 253 in 5482fac
| elif args.command == "install": | |
| return commands.install( | |
| None, | |
| None, | |
| args.package_spec, | |
| paths.ctx.bin_dir, | |
| paths.ctx.man_dir, | |
| args.python, | |
| pip_args, | |
| venv_args, |
... but not to the
upgrade command:Lines 294 to 304 in 5482fac
| elif args.command == "upgrade": | |
| return commands.upgrade( | |
| venv_dirs, | |
| args.python, | |
| pip_args, | |
| verbose, | |
| include_injected=args.include_injected, | |
| force=args.force, | |
| install=args.install, | |
| python_flag_passed=python_flag_passed, | |
| ) |
It might be enough to add the required function arguments so that the venv_args from the main.py can be passed through all the way to the install call here:
pipx/src/pipx/commands/upgrade.py
Lines 111 to 114 in 5482fac
| if install: | |
| commands.install( | |
| venv_dir=None, | |
| venv_args=[], |