tweag / FawltyDeps

Python dependency checker
Other
187 stars 13 forks source link

Handle single PEP-508 styled requirements #60

Open Nour-Mws opened 1 year ago

Nour-Mws commented 1 year ago

From https://github.com/tweag/FawltyDeps/pull/34#discussion_r1065626971:

We are starting to use parse_requirements_contents() from many different places now, and it seems most of them do not expect a full file contents a la requirements.txt, but rather expect a single (or multiple?) specifiers formatted according to PEP508. Suggest maybe (in a separate PR) to make a separate parse_pep508_specifier() that calls Requirement.parse(s) direcly?

jherland commented 1 year ago

Update after #227 was merged:

The real questions are:

  1. what kind of specifications do the various file formats use/need?
  2. and how are we currently supporting these?

For 1, setuptools^1 specifies that pyproject.toml (following PEP 621), setup.cfg, and setup.py all need to comply with PEP 508 (and PEP 440) for their dependency specifications.

For 2, currently in our code:

mknorps commented 2 months ago

@mknorps @jherland Check what is the status of this problem right now.

obscurerichard commented 2 months ago

Dealing with the full gamut of PEP-508 styled requirements is probably going to be a tar pit since the only full implementation of this is probably in pip internals that are not exposed with a stable and supported API. I've worked on practical projects (such as https://github.com/freezingsaddles/freezing-web for example) where the setup.py used to rely on pip internal libraries to parse requirements.txt but then broke when the pip internals changed.

So trying to get full support for this might be a lot of effort for relatively low reward.

jherland commented 3 weeks ago

Agree with @obscurerichard: we certainly don't want to dig further into pip internals than we absolutely have to.

With https://github.com/tweag/FawltyDeps/pull/445 we migrate parse_one_req() from depending on pkg_resources to depending on packaging.requirements instead. packaging.requirements claims to implement https://packaging.python.org/en/latest/specifications/dependency-specifiers/#dependency-specifiers which itself claims to supersede PEP 508. Given the deprecation of pkg_resources, this is clearly the better alternative AFAICS.

This reimplementation of parse_one_req() affects its callers: parse_setup_py() and parse_pyproject_toml(), but AFAICS there is no user-visible effect of this change.

parse_requirements_txt() remains on pip-requirements-parser as that provides a better API for parsing full files (rather than disparate specifiers), and the ugly temp file hack in parse_setup_cfg() also remains for now.

I suggest we either close this bug, or refocus it on the two remaining tasks mentioned above: