Dependency resolution regression in 0.1.33 #3127
Comments
|
Thanks, I’ll have to look at the changelog to see what’s up. My initial reaction is that there’s probably a real problem with that package? But that (by chance?) we went down a different resolution path in the prior version — probably due to incorporating pip’s heuristics around how we prioritize packages. |
|
(But need to revisit when I’m back at my laptop.) |
|
Can I ask why you're running with |
Running with no build isolation can speed up repeated builds and is especially helpful when working with compiled extensions because it does not create a throwaway temporary directory for the build but maintains a set build directory, allowing for much faster rebuilds. I don't believe installing setuptools is the right solution here as it is not a direct dependency of our project (neither is it in the reproducer). |
|
I'll take a closer look, but:
I don't think this is the right criteria though, is it? If you run with |
|
I think the |
charliermarsh
added
bug
https://pip.pypa.io/en/stable/cli/pip_install/#cmdoption-no-build-isolation --no-build-isolation means you must install all build dependencies manually. setuptools might be special cased by pip due to legacy reasons, but even that is not required and newer pip is allowed to break here too. Although exact resolver/backtracking behavior is going to be very installer specific. You must manually manage your build dependencies for all packages you resolve if you use it. In practice this tends to be not bad as build dependencies are relatively uncommon that I normally do, |
No, the build isolation setting is not transitive. It only holds for the current package. At least that's what I've gotten to learn from pip. https://pip.pypa.io/en/stable/cli/pip_install/#options just states:
In the reproducer above, the only build dependency (scikit-build-core) is installed prior to the main package install. |
|
I think we should avoid trying to build this package at all (i.e., I agree that this shouldn't require setuptools), but you definitely need to install all build dependencies for all transitive dependencies that will be built from source, if you run with |
|
The issue appears to be uv 0.1.33 is selecting an old version of pytest: Even though the requirements included a higher lower bound: In uv 0.1.32 a much higher version of pytest is selected (that matches the constraints of the lower bound): The requirement for atomicwrites was removed for non-Windows platforms in pytest 5.3.0. |
Oh, @charliermarsh already beat me to while I was testing and writing the post. |
|
Haha yeah. That matches my diagnosis above. Because of the heuristics (which are clearly off a bit), we're fully resolving |
I would also suspect this to be the case. |
|
Yeah, there's something for us to fix here. I'll look into it. |
Thanks for pointing that out! Didn't know that before 🙌 |
I don't follow this. My experience using pip is it is transitive and I've just confirmed this. The easiest way is to test this manually by installing library A that has dependency B which depends on some build dependency and avoid installing it yourself. pip will crash here even though A does not have any build dependencies itself. This is a little tricky to pull off with pip as most recent libraries have wheels so you end up needing to look for interesting example or use --no-binary to avoid choosing the wheel. setuptools is rather special and pip often pre-seeds that one, but that's legacy choice that future pips/python versions may stop doing entirely. I think only recent python versions/pip avoid pre-seeding setuptools. |
Yeah. That was totally a misconception on my end based on the fact that up until the most recent versions of pip, setuptools was always special cased. So I guess I never really noticed the explicit dependency there. |
As someone thinking about Python packaging resolution for awhile I reallly find the optionality of extras to be very very annoying, it makes writing all resolvers significantly more complex. |
|
Ok, I tested it out on your project, and #3133 should fix it. I think there's a "deeper" fix I want to pursue here for some other pathological cases, but that should be good for now. |
## Summary We weren't setting a priority for editables, so they were being visited last. I think there's still a problem whereby we're not aggressive enough in visiting recursive extras (and, in fact, that's making it really hard to write a test -- I wrote a test, but the most-reduced case still fails, and I'd need to add a layer of indirection to make it fail-on-main-but-pass-on-this-branch), but that problem likely already existed on main prior to #3087, so I just want to get this quick fix out now. Closes #3127. ## Test Plan - `git clone https://github.com/cda-tum/mqt-core.git` - `cargo run venv` - `cargo run pip install 'scikit-build-core[pyproject]>=0.8.1' 'setuptools_scm>=7' 'pybind11>=2.12' --resolution=lowest-direct` - `cargo run pip install --no-build-isolation '-ve.[test,qiskit,evaluation,coverage]' --resolution=lowest-direct`
|
Yeah. I can confirm that this fixes all the regressions we were seeing. |
Hey 👋🏼
thanks for all the work on
uv! We recently incorporated it into all the tools we develop as part of our research (https://mqt.readthedocs.io) and the transition was really smooth! 😎However, I believe the latest version (specifically the changes from #3087) might have broken our CI pipelines.
For a production workflow run failing, see: https://github.com/cda-tum/mqt-core/actions/runs/8744279279/job/23998070565
I tried to construct a minimal reproducible example from what I could gather. The following is the best I could come up with.
Given the following
pyproject.tomland a minimal
CMakeLists.txtThe following chain of commands will error out
See below for the complete output.
This is on
uvversion 0.1.33 running on Ubuntu 22.04.Tested to work on version 0.1.32.
My guess would be that it has to do with the recursive self dependencies that are now resolved differently, for whatever reason.
Interestingly, running with build isolation, i.e.
works just fine.
Would be nice to know if this change was intentional and I am missing something or if there is something to be fixed here (at least the scenario above was working in previous versions). Let me know if you need any more info!
The full error log of the
uv pip install '-ve.[test,coverage]' --resolution=lowest-direct --no-build-isolationcommand is as follows:The text was updated successfully, but these errors were encountered: