From 7050f0b7e5230aa3a794b79348ed6e0794f5dcc0 Mon Sep 17 00:00:00 2001
From: Daniel Hollas <danekhollas@gmail.com>
Date: Wed, 13 Nov 2024 15:18:26 +0000
Subject: [PATCH] Fix pre-commit in the image (#507)

* mv /etc/pip.conf ~/.config/pip/pip.conf
* add new test
---
 stack/base/Dockerfile | 15 +++++++++++++--
 tests/conftest.py     |  4 ++--
 tests/test_base.py    | 23 +++++++++++++++++++++--
 3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/stack/base/Dockerfile b/stack/base/Dockerfile
index d8167370..00257807 100644
--- a/stack/base/Dockerfile
+++ b/stack/base/Dockerfile
@@ -42,8 +42,19 @@ RUN cat /opt/requirements.txt | xargs -I{} conda config --system --add pinned_pa
 
 # Configure pip to use the same requirements file as constraints file.
 ENV PIP_CONSTRAINT /opt/requirements.txt
-# Ensure that pip installs packages to ~/.local by default
-COPY pip.conf /etc/pip.conf
+# Ensure that pip installs packages to '~/.local/lib/python3.X/site-packages/' by default
+# by implicitly passing the '--user' option to 'pip install'
+# Otherwise, pip would install into /opt/conda and such packages would be lost
+# when the container exits.
+# NOTE: We specifically chose the location '/opt/conda/pip.conf'
+# which represents the 'site' config file when VIRTUAL_ENV is not set, per:
+# https://pip.pypa.io/en/stable/topics/configuration/#configuration-files
+# Other locations such as '~/.config/pip/pip.conf' or '/etc/pip.conf' would interfere with virtual environments,
+# for example those used by pre-commit.
+# We can't use the PIP_USER env variable for the same reason.
+# To better understand this, try running `pip config debug` and see
+# https://github.com/aiidalab/aiidalab-docker-stack/issues/501
+COPY pip.conf "${CONDA_DIR}/pip.conf"
 
 # Upgrade pip and mamba to latest
 # Update async_generator, certipy to satisfy `pip check`
diff --git a/tests/conftest.py b/tests/conftest.py
index de911d5e..dccaa036 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -112,11 +112,11 @@ def pip_install(aiidalab_exec, nb_user):
     def _pip_install(pkg, **args):
         nonlocal package
         package = pkg
-        return aiidalab_exec(f"pip install {pkg}", **args)
+        return aiidalab_exec(f"pip install {pkg}", user=nb_user, **args)
 
     yield _pip_install
     if package:
-        aiidalab_exec(f"pip uninstall --yes {package}")
+        aiidalab_exec(f"pip uninstall --yes {package}", user=nb_user)
 
 
 @pytest.fixture(scope="session")
diff --git a/tests/test_base.py b/tests/test_base.py
index 9017a41d..481e6525 100644
--- a/tests/test_base.py
+++ b/tests/test_base.py
@@ -1,11 +1,19 @@
 """This module contains tests for the base image, which are AiiDA and package management related tests."""
 
+import email
 import json
 
 import pytest
 from packaging.version import parse
 
 
+@pytest.fixture
+def venv(tmp_path, aiidalab_exec):
+    venv_path = tmp_path / ".venv"
+    aiidalab_exec(f"python -m venv {venv_path}")
+    return venv_path
+
+
 @pytest.mark.parametrize("pkg_manager", ["pip", "mamba"])
 def test_prevent_installation_of_aiida(
     aiidalab_exec, nb_user, aiida_version, pkg_manager
@@ -76,8 +84,6 @@ def test_path_local_pip(aiidalab_exec, nb_user):
 
 def test_pip_user_install(aiidalab_exec, pip_install, nb_user):
     """Test that pip installs packages to ~/.local/ by default"""
-    import email
-
     # We use 'tuna' as an example of python-only package without dependencies
     pkg = "tuna"
     pip_install(pkg)
@@ -86,3 +92,16 @@ def test_pip_user_install(aiidalab_exec, pip_install, nb_user):
     # `pip show` output is in the RFC-compliant email header format
     msg = email.message_from_string(output)
     assert msg.get("Location").startswith(f"/home/{nb_user}/.local/")
+
+
+def test_pip_install_in_venv(aiidalab_exec, venv, nb_user):
+    """Test that pip installs packages to an activated venv"""
+
+    pkg = "tuna"
+    pip = venv / "bin/pip"
+
+    aiidalab_exec(f"{pip} install {pkg}")
+
+    output = aiidalab_exec(f"{pip} show {pkg}")
+    msg = email.message_from_string(output)
+    assert msg.get("Location").startswith(f"{venv}/lib")