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

pip doesn't support relative paths in direct URL references #6658

Open
Qix- opened this issue Jun 28, 2019 · 8 comments
Open

pip doesn't support relative paths in direct URL references #6658

Qix- opened this issue Jun 28, 2019 · 8 comments
Labels
project: vendored dependency Related to a vendored dependency state: needs discussion This needs some more discussion type: bug A confirmed bug or unintended behavior

Comments

@Qix-
Copy link

Qix- commented Jun 28, 2019

Environment

  • pip version: 19.1.1
  • Python version: 3.7
  • OS: MacOS Mojave

Description
I'm not sure if this is a six bug or a Pip bug. Excuse me if it belongs to six.

Pip seems to allow local paths in install_requires via name @ ./some/path, but the URL parsing is terribly broken.

def url_to_path(url):
# type: (str) -> str
"""
Convert a file: URL to a path.
"""
assert url.startswith('file:'), (
"You can only turn file: urls into filenames (not %r)" % url)
_, netloc, path, _, _ = urllib_parse.urlsplit(url)
if not netloc or netloc == 'localhost':
# According to RFC 8089, same as empty authority.
netloc = ''
elif sys.platform == 'win32':
# If we have a UNC path, prepend UNC share notation.
netloc = '\\\\' + netloc
else:
raise ValueError(
'non-local file URIs are not supported on this platform: %r'
% url
)

In this function, it uses urlsplit to get the individual components of the incoming URL.

Here's what that looks like with a few pieces of input:

./foo/bar -> SplitResult(scheme='', netloc='', path='./foo/bar', query='', fragment='')
file:./foo/bar -> SplitResult(scheme='file', netloc='', path='./foo/bar', query='', fragment='')
file://./foo/bar -> SplitResult(scheme='file', netloc='.', path='/foo/bar', query='', fragment='')

Notice the last one results in the netloc being . instead of empty and the path being absolute, not local. This trips the error regarding non-local paths. That's all fine and well - I can use the second form to satisfy the conditional logic (though it really ought to support the first as well).

However, there's conflicting logic elsewhere...

parsed_url = urlparse.urlparse(req.url)
if parsed_url.scheme == "file":
if urlparse.urlunparse(parsed_url) != req.url:
raise InvalidRequirement("Invalid URL given")

This is the logic that fails even though we satisfy the prior logic.

Here's a test function that shows the problem:

from six.moves.urllib import parse as urllib_parse

def tryparse(url):
    print(url)
    parsed = urllib_parse.urlparse(url)
    unparsed = urllib_parse.urlunparse(parsed)
    parsed_again = urllib_parse.urlparse(unparsed)
    print(parsed)
    print(unparsed)
    print(parsed_again)

Here's the output for ./foo/bar:

>>> tryparse('./foo/bar')
./foo/bar
ParseResult(scheme='', netloc='', path='./foo/bar', params='', query='', fragment='')
./foo/bar
ParseResult(scheme='', netloc='', path='./foo/bar', params='', query='', fragment='')

All good, though it doesn't satisfy the first function's logic of requiring a scheme of file:.

Here's the output for file:./foo/bar:

>>> tryparse('file:./foo/bar')
file:./foo/bar
ParseResult(scheme='file', netloc='', path='./foo/bar', params='', query='', fragment='')
file:///./foo/bar
ParseResult(scheme='file', netloc='', path='/./foo/bar', params='', query='', fragment='')

Oops! Notice how, when we "unparse" the result from the first parse call, our path becomes absolute file:///....

This is why the second mentioned check fails - the path is not local. I believe this to be a bug in six but can be mitigated in Pip by allowing scheme in ['file', ''] and instructing users to use the ./foo/bar URI form.

Given these two contradictory pieces of logic, it's impossible to use local paths in install_requires keys in either distutils or setuptools configurations.

Expected behavior
I should be able to do name @ ./some/path (or, honestly, simply ./some/path) to specify a vendored package local to my codebase.

How to Reproduce

#!/usr/bin/env bash
mkdir /tmp/pip-uri-repro && cd /tmp/pip-uri-repro

mkdir -p foo/bar

cat > requirements.txt <<EOF
./foo
EOF

cat > foo/setup.py <<EOF
#!/usr/bin/env python
from setuptools import setup
setup(
    name="foo",
    version="0.1",
    install_requires=[
        "bar @ file:./bar"
    ]
)
EOF

cat > foo/bar/setup.py <<EOF
#!/usr/bin/env python
from setuptools import setup
setup(
    name="bar",
    version="0.1"
)
EOF

# (OUTPUT 1)
pip install -r requirements.txt

cat > foo/setup.py <<EOF
#!/usr/bin/env python
from setuptools import setup
setup(
    name="foo",
    version="0.1",
    install_requires=[
        # we're forced to use an absolute path
        # to make the "Invalid URL" error go
        # away, which isn't right anyway (the
        # error that is raised as a result
        # is justified)
        "bar @ file://./bar"
    ]
)
EOF

# (OUTPUT 2)
pip install -r requirements.txt

Output

From the first pip install:

Processing ./foo
    ERROR: Complete output from command python setup.py egg_info:
    ERROR: error in foo setup command: 'install_requires' must be a string or list of strings containing valid project/version requirement specifiers; Invalid URL given

From the second pip install:

Processing ./foo
ERROR: Exception:
Traceback (most recent call last):
  File "/private/tmp/repro-pip-egg/env3/lib/python3.7/site-packages/pip/_internal/cli/base_command.py", line 178, in main
    status = self.run(options, args)
  File "/private/tmp/repro-pip-egg/env3/lib/python3.7/site-packages/pip/_internal/commands/install.py", line 352, in run
    resolver.resolve(requirement_set)
  File "/private/tmp/repro-pip-egg/env3/lib/python3.7/site-packages/pip/_internal/resolve.py", line 131, in resolve
    self._resolve_one(requirement_set, req)
  File "/private/tmp/repro-pip-egg/env3/lib/python3.7/site-packages/pip/_internal/resolve.py", line 294, in _resolve_one
    abstract_dist = self._get_abstract_dist_for(req_to_install)
  File "/private/tmp/repro-pip-egg/env3/lib/python3.7/site-packages/pip/_internal/resolve.py", line 242, in _get_abstract_dist_for
    self.require_hashes
  File "/private/tmp/repro-pip-egg/env3/lib/python3.7/site-packages/pip/_internal/operations/prepare.py", line 256, in prepare_linked_requirement
    path = url_to_path(req.link.url)
  File "/private/tmp/repro-pip-egg/env3/lib/python3.7/site-packages/pip/_internal/download.py", line 521, in url_to_path
    % url
ValueError: non-local file URIs are not supported on this platform: 'file://./bar'

EDIT:

Just found out that RFC 3986 specifies that relative path URIs are not permitted with the file: scheme, so technically six should be erroring out on file:./foo/bar.

However, that means, technically, I should be able to do the following in my setup.py:

PKG_DIR = os.path.dirname(os.path.abspath(__file__))
install_requires = [
    f"name @ file://{PKG_DIR}/foo/bar"
]

However, pip seems to be creating a "clean" copy of the package in /tmp, so we get something like file:///tmp/pip-req-build-9u3z545j/foo/bar.

Running that through our test function, we satisfy the second function's conditional:

>>> tryparse('file:///tmp/pip-req-build-9u3z545j/foo/bar')
file:///tmp/pip-req-build-9u3z545j/foo/bar
ParseResult(scheme='file', netloc='', path='/tmp/pip-req-build-9u3z545j/foo/bar', params='', query='', fragment='')
file:///tmp/pip-req-build-9u3z545j/foo/bar
ParseResult(scheme='file', netloc='', path='/tmp/pip-req-build-9u3z545j/foo/bar', params='', query='', fragment='')

Everything is good there. The "unparse" yields the same result, and the netloc requirements are met for the first function's conditional.

However, we're still met with an Invalid URL error, even though the second function's logic is satisfied.

Since pip (or distutils or setuptools or whatever) swallows output, I went ahead and did the following in my setup.py

import os
PKG_DIR = os.path.dirname(os.path.abspath(__file__))
assert False, os.system(f"find {PKG_DIR}")

Which verifies that all of the files are there, as expected - so it can't be a file missing or something. The line above that has "Invalid URL given" is the only place in the codebase that string shows up.

At this point, I'm not sure what the problem is.

@triage-new-issues triage-new-issues bot added the S: needs triage Issues/PRs that need to be triaged label Jun 28, 2019
@Qix-
Copy link
Author

Qix- commented Jun 28, 2019

Okay, I see the problem. setuptools, pkg-resources and pip all use slightly different versions of the packaging library.

In pip, it's the version I showed above.

However, in everything else, it's the following (I'm not sure which is the "newer" one, but the following logic is very limiting and not fully compliant as per RFC 3986 as file:/// should be allowed, implying an empty netloc):

        if req.url:
            parsed_url = urlparse.urlparse(req.url)
            if not (parsed_url.scheme and parsed_url.netloc) or (
                    not parsed_url.scheme and not parsed_url.netloc):
                raise InvalidRequirement("Invalid URL given")

🙄

That means since my filepath has file:///foo/bar and not file://localhost/foo/bar then it fails.

Here is the complete solution:

import os
from setuptools import setup

PKG_DIR = os.path.dirname(os.path.abspath(__file__))

setup(
    install_requires=[
        f'foo @ file://localhost{PKG_DIR}/foo/bar'
    ]
)

This is pretty bad UX mixed in with ambiguous and time-wasting errors.

How can we improve this situation?

@Qix- Qix- changed the title It's impossible to write "package @ ./local/path" in a way pip accepts It's ~~impossible~~ ridiculously cumbersome to write "package @ ./local/path" in a way pip accepts Jun 28, 2019
@ryanaklein
Copy link

@Qix- glad you found this! I was beating my head against the wall trying all the same formats. This is my alternative option to #6162 and the deprecation of dependency_links.

We're trying to set up a private repo and don't have our own internal server. Our solution is to publish packages to s3 and then to consume them we download them, put them in a local folder, then add them to install_requires.

I'm sure there are many other uses cases that would benefit from an intuitive way to install local packages.

@Qix-
Copy link
Author

Qix- commented Nov 26, 2019

@ryanaklein I would actually suggest ignoring all of the unresearch negativity towards git submodules and try them out (assuming you're using Git). If you stop thinking about them as branches and start thinking about them as tags (or releases), they begin to work really well. They're very frequently used in the C/C++ world, and we vendored Python packages using them pretty successfully (aside from the above bug, of course!).

Might cut down on the network/$$ costs of S3 :)

@chrahunt chrahunt added project: vendored dependency Related to a vendored dependency state: needs discussion This needs some more discussion type: bug A confirmed bug or unintended behavior labels Dec 8, 2019
@triage-new-issues triage-new-issues bot removed the S: needs triage Issues/PRs that need to be triaged label Dec 8, 2019
@chrahunt
Copy link
Member

chrahunt commented Dec 8, 2019

Expected behavior
I should be able to do name @ ./some/path (or, honestly, simply ./some/path) to specify a vendored package local to my codebase.

For the direct URL reference (name @ ./some/path) there are two places where work is happening:

  1. URI parsing packaging#120 which tracks rejection of valid PEP 508 direct url references
  2. on the pip side we need to interpret relative paths relative to something, I started the discussion here to get feedback on what makes the most sense. We could track the eventual action here or in a new dedicated issue.

The latter wouldn't be acceptable per PEP 508, so it would be hard to justify supporting much less get it working across all tools.

This is pretty bad UX mixed in with ambiguous and time-wasting errors.
How can we improve this situation?

#5204 should help with this general question from a pip standpoint.

@chrahunt chrahunt changed the title It's ~~impossible~~ ridiculously cumbersome to write "package @ ./local/path" in a way pip accepts pip doesn't support relative paths in direct URL references Dec 8, 2019
@piotr-dobrogost
Copy link

Oops! Notice how, when we "unparse" the result from the first parse call, our path becomes absolute file:///...

I think this is due to the CPython bug raised in issue 22852 – "urllib.parse wrongly strips empty #fragment, ?query, //netloc"

That bug seems to cause also issue #3783 – see this comment.

@hackermd
Copy link

hackermd commented May 15, 2020

What is the status of this issue? A solution for resolving local dependencies that are not on PyPI is urgently needed, for example in the context of monolithic repositories.

Note that npm has implemented this feature in a similar way and dependencies can be specified in package.json using a local path.

@uranusjr
Copy link
Member

Please see this comment above to know what needs to happen before this has a chance to be implmenented. pip can do nothing before that.

@ofek
Copy link
Contributor

ofek commented May 23, 2022

FYI Hatch now supports relative paths using the new context formatting capability https://hatch.pypa.io/latest/config/dependency/#local

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project: vendored dependency Related to a vendored dependency state: needs discussion This needs some more discussion type: bug A confirmed bug or unintended behavior
Projects
None yet
Development

No branches or pull requests

7 participants