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

Prevent pipx from erroneously listing other apps installed outside of pipx #650

Merged
merged 4 commits into from
Mar 23, 2021

Conversation

itsayellow
Copy link
Contributor

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

Summary of changes

Closes #564, #548.

This fixes some code goofs that allowed pipx to proclaim that pre-existing binaries in ~/.local/bin were part of a pipx install when they were in fact installed previously using pip install --user.

First fix is not to search for apps of dependencies if we didn't include dependencies on install.

Second fix is to fall back to checking non-symlinks only on non-symlink systems. Previously we would check non-symlinks always if we encountered a file that was not a symlink, which was not the intent of the if statement.

Test plan

Tested by running:

pip install --user pipx
pip install --user keyring
pipx install twine

pipx should only list the app 'twine' associated with venv 'twine'. 'keyring' is a dependency and should not be listed.

This PR:

> pipx install twine
  installed package twine 3.4.1, Python 3.9.0
  These apps are now globally available
    - twine
done! ✨ 🌟 ✨
> pipx list
venvs are in /home/mclapp/.local/pipx/venvs
apps are exposed on your $PATH at /home/mclapp/.local/bin
   package twine 3.4.1, Python 3.9.0
    - twine

pipx 0.16.1.0

> pipx install twine  
  installed package twine 3.4.1, Python 3.9.0
  These apps are now globally available
    - keyring
    - twine
done! ✨ 🌟 ✨
> pipx list
venvs are in /home/mclapp/.local/pipx/venvs
apps are exposed on your $PATH at /home/mclapp/.local/bin
   package twine 3.4.1, Python 3.9.0
    - keyring
    - twine

@itsayellow itsayellow changed the title List existing fix Prevent pipx from erroneously listing other apps installed outside of pipx Mar 21, 2021
@itsayellow itsayellow merged commit efbfb27 into pypa:master Mar 23, 2021
@itsayellow itsayellow deleted the list-existing-fix branch March 23, 2021 04:05
@@ -238,7 +241,7 @@ def _get_exposed_app_paths_for_package(
# Windows, we use a less strict check if we don't have a symlink.
if _can_symlink(local_bin_dir) and b.is_symlink():
is_same_file = b.resolve().parent.samefile(venv_bin_path)
else:
elif not _can_symlink(local_bin_dir):
Copy link
Contributor

@pdecat pdecat Apr 10, 2021

Choose a reason for hiding this comment

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

This breaks the test below:

pipx install py-spy
...
Traceback (most recent call last):
  File "/home/patrick/.local/bin/pipx", line 8, in <module>
    sys.exit(cli())
  File "/home/patrick/.local/lib/python3.9/site-packages/pipx/main.py", line 762, in cli
    return run_pipx_command(parsed_pipx_args)
  File "/home/patrick/.local/lib/python3.9/site-packages/pipx/main.py", line 193, in run_pipx_command
    return commands.install(
  File "/home/patrick/.local/lib/python3.9/site-packages/pipx/commands/install.py", line 69, in install
    run_post_install_actions(
  File "/home/patrick/.local/lib/python3.9/site-packages/pipx/commands/common.py", line 377, in run_post_install_actions
    package_summary, _ = get_venv_summary(
  File "/home/patrick/.local/lib/python3.9/site-packages/pipx/commands/common.py", line 201, in get_venv_summary
    exposed_app_paths = _get_exposed_app_paths_for_package(
  File "/home/patrick/.local/lib/python3.9/site-packages/pipx/commands/common.py", line 249, in _get_exposed_app_paths_for_package
    if is_same_file:
UnboundLocalError: local variable 'is_same_file' referenced before assignment

Using current master https://github.com/pipxproject/pipx/blame/13b6a271fcddad00cad6eab82fc2714cf6ad4503/src/pipx/commands/common.py#L249

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants