Skip to content

Commit

Permalink
Fix recursive PEP 508 VCS resolution
Browse files Browse the repository at this point in the history
- Recursively resolve PEP 508 URLs in subdependencies
  - Fix `piptools` cache storage of VCS dependencies as results
- Avoid saving VCS dependency versions to `Pipfile.lock`
- Fixes #3396
- Fixes #4217

Signed-off-by: Dan Ryan <dan.ryan@canonical.com>
  • Loading branch information
techalchemy committed May 5, 2020
1 parent 02cca98 commit 9ae55fd
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 10 deletions.
1 change: 1 addition & 0 deletions news/3396.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Dependencies with direct ``PEP508`` compliant VCS URLs specified in their ``install_requires`` will now be successfully locked during the resolution process.
1 change: 1 addition & 0 deletions news/4217.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed a bug which caused versions from VCS dependencies to be included in ``Pipfile.lock`` inadvertently.
2 changes: 2 additions & 0 deletions pipenv/patched/piptools/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,8 @@ def format_requirement(ireq, marker=None, hashes=None):
"""
if ireq.editable:
line = "-e {}".format(ireq.link.url)
elif ireq.link and ireq.link.is_vcs:
line = str(ireq.req)
elif is_url_requirement(ireq):
line = ireq.link.url
else:
Expand Down
16 changes: 12 additions & 4 deletions pipenv/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,10 @@ def pip_options(self):
if self._pip_options is None:
pip_options, _ = self.pip_command.parser.parse_args(self.pip_args)
pip_options.cache_dir = environments.PIPENV_CACHE_DIR
pip_options.no_python_version_warning = True
pip_options.no_input = True
pip_options.progress_bar = "off"
pip_options.ignore_requires_python = True
self._pip_options = pip_options
return self._pip_options

Expand All @@ -760,7 +764,7 @@ def repository(self):
from pipenv.patched.piptools.repositories.pypi import PyPIRepository
self._repository = PyPIRepository(
self.pip_args, use_json=False, session=self.session,
build_isolation=self.pip_options.build_isolation
build_isolation=False
)
return self._repository

Expand Down Expand Up @@ -1047,13 +1051,17 @@ def format_requirement_for_lockfile(req, markers_lookup, index_lookup, hashes=No
if hashes:
entry["hashes"] = sorted(set(hashes))
entry["name"] = name
if index: # and index != next(iter(project.sources), {}).get("name"):
if index:
entry.update({"index": index})
if markers:
entry.update({"markers": markers})
entry = translate_markers(entry)
if req.vcs or req.editable and entry.get("index"):
del entry["index"]
if req.vcs or req.editable:
for key in ("index", "version"):
try:
del entry[key]
except KeyError:
pass
return name, entry


Expand Down
13 changes: 9 additions & 4 deletions tasks/vendoring/patches/patched/piptools.patch
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ index 430b4bb..015ff7a 100644
from . import click
from .exceptions import IncompatibleRequirements
diff --git a/pipenv/patched/piptools/utils.py b/pipenv/patched/piptools/utils.py
index 7733447..aa93ec8 100644
index 7733447..e6f232f 100644
--- a/pipenv/patched/piptools/utils.py
+++ b/pipenv/patched/piptools/utils.py
@@ -1,14 +1,19 @@
Expand Down Expand Up @@ -796,13 +796,13 @@ index 7733447..aa93ec8 100644
+ Formats a packaging.requirements.Requirement with a lowercase name.
+
+ This is simply a copy of
+ https://github.com/pypa/pipenv/patched/packaging/blob/pipenv/patched/16.8/packaging/requirements.py#L109-L124
+ https://github.com/pypa/pipenv/patched/pipenv/patched/packaging/blob/pipenv/patched/pipenv/patched/16.8/packaging/requirements.py#L109-L124
+ modified to lowercase the dependency name.
+
+ Previously, we were invoking the original Requirement.__str__ method and
+ lowercasing the entire result, which would lowercase the name, *and* other,
+ important stuff that should not be lowercased (such as the marker). See
+ this issue for more information: https://github.com/pypa/pipenv/patched/pipenv/issues/2113.
+ this issue for more information: https://github.com/pypa/pipenv/patched/pipenv/patched/pipenv/issues/2113.
+ """
+ parts = [requirement.name.lower()]
+
Expand All @@ -822,7 +822,12 @@ index 7733447..aa93ec8 100644


def is_url_requirement(ireq):
@@ -80,10 +184,10 @@ def format_requirement(ireq, marker=None, hashes=None):
@@ -77,13 +181,15 @@ def format_requirement(ireq, marker=None, hashes=None):
"""
if ireq.editable:
line = "-e {}".format(ireq.link.url)
+ elif ireq.link and ireq.link.is_vcs:
+ line = str(ireq.req)
elif is_url_requirement(ireq):
line = ireq.link.url
else:
Expand Down
20 changes: 19 additions & 1 deletion tests/integration/test_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,9 @@ def test_lock_editable_vcs_with_extras_without_install(PipenvInstance):
assert "socks" in p.lockfile["default"]["requests"]["extras"]
c = p.pipenv('install')
assert c.return_code == 0
assert "requests" in p.lockfile["default"]
# For backward compatibility we want to make sure not to include the 'version' key
assert "version" not in p.lockfile["default"]["requests"]


@pytest.mark.vcs
Expand Down Expand Up @@ -658,4 +661,19 @@ def test_lock_after_update_source_name(PipenvInstance):
c = p.pipenv("lock --clear")
assert c.return_code == 0
assert "index" in p.lockfile["default"]["six"]
assert p.lockfile["default"]["six"]["index"] == "custom", Path(p.lockfile_path).read_text() # p.lockfile["default"]["six"]
assert p.lockfile["default"]["six"]["index"] == "custom", Path(p.lockfile_path).read_text()


@pytest.mark.lock
def test_lock_nested_direct_url(PipenvInstance):
"""
The dependency 'test_package' has a declared dependency on
a PEP508 style VCS URL. This ensures that we capture the dependency
here along with its own dependencies.
"""
with PipenvInstance(chdir=True) as p:
c = p.pipenv("install test_package")
assert c.return_code == 0
assert "vistir" in p.lockfile["default"]
assert "colorama" in p.lockfie["default"]
assert "six" in p.lockfile["default"]

0 comments on commit 9ae55fd

Please sign in to comment.