Make --target functionality align with other install schemas

[dwoz]

Fix for #10110

This patch makes target install packages directly to the target directory without installing to a temporary directory and the copying things to the final install location. This means --upgrade and --ignore-installed now work the same as --prefix.

Additionally fixes: #9605 #10629

[pfmoore]

I don’t think that this PR goes in the direction I want. We should never be simply dumping a new install over an old one. Instead, we should properly support the normal uninstall/reinstall mechanism that upgrades use.

[dwoz]

I don’t think that this PR goes in the direction I want. We should never be simply dumping a new install over an old one. Instead, we should properly support the normal uninstall/reinstall mechanism that upgrades use.

See #10110 (comment)

[dwoz]

I don’t think that this PR goes in the direction I want. We should never be simply dumping a new install over an old one. Instead, we should properly support the normal uninstall/reinstall mechanism that upgrades use.

This change now supports the normal uninstall/reinstall mechanism that upgrades use.

[pfmoore]

Does this work if options like --platform are set, when the files being installed might not even be compatible with the current interpreter? I’m not sure adding to sys.path is valid in that case.

[dwoz]

Does this work if options like --platform are set, when the files being installed might not even be compatible with the current interpreter? I’m not sure adding to sys.path is valid in that case.

It seems to yes, I will look into writing a test for this case:

(venv310) dan@carbon:~/src/pip$ dpkg --print-architecture 
amd64
(venv310) dan@carbon:~/src/pip$ pip install --only-binary=:all: --platform=manylinux_2_17_aarch64 --target=foobar pygit2
Collecting pygit2
  Using cached pygit2-1.14.1-cp310-cp310-manylinux_2_17_aarch64.manylinux2014_aarch64.whl.metadata (3.4 kB)
Requirement already satisfied: cffi>=1.16.0 in ./venv310/lib/python3.10/site-packages (from pygit2) (1.16.0)
Requirement already satisfied: pycparser in ./venv310/lib/python3.10/site-packages (from cffi>=1.16.0->pygit2) (2.21)
Using cached pygit2-1.14.1-cp310-cp310-manylinux_2_17_aarch64.manylinux2014_aarch64.whl (4.8 MB)
Installing collected packages: pygit2
Successfully installed pygit2
(venv310) dan@carbon:~/src/pip$ pip install --only-binary=:all: --platform=manylinux_2_17_aarch64 --target=foobar pygit2
Requirement already satisfied: pygit2 in ./foobar (1.14.1)
Requirement already satisfied: cffi>=1.16.0 in ./venv310/lib/python3.10/site-packages (from pygit2) (1.16.0)
Requirement already satisfied: pycparser in ./venv310/lib/python3.10/site-packages (from cffi>=1.16.0->pygit2) (2.21)

[pfmoore]

I assume the point about adding the location to sys.path is to fool the code that gets metadata for installed packages into seeing the target directory. That seems like a bit of a hack. For example, are we sure that pip install --upgrade --target x requests wouldn't see a copy of requests in the main environment and uninstall that before installing requests into the target directory? Personally, I still think that the correct way of dealing with this is to treat --target as a completely new "scheme" in its own right, and not try to force it to look like one of the existing ones.

But all of this is soemwhat secondary. The reason we install into a temp directory and then copy to the correct location at the end is for safety. If the install fails (for example, building a wheel from a sdist fails with a compile error) we can discard the temporary directory and fail with no changes made to the user's system. By installing direct into the target location, we lose this possibility, and risk a failed build leaving everything in a corrupted state. And while I'd like to say that people should only use --target to populate a previously empty directory, I know this isn't how people always use it in practice, and we can't break their workflow without a deprecation period (and a reasonable alternative solution).

[dwoz]

But all of this is soemwhat secondary. The reason we install into a temp directory and then copy to the correct location at the end is for safety. If the install fails (for example, building a wheel from a sdist fails with a compile error) we can discard the temporary directory and fail with no changes made to the user's system. By installing direct into the target location, we lose this possibility, and risk a failed build leaving everything in a corrupted state. And while I'd like to say that people should only use --target to populate a previously empty directory, I know this isn't how people always use it in practice, and we can't break their workflow without a deprecation period (and a reasonable alternative solution).

If this is the case, why would installing into site-packages not work the same way? If a build failure would leave the system in an un-wanted state, wouldn't that be true across the board?

I do not believe it has ever been documented that --target should be used with an empty directory. What makes you assume that is how most people (or any people) are using it? From what I've seen in the hand full of issues filed about --target not working as people expect. They've all been expecting it to work the same as installing without --target.

[pfmoore]

If this is the case, why would installing into site-packages not work the same way? If a build failure would leave the system in an un-wanted state, wouldn't that be true across the board?

Sorry, I wasn't clear. One of the important properties of pip install is that if something fails part way through, the target is unaltered. I believe the way that is handled for --target is by the current mechanism of installing in a temporary location and moving things into place. Maybe it's done differently when --target isn't involved, I'd have to check the code to be sure. By removing the temporary staging area, you break that guarantee, and I don't see anything to replace it in your PR. Maybe there is and I'm missing it, but you need to confirm that. And yes, we should probably have tests for that, but it's quite possible we don't. Sorry about that.

I do not believe it has ever been documented that --target should be used with an empty directory.

Correct, it hasn't. I believe¹ the original intended use case was to populate an empty directory for vendoring, and we only ever really made sure --target worked sanely for that use case. But we didn't document it, and as you say people have used it for other things since, and it doesn't always work properly. This is why I personally want to replace --target with something that is designed to work just like installing into an environment. But that means handling the process of reading and managing the installation metadata, and tools like importlib.metadata only work with an actual environment, which is why it's tricky.

We can patch things over a bit in the meanwhile, and maybe we should. But I'm not convinced we're not just going to trade one set of problems for another.

Anyway, I think we've reached a point where you're not going to convince me that your approach works, and I'm afraid I don't have time to take you through the process of writing the implementation I was hoping to get round to writing myself. Maybe one of the other pip maintainers will support this approach - I won't block it if they do, I'm very aware that I might be holding out for something that's impractical given our resources, so I want to leave the way open for the lower bar of "at least one maintainer is OK with this approach" as an option.

Footnotes

1. I've been involved with pip for many years, and what I mean here is "I'm pretty sure I recall conversations to this effect from a long time ago" 😉 ↩

[dwoz]

Anyway, I think we've reached a point where you're not going to convince me that your approach works, and I'm afraid I don't have time to take you through the process of writing the implementation I was hoping to get round to writing myself. Maybe one of the other pip maintainers will support this approach - I won't block it if they do, I'm very aware that I might be holding out for something that's impractical given our resources, so I want to leave the way open for the lower bar of "at least one maintainer is OK with this approach" as an option.

Fair enough. For what it's worth, I'm happy to jump through whatever hoops are needed to validate this approach and/or work on improving it (including potentially taking another approach).