pypa / pipx

Install and Run Python Applications in Isolated Environments
https://pipx.pypa.io
MIT License
10.13k stars 408 forks source link

Handle package containing different deps, each containing an app of same name #540

Open itsayellow opened 3 years ago

itsayellow commented 3 years ago

Describe the bug

kaggle contains two separate deps: slugify and python-slugify. Each of these separate python packages contain an app with the name slugify.

When pipx catalogs all of the apps_of_dependencies it lists slugify twice in the list because it comes from separate dependencies.

If pipx installs kaggle with switch --include-deps it tries to write the slugify link twice, once from each dep. It gives the following warning during install:

  Overwriting file C:\Users\mclapp\.local\bin\slugify with C:\Users\mclapp\.local\pipx\venvs\kaggle\Scripts\slugify

Then on uninstall, when going through the lists of apps_of_dependencies, it will encounter slugify twice. The first time it removes the binary from the pipx bin directory, and the second time when it tries to remove a non-existent file, on Windows it throws a python error and sends the user a stack trace (see below).

Solving the symptom is probably pretty easy: don't try and delete a non-existent file on Windows. macos and linux already have much more extensive checks whether the file exists in their symlink-centric part of the code.

Going further upstream, we could also make sure that we pass apps_of_dependencies through a set() to eliminate redundant entries. I'm curious if there are other thoughts on handling this annoying edge case properly.

How to reproduce

formica:pipx pipx install kaggle==1.5.9 --include-deps
  Overwriting file C:\Users\mclapp\.local\bin\slugify with C:\Users\mclapp\.local\pipx\venvs\kaggle\Scripts\slugify
done!
creating virtual environment...
installing kaggle from spec 'kaggle==1.5.9'...
  installed package kaggle 1.5.9, Python 3.9.0
  These apps are now globally available
    - chardetect.exe
    - kaggle.exe
    - slugify
    - slugify.exe
    - tqdm.exe
formica:pipx pipx uninstall kaggle
Traceback (most recent call last):
  File "c:\program files\python39\lib\runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "c:\program files\python39\lib\runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "C:\Users\mclapp\AppData\Roaming\Python\Python39\Scripts\pipx.exe\__main__.py", line 7, in <module>
  File "C:\Users\mclapp\AppData\Roaming\Python\Python39\site-packages\pipx\main.py", line 609, in cli
    return run_pipx_command(parsed_pipx_args)
  File "C:\Users\mclapp\AppData\Roaming\Python\Python39\site-packages\pipx\main.py", line 192, in run_pipx_command
    return commands.uninstall(venv_dir, constants.LOCAL_BIN_DIR, verbose)
  File "C:\Users\mclapp\AppData\Roaming\Python\Python39\site-packages\pipx\commands\uninstall.py", line 62, in uninstall
    file.unlink()
  File "c:\program files\python39\lib\pathlib.py", line 1343, in unlink
    self._accessor.unlink(self)
FileNotFoundError: [WinError 2] The system cannot find the file specified: 'C:\\Users\\mclapp\\.local\\bin\\slugify'
formica:pipx

Expected behavior

No stacktrace.

cs01 commented 3 years ago

Going further upstream, we could also make sure that we pass apps_of_dependencies through a set() to eliminate redundant entries. I'm curious if there are other thoughts on handling this annoying edge case properly.

It seems like whatever we do in this case we are in a pickle. If two apps have the same name, with what we must assume are not the same app content, then that should probably be a separate warning.

How about saying: Detected multiple packages with the same name app "slugify". pipx is using the first app it encountered associated with package kaggle and ignoring other apps with that name due to the name conflict.

itsayellow commented 3 years ago

We have a similar problem with colliding apps from different venvs (especially with --include-deps). Currently if we install 2 venvs using --include-deps with the same dependency apps, one will overwrite the other. Then if we uninstall one of these venvs, it will delete the dependency binary, even if the other package still exists.

itsayellow commented 3 years ago

Changing name since the original reason for the Issue has been fixed.

gaborbernat commented 9 months ago

PR welcome.