Update PEP 508 dependency parsing implementation

Signed-off-by: Dan Ryan <dan@danryan.co>

news/3148.bugfix.rst

@@ -1 +0,0 @@
-Fixed resolution of direct-url dependencies in ``setup.py`` files to respect ``PEP-508`` style URL dependencies.

news/3148.feature.rst

@@ -0,0 +1 @@
+Added support for resolution of direct-url dependencies in ``setup.py`` files to respect ``PEP-508`` style URL dependencies.

pipenv/utils.py

@@ -235,18 +235,26 @@ def resolve_separate(req):
     This includes non-editable urls to zip or tarballs, non-editable paths, etc.
     """
 
+    from .vendor.requirementslib.models.utils import _requirement_to_str_lowercase_name
+    from .vendor.requirementslib.models.requirements import Requirement
     constraints = set()
+    lockfile_update = {}
     if req.is_file_or_url and not req.is_vcs:
         setup_info = req.run_requires()
         requirements = [v for v in setup_info.get("requires", {}).values()]
         for r in requirements:
             if getattr(r, "url", None) and not getattr(r, "editable", False):
-                from .vendor.requirementslib.models.requirements import Requirement
-                requirement = Requirement.from_line("".join(str(r).split()))
-                constraints |= resolve_separate(requirement)
+                requirement = Requirement.from_line(_requirement_to_str_lowercase_name(r))
+                constraint_update, child_lockfile = resolve_separate(requirement)
+                constraints |= constraint_update
+                lockfile_update.update(child_lockfile)
+                # for local packages with setup.py files and potential direct url deps:
+                if req.editable and requirement.is_direct_url:
+                    name, entry = requirement.pipfile_entry
+                    lockfile_update[name] = entry
                 continue
-            constraints.add(str(r))
-    return constraints
+            constraints.add(_requirement_to_str_lowercase_name(r))
+    return constraints, lockfile_update
 
 
 def get_resolver_metadata(deps, index_lookup, markers_lookup, project, sources):
@@ -264,9 +272,11 @@ def get_resolver_metadata(deps, index_lookup, markers_lookup, project, sources):
         req = Requirement.from_line(dep)
         if req.is_file_or_url and not req.is_vcs:
             # TODO: This is a significant hack, should probably be reworked
-            constraints |= resolve_separate(req)
+            constraint_update, lockfile_update = resolve_separate(req)
+            constraints |= constraint_update
             name, entry = req.pipfile_entry
             skipped[name] = entry
+            skipped.update(lockfile_update)
             continue
         constraints.add(req.constraint_line)
 
@@ -471,31 +481,43 @@ class Resolver(object):
             return False
         return True
 
+    def get_hash(self, ireq, ireq_hashes=None):
+        """
+        Retrieve hashes for a specific ``InstallRequirement`` instance.
+
+        :param ireq: An ``InstallRequirement`` to retrieve hashes for
+        :type ireq: :class:`~pip_shims.InstallRequirement`
+        :return: A set of hashes.
+        :rtype: Set
+        """
+
+        # We _ALWAYS MUST PRIORITIZE_ the inclusion of hashes from local sources
+        # PLEASE *DO NOT MODIFY THIS* TO CHECK WHETHER AN IREQ ALREADY HAS A HASH
+        # RESOLVED. The resolver will pull hashes from PyPI and only from PyPI.
+        # The entire purpose of this approach is to include missing hashes.
+        # This fixes a race condition in resolution for missing dependency caches
+        # see pypa/pipenv#3289
+        if self._should_include_hash(ireq) and (
+            not ireq_hashes or ireq.link.scheme == "file"
+        ):
+            if not ireq_hashes:
+                ireq_hashes = set()
+            new_hashes = self.resolver.repository._hash_cache.get_hash(ireq.link)
+            add_to_set(ireq_hashes, new_hashes)
+        else:
+            ireq_hashes = set(ireq_hashes)
+        # The _ONLY CASE_ where we flat out set the value is if it isn't present
+        # It's a set, so otherwise we *always* need to do a union update
+        if ireq not in self.hashes:
+            return ireq_hashes
+        else:
+            return self.hashes[ireq] | ireq_hashes
+
     def resolve_hashes(self):
         if self.results is not None:
             resolved_hashes = self.resolver.resolve_hashes(self.results)
             for ireq, ireq_hashes in resolved_hashes.items():
-                # We _ALWAYS MUST PRIORITIZE_ the inclusion of hashes from local sources
-                # PLEASE *DO NOT MODIFY THIS* TO CHECK WHETHER AN IREQ ALREADY HAS A HASH
-                # RESOLVED. The resolver will pull hashes from PyPI and only from PyPI.
-                # The entire purpose of this approach is to include missing hashes.
-                # This fixes a race condition in resolution for missing dependency caches
-                # see pypa/pipenv#3289
-                if self._should_include_hash(ireq) and (
-                    not ireq_hashes or ireq.link.scheme == "file"
-                ):
-                    if not ireq_hashes:
-                        ireq_hashes = set()
-                    new_hashes = self.resolver.repository._hash_cache.get_hash(ireq.link)
-                    add_to_set(ireq_hashes, new_hashes)
-                else:
-                    ireq_hashes = set(ireq_hashes)
-                # The _ONLY CASE_ where we flat out set the value is if it isn't present
-                # It's a set, so otherwise we *always* need to do a union update
-                if ireq not in self.hashes:
-                    self.hashes[ireq] = ireq_hashes
-                else:
-                    self.hashes[ireq] |= ireq_hashes
+                self.hashes[ireq] = self.get_hash(ireq, ireq_hashes=ireq_hashes)
             return self.hashes
 
 
@@ -624,10 +646,12 @@ def get_locked_dep(dep, pipfile_section, prefer_pipfile=False):
         version = entry.get("version", "") if entry else ""
     else:
         version = entry if entry else ""
-    lockfile_version = lockfile_entry.get("version", "")
+    lockfile_name, lockfile_dict = lockfile_entry.copy().popitem()
+    lockfile_version = lockfile_dict.get("version", "")
     # Keep pins from the lockfile
     if prefer_pipfile and lockfile_version != version and version.startswith("=="):
-        lockfile_version = version
+        lockfile_dict["version"] = version
+    lockfile_entry[lockfile_name] = lockfile_dict
     return lockfile_entry
 
 
@@ -640,8 +664,10 @@ def prepare_lockfile(results, pipfile, lockfile):
             lockfile_entry = get_locked_dep(dep, pipfile)
             name = next(iter(k for k in lockfile_entry.keys()))
             current_entry = lockfile.get(name)
-            if not current_entry or not is_vcs(current_entry):
-                lockfile.update(lockfile_entry)
+            if current_entry and not is_vcs(current_entry):
+                lockfile[name].update(lockfile_entry[name])
+            else:
+                lockfile[name] = lockfile_entry[name]
     return lockfile
 
 
@@ -751,11 +777,10 @@ def venv_resolve_deps(
     lockfile[lockfile_section] = prepare_lockfile(results, pipfile, lockfile[lockfile_section])
     for k, v in vcs_lockfile.items():
         if k in getattr(project, vcs_section, {}):
-            lockfile[lockfile_section][k].update(v)
-            print(v)
+            if not (isinstance(v, six.string_types) and isinstance(k, Mapping)):
+                lockfile[lockfile_section][k].update(v)
         else:
             lockfile[lockfile_section][k] = v
-            print(v)
 
 
 def resolve_deps(
@@ -829,16 +854,20 @@ def resolve_deps(
         if not result.editable:
             req = Requirement.from_ireq(result)
             name = pep423_name(req.name)
-            version = str(req.get_version())
+            name, pf_entry = req.pipfile_entry
+            if req.specifiers:
+                version = str(req.get_version())
+            else:
+                version = None
             index = index_lookup.get(result.name)
             req.index = index
             collected_hashes = []
             if result in hashes:
                 collected_hashes = list(hashes.get(result))
-            elif resolver._should_include_hashes(result):
+            elif resolver._should_include_hash(result):
                 try:
-                    hash_map = resolver.resolver.resolve_hashes([result])
-                    collected_hashes = next(iter(hash_map.values()), [])
+                    hash_map = resolver.get_hash(result)
+                    collected_hashes = list(hash_map)
                 except (ValueError, KeyError, IndexError, ConnectionError):
                     pass
             elif any(
@@ -866,13 +895,17 @@ def resolve_deps(
                             ), err=True
                         )
             req.hashes = sorted(set(collected_hashes))
-            name, _entry = req.pipfile_entry
             entry = {}
-            if isinstance(_entry, six.string_types):
-                entry["version"] = _entry.lstrip("=")
+            if isinstance(pf_entry, six.string_types):
+                entry["version"] = pf_entry.lstrip("=")
             else:
-                entry.update(_entry)
-                entry["version"] = version
+                entry.update(pf_entry)
+                if version is not None:
+                    entry["version"] = version
+                if req.is_direct_url:
+                    entry["file"] = req.req.uri
+            if collected_hashes:
+                entry["hashes"] = sorted(set(collected_hashes))
             entry["name"] = name
             # if index:
             #     d.update({"index": index})
@@ -1442,15 +1475,13 @@ def clean_resolved_dep(dep, is_top_level=False, pipfile_entry=None):
     # In case we lock a uri or a file when the user supplied a path
     # remove the uri or file keys from the entry and keep the path
     if pipfile_entry and any(k in pipfile_entry for k in ["file", "path"]):
-        fs_key = next((k for k in ["path", "file"] if k in pipfile_entry), None)
-        lockfile_key = next((k for k in ["uri", "file", "path"] if k in lockfile), None)
-        if fs_key != lockfile_key:
-            try:
-                del lockfile[lockfile_key]
-            except KeyError:
-                # pass when there is no lock file, usually because it's the first time
-                pass
+        fs_key = next(iter(k for k in ["path", "file"] if k in dep), None)
+        if fs_key is not None:
             lockfile[fs_key] = pipfile_entry[fs_key]
+    elif any(k in dep for k in ["file", "path"]):
+        fs_key = next(iter(k for k in ["path", "file"] if k in dep), None)
+        if fs_key is not None:
+            lockfile[fs_key] = dep[fs_key]
 
     # If a package is **PRESENT** in the pipfile but has no markers, make sure we
     # **NEVER** include markers in the lockfile

pipenv/vendor/requirementslib/models/requirements.py

@@ -104,6 +104,7 @@ class Line(object):
         self.parsed_marker = None  # type: Optional[Marker]
         self.preferred_scheme = None  # type: Optional[str]
         self.requirement = None  # type: Optional[PackagingRequirement]
+        self.is_direct_url = False  # type: bool
         self._parsed_url = None  # type: Optional[urllib_parse.ParseResult]
         self._setup_cfg = None  # type: Optional[str]
         self._setup_py = None  # type: Optional[str]
@@ -139,9 +140,12 @@ class Line(object):
     @property
     def line_with_prefix(self):
         # type: () -> str
+        line = self.line
+        if self.is_direct_url:
+            line = self.link.url
         if self.editable:
-            return "-e {0}".format(self.line)
-        return self.line
+            return "-e {0}".format(line)
+        return line
 
     @property
     def base_path(self):
@@ -235,7 +239,22 @@ class Line(object):
         :rtype: None
         """
 
-        self.line, extras = pip_shims.shims._strip_extras(self.line)
+        extras = None
+        if "@" in self.line:
+            parsed = urllib_parse.urlparse(add_ssh_scheme_to_git_uri(self.line))
+            if not parsed.scheme:
+                name, _, line = self.line.partition("@")
+                name = name.strip()
+                line = line.strip()
+                if is_vcs(line) or is_valid_url(line):
+                    self.is_direct_url = True
+                name, extras = pip_shims.shims._strip_extras(name)
+                self.name = name
+                self.line = line
+            else:
+                self.line, extras = pip_shims.shims._strip_extras(self.line)
+        else:
+            self.line, extras = pip_shims.shims._strip_extras(self.line)
         if extras is not None:
             self.extras = parse_extras(extras)
 
@@ -253,6 +272,8 @@ class Line(object):
             name, _, url = self.line.partition("@")
             if self.name is None:
                 self.name = name
+                if is_valid_url(url):
+                    self.is_direct_url = True
             line = url.strip()
             parsed = urllib_parse.urlparse(line)
         self._parsed_url = parsed
@@ -394,18 +415,18 @@ class Line(object):
             ireq = pip_shims.shims.install_req_from_line(self.line)
         elif (self.is_file or self.is_url) and not self.is_vcs:
             line = self.line
+            if self.is_direct_url:
+                line = self.link.url
             scheme = self.preferred_scheme if self.preferred_scheme is not None else "uri"
-            if self.setup_py:
-                line = os.path.dirname(os.path.abspath(self.setup_py))
-            elif self.setup_cfg:
-                line = os.path.dirname(os.path.abspath(self.setup_cfg))
-            elif self.pyproject_toml:
-                line = os.path.dirname(os.path.abspath(self.pyproject_toml))
+            local_line = next(iter([
+                os.path.dirname(os.path.abspath(f)) for f in [
+                    self.setup_py, self.setup_cfg, self.pyproject_toml
+                ] if f is not None
+            ]), None)
+            line = local_line if local_line is not None else self.line
             if scheme == "path":
                 if not line and self.base_path is not None:
                     line = os.path.abspath(self.base_path)
-                # if self.extras:
-                    # line = pip_shims.shims.path_to_url(line)
             else:
                 if self.link is not None:
                     line = self.link.url_without_fragment
@@ -414,8 +435,6 @@ class Line(object):
                         line = self.uri
                     else:
                         line = self.path
-            if self.extras:
-                line = "{0}[{1}]".format(line, ",".join(sorted(set(self.extras))))
             if self.editable:
                 ireq = pip_shims.shims.install_req_from_editable(self.link.url)
             else:
@@ -435,9 +454,9 @@ class Line(object):
         # type: () -> None
         if self._ireq is None:
             self._ireq = self.get_ireq()
-        # if self._ireq is not None:
-        #     if self.requirement is not None and self._ireq.req is None:
-        #         self._ireq.req = self.requirement
+        if self._ireq is not None:
+            if self.requirement is not None and self._ireq.req is None:
+                self._ireq.req = self.requirement
 
     def _parse_wheel(self):
         # type: () -> Optional[str]
@@ -499,6 +518,8 @@ class Line(object):
         # type: () -> Optional[PackagingRequirement]
         name = self.name if self.name else self.link.egg_fragment
         url = self.uri if self.uri else unquote(self.link.url)
+        if self.is_direct_url:
+            url = self.link.url
         if not name:
             raise ValueError(
                 "pipenv requires an #egg fragment for version controlled "
@@ -573,7 +594,12 @@ class Line(object):
             self.relpath = relpath
             self.path = path
             self.uri = uri
-            self._link = link
+            if self.is_direct_url and self.name is not None:
+                self._link = create_link(
+                    build_vcs_uri(vcs=vcs, uri=uri, ref=ref, extras=self.extras, name=self.name)
+                )
+            else:
+                self._link = link
 
     def parse_markers(self):
         # type: () -> None
@@ -596,6 +622,7 @@ class Line(object):
         self.parse_requirement()
         self.parse_ireq()
 
+
 @attr.s(slots=True)
 class NamedRequirement(object):
     name = attr.ib()  # type: str
@@ -1309,19 +1336,20 @@ class FileRequirement(object):
 @attr.s(slots=True)
 class VCSRequirement(FileRequirement):
     #: Whether the repository is editable
-    editable = attr.ib(default=None)
+    editable = attr.ib(default=None)  # type: Optional[bool]
     #: URI for the repository
-    uri = attr.ib(default=None)
+    uri = attr.ib(default=None)  # type: Optional[str]
     #: path to the repository, if it's local
-    path = attr.ib(default=None, validator=attr.validators.optional(validate_path))
+    path = attr.ib(default=None, validator=attr.validators.optional(validate_path))  # type: Optional[str]
     #: vcs type, i.e. git/hg/svn
-    vcs = attr.ib(validator=attr.validators.optional(validate_vcs), default=None)
+    vcs = attr.ib(validator=attr.validators.optional(validate_vcs), default=None)  # type: Optional[str]
     #: vcs reference name (branch / commit / tag)
-    ref = attr.ib(default=None)
+    ref = attr.ib(default=None)  # type: Optional[str]
     #: Subdirectory to use for installation if applicable
-    subdirectory = attr.ib(default=None)
-    _repo = attr.ib(default=None)
-    _base_line = attr.ib(default=None)
+    subdirectory = attr.ib(default=None)  # type: Optional[str]
+    _repo = attr.ib(default=None)  # type: Optional['VCSRepository']
+    _base_line = attr.ib(default=None)  # type: Optional[str]
+    _parsed_line = attr.ib(default=None)  # type: Optional[Line]
     name = attr.ib()
     link = attr.ib()
     req = attr.ib()
@@ -1556,16 +1584,32 @@ class VCSRequirement(FileRequirement):
     @classmethod
     def from_line(cls, line, editable=None, extras=None):
         relpath = None
+        parsed_line = Line(line)
+        if editable:
+            line.editable = editable
+        if extras:
+            line.extras = extras
         if line.startswith("-e "):
             editable = True
             line = line.split(" ", 1)[1]
+        if "@" in line:
+            parsed = urllib_parse.urlparse(add_ssh_scheme_to_git_uri(line))
+            if not parsed.scheme:
+                possible_name, _, line = line.partition("@")
+                possible_name = possible_name.strip()
+                line = line.strip()
+                possible_name, extras = pip_shims.shims._strip_extras(possible_name)
+                name = possible_name
+                line = "{0}#egg={1}".format(line, name)
         vcs_type, prefer, relpath, path, uri, link = cls.get_link_from_line(line)
         if not extras and link.egg_fragment:
             name, extras = pip_shims.shims._strip_extras(link.egg_fragment)
-            if extras:
-                extras = parse_extras(extras)
         else:
-            name = link.egg_fragment
+            name, _ = pip_shims.shims._strip_extras(link.egg_fragment)
+        if extras:
+            extras = parse_extras(extras)
+        else:
+            line, extras = pip_shims.shims._strip_extras(line)
         subdirectory = link.subdirectory_fragment
         ref = None
         if "@" in link.path and "@" in uri:
@@ -1576,8 +1620,9 @@ class VCSRequirement(FileRequirement):
                 ref = _ref
         if relpath and "@" in relpath:
             relpath, ref = relpath.rsplit("@", 1)
+
         creation_args = {
-            "name": name,
+            "name": name if name else parsed_line.name,
             "path": relpath or path,
             "editable": editable,
             "extras": extras,
@@ -1585,7 +1630,8 @@ class VCSRequirement(FileRequirement):
             "vcs_type": vcs_type,
             "line": line,
             "uri": uri,
-            "uri_scheme": prefer
+            "uri_scheme": prefer,
+            "parsed_line": parsed_line
         }
         if relpath:
             creation_args["relpath"] = relpath
@@ -1616,6 +1662,8 @@ class VCSRequirement(FileRequirement):
                 else "{0}"
             )
             base = final_format.format(self.vcs_uri)
+        elif self._parsed_line is not None and self._parsed_line.is_direct_url:
+            return self._parsed_line.line_with_prefix
         elif getattr(self, "_base_line", None):
             base = self._base_line
         else:
@@ -1782,8 +1830,15 @@ class Requirement(object):
         # Installable local files and installable non-vcs urls are handled
         # as files, generally speaking
         line_is_vcs = is_vcs(line)
+        is_direct_url = False
         # check for pep-508 compatible requirements
         name, _, possible_url = line.partition("@")
+        name = name.strip()
+        if possible_url is not None:
+            possible_url = possible_url.strip()
+            is_direct_url = is_valid_url(possible_url)
+            if not line_is_vcs:
+                line_is_vcs = is_vcs(possible_url)
         r = None  # type: Optional[Union[VCSRequirement, FileRequirement, NamedRequirement]]
         if is_installable_file(line) or (
             (is_valid_url(possible_url) or is_file_url(line) or is_valid_url(line)) and
@@ -1846,6 +1901,9 @@ class Requirement(object):
         if hashes:
             args["hashes"] = hashes  # type: ignore
         cls_inst = cls(**args)
+        if is_direct_url:
+            setup_info = cls_inst.run_requires()
+            cls_inst.specifiers = "=={0}".format(setup_info.get("version"))
         return cls_inst
 
     @classmethod

pipenv/vendor/requirementslib/models/vcs.py

@@ -83,7 +83,10 @@ class VCSRepository(object):
         new_defaults = [False,] + list(run_command_defaults)[1:]
         new_defaults = tuple(new_defaults)
         if six.PY3:
-            pip_vcs.VersionControl.run_command.__defaults__ = new_defaults
+            try:
+                pip_vcs.VersionControl.run_command.__defaults__ = new_defaults
+            except AttributeError:
+                pip_vcs.VersionControl.run_command.__func__.__defaults__ = new_defaults
         else:
             pip_vcs.VersionControl.run_command.__func__.__defaults__ = new_defaults
         sys.modules[target_module] = pip_vcs

tests/integration/test_install_twists.py

@@ -67,7 +67,7 @@ testpipenv = {path = ".", editable = true, extras = ["dev"]}
 @pytest.mark.local
 @pytest.mark.needs_internet
 @flaky
-class TestDependencyLinks(object):
+class TestDirectDependencies(object):
     """Ensure dependency_links are parsed and installed.
 
     This is needed for private repo dependencies.
@@ -85,18 +85,15 @@ setup(
     version='0.1',
     packages=[],
     install_requires=[
-        'test-private-dependency'
-    ],
-    dependency_links=[
         '{0}'
-    ]
+    ],
 )
             """.strip().format(deplink)
             fh.write(contents)
 
     @staticmethod
     def helper_dependency_links_install_test(pipenv_instance, deplink):
-        TestDependencyLinks.helper_dependency_links_install_make_setup(pipenv_instance, deplink)
+        TestDirectDependencies.helper_dependency_links_install_make_setup(pipenv_instance, deplink)
         c = pipenv_instance.pipenv("install -v -e .")
         assert c.return_code == 0
         assert "test-private-dependency" in pipenv_instance.lockfile["default"]
@@ -108,18 +105,20 @@ setup(
         """
         with temp_environ(), PipenvInstance(pypi=pypi, chdir=True) as p:
             os.environ['PIP_PROCESS_DEPENDENCY_LINKS'] = '1'
-            TestDependencyLinks.helper_dependency_links_install_test(
+            os.environ["PIP_NO_BUILD_ISOLATION"] = '1'
+            TestDirectDependencies.helper_dependency_links_install_test(
                 p,
-                'git+https://github.com/atzannes/test-private-dependency@v0.1#egg=test-private-dependency-v0.1'
+                'test-private-dependency-v0.1@ git+https://github.com/atzannes/test-private-dependency@v0.1'
             )
 
     @pytest.mark.needs_github_ssh
     def test_ssh_dependency_links_install(self, PipenvInstance, pypi):
         with temp_environ(), PipenvInstance(pypi=pypi, chdir=True) as p:
             os.environ['PIP_PROCESS_DEPENDENCY_LINKS'] = '1'
-            TestDependencyLinks.helper_dependency_links_install_test(
+            os.environ["PIP_NO_BUILD_ISOLATION"] = '1'
+            TestDirectDependencies.helper_dependency_links_install_test(
                 p,
-                'git+ssh://git@github.com/atzannes/test-private-dependency@v0.1#egg=test-private-dependency-v0.1'
+                'test-private-dependency-v0.1@ git+ssh://git@github.com/atzannes/test-private-dependency@v0.1'
             )