Make the Python resolver respect any __glibc constraint #566
Conversation
✅ Deploy Preview for conda-lock ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
tests/test_conda_lock.py
Outdated
| manylinux_version = [int(each) for each in manylinux_match.groups()] | ||
| # Make sure the manylinux wheel was built with glibc <= 2.17 | ||
| # since that is what the virtual package spec requires | ||
| assert manylinux_version <= [2, 17] |
There was a problem hiding this comment.
Why <= and not ==? Are they backwards-compatible?
There was a problem hiding this comment.
Yes, we could change to ==. I guess I was being overly pedantic since I specified the constraint as glibc max is 2.17. Thinking about it, it wouldn't work in general (the test, not the logic) if that package was only built for 2014 (which is an alias but still).
There was a problem hiding this comment.
I am asking it as a genuine question rather than criticism. Like semantically, if the __glibc has whatever version, does that mean that I can use wheels for anything <= that version, or does it need to be an exact match?
There was a problem hiding this comment.
Yes, my understanding is that GLIBC is backward compatible. See here for example: https://developers.redhat.com/blog/2019/08/01/how-the-gnu-c-library-handles-backward-compatibility.
There was a problem hiding this comment.
Thanks a lot for the explanation.
I don't understand the low-level details, but I think the following overview may be accurate:
There is some Conda magic to detect when a package was compiled using glibc. (I'm not sure exactly what magic, but glibc is only relevant for Linux platforms.) In this case it inserts a dependency that looks something like __glibc >=2.17,<3.0.a0 in case glibc v2.17 was the version used, because later versions of glibc v2 can always run executables built with earlier versions.
An example package depending on glibc is tzcode. The corresponding dependency in the distributed file under info-tzcode-2023c-h0b41bf4_0.tar.zst/info/index.json is the __glibc >=2.17,<3.0.a0 mentioned above.
Now glibc is a system-level dependency not managed by Conda. So when it comes time to install a Conda package on a Linux platform, Conda looks at the system's glibc version and adds a virtual package named __glibc to all environments with the corresponding system version. This way Conda's solver prevents installation of packages built with a version of glibc that's newer than the one installed on the system.
Finally, for pip-installed packages we need to determine which wheels are usable by conda-lock. We use fake virtual packages so that the assumed system-level dependencies are independent of the machine used to generate the lockfile. The strategy here is to determine the version of the fake __glibc virtual package. We then use this to filter the wheels.
Assuming that the fake __glibc version is 2.X and the wheel version is 2.Y, then the constraint __glibc >=2.Y,<3.0.a0 is satisfied whenever X>=Y, so we filter for wheels with glibc version <=2.X. The manylinux spec of PEP 600 defines how to determine the glibc version from the tag.
Assuming this is correct, I wonder if we can include it, or perhaps a link to this comment?
conda_lock/pypi_solver.py
Outdated
| for tag in MANYLINUX_TAGS: | ||
| if tag[0] == "_": | ||
| # Compare to see if glibc_version is greater than this version | ||
| if tag[1:].split("_") > glibc_version_splits: |
There was a problem hiding this comment.
I'm concerned about string comparisons here, like "9" < "10" being false. We should probably use packaging.version.Version. (See the test I wrote below.)
There was a problem hiding this comment.
Yes, we can use that. It seemed a bit heavyweight for this. We could do map(int, tag[1:].split("_") which should solve this issue that I missed (sorry :( ).
tests/test_conda_lock.py
Outdated
| manylinux_version = [int(each) for each in manylinux_match.groups()] | ||
| # Make sure the manylinux wheel was built with glibc <= 2.17 | ||
| # since that is what the virtual package spec requires | ||
| assert manylinux_version <= [2, 17] |
There was a problem hiding this comment.
Yes, we could change to ==. I guess I was being overly pedantic since I specified the constraint as glibc max is 2.17. Thinking about it, it wouldn't work in general (the test, not the logic) if that package was only built for 2014 (which is an alias but still).
conda_lock/pypi_solver.py
Outdated
| for tag in MANYLINUX_TAGS: | ||
| if tag[0] == "_": | ||
| # Compare to see if glibc_version is greater than this version | ||
| if tag[1:].split("_") > glibc_version_splits: |
There was a problem hiding this comment.
Yes, we can use that. It seemed a bit heavyweight for this. We could do map(int, tag[1:].split("_") which should solve this issue that I missed (sorry :( ).
|
There were a few things I still wasn't entirely satisfied with, so I opened romain-intel#1 |
|
Commented on that PR -- thanks a ton for putting so much work into this. |
|
I was wondering if there is any case where we should specify that no glibc dependency allowed. This seems like an extreme edge case, like if we were constructing an environment to be copied into a stock Alpine Docker image. Probably this could be accomplished by setting the fake virtual package glibc version to 0. Probably not worth worrying about it at this point. |
PR conda#541 added support for finding wheels with GLIBC 2.28. However, as rightly pointed out in that PR: "Note this could pose an issue if the glibc on the machine you are doing conda lock install does not have a recent enough glibc" This PR aims to solve this issue by propagating the GLIBC version specified through the virtual package spec to Conda all the way down to the pypi solver. To make all tests pass, the default GLIBC version is also raised to 2.28. We could alternatively keep it fixed and provide a virtual-packages.yml file for the test in PR conda#541 but it feels like both "defaults" should be the same (for pip and conda).
Co-authored-by: Ben Mares <services-git-throwaway1@tensorial.com>
* Refactor manylinux tag logic * Run doctests * Remove unused logger * Clarify that legacy manylinux tags aren't glibc versions
romain-intel
force-pushed
the
fix/respect-glibc-version
branch
from
48c8a69 to
ca4d4cd
Compare
|
I merged your changes and rebased this on top of the latest main branch so should be all set to go. Thanks for working with me on this one :). For no glibc, yes, you could I suppose set a lower enough version (so anything prior to 2 probably) and that would work. It would basically not generate any of the manylinux tags and should therefore use more source distributions. |
|
I'm going to merge this once it goes green. Thanks so much @romain-intel for the excellent collaboration! |
|
Thank YOU for being so quick on this! |
PR #541 added support for finding wheels with GLIBC 2.28. However, as rightly pointed out in that PR:
"Note this could pose an issue if the glibc on the machine you are doing conda lock install does not have a recent enough glibc"
This PR aims to solve this issue by propagating the GLIBC version specified through the virtual package spec to Conda all the way down to the pypi solver.
To make all tests pass, the default GLIBC version is also raised to 2.28. We could alternatively keep it fixed and provide a virtual-packages.yml file for the test in PR #541 but it feels like both "defaults" should be the same (for pip and conda).