Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pip-compile is non-deterministic with multiple declarations of a dependency specifying extras #1451

Closed
gizmo385 opened this issue Jul 14, 2021 · 4 comments · Fixed by #1486
Closed
Labels
bug Something is not working resolver Related to dependency resolver

Comments

@gizmo385
Copy link

gizmo385 commented Jul 14, 2021

Since pip-tools 5.1.2 (c0b33e7), running pip-compile against a requirements.in where there are multiple declarations of a dependency with different extras, the resulting requirements.txt will be a non-deterministic choice between the resolved dependencies of the different declared sets of extras.

Attached to this issue is both a tar file with a minimal reproduction of the issue as well as a log demonstrating reproduction of the issue with the attached example.

Minimal Reproduction
Log demonstrating reproduction of the issue

Environment Versions

  1. macOS Big Sur 11.3.1 (20E241)
  2. Python version: Python 3.7.0
  3. pip version: pip 21.1.3 from /Users/MBP-CChapline/workspace/bug-repro/venv/lib/python3.7/site-packages/pip (python 3.7)
  4. pip-tools version: pip-compile, version 6.2.0

Steps to replicate

This is a minimal reproduction of the issue:

  1. Create a common-package folder with a setup.py that declares 2 extras with different dependencies:
import setuptools

setuptools.setup(
    name="our-cool-common-package",
    description="Some common package",
    packages=setuptools.find_packages(),
    extras_require={
        'extra1': ['requests'],
        'extra2': ['typing']
    }
)
  1. Create requirements-second.in like so:
./common-package[extra2]
  1. Create requirements-main.in like so:
-r ./requirements-second.in
./common-package[extra1]
  1. Run pip-compile requirements-main.in repeatedly and observe the outputted requirement sets.

Expected result

The expected result is that the final requirement set generated by pip-compile will include the union of the extras declared throughout included requirements.in files. So for the above example, it would be expected that the generated requirements.txt would include both requests (from extra1) and typing (From extra2).

Actual result

The actual result is that the resulting set of dependencies is a non-deterministic choice between the different declarations of the common-package[<extras>] dependency. The collapsed details below show example output of this occurring.

Example non-deterministic output
~/workspace/bug-repro ⌚ 10:37:38
$ ./venv/bin/pip-compile requirements-main.in
/Users/MBP-CChapline/workspace/bug-repro/venv/lib/python3.7/site-packages/pip/_internal/operations/prepare.py:226: PipDeprecationWarning: DEPRECATION: A future pip version will change local packages to be built in-place without first copying to a temporary directory. We recommend you use --use-feature=in-tree-build to test your packages with this new behavior before it becomes the default.
 pip 21.3 will remove support for this functionality. You can find discussion regarding this at https://github.com/pypa/pip/issues/7555.
  issue=7555
#
# This file is autogenerated by pip-compile with python 3.7
# To update, run:
#
#    pip-compile requirements-main.in
#
certifi==2021.5.30
    # via requests
charset-normalizer==2.0.1
    # via requests
idna==3.2
    # via requests
file:///Users/MBP-CChapline/workspace/bug-repro/common-package
    # via
    #   -r ./requirements-second.in
    #   -r requirements-main.in
requests==2.26.0
    # via our-cool-common-package
urllib3==1.26.6
    # via requests

~/workspace/bug-repro ⌚ 10:37:50
$ ./venv/bin/pip-compile requirements-main.in
/Users/MBP-CChapline/workspace/bug-repro/venv/lib/python3.7/site-packages/pip/_internal/operations/prepare.py:226: PipDeprecationWarning: DEPRECATION: A future pip version will change local packages to be built in-place without first copying to a temporary directory. We recommend you use --use-feature=in-tree-build to test your packages with this new behavior before it becomes the default.
 pip 21.3 will remove support for this functionality. You can find discussion regarding this at https://github.com/pypa/pip/issues/7555.
  issue=7555
#
# This file is autogenerated by pip-compile with python 3.7
# To update, run:
#
#    pip-compile requirements-main.in
#
certifi==2021.5.30
    # via requests
charset-normalizer==2.0.1
    # via requests
idna==3.2
    # via requests
file:///Users/MBP-CChapline/workspace/bug-repro/common-package
    # via
    #   -r ./requirements-second.in
    #   -r requirements-main.in
requests==2.26.0
    # via our-cool-common-package
urllib3==1.26.6
    # via requests

~/workspace/bug-repro ⌚ 10:37:53
$ ./venv/bin/pip-compile requirements-main.in
/Users/MBP-CChapline/workspace/bug-repro/venv/lib/python3.7/site-packages/pip/_internal/operations/prepare.py:226: PipDeprecationWarning: DEPRECATION: A future pip version will change local packages to be built in-place without first copying to a temporary directory. We recommend you use --use-feature=in-tree-build to test your packages with this new behavior before it becomes the default.
 pip 21.3 will remove support for this functionality. You can find discussion regarding this at https://github.com/pypa/pip/issues/7555.
  issue=7555
#
# This file is autogenerated by pip-compile with python 3.7
# To update, run:
#
#    pip-compile requirements-main.in
#
file:///Users/MBP-CChapline/workspace/bug-repro/common-package
    # via
    #   -r ./requirements-second.in
    #   -r requirements-main.in
typing==3.7.4.3
    # via our-cool-common-package

Additional details:

This issue appears to have been introduced in version 5.1.2 of pip-tools, specifically in commit c0b33e7. During our debugging of this issue, it appears that the patch collapsed below may solve the immediate issue, but we're unsure if it could cause other regressions in pip-compile.

Potential patch
diff --git a/piptools/resolver.py b/piptools/resolver.py
index 14971b0..51b00e2 100644
--- a/piptools/resolver.py
+++ b/piptools/resolver.py
@@ -76,11 +76,12 @@ def combine_install_requirements(

     for ireq in source_ireqs[1:]:
         # NOTE we may be losing some info on dropped reqs here
-        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)
+        if combined_ireq.req is not None and ireq.req is not None:
+            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(sorted({*combined_ireq.extras, *ireq.extras}))
@@ -227,11 +228,6 @@ class Resolver:

         """
         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.

A PR with the above change has been opened as #1452 😊 .

@AndydeCleyre
Copy link
Contributor

@richafrank @auvipy

Can you please take a look at this, and at #1452 ? There's a little more info at #1478 as well.

@AndydeCleyre
Copy link
Contributor

AndydeCleyre commented Sep 14, 2021

I notice in the combine_install_requirements we are directly setting extras as a tuple, which is odd as that attribute should really be a set. Not that it would fix this issue. But I wonder if we ought to be making use of one of the InstallRequirements constructors/helpers rather than assigning attributes here. I also notice that while the combined_ireq seems to get all the extras, the combined_ireq.req does not.

EDIT: Those don't seem to be relevant anyway.

@richafrank
Copy link
Contributor

Thanks @AndydeCleyre . I'll try to take a look later this week.

@richafrank
Copy link
Contributor

@AndydeCleyre I put up #1486 for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working resolver Related to dependency resolver
Projects
None yet
4 participants