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

Add forks to lockfile, don't read them yet #5480

Merged
merged 3 commits into from
Jul 30, 2024
Merged

Conversation

konstin
Copy link
Member

@konstin konstin commented Jul 26, 2024

Add the forks to the lockfile, without using them yet, which we'll add in the next PR.

Please review commit-by-commit

Part of #5180 (comment)

@konstin konstin changed the base branch from main to konsti/unify-resolutions-later July 26, 2024 13:15
@konstin konstin added the preview Experimental behavior label Jul 26, 2024
Base automatically changed from konsti/unify-resolutions-later to main July 26, 2024 14:29
@konstin konstin force-pushed the konsti/add-forks-to-lockfile branch 8 times, most recently from 8aff9ae to f521fdf Compare July 29, 2024 13:50
@konstin konstin marked this pull request as ready for review July 29, 2024 13:51
@konstin konstin requested a review from BurntSushi July 29, 2024 13:51
@konstin konstin added the bug Something isn't working label Jul 29, 2024
Copy link
Member

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all LGTM. I'll mourn the lock file's simplicity. Markers are everywhere!

@BurntSushi
Copy link
Member

Thinking about this a bit more, how does this interact with uv pip compile --universal? We don't write the fork markers to requirements.txt, so does that mean uv pip compile --universal is unstable?

@konstin
Copy link
Member Author

konstin commented Jul 30, 2024

Yes, --universal is unstable, and i don't have a good solution for fixing that:

Example: Packse scenario "preferences-dependent-forking":

First pass of uv pip compile --universal forking.in -o forking.txt:

# This file was autogenerated by uv via the following command:
#    uv pip compile --universal forking.in -o forking.txt
preferences-dependent-forking==0.0.0
    # via -r forking.in
preferences-dependent-forking-bar==1.0.0
    # via
    #   preferences-dependent-forking
    #   preferences-dependent-forking-cleaver
preferences-dependent-forking-cleaver==1.0.0
    # via preferences-dependent-forking
preferences-dependent-forking-foo==1.0.0
    # via
    #   preferences-dependent-forking
    #   preferences-dependent-forking-cleaver
preferences-dependent-forking-foo==2.0.0
    # via preferences-dependent-forking

Second pass of uv pip compile --universal forking.in -o forking.txt:

# This file was autogenerated by uv via the following command:
#    uv pip compile --universal forking.in -o forking.txt
preferences-dependent-forking==0.0.0
    # via -r forking.in
preferences-dependent-forking-bar==1.0.0
    # via
    #   preferences-dependent-forking
    #   preferences-dependent-forking-cleaver
preferences-dependent-forking-cleaver==1.0.0
    # via preferences-dependent-forking
preferences-dependent-forking-foo==1.0.0
    # via
    #   preferences-dependent-forking
    #   preferences-dependent-forking-cleaver

Add the overall list of forks to the lockfiles, so we can use them as preferences to avoid instabilities in the next step.

I've ordered the structs so that metadata is on top, and payload at the bottom, as it is in the lockfile itself.
To set the preferences correctly, we need to know which version belongs to which fork. Instead of tracing that through the graph, we save this information in the lockfile to indicate to the user how and why there are multiple version for a package.
This gives the resolver access to the fork preferences from the lockfile.
@konstin konstin force-pushed the konsti/add-forks-to-lockfile branch from f521fdf to 4b1ca1f Compare July 30, 2024 10:58
@konstin konstin enabled auto-merge (squash) July 30, 2024 10:59
@konstin konstin merged commit dedd913 into main Jul 30, 2024
57 checks passed
@konstin konstin deleted the konsti/add-forks-to-lockfile branch July 30, 2024 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working preview Experimental behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants