req: improve extras/markers/link handling #5788

benoit-pierre · 2018-09-16T22:43:18Z

ensure this information is not duplicated at both the InstallRequirement and InstallRequirement.req levels
make the output more consistent, and not dependent on whether an InstallRequirement was created with install_req_from_line or install_req_from_req

Before:

package@url -> Collecting package@ url from url (from ...) / Collecting ... (from package@ url->...)
package[extra]>=1.0; marker -> Collecting package[extra] / Collecting .. (from package[extra]) (when used from the command line), but Collecting package[extra]; marker / Collecting .. (from package[extra]; marker->...) when coming from a package requirements
package-0.8-py2.py3-none-any.whl[test]; marker -> Processing package-0.8-py2.py3-none-any.whl / Collecting ... (from package==0.8)

Basically the format will now consistently looks like: Collecting package==2.0 from URL (from ...) / Collecting ... (from package[extra]==2.0).

benoit-pierre · 2018-09-16T23:04:53Z

While working on this I noticed a few things that don't make sense (to me):

you can't install from an URL with extras: e.g. pip install 'https://files.pythonhosted.org/packages/96/09/31e7455012feb2022bb5404308c5c6026ab192840927a46471c95903c31e/plover-4.0.0.dev8-py3-none-any.whl [gui_qt]' (but you can use environment markers: pip install 'https://files.pythonhosted.org/packages/96/09/31e7455012feb2022bb5404308c5c6026ab192840927a46471c95903c31e/plover-4.0.0.dev8-py3-none-any.whl; "linux" in sys_platform')
and you can't use a PEP 508 direct URL as a workaround for above: e.g. pip install 'plover[gui_qt] @ https://files.pythonhosted.org/packages/96/09/31e7455012feb2022bb5404308c5c6026ab192840927a46471c95903c31e/plover-4.0.0.dev8-py3-none-any.whl' (because the code will assume the whole requirement is a local archive/wheel path)
when using pip install bad/path/to/package-0.8-py2.py3-none-any.whl, a warning will be shown about the requirement looking like a filename, but said filename not existing: why not error-out right there? The code will try to handle it like a filename anyway, and fail later with an EnvironmentError.

wkschwartz · 2018-10-06T17:28:24Z

Ref to this comment: https://github.com/pypa/pip/pull/5571/files#r223152710

I don't have time right now to generate a minimum example. Basically, ProjectB has a install_requirements entry of the form 'ProjectA @ https://github.com/wkschwartz/ProjectA.git@master'. I created a ProjectB wheel with python setup.py bdist_wheel and kept it stored locally on my hard drive. ProjectC requires ProjectB. The first problem is that Pip cannot take requirements in the form 'ProjectB @ file://../ProjectB/dist/ProjectB-1.0.0-any-none.whl' (or, for that matter, 'ProjectB @ file:///Users/wkschwartz/ProjectB/dist/ProjectB-1.0.0-any-none.whl'), so in the venv for ProjectC I manually installed ProjectB with pip install ../ProjectB/dist/ProjectB-1.0.0-any-none.whl. This worked fine. The second problem then came when I installed ProjectC into ProjectC's venv with pip install .. That's when it generated the linked traceback.

benoit-pierre · 2018-10-06T17:35:37Z

Yes, not being able to directly use a PEP 508 URL to a wheel is a known limitation...

benoit-pierre · 2018-10-06T17:52:21Z

Wait, I meant as a direct argument on the command line, but there's another known issue with packaging: see pypa/packaging#120.

benoit-pierre · 2018-10-06T18:34:55Z

@wkschwartz : OK, so after patching pip to handle file:///localhost/ correctly on Unix:

 src/pip/_internal/download.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git i/src/pip/_internal/download.py w/src/pip/_internal/download.py
index 96f3b65c..8d8d1654 100644
--- i/src/pip/_internal/download.py
+++ w/src/pip/_internal/download.py
@@ -463,9 +463,12 @@ def url_to_path(url):
 
     _, netloc, path, _, _ = urllib_parse.urlsplit(url)
 
-    # if we have a UNC path, prepend UNC share notation
     if netloc:
-        netloc = '\\\\' + netloc
+        # if we have a UNC path, prepend UNC share notation
+        if sys.platform == 'win32':
+            netloc = '\\\\' + netloc
+        else:
+            netloc = ''
 
     path = urllib_request.url2pathname(netloc + path)
     return path

I can reproduce the issue on master with this test:

def test_pep508_url_already_installed(script):
    pkgA_path = create_basic_wheel_for_package(
        script,
        name='pkgA', version='1.0',
        depends=[], extras={},
    )
    pkgB_path = create_basic_wheel_for_package(
        script,
        name='pkgB', version='2.0',
        depends=['pkgA @ %s' % ('file://localhost' + str(pkgA_path.abspath))],
        extras={},
    )
    pkgC_path = create_basic_wheel_for_package(
        script,
        name='pkgC', version='3.0',
        depends=['pkgB'],
        extras={},
    )
    r = script.pip_install_local(
        '-v', pkgB_path
    )
    assert "Successfully installed pkgA-1.0 pkgB-2.0" in r.stdout, str(r)
    r = script.pip_install_local(
        '-v', pkgC_path
    )
    assert "Successfully installed pkgC-3.0" in r.stdout, str(r)

Will raise AttributeError: 'NoneType' object has no attribute 'netloc', but it works fine with this PR.

astrojuanlu · 2018-10-09T10:45:51Z

While working on this I noticed a few things that don't make sense (to me):

@marcromani and I were hit by this yesterday. Is it an issue that has already been reported?

Also, I see some comments here and there about how problematic it is to properly support PEP 508 URL requirements opposed to dependency_links, but I assume the roadmap is to go on with them, is that correct?

news/5788.bugfix

WeatherGod · 2018-10-16T21:17:24Z

src/pip/_internal/req/req_install.py

+        self.extras = {
+            pkg_resources.safe_extra(extra) for extra in extras
+        }
+        self.extras = set(extras)


This doesn't seem right, or at least, it is redundant?

Good catch.

WeatherGod · 2018-10-16T21:19:27Z

src/pip/_internal/req/req_install.py

-        s = str(self.req)
+        s = self.req.name
+        if self.extras:
+            s += '[' + ','.join(sorted(self.extras)) + ']'


This would be better as s += '[%s]' % ','.join(sorted(self.extras)), and it improves performance.

benoit-pierre · 2018-10-17T13:25:59Z

Rebased on #5893.

* allow calling it with `comes_from=None` (easier for testing) * handle the case where `comes_from.link is None` (which can happen if a package using a PEP 508 URL requirement is already installed)

- ensure this information is not duplicated at both the `InstallRequirement` and `InstallRequirement.req` levels - make the output more consistent, and not dependent on whether an `InstallRequirement` was created with `install_req_from_line` or `install_req_from_req`

BrownTruck · 2018-11-16T08:30:06Z

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

wkschwartz · 2018-11-28T16:07:18Z

Does the closing of this PR mean that #5788 (comment) won't get fixed, or is there another PR taking care of this?

benoit-pierre · 2018-11-28T16:14:06Z

It means someone else will have to do the work of pushing this and other related changes.

astrojuanlu · 2018-11-28T16:16:35Z

Thanks @benoit-pierre for the effort and dedication, these are no easy tasks and at least the next person that takes care of them will have your work as a starting point.

cjerdonek · 2019-03-20T09:51:39Z

@wkschwartz See #5889 and associated PR's.

lock · 2019-05-28T05:08:33Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

benoit-pierre force-pushed the improve_extras/markers/links_handling branch from 68f941f to 5bcb430 Compare September 18, 2018 12:08

benoit-pierre mentioned this pull request Sep 24, 2018

Updating remote links with new URLs for PEP508 functionallity #5780

Open

benoit-pierre mentioned this pull request Oct 6, 2018

Allow PEP508 url dependencies in install_requires #5571

Merged

astrojuanlu mentioned this pull request Oct 9, 2018

_pip install_ broken beyond normal repair by Jupyter Notebook Extensions (AttributeError: _DistInfoDistribution__dep_map) #5839

Closed

benoit-pierre mentioned this pull request Oct 16, 2018

AttributeError: 'NoneType' object has no attribute 'netloc' #5889

Closed

cjerdonek reviewed Oct 16, 2018

View reviewed changes

news/5788.bugfix Show resolved Hide resolved

WeatherGod reviewed Oct 16, 2018

View reviewed changes

benoit-pierre force-pushed the improve_extras/markers/links_handling branch from 5bcb430 to 6fe66ea Compare October 17, 2018 13:25

improve handling of file URIs

fe7584b

benoit-pierre force-pushed the improve_extras/markers/links_handling branch from 6fe66ea to 6980ce3 Compare October 18, 2018 23:05

benoit-pierre added 3 commits October 19, 2018 13:07

req: fix possible exception in install_req_from_req

92153ed

* allow calling it with `comes_from=None` (easier for testing) * handle the case where `comes_from.link is None` (which can happen if a package using a PEP 508 URL requirement is already installed)

req: minor cleanups

a84621c

benoit-pierre force-pushed the improve_extras/markers/links_handling branch from 6980ce3 to 590bc3a Compare October 19, 2018 18:37

BrownTruck added the needs rebase or merge PR has conflicts with current master label Nov 16, 2018

benoit-pierre closed this Nov 28, 2018

rouge8 mentioned this pull request Apr 22, 2019

Re-install PEP 508 wheel dependencies when the version changes #6402

Closed

lock bot added the auto-locked Outdated issues that have been locked by automation label May 28, 2019

lock bot locked as resolved and limited conversation to collaborators May 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

req: improve extras/markers/link handling #5788

req: improve extras/markers/link handling #5788

benoit-pierre commented Sep 16, 2018

benoit-pierre commented Sep 16, 2018

wkschwartz commented Oct 6, 2018

benoit-pierre commented Oct 6, 2018

benoit-pierre commented Oct 6, 2018

benoit-pierre commented Oct 6, 2018

astrojuanlu commented Oct 9, 2018 •

edited

Loading

WeatherGod Oct 16, 2018

benoit-pierre Oct 16, 2018

WeatherGod Oct 16, 2018

benoit-pierre commented Oct 17, 2018

BrownTruck commented Nov 16, 2018

wkschwartz commented Nov 28, 2018

benoit-pierre commented Nov 28, 2018

astrojuanlu commented Nov 28, 2018

cjerdonek commented Mar 20, 2019

lock bot commented May 28, 2019

req: improve extras/markers/link handling #5788

req: improve extras/markers/link handling #5788

Conversation

benoit-pierre commented Sep 16, 2018

benoit-pierre commented Sep 16, 2018

wkschwartz commented Oct 6, 2018

benoit-pierre commented Oct 6, 2018

benoit-pierre commented Oct 6, 2018

benoit-pierre commented Oct 6, 2018

astrojuanlu commented Oct 9, 2018 • edited Loading

WeatherGod Oct 16, 2018

Choose a reason for hiding this comment

benoit-pierre Oct 16, 2018

Choose a reason for hiding this comment

WeatherGod Oct 16, 2018

Choose a reason for hiding this comment

benoit-pierre commented Oct 17, 2018

BrownTruck commented Nov 16, 2018

wkschwartz commented Nov 28, 2018

benoit-pierre commented Nov 28, 2018

astrojuanlu commented Nov 28, 2018

cjerdonek commented Mar 20, 2019

lock bot commented May 28, 2019

astrojuanlu commented Oct 9, 2018 •

edited

Loading