search

pypa / pip

The Python package installer
https://pip.pypa.io/
MIT License
9.46k stars 3.01k forks source link

Add check for `vendored()` calls in `pip/_vendor/__init__.py` #8048

Open uranusjr opened 4 years ago

uranusjr commented 4 years ago

https://github.com/pypa/pip/pull/8045#issuecomment-613305096

We have been missing the vendored() call updates too many times that this should be beneficial. But at least I am not really motivated enough to put work on this, so it would be nice if some of our downstream distributors can work on this :p

SanketDG commented 4 years ago

What would be the data source for automating this? Where would I get the values upon which vendored() is to be called?

uranusjr commented 4 years ago

IIUC it is supposed to be called for all importable modules in pip._vendor

SanketDG commented 4 years ago

IIUC it is supposed to be called for all importable modules in pip._vendor

@uranusjr There seems to be a lot of importable modules in pip._vendored. Are we supposed to call them all in vendored() in __init__.py?

xavfernandez commented 4 years ago

I'd suggest using the output of rg -e "from pip._vendor|import pip._vendor" src/pip/ to check if those modules are correctly vendored(). Apparently idna, packaging.requirements/tags/utils aren't :o

SanketDG commented 4 years ago

Grep command for portability:

grep -rE "from pip._vendor" src/pip

Some more that don't seem to be included:

pyparsing chardet certifi

Stupid question, how does not including them not break pip?

xavfernandez commented 4 years ago

Stupid question, how does not including them not break pip?

Not stupid at all. The vendored() calls are only useful for "debundled" versions of pip (i.e. the ones packaged & distributed by distributions like Debian or Archlinux, etc).

SanketDG commented 4 years ago

The vendored() calls are only useful for "debundled" versions of pip (i.e. the ones packaged & distributed by distributions like Debian or Archlinux, etc).

I see. So it would only break those packages. Looks like @pradyunsg already figured out the missing reqs in https://github.com/pypa/pip/issues/8040#issuecomment-613278877

pradyunsg commented 4 years ago

@SanketDG not exactly.

Those are merely updates to our already-vendored packages. This issue is more about indicating them correctly, so that pip work well even when downstream distributors do things to it that we don't want them to.

SanketDG commented 4 years ago

This issue is more about indicating them correctly, so that pip work well even when downstream distributors do things to it that we don't want them to.

Right, I understand. I could write up a quick script that would check for all importable unique modules in vendor against all the vendored() calls. But my worry is that there could be edge cases at such a crude check.