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

bug: Removed --python flag dependency when --force is passed #1306

Merged
merged 24 commits into from
Mar 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
101f520
Removed --python flag dependency when --force is passed
swaraj-raft Mar 27, 2024
7ccbe2d
Changed default value of python arg to empty string
swaraj-raft Mar 28, 2024
97ee387
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 28, 2024
9b8e28f
Merge branch 'main' into main
SwarajBaral Mar 28, 2024
34f6aab
Updated install code to infer python flag from main
swaraj-raft Mar 28, 2024
55ce577
Synced branch with main
swaraj-raft Mar 28, 2024
bcd4152
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 28, 2024
3e2252e
Fixed python_flag_passed assignment
swaraj-raft Mar 28, 2024
4b24d24
Synced branch and fixed python_flag_passed default assignment
swaraj-raft Mar 28, 2024
e6bda7e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 28, 2024
a2c5b50
Removed Optional typing for python_flag_passed param in install
swaraj-raft Mar 28, 2024
9747852
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 28, 2024
50a31ac
Passed the correct value of python_flag_passed param in all invocations
swaraj-raft Mar 28, 2024
6d81069
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 28, 2024
82b595d
Passed the python_flag_passed param to upgrade and reinstall
swaraj-raft Mar 29, 2024
1b79ced
Synced branch
swaraj-raft Mar 29, 2024
1da91ec
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 29, 2024
39c021a
Added test case
swaraj-raft Mar 30, 2024
0cc5993
Merge branch 'pypa:main' into main
SwarajBaral Mar 30, 2024
5362755
Merge branch 'main' of https://github.com/SwarajBaral/pipx
swaraj-raft Mar 30, 2024
5f8c046
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 30, 2024
414d49e
Refactored test case and changelog
swaraj-raft Mar 31, 2024
568a8db
Sync branch
swaraj-raft Mar 31, 2024
5bbd066
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 31, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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