From eea5d57b9fe5549fb9402feff486fe0b6d459419 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Sun, 28 Jan 2018 16:13:46 -0500 Subject: [PATCH] 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'