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

Backtracking when package has conflicting runtime dependencies #10719

Closed
1 task done
pitrou opened this issue Dec 9, 2021 · 12 comments
Closed
1 task done

Backtracking when package has conflicting runtime dependencies #10719

pitrou opened this issue Dec 9, 2021 · 12 comments
Labels
C: dependency resolution About choosing which dependencies to install state: needs discussion This needs some more discussion

Comments

@pitrou
Copy link

pitrou commented Dec 9, 2021

Description

The oldest-supported-numpy package version 0.13 has conflicting requirements in its setup.cfg:
https://github.com/scipy/oldest-supported-numpy/blob/v0.13/setup.cfg#L43-L50

Those two lines conflict on s390x and Python 3.8:

    numpy==1.17.5; python_version=='3.8' and platform_machine=='s390x' and platform_python_implementation != 'PyPy'
    numpy==1.17.3; python_version=='3.8' and (platform_machine!='arm64' or platform_system!='Darwin') and platform_machine!='aarch64' and platform_python_implementation != 'PyPy'

When copying those directly in a requirements file, you get the expected error:

$ python -c "import platform; print(platform.machine())"
s390x
$ cat requirements-build.txt 
    numpy==1.17.5; python_version=='3.8' and platform_machine=='s390x' and platform_python_implementation != 'PyPy'
    numpy==1.17.3; python_version=='3.8' and (platform_machine!='arm64' or platform_system!='Darwin') and platform_machine!='aarch64' and platform_python_implementation != 'PyPy'

$ pip install -r requirements-build.txt 
Collecting numpy==1.17.5
  Using cached numpy-1.17.5.zip (6.4 MB)
  Preparing metadata (setup.py) ... done
ERROR: Cannot install numpy==1.17.3 and numpy==1.17.5 because these package versions have conflicting dependencies.

The conflict is caused by:
    The user requested numpy==1.17.5
    The user requested numpy==1.17.3

To fix this you could try to:
1. loosen the range of package versions you've specified
2. remove package versions to allow pip attempt to solve the dependency conflict

ERROR: ResolutionImpossible: for help visit https://pip.pypa.io/en/latest/user_guide/#fixing-conflicting-dependencies

However, when installing said package, pip silently downgrades the package to the previous version without conflicting requirements:

$ pip install oldest-supported-numpy      
Collecting oldest-supported-numpy
  Using cached oldest_supported_numpy-0.13-py3-none-any.whl (3.8 kB)
Collecting numpy==1.17.5
  Using cached numpy-1.17.5.zip (6.4 MB)
  Preparing metadata (setup.py) ... done
Collecting oldest-supported-numpy
  Using cached oldest_supported_numpy-0.12-py3-none-any.whl (3.8 kB)
Collecting numpy==1.17.3
  Using cached numpy-1.17.3.zip (6.4 MB)
  Preparing metadata (setup.py) ... done
Building wheels for collected packages: numpy
[etc.]

It should at least display a warning about the unexpected downgrade (or perhaps better, error out).

Expected behavior

No response

pip version

21.3.1

Python version

3.8.10

OS

Ubuntu 20.04.3

How to Reproduce

See description above.

Output

No response

Code of Conduct

@pitrou pitrou added S: needs triage Issues/PRs that need to be triaged type: bug A confirmed bug or unintended behavior labels Dec 9, 2021
@uranusjr
Copy link
Member

uranusjr commented Dec 9, 2021

This is kind of similar to #10655, but instead of a build failure, the build (building a wheel = no-op) “succeeds” but produces an internally inconsistent result. The inconsisntency can be catched, but the problem is what pip should do when this happens.

@uranusjr uranusjr added C: dependency resolution About choosing which dependencies to install state: needs discussion This needs some more discussion and removed type: bug A confirmed bug or unintended behavior S: needs triage Issues/PRs that need to be triaged labels Dec 9, 2021
@uranusjr uranusjr changed the title pip silently downgrades to previous package version when conflicting requirements in install_requires pip backtracks when package contains internally inconsistent dependencies Dec 9, 2021
@uranusjr uranusjr changed the title pip backtracks when package contains internally inconsistent dependencies pip backtracks when package contains internally conflicting dependencies Dec 9, 2021
@pradyunsg
Copy link
Member

pradyunsg commented Dec 9, 2021

I don't see how this is related to #10655.

This is basically equivalent to:

  • A 2.0 depends on B==1 and B==2
  • A 1.0 depends on B==1

(A is oldest-supported-numpy, B is numpy)

Here, pip is backtracking on the first conflict and moving on to the second item. The only sort of thing that can help here is #10258, but even then, this isn't going to be something that errors out explicitly.

@uranusjr
Copy link
Member

uranusjr commented Dec 9, 2021

Judging from the original issue title, what OP wants is for pip to not look for A 1.0 when A 2.0 fails to resolve, but fail immediately. #10258 (as it is implemented right now) does not help this, because there are no conflicts in the end result; the key thing that needs to change is, when A 2.0 fails, the resolver needs to stop instead of backtracking, which is the same-ish expectation in #10655.

@pradyunsg pradyunsg changed the title pip backtracks when package contains internally conflicting dependencies Backtracking when package has conflicting dependencies Dec 12, 2021
@pradyunsg pradyunsg changed the title Backtracking when package has conflicting dependencies Backtracking when package has conflicting runtime dependencies Dec 12, 2021
@pradyunsg
Copy link
Member

pradyunsg commented Dec 12, 2021

when A 2.0 fails

But there's never a "failure" -- at least, as I use that in my vocabulary?

#10655 is about a failure during metadata generation (i.e. a subprocess call or whatever). It does not deal with changing behaviour around inconsistent metadata or even the properly formed but not-actually-sane metadata. This issue falls in the last category. You have valid metadata (i.e. that can be read, parsed and processed) but that would never generate a proper result.

the resolver needs to stop instead of backtracking, which is the same-ish expectation in #10655.

Yes, but... the main difference is what the level of the issue is. The situation in #10655 is that the package doesn't build metadata successfully.

The situation here is that a package has an unsatisfiable runtime dependencies on one version and a satisfiable set on other versions. This is almost definitionally the case where the resolver needs to backtrack, as I understand it, so... 🤷🏽

If you're suggesting is that pip have some hueristic to detect "bad" package dependency declarations and fail / warn on those versions... I don't like that but I also don't hate it? It does sound like something that would involve a decent amount of complexity/work to figure out and only be genuinely better in some edge cases like this, with a risk of having unintended effects (depending on the heuristic, obviously). This heuristic can't be something like "disallow backtracking on 'top-level' packages" or whatever -- not least because I'm pretty sure a decent chunk of our tests involve backtracking on a 'top-level' package.

If all we want is presentation of dependency conflicts, then #10258 cover this and we can fold this issue into #9254.

@pfmoore
Copy link
Member

pfmoore commented Dec 12, 2021

I'm afraid1 I agree with @pradyunsg here. How is the situation here any different than a package with a dependency on click>=9000? In both cases, the dependencies cannot be satisfied, and that is exactly what backtracking is meant to handle.

Yes, the case of depending on X==1.0 and X==2.0 is logically impossible to satisfy. Something should tell the user about that. But I'd argue that the backend could do that at sdist generation time, just as easily (and more usefully, because then we wouldn't get broken packages uploaded) as pip can do it at runtime2.

But the OP's case wasn't a simple one. The quoted requirements were:

    numpy==1.17.5; python_version=='3.8' and platform_machine=='s390x' and platform_python_implementation != 'PyPy'
    numpy==1.17.3; python_version=='3.8' and (platform_machine!='arm64' or platform_system!='Darwin') and platform_machine!='aarch64' and platform_python_implementation != 'PyPy'

That's not something you're going to be able to validate in advance.

If you're suggesting is that pip have some hueristic to detect "bad" package dependency declarations and fail / warn on those versions... I don't like that but I also don't hate it?

One thing we could potentially do is in _check_metadata_consistency, look for incompatible requirements after markers have been resolved, and raise MetadataInconsistent if any are found. I'd strongly advocate for the function that checks that to be added to packaging, though - if it's useful at all, it will be useful in more places than pip, and I think we should have a single implementation so we don't get inconsistent views on what should be checked.

I don't think we should do anything if our standard handling of MetadataInconsistent isn't good enough for this case. I don't think this is a significant enough situation to warrant special-case code in pip. After all, it's ultimately a bug in the package, not a problem that pip can deal with.

Footnotes

  1. I say "I'm afraid", because I understand how the behaviour we have at the moment is not a good user experience

  2. And by "just as easily" I mean that it's actually not that easy - a general "these two specifier sets are inconsistent" is far from trivial, although catching the simpler cases might be possible and useful.

@pradyunsg
Copy link
Member

That's not something you're going to be able to validate in advance.

For the specific case of this package, they actually can -- scipy/oldest-supported-numpy#34

@pfmoore
Copy link
Member

pfmoore commented Dec 12, 2021

OK, for a given set of supported environments. That's a very good approach, but something only the individual project can do, which I agree is the correct solution here. My comment was more about the backend doing the validation at build time, not the project itself.

@pradyunsg
Copy link
Member

I understand how the behaviour we have at the moment is not a good user experience

Likewise.

I'd strongly advocate for the function that checks that to be added to packaging, though - if it's useful at all, it will be useful in more places than pip, and I think we should have a single implementation so we don't get inconsistent views on what should be checked.

I tend to agree, however, I'm still not sure that we should trigger an error condition here (i.e. fail the entire resolution process) instead of just presenting a message.

And, presenting a message is covered by #9254.

@pradyunsg
Copy link
Member

My comment was more about the backend doing the validation at build time, not the project itself.

Yup yup. I didn't intend to imply that you meant otherwise. :)

@pradyunsg
Copy link
Member

Alright. oldest-supported-numpy is now covered, thanks to scipy/oldest-supported-numpy#35.

Is there really anything actionable here, or can we fold this into #9254?

@pitrou
Copy link
Author

pitrou commented Dec 12, 2021

Just for the record, in my case a simple warning would have been satisfactory. The problem was being unable to easily diagnose the issue, since pip was doing something totally unexpected without saying anything about it (and since multiple packages were involved, finding the root cause wasn't trivial).

@pradyunsg
Copy link
Member

pradyunsg commented Dec 13, 2021

Consolidating this into #9254 (which has an open PR for it) then! Thanks @pitrou for filing this, and triggering an interesting discussion! :)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: dependency resolution About choosing which dependencies to install state: needs discussion This needs some more discussion
Projects
None yet
Development

No branches or pull requests

4 participants