diff --git a/piptools/repositories/base.py b/piptools/repositories/base.py index dd73ff32e..6ac06bad9 100644 --- a/piptools/repositories/base.py +++ b/piptools/repositories/base.py @@ -44,3 +44,11 @@ def allow_all_wheels(self): """ Monkey patches pip.Wheel to allow wheels from all platforms and Python versions. """ + + def copy_ireq_dependencies(self, source, dest): + """ + Notifies the repository that `dest` is a copy of `source`, and so it + has the same dependencies. Otherwise, once we prepare an ireq to assign + it its name, we would lose track of those dependencies on combining + that ireq with others. + """ diff --git a/piptools/repositories/pypi.py b/piptools/repositories/pypi.py index b3a1cc76b..33d875e38 100644 --- a/piptools/repositories/pypi.py +++ b/piptools/repositories/pypi.py @@ -178,6 +178,10 @@ def resolve_reqs(self, download_dir, ireq, wheel_cache): upgrade_strategy="to-satisfy-only", ) results = resolver._resolve_one(reqset, ireq) + if not ireq.prepared: + # If still not prepared, e.g. a constraint, do enough to assign + # the ireq a name: + resolver._get_abstract_dist_for(ireq) if PIP_VERSION[:2] <= (20, 0): reqset.cleanup_files() @@ -235,6 +239,13 @@ def get_dependencies(self, ireq): return self._dependencies_cache[ireq] + def copy_ireq_dependencies(self, source, dest): + try: + self._dependencies_cache[dest] = self._dependencies_cache[source] + except KeyError: + # `source` may not be in cache yet. + pass + def _get_project(self, ireq): """ Return a dict of a project info from PyPI JSON API for a given diff --git a/piptools/resolver.py b/piptools/resolver.py index 37bc21ee5..dfd14077f 100644 --- a/piptools/resolver.py +++ b/piptools/resolver.py @@ -4,7 +4,7 @@ import copy import os from functools import partial -from itertools import chain, count +from itertools import chain, count, groupby from pip._internal.req.constructors import install_req_from_line @@ -14,7 +14,6 @@ UNSAFE_PACKAGES, format_requirement, format_specifier, - full_groupby, is_pinned_requirement, is_url_requirement, key_from_ireq, @@ -45,7 +44,7 @@ def __str__(self): return repr([self.key, self.specifier, self.extras]) -def combine_install_requirements(ireqs): +def combine_install_requirements(repository, ireqs): """ Return a single install requirement that reflects a combination of all the inputs. @@ -56,12 +55,21 @@ def combine_install_requirements(ireqs): for ireq in ireqs: source_ireqs.extend(getattr(ireq, "_source_ireqs", [ireq])) + # Optimization. Don't bother with combination logic. + 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]) + repository.copy_ireq_dependencies(source_ireqs[0], combined_ireq) + 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.req.specifier &= ireq.req.specifier + if combined_ireq.constraint: + # We don't find dependencies for constraint ireqs, so copy them + # from non-constraints: + repository.copy_ireq_dependencies(ireq, combined_ireq) combined_ireq.constraint &= ireq.constraint # Return a sorted, de-duped tuple of extras combined_ireq.extras = tuple( @@ -117,12 +125,7 @@ def __init__( @property def constraints(self): return set( - self._group_constraints( - chain( - sorted(self.our_constraints, key=str), - sorted(self.their_constraints, key=str), - ) - ) + self._group_constraints(chain(self.our_constraints, self.their_constraints)) ) def resolve_hashes(self, ireqs): @@ -220,8 +223,22 @@ def _group_constraints(self, constraints): flask~=0.7 """ - for _, ireqs in full_groupby(constraints, key=key_from_ireq): - yield combine_install_requirements(ireqs) + constraints = list(constraints) + 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(x), not x.editable))), + key=key_from_ireq, + ): + yield combine_install_requirements(self.repository, ireqs) def _resolve_one_round(self): """ @@ -256,7 +273,7 @@ def _resolve_one_round(self): for best_match in best_matches: their_constraints.extend(self._iter_dependencies(best_match)) # Grouping constraints to make clean diff between rounds - theirs = set(self._group_constraints(sorted(their_constraints, key=str))) + theirs = set(self._group_constraints(their_constraints)) # NOTE: We need to compare RequirementSummary objects, since # InstallRequirement does not define equality diff --git a/piptools/utils.py b/piptools/utils.py index 77334474f..00d1ab1be 100644 --- a/piptools/utils.py +++ b/piptools/utils.py @@ -3,7 +3,7 @@ import sys from collections import OrderedDict -from itertools import chain, groupby +from itertools import chain import six from click.utils import LazyFile @@ -144,11 +144,6 @@ def as_tuple(ireq): return name, version, extras -def full_groupby(iterable, key=None): - """Like groupby(), but sorts the input on the group key first.""" - return groupby(sorted(iterable, key=key), key=key) - - def flat_map(fn, collection): """Map a function over a collection and flatten the result by one-level""" return chain.from_iterable(map(fn, collection)) diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 3b3771ea7..0b1d76adb 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -219,6 +219,53 @@ def test_editable_package(pip_conf, runner): assert "small-fake-a==0.1" in out.stderr +def test_editable_package_without_non_editable_duplicate(pip_conf, runner): + """ + piptools keeps editable requirement, + without also adding a duplicate "non-editable" requirement variation + """ + fake_package_dir = os.path.join(PACKAGES_PATH, "small_fake_a") + fake_package_dir = path_to_url(fake_package_dir) + with open("requirements.in", "w") as req_in: + # small_fake_with_unpinned_deps also requires small_fake_a + req_in.write( + "-e " + + fake_package_dir + + "\nsmall_fake_with_unpinned_deps" # require editable fake package + ) + + out = runner.invoke(cli, ["-n"]) + + assert out.exit_code == 0 + assert fake_package_dir in out.stderr + # Shouldn't include a non-editable small-fake-a==. + assert "small-fake-a==" not in out.stderr + + +def test_editable_package_constraint_without_non_editable_duplicate(pip_conf, runner): + """ + piptools keeps editable constraint, + without also adding a duplicate "non-editable" requirement variation + """ + fake_package_dir = os.path.join(PACKAGES_PATH, "small_fake_a") + fake_package_dir = path_to_url(fake_package_dir) + with open("constraints.txt", "w") as constraints: + constraints.write("-e " + fake_package_dir) # require editable fake package + + with open("requirements.in", "w") as req_in: + req_in.write( + "-c constraints.txt" # require editable fake package + "\nsmall_fake_with_unpinned_deps" # This one also requires small_fake_a + ) + + out = runner.invoke(cli, ["-n"]) + + assert out.exit_code == 0 + assert fake_package_dir in out.stderr + # Shouldn't include a non-editable small-fake-a==. + assert "small-fake-a==" not in out.stderr + + @pytest.mark.parametrize(("req_editable",), [(True,), (False,)]) def test_editable_package_in_constraints(pip_conf, runner, req_editable): """ diff --git a/tests/test_data/packages/small_fake_a/setup.py b/tests/test_data/packages/small_fake_a/setup.py new file mode 100644 index 000000000..f1b8ceeb9 --- /dev/null +++ b/tests/test_data/packages/small_fake_a/setup.py @@ -0,0 +1,3 @@ +from setuptools import setup + +setup(name="small_fake_a", version=0.1) diff --git a/tests/test_resolver.py b/tests/test_resolver.py index 5c0fe671e..fe70b1587 100644 --- a/tests/test_resolver.py +++ b/tests/test_resolver.py @@ -271,17 +271,17 @@ def test_iter_dependencies_ignores_constraints(resolver, from_line): next(res._iter_dependencies(ireq)) -def test_combine_install_requirements(from_line): +def test_combine_install_requirements(repository, 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([celery30, celery31]) + combined = combine_install_requirements(repository, [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([celery32, combined]) + combined_all = combine_install_requirements(repository, [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"