search

conda / conda-lock

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

Update vendored Poetry to 1.8.0 #613

Open romain-intel opened 4 months ago

romain-intel commented 4 months ago

Checklist

What is the idea?

Poetry 1.8.0 came out (see here: https://github.com/python-poetry/poetry/releases/tag/1.8.0) and it adds some significant improvements particularly around resolution speed (support for PEP 658 as well as range requests for repos that do not support PEP 658) as well as correctness issues (https://github.com/python-poetry/poetry/pull/9000 -- an issue that bit me more than once).

It would be nice to move from poetry 1.1.15 (the one currently vendored) to this one.

Why is this needed?

Apart from the points raised above (speed and correctness in certain situations), the version of poetry currently vendored is quite old and upgrading it would be a good idea to keep up with newer developments. Poetry now officially supports python 3.12 which would be a side benefit.

What should happen?

Nothing would change for the user except that resolving environments containing pypi and conda dependencies would probably be faster and less expensive (in network cost). More environments would probably be able to work properly as well.

Additional Context

I am happy to provide help/do this but any guidance would be helpful. I saw some scripts/README around upgrading poetry but I am not sure if there is any additional context around it. I can open a PR for it if this is something that would be accepted (ie: I don't necessarily want to spend a ton of time if it won't be looked at/reviewed and has no path to acceptance :)).

romain-intel commented 3 months ago

Waiting a tad -- poetry 1.8.0 seems to have some issues being worked our around their support for PEP 658 (lazy-wheel).

maresb commented 3 months ago

Hi @romain-intel, thanks so much for your very generous offer.

To explain the situation a bit, in retrospect I was trying to be too clever in #240 when I originally vendored Poetry. (Previously it was an external dependency which was a big mess since Poetry basically assumes that it lives in an isolated environment.)

In principle there's a tool called vendoring which is suited for automating this process. Unfortunately, Poetry itself vendors a lot of stuff, and vendoring sync doesn't handle recursive vendoring so gracefully. Since I wanted to make it easier to revendor in the future :joy: :joy: :joy: I tried to script the process. If you look at the last 8 commits in #240 you'll see that they're script-generated. (I learn from my mistakes, but it's a shame that in this case it's impacted others.)

So basically what needs to happen is:

  1. Rerun vendoring sync
  2. Sort out and organize the new dependencies
  3. Clean up the license files
  4. Update the codebase to fix breakage due to upgrading

I'd be delighted if you could help me out with this.

There are some external considerations to keep in mind:

  1. Poetry is just one of many packaging tools, and we should make a reasonable effort to not give Poetry any preferential treatment.
  2. Conda-lock will likely soon be obsolete thanks to pixi
  3. I'm really keen to merge #390 as soon as possible. I'd rather not keep rebasing that. Depending on how much the codebase needs to be changed to fix breakage, I'm concerned that this might generate merge conflicts. Ideally we'd finish out #390 first. But this is has been a really huge thorn, so if this is ready before I manage to merge #390 then I'd push it through anyways.

I think that's it. So thank you so much for your willingness to help out, and please let me know if there's anything I can do to help move things along.

romain-intel commented 1 month ago

As a quick update -- I have it working (updated to 1.8.2) except for the hash override part. I am still working on this and also trying to support markers because I can't generate the env for all platforms with the new requirements (it needs sys_platform marker support). I'll make this as a separate PR but mentioning it here as a quick update.

maresb commented 1 month ago

Amazing, thanks so much @romain-intel !!!!

romain-intel commented 1 month ago

I added an initial PR. It's not done yet but hopefully gives you a scope of the work (it wasn't too bad).