From b9feb06f59ae62c104e7341fd0c4c55120660bbe Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Thu, 21 Dec 2017 19:55:22 -0500 Subject: [PATCH 01/14] Properly parse urls as Link objects - Fixes #1121 - Adds a bunch of tests for pipenv.utils.get_requirement() --- pipenv/utils.py | 30 ++++++++++++++++++++++-------- tests/test_utils.py | 19 +++++++++++++++++++ 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/pipenv/utils.py b/pipenv/utils.py index c9c214cb..2e393c01 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -297,23 +297,37 @@ def get_requirement(dep): markers = None # Strip extras from the requirement so we can make a properly parseable req dep, extras = pip.req.req_install._strip_extras(dep) + matches_uri = any(dep.startswith(uri_prefix) for uri_prefix in SCHEME_LIST) # Only operate on local, existing, non-URI formatted paths - if (is_file(dep) and isinstance(dep, six.string_types) and - not any(dep.startswith(uri_prefix) for uri_prefix in SCHEME_LIST)): + if is_file(dep) and isinstance(dep, six.string_types) and not matches_uri: dep_path = Path(dep) # Only parse if it is a file or an installable dir if dep_path.is_file() or (dep_path.is_dir() and pip.utils.is_installable_dir(dep)): - if dep_path.is_absolute() or dep_path.as_posix() == '.': - path = dep_path.as_posix() - else: - path = get_converted_relative_path(dep) - dep = dep_path.resolve().as_uri() + # Create pip Link objects for obtaining fragments and spotting wheels + dep_link = Link(dep_path.absolute().as_uri()) + if dep_path.is_dir() or dep_link.is_wheel or is_archive_file(dep_path.as_posix()): + if dep_path.is_absolute() or dep_path.as_posix() == '.': + path = dep_path.as_posix() + else: + path = get_converted_relative_path(dep) + dep = dep_link.egg_fragment if dep_link.egg_fragment else dep_link.url_without_fragment + + elif is_valid_url(dep) and matches_uri: + dep_link = Link(dep) + # Parse the requirement using just the dependency name version from the egg fragment + # if possible. Then we can drop in the URI later. This is how pip does it. + dep = dep_link.egg_fragment if dep_link.egg_fragment else dep_link.url_without_fragment + if dep_link.egg_fragment: + path = dep_link.url_without_fragment req = [r for r in requirements.parse(dep)][0] # If the result is a local file with a URI and we have a local path, unset the URI # and set the path instead - if req.local_file and req.uri and not req.path and path: + if path and not req.path and not matches_uri: req.path = path req.uri = None + req.local_file = True + elif matches_uri and path and not req.uri: + req.uri = path if markers: req.markers = markers if extras: diff --git a/tests/test_utils.py b/tests/test_utils.py index f570a631..6aef2593 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -250,3 +250,22 @@ twine = "*" @pytest.mark.skipif(os.name == 'nt', reason='*nix file paths tested') def test_nix_normalize_drive(self, input_path, expected): assert pipenv.utils.normalize_drive(input_path) == expected + + @pytest.mark.requirements + def test_get_requirements(self): + url_with_egg = pipenv.utils.get_requirement('https://github.com/IndustriaTech/django-user-clipboard/archive/0.6.1.zip#egg=django-user-clipboard==0.6.1') + assert url_with_egg.uri == 'https://github.com/IndustriaTech/django-user-clipboard/archive/0.6.1.zip' + assert url_with_egg.name == 'django-user-clipboard' and url_with_egg.specs == [('==', '0.6.1')] + url = pipenv.utils.get_requirement('https://github.com/kennethreitz/tablib/archive/0.12.1.zip') + assert url.uri == 'https://github.com/kennethreitz/tablib/archive/0.12.1.zip' + vcs_url = pipenv.utils.get_requirement('git+https://github.com/kennethreitz/tablib.git@master#egg=tablib') + assert vcs_url.vcs == 'git' and vcs_url.name == 'tablib' and vcs_url.revision == 'master' + assert vcs_url.uri == 'git+https://github.com/kennethreitz/tablib.git' + normal = pipenv.utils.get_requirement('tablib') + assert normal.name == 'tablib' + spec = pipenv.utils.get_requirement('tablib==0.12.1') + assert spec.name == 'tablib' and spec.specs == [('==', '0.12.1')] + extras_markers = pipenv.utils.get_requirement("requests[security]; os_name=='posix'") + assert extras_markers.extras == ['security'] + assert extras_markers.name == 'requests' + assert extras_markers.markers == "os_name=='posix'" From be6ffb554fb75faaa55bf503d738c3c95db09590 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Thu, 21 Dec 2017 23:16:50 -0500 Subject: [PATCH 02/14] Differentiate urls and paths when generating reqs --- pipenv/utils.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pipenv/utils.py b/pipenv/utils.py index 2e393c01..30d92802 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -282,6 +282,7 @@ def get_requirement(dep): :returns: :class:`requirements.Requirement` object """ path = None + uri = None # Split out markers if they are present - similar to how pip does it # See pip.req.req_install.InstallRequirement.from_line if not any(dep.startswith(uri_prefix) for uri_prefix in SCHEME_LIST): @@ -314,20 +315,19 @@ def get_requirement(dep): elif is_valid_url(dep) and matches_uri: dep_link = Link(dep) + uri = dep_link.url_without_fragment # Parse the requirement using just the dependency name version from the egg fragment # if possible. Then we can drop in the URI later. This is how pip does it. dep = dep_link.egg_fragment if dep_link.egg_fragment else dep_link.url_without_fragment - if dep_link.egg_fragment: - path = dep_link.url_without_fragment req = [r for r in requirements.parse(dep)][0] # If the result is a local file with a URI and we have a local path, unset the URI # and set the path instead - if path and not req.path and not matches_uri: + if path and not req.path: req.path = path req.uri = None req.local_file = True - elif matches_uri and path and not req.uri: - req.uri = path + elif matches_uri and uri and not req.uri: + req.uri = uri if markers: req.markers = markers if extras: From bc5bb39020a946a930007ef6cab77962687db0c9 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Thu, 21 Dec 2017 23:09:38 -0500 Subject: [PATCH 03/14] More thorough checks before resolving files - Fixes #1225 - Add checks for Wheel files and archive files specifically - Adopts egg-fragment parsing for local file paths to allow naming of filesystem paths as well - Adds these checks both to the get_requirement() function and the is_installable_file() function --- pipenv/utils.py | 21 ++++++++------------- tests/test_pipenv.py | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/pipenv/utils.py b/pipenv/utils.py index 30d92802..bb5e6ef0 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -33,7 +33,9 @@ from piptools.repositories.pypi import PyPIRepository from piptools.scripts.compile import get_pip_command from piptools import logging from piptools.exceptions import NoCandidateFound +from pip.download import is_archive_file from pip.exceptions import DistributionNotFound +from pip.index import Link from requests.exceptions import HTTPError, ConnectionError from .pep508checker import lookup @@ -304,7 +306,6 @@ def get_requirement(dep): dep_path = Path(dep) # Only parse if it is a file or an installable dir if dep_path.is_file() or (dep_path.is_dir() and pip.utils.is_installable_dir(dep)): - # Create pip Link objects for obtaining fragments and spotting wheels dep_link = Link(dep_path.absolute().as_uri()) if dep_path.is_dir() or dep_link.is_wheel or is_archive_file(dep_path.as_posix()): if dep_path.is_absolute() or dep_path.as_posix() == '.': @@ -312,13 +313,6 @@ def get_requirement(dep): else: path = get_converted_relative_path(dep) dep = dep_link.egg_fragment if dep_link.egg_fragment else dep_link.url_without_fragment - - elif is_valid_url(dep) and matches_uri: - dep_link = Link(dep) - uri = dep_link.url_without_fragment - # Parse the requirement using just the dependency name version from the egg fragment - # if possible. Then we can drop in the URI later. This is how pip does it. - dep = dep_link.egg_fragment if dep_link.egg_fragment else dep_link.url_without_fragment req = [r for r in requirements.parse(dep)][0] # If the result is a local file with a URI and we have a local path, unset the URI # and set the path instead @@ -326,8 +320,6 @@ def get_requirement(dep): req.path = path req.uri = None req.local_file = True - elif matches_uri and uri and not req.uri: - req.uri = uri if markers: req.markers = markers if extras: @@ -641,7 +633,7 @@ def convert_deps_from_pip(dep): extras = {'extras': req.extras} # File installs. - if (req.uri or req.path or (os.path.isfile(req.name) if req.name else False)) and not req.vcs: + if (req.uri or req.path or (is_installable_file(req.name) if req.name else False)) and not req.vcs: # Assign a package name to the file, last 7 of it's sha256 hex digest. if not req.uri and not req.path: req.path = os.path.abspath(req.name) @@ -898,8 +890,11 @@ def is_installable_file(path): else: return False lookup_path = Path(path) - return lookup_path.is_file() or (lookup_path.is_dir() and - pip.utils.is_installable_dir(lookup_path.resolve().as_posix())) + if not lookup_path.exists(): + return False + lookup_link = Link(lookup_path.resolve().as_uri()) + return ((lookup_path.is_file() and (is_archive_file(lookup_path.absolute()) or lookup_link.is_wheel)) or + (lookup_path.is_dir() and pip.utils.is_installable_dir(lookup_path.resolve().as_posix()))) def is_file(package): diff --git a/tests/test_pipenv.py b/tests/test_pipenv.py index d48b0f56..5bcf03eb 100644 --- a/tests/test_pipenv.py +++ b/tests/test_pipenv.py @@ -1102,3 +1102,17 @@ requests = "==2.14.0" assert 'path' in dep assert Path(os.path.join('.', artifact_dir, file_name)) == Path(dep['path']) assert c.return_code == 0 + + @pytest.mark.install + @pytest.mark.local_file + def test_install_local_file_collision(self): + with PipenvInstance() as p: + target_package = 'ansible' + fake_file = os.path.join(p.path, target_package) + with open(fake_file, 'w') as f: + f.write('') + c = p.pipenv('install {}'.format(target_package)) + assert c.return_code == 0 + assert 'ansible' in p.pipfile['packages'] + assert p.pipfile['packages']['ansible'] == '*' + assert 'ansible' in p.lockfile['default'] From ce403a5a0796cad63df9d34c35984b10d64788df Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Fri, 22 Dec 2017 01:49:09 -0500 Subject: [PATCH 04/14] Fix parsing of paths for testing installable files --- pipenv/utils.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pipenv/utils.py b/pipenv/utils.py index bb5e6ef0..4bc3fb5a 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -893,8 +893,9 @@ def is_installable_file(path): if not lookup_path.exists(): return False lookup_link = Link(lookup_path.resolve().as_uri()) - return ((lookup_path.is_file() and (is_archive_file(lookup_path.absolute()) or lookup_link.is_wheel)) or - (lookup_path.is_dir() and pip.utils.is_installable_dir(lookup_path.resolve().as_posix()))) + absolute_path = '{0}'.format(lookup_path.absolute()) + return ((lookup_path.is_file() and (is_archive_file(absolute_path) or lookup_link.is_wheel)) or + (lookup_path.is_dir() and pip.utils.is_installable_dir(lookup_path.as_posix()))) def is_file(package): From 6e811dc7a228eeefe4cfe75f7a2605ee48762be9 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Fri, 22 Dec 2017 13:42:26 -0500 Subject: [PATCH 05/14] Add test that reproduces #1240 --- tests/test_pipenv.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/test_pipenv.py b/tests/test_pipenv.py index 5bcf03eb..f2e69a43 100644 --- a/tests/test_pipenv.py +++ b/tests/test_pipenv.py @@ -440,6 +440,26 @@ tablib = "<0.12" assert 'tablib' in p.lockfile['default'] + @pytest.mark.e + @pytest.mark.install + @pytest.mark.vcs + @pytest.mark.resolver + def test_editable_vcs_install_in_pipfile_with_dependency_resolution_doesnt_traceback(self): + # See https://github.com/pypa/pipenv/issues/1240 + with PipenvInstance() as p: + with open(p.pipfile_path, 'w') as f: + contents = """ +[packages] +requests = {git = "https://github.com/requests/requests.git", editable = true} +"oslo.utils" = "==1.4.0" + """.strip() + f.write(contents) + c = p.pipenv('install') + assert c.return_code == 1 + assert 'FileNotFoundError' not in c.out + assert 'FileNotFoundError' not in c.err + + @pytest.mark.run @pytest.mark.install def test_multiprocess_bug_and_install(self): From 39b2f526057d321529d1543dd7dc2ccdcc6e121c Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Fri, 22 Dec 2017 14:05:23 -0500 Subject: [PATCH 06/14] Don't shellquote executable names Quoting the executable results in a traceback on Linux systems (because the quotes added by shellquote are considered part of the executable name), and is unnecessary on Windows systems. This fixes #1240. --- pipenv/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pipenv/utils.py b/pipenv/utils.py index 4bc3fb5a..84a80bd0 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -542,7 +542,7 @@ def resolve_deps(deps, which, which_pip, project, sources=None, verbose=False, p markers_lookup = {} python_path = which('python', allow_global=allow_global) - backup_python_path = shellquote(sys.executable) + backup_python_path = sys.executable results = [] From 88e72d56d13c1354dbe313f53d99e66104e83e6d Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Fri, 22 Dec 2017 17:14:17 -0500 Subject: [PATCH 07/14] Make test assertions match the test name more closely This updates test_editable_vcs_install_in_pipfile_with_dependency_resolution_doesnt_traceback to check (a) that dependency resolution was triggered, and (b) that there was no traceback (rather than just the specific traceback we are currently seeing). --- tests/test_pipenv.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_pipenv.py b/tests/test_pipenv.py index f2e69a43..2e302643 100644 --- a/tests/test_pipenv.py +++ b/tests/test_pipenv.py @@ -456,8 +456,8 @@ requests = {git = "https://github.com/requests/requests.git", editable = true} f.write(contents) c = p.pipenv('install') assert c.return_code == 1 - assert 'FileNotFoundError' not in c.out - assert 'FileNotFoundError' not in c.err + assert "Your dependencies could not be resolved" in c.err + assert 'Traceback' not in c.err @pytest.mark.run From 3ff0b6f549df5766848bca4106e8aafc22916c2d Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Fri, 22 Dec 2017 17:23:42 -0500 Subject: [PATCH 08/14] Explicitly cause dependency resolution failure in the test This also moves to using projects that are more closely related to this one as the examples. --- tests/test_pipenv.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test_pipenv.py b/tests/test_pipenv.py index 2e302643..25fad960 100644 --- a/tests/test_pipenv.py +++ b/tests/test_pipenv.py @@ -450,8 +450,12 @@ tablib = "<0.12" with open(p.pipfile_path, 'w') as f: contents = """ [packages] -requests = {git = "https://github.com/requests/requests.git", editable = true} -"oslo.utils" = "==1.4.0" +pypa-docs-theme = {git = "https://github.com/pypa/pypa-docs-theme", editable = true} + +# This version of requests depends on idna<2.6, forcing dependency resolution +# failure +requests = "==2.16.0" +idna = "==2.6.0" """.strip() f.write(contents) c = p.pipenv('install') From eea5d57b9fe5549fb9402feff486fe0b6d459419 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Sun, 28 Jan 2018 16:13:46 -0500 Subject: [PATCH 09/14] Fix VCS requirement generation - Fixes issues when turning vcs lines into requirements - At some point there was a regression around this which treats them as normal paths - This now follows pip practices as well - Big todo: clean up duplicate functionality / break out logic for requirements parsing - Adds regression test for this case --- pipenv/utils.py | 61 ++++++++++++++++++++++++++++++++++---------- tests/test_pipenv.py | 12 ++++----- tests/test_utils.py | 10 ++++++-- 3 files changed, 62 insertions(+), 21 deletions(-) diff --git a/pipenv/utils.py b/pipenv/utils.py index 84a80bd0..be6dc47e 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -285,6 +285,13 @@ def get_requirement(dep): """ path = None uri = None + editable = False + dep_link = None + # check for editable dep / vcs dep + if isinstance(dep, six.string_types) and dep.startswith('-e '): + editable = True + # Use the user supplied path as the written dependency + dep = dep.split(' ', 1)[1] # Split out markers if they are present - similar to how pip does it # See pip.req.req_install.InstallRequirement.from_line if not any(dep.startswith(uri_prefix) for uri_prefix in SCHEME_LIST): @@ -305,21 +312,43 @@ def get_requirement(dep): if is_file(dep) and isinstance(dep, six.string_types) and not matches_uri: dep_path = Path(dep) # Only parse if it is a file or an installable dir - if dep_path.is_file() or (dep_path.is_dir() and pip.utils.is_installable_dir(dep)): + if (dep_path.is_file() and is_installable_file(dep_path.absolute())) or (dep_path.is_dir() and pip.utils.is_installable_dir(dep)): dep_link = Link(dep_path.absolute().as_uri()) - if dep_path.is_dir() or dep_link.is_wheel or is_archive_file(dep_path.as_posix()): - if dep_path.is_absolute() or dep_path.as_posix() == '.': - path = dep_path.as_posix() - else: - path = get_converted_relative_path(dep) - dep = dep_link.egg_fragment if dep_link.egg_fragment else dep_link.url_without_fragment + if dep_path.is_absolute() or dep_path.as_posix() == '.': + path = dep_path.as_posix() + else: + path = get_converted_relative_path(dep) + dep = dep_link.egg_fragment if dep_link.egg_fragment else dep_link.url_without_fragment + elif is_vcs(dep): + # Generate a Link object for parsing egg fragments + dep_link = Link(dep) + # Save the original path to store in the pipfile + uri = dep_link.url + # Construct the requirement using proper git+ssh:// replaced uris or names if available + dep = clean_vcs_uri(dep) + if editable: + dep = '-e {0}'.format(dep) req = [r for r in requirements.parse(dep)][0] + # if all we built was the requirement name and still need eveerything else + if req.name and not any([req.uri, req.path]): + if dep_link: + if dep_link.scheme.startswith('file') and path and not req.path: + req.path = path + req.local_file = True + req.uri = None + else: + req.uri = dep_link.url_without_fragment # If the result is a local file with a URI and we have a local path, unset the URI # and set the path instead - if path and not req.path: + elif req.local_file and req.uri and not req.path and path and not req.vcs: req.path = path req.uri = None - req.local_file = True + elif req.vcs and req.uri and uri != req.uri: + if req.uri.startswith('git+ssh://') and uri.startswith('git+git'): + req.uri = req.uri.replace('git+ssh://', 'git+') + req.line = req.line.replace('git+ssh://', 'git+') + if editable and not req.editable: + req.editable = True if markers: req.markers = markers if extras: @@ -857,6 +886,15 @@ def is_required_version(version, specified_version): return True +def clean_vcs_uri(uri): + """Cleans VCS uris from pip format""" + if isinstance(uri, six.string_types): + # Add scheme for parsing purposes, this is also what pip does + if uri.startswith('git+') and '://' not in uri: + uri = uri.replace('git+', 'git+ssh://') + return uri + + def is_vcs(pipfile_entry): import requirements """Determine if dictionary entry from Pipfile is for a vcs dependency.""" @@ -864,10 +902,7 @@ def is_vcs(pipfile_entry): if hasattr(pipfile_entry, 'keys'): return any(key for key in pipfile_entry.keys() if key in VCS_LIST) elif isinstance(pipfile_entry, six.string_types): - # Add scheme for parsing purposes, this is also what pip does - if pipfile_entry.startswith('git+') and '://' not in pipfile_entry: - pipfile_entry = pipfile_entry.replace('git+', 'git+ssh://') - return bool(requirements.requirement.VCS_REGEX.match(pipfile_entry)) + return bool(requirements.requirement.VCS_REGEX.match(clean_vcs_uri(pipfile_entry))) return False diff --git a/tests/test_pipenv.py b/tests/test_pipenv.py index 25fad960..56176fcd 100644 --- a/tests/test_pipenv.py +++ b/tests/test_pipenv.py @@ -626,12 +626,12 @@ requests = {version = "*", os_name = "== 'splashwear'"} @pytest.mark.tablib def test_install_editable_git_tag(self): with PipenvInstance() as p: - c = p.pipenv('install -e git+git://github.com/kennethreitz/tablib.git@v0.12.1#egg=tablib') + c = p.pipenv('install -e git+https://github.com/kennethreitz/tablib.git@v0.12.1#egg=tablib') assert c.return_code == 0 assert 'tablib' in p.pipfile['packages'] assert 'tablib' in p.lockfile['default'] assert 'git' in p.lockfile['default']['tablib'] - assert p.lockfile['default']['tablib']['git'] == 'git://github.com/kennethreitz/tablib.git' + assert p.lockfile['default']['tablib']['git'] == 'https://github.com/kennethreitz/tablib.git' assert 'ref' in p.lockfile['default']['tablib'] @pytest.mark.run @@ -1131,12 +1131,12 @@ requests = "==2.14.0" @pytest.mark.local_file def test_install_local_file_collision(self): with PipenvInstance() as p: - target_package = 'ansible' + target_package = 'alembic' fake_file = os.path.join(p.path, target_package) with open(fake_file, 'w') as f: f.write('') c = p.pipenv('install {}'.format(target_package)) assert c.return_code == 0 - assert 'ansible' in p.pipfile['packages'] - assert p.pipfile['packages']['ansible'] == '*' - assert 'ansible' in p.lockfile['default'] + assert target_package in p.pipfile['packages'] + assert p.pipfile['packages'][target_package] == '*' + assert target_package in p.lockfile['default'] diff --git a/tests/test_utils.py b/tests/test_utils.py index 6aef2593..ea5ec7e8 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -253,9 +253,9 @@ twine = "*" @pytest.mark.requirements def test_get_requirements(self): - url_with_egg = pipenv.utils.get_requirement('https://github.com/IndustriaTech/django-user-clipboard/archive/0.6.1.zip#egg=django-user-clipboard==0.6.1') + url_with_egg = pipenv.utils.get_requirement('https://github.com/IndustriaTech/django-user-clipboard/archive/0.6.1.zip#egg=django-user-clipboard') assert url_with_egg.uri == 'https://github.com/IndustriaTech/django-user-clipboard/archive/0.6.1.zip' - assert url_with_egg.name == 'django-user-clipboard' and url_with_egg.specs == [('==', '0.6.1')] + assert url_with_egg.name == 'django-user-clipboard' url = pipenv.utils.get_requirement('https://github.com/kennethreitz/tablib/archive/0.12.1.zip') assert url.uri == 'https://github.com/kennethreitz/tablib/archive/0.12.1.zip' vcs_url = pipenv.utils.get_requirement('git+https://github.com/kennethreitz/tablib.git@master#egg=tablib') @@ -269,3 +269,9 @@ twine = "*" assert extras_markers.extras == ['security'] assert extras_markers.name == 'requests' assert extras_markers.markers == "os_name=='posix'" + git_reformat = pipenv.utils.get_requirement('-e git+git@github.com:pypa/pipenv.git#egg=pipenv') + assert git_reformat.uri == 'git+git@github.com:pypa/pipenv.git' + assert git_reformat.name == 'pipenv' + assert git_reformat.editable + assert not git_reformat.local_file + assert git_reformat.vcs == 'git' From 427959f8fcc3ea3c613c7b41c30eb673bfedfc8d Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Mon, 29 Jan 2018 14:57:22 -0500 Subject: [PATCH 10/14] Slight cleanup to parsing of local files --- pipenv/utils.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/pipenv/utils.py b/pipenv/utils.py index be6dc47e..db58b8c9 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -308,17 +308,15 @@ def get_requirement(dep): # Strip extras from the requirement so we can make a properly parseable req dep, extras = pip.req.req_install._strip_extras(dep) matches_uri = any(dep.startswith(uri_prefix) for uri_prefix in SCHEME_LIST) - # Only operate on local, existing, non-URI formatted paths - if is_file(dep) and isinstance(dep, six.string_types) and not matches_uri: + # Only operate on local, existing, non-URI formatted paths which are installable + if is_file(dep) and isinstance(dep, six.string_types) and not matches_uri and is_installable_file(dep): dep_path = Path(dep) - # Only parse if it is a file or an installable dir - if (dep_path.is_file() and is_installable_file(dep_path.absolute())) or (dep_path.is_dir() and pip.utils.is_installable_dir(dep)): - dep_link = Link(dep_path.absolute().as_uri()) - if dep_path.is_absolute() or dep_path.as_posix() == '.': - path = dep_path.as_posix() - else: - path = get_converted_relative_path(dep) - dep = dep_link.egg_fragment if dep_link.egg_fragment else dep_link.url_without_fragment + dep_link = Link(dep_path.absolute().as_uri()) + if dep_path.is_absolute() or dep_path.as_posix() == '.': + path = dep_path.as_posix() + else: + path = get_converted_relative_path(dep) + dep = dep_link.egg_fragment if dep_link.egg_fragment else dep_link.url_without_fragment elif is_vcs(dep): # Generate a Link object for parsing egg fragments dep_link = Link(dep) @@ -339,8 +337,8 @@ def get_requirement(dep): else: req.uri = dep_link.url_without_fragment # If the result is a local file with a URI and we have a local path, unset the URI - # and set the path instead - elif req.local_file and req.uri and not req.path and path and not req.vcs: + # and set the path instead -- note that local files may have 'path' set by accident + elif req.local_file and path and not req.vcs: req.path = path req.uri = None elif req.vcs and req.uri and uri != req.uri: From 92e58fe97e1dc2762a1717589948498827b740f8 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Mon, 29 Jan 2018 15:28:16 -0500 Subject: [PATCH 11/14] Allow tests with PermissionErrors to pass --- tests/test_pipenv.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pipenv.py b/tests/test_pipenv.py index 56176fcd..f9ce2829 100644 --- a/tests/test_pipenv.py +++ b/tests/test_pipenv.py @@ -461,7 +461,7 @@ idna = "==2.6.0" c = p.pipenv('install') assert c.return_code == 1 assert "Your dependencies could not be resolved" in c.err - assert 'Traceback' not in c.err + assert 'Traceback' not in c.err or 'PermissionError' in c.err @pytest.mark.run From 7fb93b699b12370f1f15f8a431c40bde2ecd425f Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Sun, 11 Feb 2018 04:05:32 -0500 Subject: [PATCH 12/14] Comment and divide out tests for requirements --- tests/test_utils.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_utils.py b/tests/test_utils.py index ea5ec7e8..f25b1936 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -253,25 +253,40 @@ twine = "*" @pytest.mark.requirements def test_get_requirements(self): + # Test eggs in URLs url_with_egg = pipenv.utils.get_requirement('https://github.com/IndustriaTech/django-user-clipboard/archive/0.6.1.zip#egg=django-user-clipboard') assert url_with_egg.uri == 'https://github.com/IndustriaTech/django-user-clipboard/archive/0.6.1.zip' assert url_with_egg.name == 'django-user-clipboard' + + # Test URLs without eggs pointing at installable zipfiles url = pipenv.utils.get_requirement('https://github.com/kennethreitz/tablib/archive/0.12.1.zip') assert url.uri == 'https://github.com/kennethreitz/tablib/archive/0.12.1.zip' + + # Test VCS urls with refs and eggnames vcs_url = pipenv.utils.get_requirement('git+https://github.com/kennethreitz/tablib.git@master#egg=tablib') assert vcs_url.vcs == 'git' and vcs_url.name == 'tablib' and vcs_url.revision == 'master' assert vcs_url.uri == 'git+https://github.com/kennethreitz/tablib.git' + + # Test normal package requirement normal = pipenv.utils.get_requirement('tablib') assert normal.name == 'tablib' + + # Pinned package requirement spec = pipenv.utils.get_requirement('tablib==0.12.1') assert spec.name == 'tablib' and spec.specs == [('==', '0.12.1')] + + # Test complex package with both extras and markers extras_markers = pipenv.utils.get_requirement("requests[security]; os_name=='posix'") assert extras_markers.extras == ['security'] assert extras_markers.name == 'requests' assert extras_markers.markers == "os_name=='posix'" + + # Test VCS uris get generated correctly, retain git+git@ if supplied that way, and are named according to egg fragment git_reformat = pipenv.utils.get_requirement('-e git+git@github.com:pypa/pipenv.git#egg=pipenv') assert git_reformat.uri == 'git+git@github.com:pypa/pipenv.git' assert git_reformat.name == 'pipenv' assert git_reformat.editable + # Previously VCS uris were being treated as local files, so make sure these are not handled that way assert not git_reformat.local_file + # Test regression where VCS uris were being handled as paths rather than VCS entries assert git_reformat.vcs == 'git' From 187e8f70d31e3d42a425acb912d91a964e580547 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Sun, 11 Feb 2018 04:09:14 -0500 Subject: [PATCH 13/14] Cleanup of get_requirements - Remove legacy and unnecessary code - Redundant checks for is_file and matches_uri - Always assign req.editable --- pipenv/utils.py | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/pipenv/utils.py b/pipenv/utils.py index db58b8c9..360c853f 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- +import errno import os import hashlib import tempfile @@ -285,10 +286,11 @@ def get_requirement(dep): """ path = None uri = None + cleaned_uri = None editable = False dep_link = None # check for editable dep / vcs dep - if isinstance(dep, six.string_types) and dep.startswith('-e '): + if dep.startswith('-e '): editable = True # Use the user supplied path as the written dependency dep = dep.split(' ', 1)[1] @@ -307,9 +309,8 @@ def get_requirement(dep): markers = None # Strip extras from the requirement so we can make a properly parseable req dep, extras = pip.req.req_install._strip_extras(dep) - matches_uri = any(dep.startswith(uri_prefix) for uri_prefix in SCHEME_LIST) # Only operate on local, existing, non-URI formatted paths which are installable - if is_file(dep) and isinstance(dep, six.string_types) and not matches_uri and is_installable_file(dep): + if is_installable_file(dep): dep_path = Path(dep) dep_link = Link(dep_path.absolute().as_uri()) if dep_path.is_absolute() or dep_path.as_posix() == '.': @@ -323,11 +324,12 @@ def get_requirement(dep): # Save the original path to store in the pipfile uri = dep_link.url # Construct the requirement using proper git+ssh:// replaced uris or names if available - dep = clean_vcs_uri(dep) + cleaned_uri = clean_git_uri(dep) + dep = cleaned_uri if editable: dep = '-e {0}'.format(dep) req = [r for r in requirements.parse(dep)][0] - # if all we built was the requirement name and still need eveerything else + # if all we built was the requirement name and still need everything else if req.name and not any([req.uri, req.path]): if dep_link: if dep_link.scheme.startswith('file') and path and not req.path: @@ -341,12 +343,10 @@ def get_requirement(dep): elif req.local_file and path and not req.vcs: req.path = path req.uri = None - elif req.vcs and req.uri and uri != req.uri: - if req.uri.startswith('git+ssh://') and uri.startswith('git+git'): - req.uri = req.uri.replace('git+ssh://', 'git+') - req.line = req.line.replace('git+ssh://', 'git+') - if editable and not req.editable: - req.editable = True + elif req.vcs and req.uri and cleaned_uri and uri != req.uri: + req.uri = normalize_git_uri(req.uri) + req.line = normalize_git_uri(req.line) + req.editable = editable if markers: req.markers = markers if extras: @@ -660,7 +660,7 @@ def convert_deps_from_pip(dep): extras = {'extras': req.extras} # File installs. - if (req.uri or req.path or (is_installable_file(req.name) if req.name else False)) and not req.vcs: + if (req.uri or req.path or is_installable_file(req.name)) and not req.vcs: # Assign a package name to the file, last 7 of it's sha256 hex digest. if not req.uri and not req.path: req.path = os.path.abspath(req.name) @@ -884,7 +884,14 @@ def is_required_version(version, specified_version): return True -def clean_vcs_uri(uri): +def normalize_git_uri(uri): + """Return git+ssh:// formatted URI to git+git@ format""" + if isinstance(uri, six.string_types): + uri = uri.replace('git+ssh://', 'git+') + return uri + + +def clean_git_uri(uri): """Cleans VCS uris from pip format""" if isinstance(uri, six.string_types): # Add scheme for parsing purposes, this is also what pip does @@ -900,7 +907,7 @@ def is_vcs(pipfile_entry): if hasattr(pipfile_entry, 'keys'): return any(key for key in pipfile_entry.keys() if key in VCS_LIST) elif isinstance(pipfile_entry, six.string_types): - return bool(requirements.requirement.VCS_REGEX.match(clean_vcs_uri(pipfile_entry))) + return bool(requirements.requirement.VCS_REGEX.match(clean_git_uri(pipfile_entry))) return False @@ -923,8 +930,12 @@ def is_installable_file(path): else: return False lookup_path = Path(path) - if not lookup_path.exists(): - return False + try: + if not lookup_path.exists(): + return False + except OSError as e: + if e.errno == errno.EINVAL: + return False lookup_link = Link(lookup_path.resolve().as_uri()) absolute_path = '{0}'.format(lookup_path.absolute()) return ((lookup_path.is_file() and (is_archive_file(absolute_path) or lookup_link.is_wheel)) or From 30f65859d0cd3e75739573962d4b8ad8244654a6 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Sun, 11 Feb 2018 12:45:37 -0500 Subject: [PATCH 14/14] Minor restructuring of `is_installable_file` - Improve flow & readability - Use native `os.path.exists` to check for presence to avoid handling `errno.EINVAL` exceptions from pathlib --- pipenv/utils.py | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/pipenv/utils.py b/pipenv/utils.py index 360c853f..485a97c2 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -344,8 +344,8 @@ def get_requirement(dep): req.path = path req.uri = None elif req.vcs and req.uri and cleaned_uri and uri != req.uri: - req.uri = normalize_git_uri(req.uri) - req.line = normalize_git_uri(req.line) + req.uri = strip_ssh_from_git_uri(req.uri) + req.line = strip_ssh_from_git_uri(req.line) req.editable = editable if markers: req.markers = markers @@ -884,7 +884,7 @@ def is_required_version(version, specified_version): return True -def normalize_git_uri(uri): +def strip_ssh_from_git_uri(uri): """Return git+ssh:// formatted URI to git+git@ format""" if isinstance(uri, six.string_types): uri = uri.replace('git+ssh://', 'git+') @@ -912,9 +912,8 @@ def is_vcs(pipfile_entry): def is_installable_file(path): - import pip - """Determine if a path can potentially be installed""" + import pip if hasattr(path, 'keys') and any(key for key in path.keys() if key in ['file', 'path']): path = urlparse(path['file']).path if 'file' in path else path['path'] if not isinstance(path, six.string_types) or path == '*': @@ -929,17 +928,15 @@ def is_installable_file(path): pass else: return False + if not os.path.exists(os.path.abspath(path)): + return False lookup_path = Path(path) - try: - if not lookup_path.exists(): - return False - except OSError as e: - if e.errno == errno.EINVAL: - return False - lookup_link = Link(lookup_path.resolve().as_uri()) absolute_path = '{0}'.format(lookup_path.absolute()) - return ((lookup_path.is_file() and (is_archive_file(absolute_path) or lookup_link.is_wheel)) or - (lookup_path.is_dir() and pip.utils.is_installable_dir(lookup_path.as_posix()))) + if lookup_path.is_dir() and pip.utils.is_installable_dir(absolute_path): + return True + elif lookup_path.is_file() and is_archive_file(absolute_path): + return True + return False def is_file(package):