diff --git a/news/3148.bugfix.rst b/news/3148.bugfix.rst deleted file mode 100644 index 1f0f4a62..00000000 --- a/news/3148.bugfix.rst +++ /dev/null @@ -1 +0,0 @@ -Fixed resolution of direct-url dependencies in ``setup.py`` files to respect ``PEP-508`` style URL dependencies. diff --git a/news/3148.feature.rst b/news/3148.feature.rst new file mode 100644 index 00000000..e33434db --- /dev/null +++ b/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. diff --git a/pipenv/utils.py b/pipenv/utils.py index 602338e7..462d114a 100644 --- a/pipenv/utils.py +++ b/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 diff --git a/pipenv/vendor/requirementslib/models/requirements.py b/pipenv/vendor/requirementslib/models/requirements.py index 81ff2b95..82925368 100644 --- a/pipenv/vendor/requirementslib/models/requirements.py +++ b/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 diff --git a/pipenv/vendor/requirementslib/models/vcs.py b/pipenv/vendor/requirementslib/models/vcs.py index e53927e7..aa9e051e 100644 --- a/pipenv/vendor/requirementslib/models/vcs.py +++ b/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 diff --git a/tests/integration/test_install_twists.py b/tests/integration/test_install_twists.py index 0879a265..bbe626cb 100644 --- a/tests/integration/test_install_twists.py +++ b/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' )