pre-commit / pre-commit-hooks

Some out-of-the-box hooks for pre-commit
MIT License
5.18k stars 691 forks source link

`requirements-txt-fixer` ordering breaks using --index-url and --extra-index-url together in requirements.txt #612

Open redfungus opened 3 years ago

redfungus commented 3 years ago

If you use both the --index-url and --extra-index-url flags together in a requirements.txt file the hook will put --extra-index-url above the --index-url which causes the url set to index-url be skipped. I suppose this is because the flags are also sorted alphabetically.

asottile commented 3 years ago

note that --extra-index-url is unsafe and should generally never be used

a special case could be added, though my thought is that supporting something that shouldn't be used seems like unnecessary work

redfungus commented 3 years ago

@asottile I did not know about the security problems! Thank you for the link!

robin-snt commented 2 years ago

Is there a pre-commit hook for detecting if the developer has incorrectly configured pip to run with --extra-index-url instead of --index-url?

I mean in addition to simply reading the requirements files, so it would also detect if the global pip config has been incorrectly configured?

asottile commented 2 years ago

something like that doesn't really make sense as a hook -- looking at global settings doesn't make sense for something that's supposed to check source code

plus you'd have to know to configure such a thing and at that point you might as well just turn off the bad setting -- it's not something you're going to "accidentally" commit one day

robin-snt commented 2 years ago

@asottile Your insight is highly appreciated!

renegaderyu commented 1 year ago

@asottile please review if you have time. I'm hoping this PR is simple enough and goes with the spirit of the special case you mentioned. Also, I'd appreciate if you could label w/ hacktoberfest-accepted so I can get a tree planted, thanks.

asottile commented 1 year ago

I'm not going to review something which doesn't pass tests

renegaderyu commented 1 year ago

@asottile Apologies for not seeing the failing tests before asking. I think its ready now.