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

pip install --target has inconsistent "file merging" #13224

Open
1 task done
asottile opened this issue Feb 16, 2025 · 1 comment
Open
1 task done

pip install --target has inconsistent "file merging" #13224

asottile opened this issue Feb 16, 2025 · 1 comment
Labels
C: target pip install's --target option's behaviour handling type: bug A confirmed bug or unintended behavior

Comments

@asottile
Copy link
Contributor

Description

compare the outputs of these two scenarios

(setup beforehand)

python3.10 -m venv venv310
python3.12 -m venv venv312

with cffi as the package:

$ rm -rf out; venv310/bin/pip install --target out cffi && venv312/bin/pip install --target out cffi; find out -name '*.so'
Collecting cffi
  Using cached cffi-1.17.1-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (1.5 kB)
Collecting pycparser (from cffi)
  Using cached pycparser-2.22-py3-none-any.whl.metadata (943 bytes)
Using cached cffi-1.17.1-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (446 kB)
Using cached pycparser-2.22-py3-none-any.whl (117 kB)
Installing collected packages: pycparser, cffi
Successfully installed cffi-1.17.1 pycparser-2.22
Collecting cffi
  Using cached cffi-1.17.1-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (1.5 kB)
Collecting pycparser (from cffi)
  Using cached pycparser-2.22-py3-none-any.whl.metadata (943 bytes)
Using cached cffi-1.17.1-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (479 kB)
Using cached pycparser-2.22-py3-none-any.whl (117 kB)
Installing collected packages: pycparser, cffi
Successfully installed cffi-1.17.1 pycparser-2.22
WARNING: Target directory /tmp/z/zz/out/cffi already exists. Specify --upgrade to force replacement.
WARNING: Target directory /tmp/z/zz/out/pycparser-2.22.dist-info already exists. Specify --upgrade to force replacement.
WARNING: Target directory /tmp/z/zz/out/cffi-1.17.1.dist-info already exists. Specify --upgrade to force replacement.
WARNING: Target directory /tmp/z/zz/out/pycparser already exists. Specify --upgrade to force replacement.
out/_cffi_backend.cpython-312-x86_64-linux-gnu.so
out/_cffi_backend.cpython-310-x86_64-linux-gnu.so

with markupsafe as the package:

$ rm -rf out; venv310/bin/pip install --target out cffi && venv312/bin/pip install --target out cffi; find out -name '*.so'
rm -rf out; venv310/bin/pip install --target out markupsafe && venv312/bin/pip install --target out markupsafe; find out -name '*.so'
Collecting markupsafe
  Using cached MarkupSafe-3.0.2-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (4.0 kB)
Using cached MarkupSafe-3.0.2-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (20 kB)
Installing collected packages: markupsafe
Successfully installed markupsafe-3.0.2
Collecting markupsafe
  Using cached MarkupSafe-3.0.2-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (4.0 kB)
Using cached MarkupSafe-3.0.2-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (23 kB)
Installing collected packages: markupsafe
Successfully installed markupsafe-3.0.2
WARNING: Target directory /tmp/z/zz/out/markupsafe already exists. Specify --upgrade to force replacement.
WARNING: Target directory /tmp/z/zz/out/MarkupSafe-3.0.2.dist-info already exists. Specify --upgrade to force replacement.
out/markupsafe/_speedups.cpython-310-x86_64-linux-gnu.so

the important difference here is with cffi pip seems to "properly" merge the two installs such that we get the version-specific .so files for both versions. I say "properly" because it's not really well defined what should happen here (since pip could probably see that cffi is "already installed" and not bother unzipping at all -- even though it is installed for a different python version). in the markupsafe version we don't get the .so file for the second installation because pip stops its unzip when it hits a folder boundary

my use case for this is to build lambda zips for multiple python versions during an upgrade -- I'll likely work around this by using --target with multiple directories and then merging them myself

Expected behavior

there's a couple ways this could go -- I do expect a hard error rather than a warning in any of them (refuse the temptation to guess!)

I think either:

  • merge the installs into --target (perhaps more surprising, but it's the behaviour I actually want)
  • notice the package is already there and refuse to install over it and require --upgrade (this would be further from my use case -- but at least explicit that I would need to do the directory merging myself later!)

a data point -- this "works" for a prefix with multiple pythons installed to it -- but presumably only because the site-packages directories are separate (though they aren't on windows -- but I don't really care about the windows behaviour so I'm not going to bother checking it!)

pip version

25.0.1

Python version

3.10.16, 3.12.6

OS

ubuntu 24.04

How to Reproduce

see description

Output

see description

Code of Conduct

@asottile asottile added S: needs triage Issues/PRs that need to be triaged type: bug A confirmed bug or unintended behavior labels Feb 16, 2025
@pfmoore
Copy link
Member

pfmoore commented Feb 16, 2025

Generally, I would only ever advise using --target on an empty destination directory. That's not an official policy1, but it's what I would consider the safe approach.

If someone were to create a PR improving the way we handle installing using --target into a non-empty directory, it would probably be considered, although I can't promise it would be accepted, and I suspect there would be a debate over what counts as an "improvement" in any case 🙁 And it's unlikely that anything in this area would get a high priority.

Footnotes

  1. At least, I don't think it is - we've talked about that being the only safe option and from what I recall the other pip maintainers were broadly in agreement, but I don't think we ever formalised it.

@sbidoul sbidoul added C: target pip install's --target option's behaviour handling and removed S: needs triage Issues/PRs that need to be triaged labels Feb 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: target pip install's --target option's behaviour handling type: bug A confirmed bug or unintended behavior
Projects
None yet
Development

No branches or pull requests

3 participants