diff --git a/piptools/resolver.py b/piptools/resolver.py index d76a8b010..012e9f030 100644 --- a/piptools/resolver.py +++ b/piptools/resolver.py @@ -54,7 +54,7 @@ def __str__(self) -> str: def combine_install_requirements( - repository: BaseRepository, ireqs: Iterable[InstallRequirement] + ireqs: Iterable[InstallRequirement], ) -> InstallRequirement: """ Return a single install requirement that reflects a combination of @@ -70,37 +70,66 @@ def combine_install_requirements( if len(source_ireqs) == 1: return source_ireqs[0] - # deepcopy the accumulator so as to not modify the inputs - combined_ireq = copy.deepcopy(source_ireqs[0]) + link_attrs = { + attr: getattr(source_ireqs[0], attr) for attr in ("link", "original_link") + } + + constraint = source_ireqs[0].constraint + extras = set(source_ireqs[0].extras) + # deepcopy the accumulator req so as to not modify the inputs + req = copy.deepcopy(source_ireqs[0].req) for ireq in source_ireqs[1:]: # NOTE we may be losing some info on dropped reqs here - if combined_ireq.req is not None and ireq.req is not None: - combined_ireq.req.specifier &= ireq.req.specifier - combined_ireq.constraint &= ireq.constraint - combined_ireq.extras = {*combined_ireq.extras, *ireq.extras} - if combined_ireq.req is not None: - combined_ireq.req.extras = set(combined_ireq.extras) + if req is not None and ireq.req is not None: + req.specifier &= ireq.req.specifier + + constraint &= ireq.constraint + extras |= ireq.extras + if req is not None: + req.extras = set(extras) + + for attr_name, attr_val in link_attrs.items(): + link_attrs[attr_name] = attr_val or getattr(ireq, attr_name) # InstallRequirements objects are assumed to come from only one source, and # so they support only a single comes_from entry. This function breaks this # model. As a workaround, we deterministically choose a single source for # the comes_from entry, and add an extra _source_ireqs attribute to keep # track of multiple sources for use within pip-tools. - if len(source_ireqs) > 1: - if any(ireq.comes_from is None for ireq in source_ireqs): - # None indicates package was directly specified. - combined_ireq.comes_from = None - else: - # Populate the comes_from field from one of the sources. - # Requirement input order is not stable, so we need to sort: - # We choose the shortest entry in order to keep the printed - # representation as concise as possible. - combined_ireq.comes_from = min( - (ireq.comes_from for ireq in source_ireqs), - key=lambda x: (len(str(x)), str(x)), - ) - combined_ireq._source_ireqs = source_ireqs + if any(ireq.comes_from is None for ireq in source_ireqs): + # None indicates package was directly specified. + comes_from = None + else: + # Populate the comes_from field from one of the sources. + # Requirement input order is not stable, so we need to sort: + # We choose the shortest entry in order to keep the printed + # representation as concise as possible. + comes_from = min( + (ireq.comes_from for ireq in source_ireqs), + key=lambda x: (len(str(x)), str(x)), + ) + + combined_ireq = InstallRequirement( + req=req, + comes_from=comes_from, + editable=source_ireqs[0].editable, + link=link_attrs["link"], + markers=source_ireqs[0].markers, + use_pep517=source_ireqs[0].use_pep517, + isolated=source_ireqs[0].isolated, + install_options=source_ireqs[0].install_options, + global_options=source_ireqs[0].global_options, + hash_options=source_ireqs[0].hash_options, + constraint=constraint, + extras=extras, + user_supplied=source_ireqs[0].user_supplied, + ) + # e.g. If the original_link was None, keep it so. Passing `link` as an + # argument to `InstallRequirement` sets it as the original_link: + combined_ireq.original_link = link_attrs["original_link"] + combined_ireq._source_ireqs = source_ireqs + return combined_ireq @@ -205,39 +234,6 @@ def resolve(self, max_rounds: int = 10) -> Set[InstallRequirement]: return results - def _get_ireq_with_name( - self, - ireq: InstallRequirement, - proxy_cache: Dict[InstallRequirement, InstallRequirement], - ) -> InstallRequirement: - """ - Return the given ireq, if it has a name, or a proxy for the given ireq - which has been prepared and therefore has a name. - - Preparing the ireq is side-effect-ful and can only be done once for an - instance, so we use a proxy instead. combine_install_requirements may - use the given ireq as a template for its aggregate result, mutating it - further by combining extras, etc. In that situation, we don't want that - aggregate ireq to be prepared prior to mutation, since its dependencies - will be frozen with those of only a subset of extras. - - i.e. We both want its name early (via preparation), but we also need to - prepare it after any mutation for combination purposes. So we use a - proxy here for the early preparation. - """ - if ireq.name is not None: - return ireq - - if ireq in proxy_cache: - return proxy_cache[ireq] - - # get_dependencies has the side-effect of assigning name to ireq - # (so we can group by the name in _group_constraints below). - name_proxy = copy.deepcopy(ireq) - self.repository.get_dependencies(name_proxy) - proxy_cache[ireq] = name_proxy - return name_proxy - def _group_constraints( self, constraints: Iterable[InstallRequirement] ) -> Iterator[InstallRequirement]: @@ -258,32 +254,21 @@ def _group_constraints( """ constraints = list(constraints) - cache: Dict[InstallRequirement, InstallRequirement] = {} - - def key_from_ireq_with_name(ireq: InstallRequirement) -> str: - """ - See _get_ireq_with_name for context. - - We use a cache per call here because it should only be necessary - the first time an ireq is passed here (later on in the round, it - will be prepared and dependencies for it calculated), but we can - save time by reusing the proxy between the sort and groupby calls - below. - """ - return key_from_ireq(self._get_ireq_with_name(ireq, cache)) + for ireq in constraints: + if ireq.name is None: + # get_dependencies has side-effect of assigning name to ireq + # (so we can group by the name below). + self.repository.get_dependencies(ireq) # Sort first by name, i.e. the groupby key. Then within each group, # sort editables first. # This way, we don't bother with combining editables, since the first # ireq will be editable, if one exists. for _, ireqs in groupby( - sorted( - constraints, - key=(lambda x: (key_from_ireq_with_name(x), not x.editable)), - ), - key=key_from_ireq_with_name, + sorted(constraints, key=(lambda x: (key_from_ireq(x), not x.editable))), + key=key_from_ireq, ): - yield combine_install_requirements(self.repository, ireqs) + yield combine_install_requirements(ireqs) def _resolve_one_round(self) -> Tuple[bool, Set[InstallRequirement]]: """ @@ -411,7 +396,11 @@ def _iter_dependencies( return if ireq.editable or is_url_requirement(ireq): - yield from self.repository.get_dependencies(ireq) + dependencies = self.repository.get_dependencies(ireq) + # Don't just yield from above. Instead, use the same `markers`-stripping + # behavior as we have for cached dependencies below. + dependency_strings = sorted(str(ireq.req) for ireq in dependencies) + yield from self._ireqs_of_dependencies(ireq, dependency_strings) return elif not is_pinned_requirement(ireq): raise TypeError(f"Expected pinned or editable requirement, got {ireq}") @@ -430,12 +419,20 @@ def _iter_dependencies( # Example: ['Werkzeug>=0.9', 'Jinja2>=2.4'] dependency_strings = self.dependency_cache[ireq] + yield from self._ireqs_of_dependencies(ireq, dependency_strings) + + def _ireqs_of_dependencies( + self, ireq: InstallRequirement, dependency_strings: List[str] + ) -> Iterator[InstallRequirement]: log.debug( "{:25} requires {}".format( format_requirement(ireq), ", ".join(sorted(dependency_strings, key=lambda s: s.lower())) or "-", ) ) + # This yields new InstallRequirements that are similar to those that + # produced the dependency_strings, but they lack `markers` on their + # underlying Requirements: for dependency_string in dependency_strings: yield install_req_from_line( dependency_string, constraint=ireq.constraint, comes_from=ireq diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 2fc70a691..cb3ce1f6b 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -1732,6 +1732,34 @@ def test_duplicate_reqs_combined( assert "test-package-1==0.1" in out.stderr +def test_local_duplicate_subdependency_combined(runner, make_package): + """ + Test pip-compile tracks subdependencies properly when install requirements + are combined, especially when local paths are passed as urls, and those reqs + are combined after getting dependencies. + + Regression test for issue GH-1505. + """ + package_a = make_package("project-a", install_requires=["pip-tools==6.3.0"]) + package_b = make_package("project-b", install_requires=["project-a"]) + + with open("requirements.in", "w") as req_in: + req_in.writelines( + [ + f"file://{package_a}#egg=project-a\n", + f"file://{package_b}#egg=project-b", + ] + ) + + out = runner.invoke(cli, ["-n"]) + + assert out.exit_code == 0 + assert "project-b" in out.stderr + assert "project-a" in out.stderr + assert "pip-tools==6.3.0" in out.stderr + assert "click" in out.stderr # dependency of pip-tools + + def test_combine_extras(pip_conf, runner, make_package): """ Ensure that multiple declarations of a dependency that specify different @@ -1764,6 +1792,73 @@ def test_combine_extras(pip_conf, runner, make_package): assert "small-fake-b==" in out.stderr +def test_combine_different_extras_of_the_same_package( + pip_conf, runner, tmpdir, make_package, make_wheel +): + """ + Loosely based on the example from https://github.com/jazzband/pip-tools/issues/1511. + """ + pkgs = [ + make_package( + "fake-colorful", + version="0.3", + ), + make_package( + "fake-tensorboardX", + version="0.5", + ), + make_package( + "fake-ray", + version="0.1", + extras_require={ + "default": ["fake-colorful==0.3"], + "tune": ["fake-tensorboardX==0.5"], + }, + ), + make_package( + "fake-tune-sklearn", + version="0.7", + install_requires=[ + "fake-ray[tune]==0.1", + ], + ), + ] + + dists_dir = tmpdir / "dists" + for pkg in pkgs: + make_wheel(pkg, dists_dir) + + with open("requirements.in", "w") as req_in: + req_in.writelines( + [ + "fake-ray[default]==0.1\n", + "fake-tune-sklearn==0.7\n", + ] + ) + + out = runner.invoke( + cli, ["--find-links", str(dists_dir), "--no-header", "--no-emit-options"] + ) + assert out.exit_code == 0 + assert ( + dedent( + """\ + fake-colorful==0.3 + # via fake-ray + fake-ray[default,tune]==0.1 + # via + # -r requirements.in + # fake-tune-sklearn + fake-tensorboardx==0.5 + # via fake-ray + fake-tune-sklearn==0.7 + # via -r requirements.in + """ + ) + == out.stderr + ) + + @pytest.mark.parametrize( ("pkg2_install_requires", "req_in_content", "out_expected_content"), ( diff --git a/tests/test_resolver.py b/tests/test_resolver.py index bf28ed6e4..216ebf240 100644 --- a/tests/test_resolver.py +++ b/tests/test_resolver.py @@ -284,33 +284,73 @@ def test_iter_dependencies_ignores_constraints(resolver, from_line): next(res._iter_dependencies(ireq)) -def test_combine_install_requirements(repository, from_line): +def test_iter_dependencies_after_combine_install_requirements( + pypi_repository, base_resolver, make_package, from_line +): + res = base_resolver([], repository=pypi_repository) + + sub_deps = ["click"] + package_a = make_package("package-a", install_requires=sub_deps) + package_b = make_package("package-b", install_requires=["package-a"]) + + local_package_a = from_line(path_to_url(package_a)) + assert [dep.name for dep in res._iter_dependencies(local_package_a)] == sub_deps + + package_a_from_b = from_line("package-a", comes_from=path_to_url(package_b)) + combined = combine_install_requirements([local_package_a, package_a_from_b]) + assert [dep.name for dep in res._iter_dependencies(combined)] == sub_deps + + +def test_iter_dependencies_after_combine_install_requirements_extras( + pypi_repository, base_resolver, make_package, from_line +): + res = base_resolver([], repository=pypi_repository) + + package_a = make_package( + "package-a", extras_require={"click": ["click"], "celery": ["celery"]} + ) + package_b = make_package("package-b", install_requires=["package-a"]) + + local_package_a = from_line(path_to_url(package_a)) + assert [dep.name for dep in res._iter_dependencies(local_package_a)] == [] + + package_a_from_b = from_line("package-a[click]", comes_from=path_to_url(package_b)) + package_a_with_other_extra = from_line("package-a[celery]") + combined = combine_install_requirements( + [local_package_a, package_a_from_b, package_a_with_other_extra] + ) + + dependency_names = {dep.name for dep in res._iter_dependencies(combined)} + assert {"celery", "click"}.issubset(dependency_names) + + +def test_combine_install_requirements(from_line): celery30 = from_line("celery>3.0", comes_from="-r requirements.in") celery31 = from_line("celery==3.1.1", comes_from=from_line("fake-package")) celery32 = from_line("celery<3.2") - combined = combine_install_requirements(repository, [celery30, celery31]) + combined = combine_install_requirements([celery30, celery31]) assert combined.comes_from == celery31.comes_from # shortest string assert set(combined._source_ireqs) == {celery30, celery31} assert str(combined.req.specifier) == "==3.1.1,>3.0" - combined_all = combine_install_requirements(repository, [celery32, combined]) + combined_all = combine_install_requirements([celery32, combined]) assert combined_all.comes_from is None assert set(combined_all._source_ireqs) == {celery30, celery31, celery32} assert str(combined_all.req.specifier) == "<3.2,==3.1.1,>3.0" -def _test_combine_install_requirements_extras(repository, with_extra, without_extra): - combined = combine_install_requirements(repository, [without_extra, with_extra]) +def _test_combine_install_requirements_extras(with_extra, without_extra): + combined = combine_install_requirements([without_extra, with_extra]) assert str(combined) == str(with_extra) assert combined.extras == with_extra.extras - combined = combine_install_requirements(repository, [with_extra, without_extra]) + combined = combine_install_requirements([with_extra, without_extra]) assert str(combined) == str(with_extra) assert combined.extras == with_extra.extras -def test_combine_install_requirements_extras_req(repository, from_line, make_package): +def test_combine_install_requirements_extras_req(from_line, make_package): """ Extras should be unioned in combined install requirements (whether or not InstallRequirement.req is None, and testing either order of the inputs) @@ -320,12 +360,10 @@ def test_combine_install_requirements_extras_req(repository, from_line, make_pac without_extra = from_line("edx-opaque-keys") assert without_extra.req is not None - _test_combine_install_requirements_extras(repository, with_extra, without_extra) + _test_combine_install_requirements_extras(with_extra, without_extra) -def test_combine_install_requirements_extras_no_req( - repository, from_line, make_package -): +def test_combine_install_requirements_extras_no_req(from_line, make_package): """ Extras should be unioned in combined install requirements (whether or not InstallRequirement.req is None, and testing either order of the inputs) @@ -337,10 +375,37 @@ def test_combine_install_requirements_extras_no_req( assert local_package_without_extra.req is None _test_combine_install_requirements_extras( - repository, local_package_with_extra, local_package_without_extra + local_package_with_extra, local_package_without_extra ) +def test_combine_install_requirements_with_paths(from_line, make_package): + name = "fake_package_b" + version = "1.0.0" + + test_package = make_package(name, version=version) + fake_package = from_line(f"{name} @ {path_to_url(test_package)}") + fake_package_name = from_line(f"{name}=={version}", comes_from=from_line(name)) + + for pair in [(fake_package, fake_package_name), (fake_package_name, fake_package)]: + combined = combine_install_requirements(pair) + assert str(combined.specifier) == str(fake_package_name.specifier) + assert str(combined.link) == str(fake_package.link) + assert str(combined.local_file_path) == str(fake_package.local_file_path) + assert str(combined.original_link) == str(fake_package.original_link) + + +def test_combine_install_requirements_for_one_package_with_multiple_extras( + from_line, +): + """Regression test for https://github.com/jazzband/pip-tools/pull/1512""" + pkg1 = from_line("ray[default]==1.1.1") + pkg2 = from_line("ray[tune]==1.1.1") + combined = combine_install_requirements([pkg1, pkg2]) + + assert str(combined) == "ray[default,tune]==1.1.1" + + def test_compile_failure_shows_provenance(resolver, from_line): """ Provenance of conflicting dependencies should be printed on failure.