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

Package name can collide with top-level directory #949

Closed
shvenkat opened this issue Oct 22, 2017 · 2 comments
Closed

Package name can collide with top-level directory #949

shvenkat opened this issue Oct 22, 2017 · 2 comments
Labels
Type: Bug 🐛 This issue is a bug.

Comments

@shvenkat
Copy link

Description

A package specified in Pipfile is not installed if there is a top-level directory with the same name. If the directory is removed/renamed, the next pipenv install installs the package.

Environment

  1. OS Type: Mac OS 10.11.6
  2. Python version: Python 3.6.0
  3. Pipenv version: pipenv, version 8.2.7

Steps to reproduce

mkdir /tmp/pipenv_test && cd /tmp/pipenv_test
echo -e "[requires]\npython_version = \"3.6\"\n\n[packages]\nrequests = \"*\"" > Pipfile
mkdir requests
PIPENV_VENV_IN_PROJECT=1 pipenv install

Expected result

Package requests is installed into the virtual environment in .venv.

Actual result

Package requests is not installed into the virtual environment in .venv. Deleting or renaming the directory requests, and running PIPENV_VENV_IN_PROJECT=1 pipenv install does result in the package requests being installed.

@nateprewitt
Copy link
Member

Thanks for opening this @shvenkat! This is yet another data point on multiple issues we have open around local file installs.

@techalchemy, this suggests to me that we probably should require a ./ minimum when people are installing local packages (e.g. ./requests). Otherwise, it's going to be difficult to disambiguate what they're referencing with the entry in the Pipfile.

@techalchemy
Copy link
Member

@nateprewitt this would align with pip's behavior around directories (files can be implicitly resolved I think). This is kind of an odd case and we had talked about attempting (and if we fail, doing the normal method) package installation if we find a directory match. Maybe that is overcomplicating though, especially given that it does not replicate the current behavior of pip.

 ◰³ tempenv-4d12275072fe  /t/new  unzip dls/tablib.zip
Archive:  dls/tablib.zip
4c300e65a50eef72b91fa1909c9f68679723955e
 ◰³ tempenv-4d12275072fe  /t/new  ls
dls  Pipfile  tablib
 ◰³ tempenv-4d12275072fe  /t/new  pip install tablib 
Collecting tablib
  Using cached tablib-0.12.1.tar.gz
Collecting odfpy (from tablib)
Collecting openpyxl (from tablib)
...

And you can see that it only resolves it if I put any explicit identifier that we are dealing with the local filesystem (os.sep is present or ./ is present), in which case pip (and in our case the Pipfile) can store a relative path but it resolves it to a file:// URI before it does anything else:

 ◰³ tempenv-4d12275072fe  /t/new  pip install tablib/
Processing ./tablib
  Requirement already satisfied (use --upgrade to upgrade): tablib==0.12.1 from file:///tmp/new/tablib in /home/hawk/.virtualenvs/tempenv-4d12275072fe/lib/python3.6/site-packages

techalchemy added a commit to techalchemy/pipenv that referenced this issue Oct 23, 2017
* Issues - pypa#949, pypa#939, pypa#936 and to a lesser extent pypa#817 pypa#540 and more
* Fixed: Local file path installation (resolves in pipfile as relative
path)
* Pass file:// URI to Requirements library for resolving

TODO:
* Ignore non-explicit directory paths lacking os.sep or ./
* Add tests
@erinxocon erinxocon added the Type: Bug 🐛 This issue is a bug. label Oct 24, 2017
techalchemy added a commit to techalchemy/pipenv that referenced this issue Nov 14, 2017
* Issues - pypa#949, pypa#939, pypa#936 and to a lesser extent pypa#817 pypa#540 and more
* Fixed: Local file path installation (resolves in pipfile as relative
path)
* Pass file:// URI to Requirements library for resolving

TODO:
* Ignore non-explicit directory paths lacking os.sep or ./
* Add tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug 🐛 This issue is a bug.
Projects
None yet
Development

No branches or pull requests

4 participants