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

Update vendored Poetry to 1.8.0 #613

Open
2 tasks done
romain-intel opened this issue Feb 26, 2024 · 5 comments
Open
2 tasks done

Update vendored Poetry to 1.8.0 #613

romain-intel opened this issue Feb 26, 2024 · 5 comments

Comments

@romain-intel
Copy link
Contributor

Checklist

  • I added a descriptive title
  • I searched open requests and couldn't find a duplicate

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 (python-poetry/poetry#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
Copy link
Contributor Author

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

@maresb
Copy link
Contributor

maresb commented Mar 4, 2024

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 😂 😂 😂 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 Support Multiple Categories for Sub-Dependencies in Lockfile  #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 Support Multiple Categories for Sub-Dependencies in Lockfile  #390 first. But this is has been a really huge thorn, so if this is ready before I manage to merge Support Multiple Categories for Sub-Dependencies in Lockfile  #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
Copy link
Contributor Author

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
Copy link
Contributor

maresb commented May 7, 2024

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

@romain-intel
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants