Updating a package that has `./` in RECORD removes site-packages

[MrMino]

Description

If you create a package that has a ./ entry in its RECORD file and install it, pip will think that the whole site-pacakges belongs to this package.

This means that when removing / updating / reinstalling it, pip will remove the whole site-packages first.

Probably the root cause of #7170.

It is easy to fall prey to this kind of bug when developing PEP-517 hooks.

Expected behavior

Warn? Refuse to install packages that have ./ in RECORD? Unsure.

pip version

21.1.2

Python version

3.8.10

OS

Ubuntu 20.04 LTS

How to Reproduce

Reproduce only in a virtualenv - this will remove your site-packages

python -m pip install wheelfile
python -c "from wheelfile import WheelFile; WheelFile(mode='w', distname='bomb', version='0').write('./', recursive=False, resolve=False)"
python -c "from wheelfile import WheelFile; WheelFile(mode='w', distname='bomb', version='1')"
python -m pip install ./bomb-0-py3-none-any.whl
python -m pip install ./bomb-1-py3-none-any.whl # 💥

Output

No response

Code of Conduct

• [X] I agree to follow the PSF Code of Conduct.

[uranusjr]

The only “proper” fix I can think of is, when installing (or uninstalling, but I think doing it on installation makes more sense) a distribution, pip needs to check whether an entry is also listed in other distributions. But that’s a pretty expensive operation (pip needs to check every RECORD line in every already-installed distribution) and most of the time wasteful, so I’m not sure that would be acceptable in general.

[pfmoore]

Wait, I'm confused here. How does an installed RECORD file end up containing ./ anyway? The specification explicitly says "directories should not be listed".

So this is basically a broken installation.

I'd start by saying this is a bug in wheelfile, as it's creating broken wheels (specifically, RECORD, but I'd argue that wheels should not contain directories, just files - unfortunately the wheel spec is somewhat vague on this detail, as with a lot of other things, but the intent is clearly that wheels only contain actual files).

I'd be fine with pip adding a check and refusing to install a wheel which contains a plain directory, but I'd view that just as a precaution. After all, users can trigger this issue by manually editing ./ into an existing RECORD file, and I'd be perfectly fine saying that if they did, any consequences are the user's fault.

PS Ignoring this issue, wheelfile looks neat!

[uranusjr]

So this is basically a broken installation.

Yes I think it’s implied in the top post, since it said It is easy to fall prey to this kind of bug when developing PEP-517 hooks. In this case, the bug is caught during development, but if such a broken wheel is distributed to PyPI, anyone that ends up installing the wheel will have their environment nuked on uninstallation. Pip is definitely not at fault when that happens, but it also probably would be nice if pip can do something to prevent such descruction.

[pradyunsg]

Pip is definitely not at fault when that happens, but it also probably would be nice if pip can do something to prevent such descruction.

Yea, being defensive here can't hurt.

[pradyunsg]

I've labelled this issue as an "deferred PR".

This label is essentially for indicating that further discussion related to this issue should be deferred until someone comes around to make a PR. This does not mean that the said PR would be accepted - it has not been determined whether this is a useful change to pip and that decision has been deferred until the PR is made.

[MrMino]

Wait, I'm confused here. How does an installed RECORD file end up containing ./ anyway? The specification explicitly says "directories should not be listed".

🤦 Ugh, true, I've just never got to that point because I based the implementation on PEPs mostly, and there's nothing in there about it (IIRC). I'll fix this in the next release.

PS Ignoring this issue, wheelfile looks neat!

Thanks :))!