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

Restricting universal lock by OS still downloads wheels from all OSes #6512

Closed
tmct opened this issue Aug 23, 2024 · 12 comments
Closed

Restricting universal lock by OS still downloads wheels from all OSes #6512

tmct opened this issue Aug 23, 2024 · 12 comments
Assignees
Labels
enhancement New feature or improvement to existing functionality good first issue Good for newcomers lock Related to universal resolution and locking

Comments

@tmct
Copy link

tmct commented Aug 23, 2024

Hi - I'm very glad to see uv's recent universal locking features, and engagement with the lockfile standard - but I've found one behaviour that seems a little odd.

Perhaps I have misunderstood how environment markers are used in universal resolution, but I am surprised by this behaviour:

mkdir temp
cd temp
uv init -p 3.10

# I only want to produce a lockfile for Linux, across Python versions
cat <<EOL >> pyproject.toml

[tool.uv]
environments = ["sys_platform == 'linux'"]
EOL

uv add torch==2.1.0 -p 3.10

Result: I still get plently Windows-specific wheels in my uv.lock lockfile, e.g. torch-2.1.0-cp310-cp310-win_amd64.whl - presumably these have all still been downloaded.

Would it be possible please not to download these Mac/Windows-only wheels for linux-only universal locks, as an example?

Many thanks,
Tom

@konstin konstin added the enhancement New feature or improvement to existing functionality label Aug 23, 2024
@zanieb
Copy link
Member

zanieb commented Aug 23, 2024

Thanks for the report!

@konstin you marked this as an enhancement, is this not a bug?

cc @charliermarsh

@zanieb zanieb added the lock Related to universal resolution and locking label Aug 23, 2024
@konstin
Copy link
Member

konstin commented Aug 23, 2024

We already never install those wheels, but we're missing the pruning of the wheels list for platforms. We're currently only prune the wheels list for the python version (#4696), but we can extend this to the platform markers also. It may not work 100% for platform marker them since markers and wheels tags don't have a perfect 1:1 mapping, but we would still trim the lockfile down by a lot.

@zanieb
Copy link
Member

zanieb commented Aug 23, 2024

Thanks for the additional context. Is this hard?

@konstin
Copy link
Member

konstin commented Aug 23, 2024

Hard to tell, hopefully it's a just a lookup table and some filtering. Ideally, you create a mapping from a marker to what tags it supports, e.g. sys_platform == 'linux' -> manylinux, musllinnux and linux, and then filter the wheels by that wheel tag.

@charliermarsh
Copy link
Member

This would be a good improvement.

@konstin konstin added the good first issue Good for newcomers label Aug 23, 2024
@tmct
Copy link
Author

tmct commented Aug 23, 2024

Thanks very much all - would be very happy to see this enhancement - none of the other "universal" lockfile tools seems to support this filtering...

@charliermarsh
Copy link
Member

This is a bit more tedious because we don't track "all the markers under which this package could be relevant" in the lockfile directly -- you have to compute it by propagating markers. So at this point in the lockfile:

// Remove wheels that don't match `requires-python` and can't be selected for
// installation.
if let Some(requires_python) = &requires_python {
    package
        .wheels
        .retain(|wheel| requires_python.matches_wheel_tag(&wheel.filename));
}

We'd need to track the supported platforms.

@konstin konstin self-assigned this Sep 2, 2024
konstin added a commit that referenced this issue Sep 3, 2024
Prep for fixing #6512. No functional changes.
konstin added a commit that referenced this issue Sep 3, 2024
Prep for fixing #6512. No functional changes.
konstin added a commit that referenced this issue Sep 3, 2024
Prep for fixing #6512. No functional changes.
konstin added a commit that referenced this issue Sep 3, 2024
Prep for fixing #6512. No functional changes.
@charliermarsh
Copy link
Member

Closed by #6957.

@tmct
Copy link
Author

tmct commented Sep 4, 2024

Thank you! Though that PR is described as "Prep for fixing #6512. No functional changes."?

@tmct
Copy link
Author

tmct commented Sep 4, 2024

Ah, now I see the subsequent #6961 and #6959 in the release notes. Very nice

@charliermarsh
Copy link
Member

Apologies, I linked the wrong PR!

@tmct
Copy link
Author

tmct commented Oct 15, 2024

Hi, quick question, which caches do I need to clear to make this happen if I forgot to restrict by OS before first sync? Thanks

Apologies, was using an old version of uv 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement to existing functionality good first issue Good for newcomers lock Related to universal resolution and locking
Projects
None yet
Development

No branches or pull requests

4 participants