Ensure all pip._vendor.* modules are mapped to debundled correspondences

[yan12125]

With the original vendored() implementation and such an initialization sequence:

vendored("packaging")
vendored("packaging.version")

In sys.modules, pip._vendor.packaging is correctly connected to the debundled packaging, while pip._vendor.packaging.version is not, as the latter is __import__ed from the existing pip._vendor.packaging module. That results in the same issue as #5429 - pip._vendor.packaging.version.Version and packaging.version.Version cannot be compared.

My patch attempts to fix this issue by skipping __import__ from the vendored name. This is safe because vendored() is called only when DEBUNDLED = True, and vendored libraries are already deleted as per debundling instructions.

[pradyunsg]

I'm okay with this in principle but this shouldn't have a news file entry.

It might be worth it to check if there's any documentation that needs to be updated for this.

[yan12125]

I'm okay with this in principle but this shouldn't have a news file entry.

OK I replaced the news entry with an empty .trivial file. Is it because debundling is not officially supported?

It might be worth it to check if there's any documentation that needs to be updated for this.

With such a change the first step of debundling

Delete everything in pip/_vendor/ except for pip/_vendor/__init__.py.

is no longer necessary. However, as keeping files of bundled dependencies does not make sense if DEBUNDLING = True is used, it's better to keep that step as-is IMO.

[pradyunsg]

Gonna go ahead and mention the various Linux folks here. Their inputs on this issue would be nice; like if they're all okay with this change etc.

Debian: @doko42 @warsaw
Fedora: @torsava @stratakis @hroncok
Arch Linux: @felixonmars (maintainer), @eli-schwartz (community contributor)

[hroncok]

Based on the rationale provided by upstream (you), Fedora keeps stuff in pip bundled. We only patch certifi to use /etc/pki/tls/certs/ca-bundle.crt and we remove certifi's cacert.pem. This is unlikely to change in the near feature. This change does not concern Fedora.

Note: I've deleted the previous comment. I just woke up and I was commenting about pipenv, not pip.

[warsaw]

I'm not really doing much Debian work these days, but I think it's fine as long as it works in both a vendored and unvendored scenario. Is that tested?

[yan12125]

I've tested this patch in both original pip (with bundled dependencies) and Arch Linux's pip (with debundling scripts in [1]). pip list -o (the command in issue #5429) runs fine. I'm not sure if Arch's debundling scritps are equivalent with Debian's or not, so testing from a Debian developer will be great.

[1] https://git.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packages/python-pip

[pradyunsg]

@yan12125 could you update this PR?

I think it's clear that this isn't a problem for anyone and this seems like a good change.

[eli-schwartz]

This is an interesting point of failure, recursively unbundled dependencies. I'm thinking of a different potential issue this would solve though -- often, users will despite our best attempts, use sudo pip install -U pip and break everything, then when pip is updated through the system package manager, it will detect some, but not all, vendored modules left over from the unclean sudo pip.

This has caused issues in the past, and I believe this change would also change pip to simply ignore the leftover, mismatching modules (because DEBUNDLED). So for this alone, I think it is a change that will benefit us.

One recent example where this matters is the progress module, which changed API (see #6319) and Arch has patched pip to contain the pip-specific changes together with the new system dependency. Importing pip._vendor.progress would therefore break pip right after we fixed it for progress itself.

[yan12125]

I believe this change would also change pip to simply ignore the leftover, mismatching modules (because DEBUNDLED)

@eli-schwartz Great point! I've just run a simple test:

0. Remove vendored libraries and modify _vendor/__init__.py as suggested in https:/pypa/pip/blob/master/src/pip/_vendor/README.rst#debundling
1. python setup.py install --user
2. touch ~/.local/lib/python3.7/site-packages/pip-19.1.dev0-py3.7.egg/pip/_vendor/progress.py

On the current master, touching an empty progress.py breaks pip, and with this patch everything works fine. Glad that my patch can fix two issues at a time :)

could you update this PR?

@pradyunsg I assume you meant rebase this PR onto the current master branch? Then it's done.

[pradyunsg]

@pradyunsg I assume you meant rebase this PR onto the current master branch? Then it's done.

Yep. Thanks!

[pradyunsg]

Thanks @yan12125 for the PR and everyone else for chiming in on this! :D

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.