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

Installing packages with relative paths fails -- discussion #939

Closed
techalchemy opened this issue Oct 19, 2017 · 4 comments
Closed

Installing packages with relative paths fails -- discussion #939

techalchemy opened this issue Oct 19, 2017 · 4 comments
Labels
Type: Discussion This issue is open for discussion.

Comments

@techalchemy
Copy link
Member

See #936 -- basically boils down to is_file checks for local files. This issue is to open a broader discussion for the proper way to handle checks for local files during installation. Previously we have had several issues (#817, #540 for instance) where users needed to install a local sdist in the project directory (in that case a file, properly speaking, as a tar.gz or a .zip), and a relative path to a local directory containing a valid setup.py (e.g. pipenv install -e ../somedirectory/ or the equivalent of pip install -e ../...).

As a consequence we currently are mainly checking only that a path exists locally before attempting to install it as a local path, no matter whether the user supplied a preceding ./ if it is in the current / project directory (we could require this), or whether the path is a file which can be installed with pip, or, if it is a directory, if that directory contains a python package at all and if so, if it can be installed with pip.

Essentially I would propose that we start the discussion with this set of options and whether they are suitable/viable:

  • Our initial position for filesystem locations was to require file:// URIs, but we budged on this because pip allows local paths and because users often use this syntax. We could enforce a local path 'uri' by enforcing ./ and ../ syntax and asking if the user meant ./path if we find a matching one before proceeding
  • We could ignore any directories in the project root that are not explicitly installed in editable mode - what are the potential consequences of this?
  • We could attempt to pass through existing directories to a pip install as we are doing now, but instead of failing and pretending to succeed, we could figure out a way to fail silently and proceed to pypi or whatever repository the user has chosen. This seems like the most idiomatic approach potentially, although I'm not sure how complex the implementation is.
  • We could be dumb and just check for the presence of setup.py in any directory that matches a requested path and, if it is missing, dont bother trying to install that directory and just query the repository

I personally am only opposed to the first option, I don't really have strong feelings on the others and could see one or any combination of those working well. There could easily be other options I'm not considering. I'd be curious to hear what others have to say

/cc @krismolendyke @kennethreitz @erinxocon @nateprewitt (when you guys have time)

@techalchemy
Copy link
Member Author

/cc @vphilippon

@techalchemy
Copy link
Member Author

See #914 also

@techalchemy techalchemy changed the title Installing packages from pypi fails if there is a project subdirectory named after the package Installing packages with relative paths fails -- discussion Oct 20, 2017
@kstohr
Copy link

kstohr commented Oct 21, 2017

Apologies, I just submitted a query on this issue... and didn't make the connection.

The user should determine when/whether local packages should be added to the pipfile explicitly rather than automatically detecting and installing which could cause issues/complications for projects in development. The above mentioned pipenv install -e ./path/to/setup.py seems intuitive to me...

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: Discussion This issue is open for discussion. 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
techalchemy added a commit to techalchemy/pipenv that referenced this issue Nov 14, 2017
@techalchemy
Copy link
Member Author

Closing this -- resolved via #958

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Discussion This issue is open for discussion.
Projects
None yet
Development

No branches or pull requests

3 participants