Skip to content

Commit

Permalink
Fix print condition for python + force flag warning (#1306)
Browse files Browse the repository at this point in the history
* Removed --python flag dependency when --force is passed

* Changed default value of python arg to empty string

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Updated install code to infer python flag from main

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fixed python_flag_passed assignment

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Removed Optional typing for python_flag_passed param in install

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Passed the correct value of python_flag_passed param in all invocations

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Passed the python_flag_passed param to upgrade and reinstall

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Added test case

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Refactored test case and changelog

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: Swaraj Baral <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Mar 31, 2024
1 parent 8fbc085 commit f8e7bfc
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 6 deletions.
1 change: 1 addition & 0 deletions changelog.d/1304.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Only show `--python` and `--force` flag warning if both flags are present
4 changes: 2 additions & 2 deletions src/pipx/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ def install(
include_dependencies: bool,
preinstall_packages: Optional[List[str]],
suffix: str = "",
python_flag_passed=False,
) -> ExitCode:
"""Returns pipx exit code."""
# package_spec is anything pip-installable, including package_name, vcs spec,
# zip file, or tar.gz file.
python_flag_was_passed = python is not None

python = python or DEFAULT_PYTHON

Expand All @@ -57,7 +57,7 @@ def install(
venv = Venv(venv_dir, python=python, verbose=verbose)
venv.check_upgrade_shared_libs(pip_args=pip_args, verbose=verbose)
if exists:
if not reinstall and force and python_flag_was_passed:
if not reinstall and force and python_flag_passed:
print(
pipx_wrap(
f"""
Expand Down
4 changes: 4 additions & 0 deletions src/pipx/commands/reinstall.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def reinstall(
python: str,
verbose: bool,
force_reinstall_shared_libs: bool = False,
python_flag_passed: bool = False,
) -> ExitCode:
"""Returns pipx exit code."""
if not venv_dir.exists():
Expand Down Expand Up @@ -74,6 +75,7 @@ def reinstall(
include_dependencies=venv.pipx_metadata.main_package.include_dependencies,
preinstall_packages=[],
suffix=venv.pipx_metadata.main_package.suffix,
python_flag_passed=python_flag_passed,
)

# now install injected packages
Expand Down Expand Up @@ -105,6 +107,7 @@ def reinstall_all(
verbose: bool,
*,
skip: Sequence[str],
python_flag_passed: bool = False,
) -> ExitCode:
"""Returns pipx exit code."""
failed: List[str] = []
Expand All @@ -124,6 +127,7 @@ def reinstall_all(
python=python,
verbose=verbose,
force_reinstall_shared_libs=first_reinstall,
python_flag_passed=python_flag_passed,
)
except PipxError as e:
print(e, file=sys.stderr)
Expand Down
6 changes: 6 additions & 0 deletions src/pipx/commands/upgrade.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ def _upgrade_venv(
force: bool,
install: bool = False,
python: Optional[str] = None,
python_flag_passed: bool = False,
) -> int:
"""Return number of packages with changed versions."""
if not venv_dir.is_dir():
Expand All @@ -122,6 +123,7 @@ def _upgrade_venv(
reinstall=False,
include_dependencies=False,
preinstall_packages=None,
python_flag_passed=python_flag_passed,
)
return 0
else:
Expand Down Expand Up @@ -186,6 +188,7 @@ def upgrade(
include_injected: bool,
force: bool,
install: bool,
python_flag_passed: bool = False,
) -> ExitCode:
"""Return pipx exit code."""

Expand All @@ -198,6 +201,7 @@ def upgrade(
force=force,
install=install,
python=python,
python_flag_passed=python_flag_passed,
)

# Any error in upgrade will raise PipxError (e.g. from venv.upgrade_package())
Expand All @@ -212,6 +216,7 @@ def upgrade_all(
include_injected: bool,
skip: Sequence[str],
force: bool,
python_flag_passed: bool = False,
) -> ExitCode:
"""Returns pipx exit code."""
venv_error = False
Expand All @@ -229,6 +234,7 @@ def upgrade_all(
include_injected=include_injected,
upgrading_all=True,
force=force,
python_flag_passed=python_flag_passed,
)

except PipxError as e:
Expand Down
15 changes: 12 additions & 3 deletions src/pipx/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,15 @@ def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.Ar
if "skip" in args:
skip_list = [canonicalize_name(x) for x in args.skip]

if "python" in args and args.python is not None:
python_flag_passed = False

if "python" in args:
python_flag_passed = bool(args.python)
fetch_missing_python = args.fetch_missing_python
try:
interpreter = find_python_interpreter(args.python, fetch_missing_python=fetch_missing_python)
interpreter = find_python_interpreter(
args.python or DEFAULT_PYTHON, fetch_missing_python=fetch_missing_python
)
args.python = interpreter
except InterpreterResolutionError as e:
logger.debug("Failed to resolve interpreter:", exc_info=True)
Expand Down Expand Up @@ -244,6 +249,7 @@ def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.Ar
include_dependencies=args.include_deps,
preinstall_packages=args.preinstall,
suffix=args.suffix,
python_flag_passed=python_flag_passed,
)
elif args.command == "inject":
return commands.inject(
Expand Down Expand Up @@ -275,6 +281,7 @@ def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.Ar
include_injected=args.include_injected,
force=args.force,
install=args.install,
python_flag_passed=python_flag_passed,
)
elif args.command == "upgrade-all":
return commands.upgrade_all(
Expand All @@ -284,6 +291,7 @@ def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.Ar
skip=skip_list,
force=args.force,
pip_args=pip_args,
python_flag_passed=python_flag_passed,
)
elif args.command == "list":
return commands.list_packages(
Expand Down Expand Up @@ -318,6 +326,7 @@ def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.Ar
local_man_dir=paths.ctx.man_dir,
python=args.python,
verbose=verbose,
python_flag_passed=python_flag_passed,
)
elif args.command == "reinstall-all":
return commands.reinstall_all(
Expand All @@ -327,6 +336,7 @@ def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.Ar
args.python,
verbose,
skip=skip_list,
python_flag_passed=python_flag_passed,
)
elif args.command == "runpip":
if not venv_dir:
Expand Down Expand Up @@ -373,7 +383,6 @@ def add_include_dependencies(parser: argparse.ArgumentParser) -> None:
def add_python_options(parser: argparse.ArgumentParser) -> None:
parser.add_argument(
"--python",
default=DEFAULT_PYTHON,
help=(
"Python to install with. Possible values can be the executable name (python3.11), "
"the version to pass to py launcher (3.11), or the full path to the executable."
Expand Down
10 changes: 9 additions & 1 deletion tests/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,17 @@ def test_passed_python_and_force_flag_warning(pipx_temp_env, capsys):
captured = capsys.readouterr()
assert "--python is ignored when --force is passed." in captured.out

assert not run_pipx_cli(["install", "black", "--force"])
captured = capsys.readouterr()
assert (
"--python is ignored when --force is passed." not in captured.out
), "Should only print warning if both flags present"

assert not run_pipx_cli(["install", "pycowsay", "--force"])
captured = capsys.readouterr()
assert "--python is ignored when --force is passed." not in captured.out
assert (
"--python is ignored when --force is passed." not in captured.out
), "Should not print warning if package does not exist yet"


def test_install_run_in_separate_directory(caplog, capsys, pipx_temp_env, monkeypatch, tmp_path):
Expand Down

0 comments on commit f8e7bfc

Please sign in to comment.