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

pip compile: change order of check to handle exact argument first #5111

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

skshetry
Copy link
Contributor

@skshetry skshetry commented Jul 16, 2024

I messed up the order of checks in #5033, due to which it failed to exclude the case of -P package, as arg.startswith("-P") check came first and skipped only the first argument.

That means that, in the following command:

uv pip compile --output-file pip_compile_uv_header.txt unpinned_uv.in -P attrs==18.1.0

The generated header would exclude -P, but keep attrs==18.1.0.

# This file was autogenerated by uv via the following command:
#    uv pip compile --output-file pip_compile_uv_header.txt unpinned_uv.in attrs==18.1.0

But we want to check for an exact match first and then only check for the case when option and value are together.

This also affected --find-links short option of style -f <uri>.

Hopefully, third times going to be a charm. 😳

Summary

Test Plan

I tested locally, and also changed one snapshot test to use -P for variation. I don't think it's worth an extra test, but can do that for sure.

Before the checks for when the option and value are together.
@zanieb zanieb added the bug Something isn't working label Jul 16, 2024
@charliermarsh charliermarsh merged commit e2dfab7 into astral-sh:main Jul 16, 2024
51 checks passed
@skshetry skshetry deleted the fix-order-exclude branch July 16, 2024 16:46
@charliermarsh
Copy link
Member

Thanks! I also missed this as reviewer, all good.

@avilaton
Copy link

I can feel the anxiety we all have to get this dependabot stuff merged, I coded most of that pr on my cellphone while doing other things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants