-
Notifications
You must be signed in to change notification settings - Fork 478
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 = false
Comparison 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/pycowsay
Expected 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=[], |