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

Option to skip dependencies with empty PyPI listing. #211

Closed
wants to merge 2 commits into from

Conversation

m000
Copy link

@m000 m000 commented Jan 7, 2022

Adds a cli and resolvelib option to allow pip-audit continue running when resolvelib deems a package as unresolvable.
This notably happens when a package links page exists, but is empty (e.g. https://pypi.org/simple/pkg_resources/).
Fixes #210.

Nit: Avoid redirection from https://pypi.org/simple/PROJECT to https://pypi.org/simple/PROJECT/.

@CLAassistant
Copy link

CLAassistant commented Jan 7, 2022

CLA assistant check
All committers have signed the CLA.

@woodruffw woodruffw added component:cli CLI components component:dep-sources Dependency sources labels Jan 7, 2022
@woodruffw
Copy link
Member

woodruffw commented Jan 7, 2022

Overall structure looks good here, thanks! FWIW, I agree with the comment you left on #210 (comment): a separate PyPINoProjectLinksError makes sense to me. Could you add that?

cc @di for signoff: I think this is also a reasonable solution to #209 that won't be overly invasive.

@woodruffw woodruffw requested review from woodruffw and di January 7, 2022 18:40
@m000
Copy link
Author

m000 commented Jan 8, 2022

I'm working to update the PR. I may have to tweak it a bit. Where the exception is handled now, the top-level package (e.g. django-admin-inline-paginator) will be marked as skipped, while we only want to skip the dependency that had no links (e.g. pkg_resources).

Also, the name of the flag needs to be updated to reflect its effects. I was thinking to make it --skip-no-links. If anyone can come up with something more descriptive, please comment.

@m000 m000 changed the title Option to skip unresolvable packages instead of raising. Option to skip dependencies with empty PyPI listing. Jan 8, 2022
@m000
Copy link
Author

m000 commented Jan 8, 2022

In the end, I had to take a slightly more complicated approach. Because resolvelib doesn't have provisions for changing it's behaviour when we feed it with an empty dependency list to choose from, I resorted to feeding it with a fake candidate, and then handling the fake appropriately, before and after resolvelib is invoked.

I have added a new exception that will be thrown if the new flag is not specified and an empty PyPI listing occurs. This is mostly informational, to tell apart from a generic ResolutionImpossible error.

I have checked with the test case of #197 (where the cause of failure is different), and the two cases are not confused. I.e. #197 will still throw the same error even when --ignore-empty is supplied.

--ignore-empty seems a descriptive enough name for the flag, if no better proposals come up.

A notable case of this is pkg_resources==0.0.0 which is installed by
Debian's pip distribution.

Currently resolvelib does not support skipping a dependency when presented
with an empty candidates list for it. So, the only viable way to achieve
the desired effect is to inject a fake candidate to make resolvelib happy,
and add appropriate handling code for it on the pip-audit side.

Exposed to cli via the --skip-empty flag.
@di
Copy link
Member

di commented Jan 10, 2022

Some thoughts:

While this works for the narrow case of pkg_resources, I'm not sure it makes makes sense to apply across all projects that don't have releases on PyPI. It's entirely possible that a user has a vulnerable version of a package installed, that PyPI knows about the vulnerability, but the project owners (for whatever reason) have removed all releases. This is rare now but may become more common if we start taking into account packages that are malicious / typosquats / etc, where it's very likely they would have no releases on PyPI after admin intervention.

I think this change would make sense if we made it more generalizable as an option to skip resolution for any dependency that isn't resolvable for whatever reason. We would still check if there is a known vulnerability or not, but wouldn't attempt to continue resolution for it. This would be similar to #139, #145, #162, etc.

@m000
Copy link
Author

m000 commented Jan 11, 2022

I think this change would make sense if we made it more generalizable as an option to skip resolution for any dependency that isn't resolvable for whatever reason. We would still check if there is a known vulnerability or not, but wouldn't attempt to continue resolution for it.

Sounds reasonable. I'll update the PR within the next few days.

@m000
Copy link
Author

m000 commented Jan 27, 2022

Sorry for the delay. I'm in the middle of moving to a new place, so my free time is less than expected. I will pick this up the soonest possible.

@di
Copy link
Member

di commented May 2, 2022

I'm going to close this for now, but feel free to start a new PR if you are able to revisit!

@di di closed this May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:cli CLI components component:dep-sources Dependency sources
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pypi_provider asks for bogus requirement pkg_resources==0.0.0
4 participants