Skip to content

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

Merged
charliermarsh merged 1 commit into
astral-sh:mainfrom
skshetry:fix-order-exclude
Jul 16, 2024
Merged

charliermarsh merged 1 commit into
astral-sh:mainfrom
skshetry:fix-order-exclude

Conversation

@skshetry

@skshetry skshetry commented Jul 16, 2024 •

Copy link
Copy Markdown
Contributor

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
@skshetry
skshetry deleted the fix-order-exclude branch July 16, 2024 16:46
@charliermarsh

Copy link
Copy Markdown
Member

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

@avilaton

Copy link
Copy Markdown

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