conda / conda-lock

Lightweight lockfile for conda environments
https://conda.github.io/conda-lock/
Other
456 stars 101 forks source link

Make the Python resolver respect any __glibc constraint #566

Closed romain-intel closed 6 months ago

romain-intel commented 6 months ago

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

netlify[bot] commented 6 months ago

Deploy Preview for conda-lock ready!

Name Link
Latest commit 0b5ccc257f486c43aeb3e62e7a714c7e3507bbb6
Latest deploy log https://app.netlify.com/sites/conda-lock/deploys/6576c9fa90b2940008ccfb0e
Deploy Preview https://deploy-preview-566--conda-lock.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

maresb commented 6 months ago

There were a few things I still wasn't entirely satisfied with, so I opened https://github.com/romain-intel/conda-lock/pull/1

romain-intel commented 6 months ago

Commented on that PR -- thanks a ton for putting so much work into this.

maresb commented 6 months ago

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.

romain-intel commented 6 months ago

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.

maresb commented 6 months ago

I'm going to merge this once it goes green. Thanks so much @romain-intel for the excellent collaboration!

romain-intel commented 6 months ago

Thank YOU for being so quick on this!