Disallow --user installs if site.ENABLE_USER_SITE is False.

[pelson]

#8794 highlighted that site.ENABLE_USER_SITE was being ignored when pip install --user is being set (when not in a virtualenvironment). This is in contrast to the behaviour for virtualenvs which raises an InstallationError since the installed packages wouldn't be on the sys.path afterwards.

This MR brings about the symmetry by preventing the user from being able to do --user when site.ENABLE_USER_SITE is set. I wasn't sure on the best testing strategy for this change, so didn't implement any tests at this point (happy to take advice).

[notatallshaw]

@uranusjr Do you still think this is the correct thing to do?

@pelson Apologies for no one ever replying, pip is currently maintained by only volunteers, I'd like to clear out our backlog of PRs, but I don't have much time either. However if this is agreed to be the right approach, it would need atleast one test, and and update to the docs.

[pelson]

@notatallshaw - totally understand 👍

In terms of the change being proposed here, I still stand by the fact that it doesn't make any sense to install to --user if user site packages is not going to be on the path. That said, I can see that the issue which has been open for ~5 years hasn't had much user traction, so I'm fine either way on whether we fix it or not.

I'd be fine to add a test here, if you have a suggestion on the best way to go about it (perhaps point me to an existing test that I can adapt?) - do you want an integration test, or a unit test?

[ichard26]

@pelson I spent five minutes on writing a test. I think something along the lines of this should work:

diff --git a/tests/functional/test_install_user.py b/tests/functional/test_install_user.py
index 0b1e025cb..436e519bc 100644
--- a/tests/functional/test_install_user.py
+++ b/tests/functional/test_install_user.py
@@ -120,6 +120,19 @@ class Tests_UserSite:
             "visible in this virtualenv." in result.stderr
         )
 
+    def test_install_user_nositepkgs_fails(
+        self, script: PipTestEnvironment, data: TestData
+    ) -> None:
+        script.environ["PYTHONNOUSERSITE"] = "1"
+        run_from = data.packages.joinpath("FSPkg")
+        result = script.pip(
+            "install", "--user", curdir, cwd=run_from, expect_error=True
+        )
+        assert (
+            "Can not perform a '--user' install. User site-packages are "
+            "disabled for this Python."
+        ) in result.stderr
+
     @pytest.mark.network
     def test_install_user_conflict_in_usersite(
         self, script: PipTestEnvironment

[ichard26]

@pelson since you were active recently, are you interested in repicking this up or not? I can push a test and changes to the docs if needed.

[pelson]

Thanks for the test @ichard26 - I added it directly and rebased.

[notatallshaw]

Windows CI failure is because of this: #13598, but Ubuntu test failure looks like because of this PR:

FAILED tests/functional/test_install_user.py::Tests_UserSite::test_install_user_nositepkgs_fails - AssertionError: Arguments not expected: env

[pelson]

Spent quite a bit of time trying to get a test for this. Since we are running scripttest, it looks like monkeypatching doesn't work, and I can't set environment variables either it seems (even with os.environ). I note that there is no test coverage for those failure scenarios (think they were introduced in 5f14682). I've invested a bit too much in this issue now, but if you have advice on how to control the environment in which the scripttest runs, I would be happy to tweak what I have. I don't see any patterns in test_install_user which I can reuse (those tests for the other failure scenarios would have helped here... 😢).

For now, I just create a temporary script with the right setup, and call main in the subprocess.

[notatallshaw]

Ah sorry to hear @pelson, I will put this PR on my review list (@ichard26 is largely unavailable right now) so your efforts aren't watered.