🐛 fix(activation): restore PKG_CONFIG_PATH that was not set before

docs/changelog/3233.bugfix.rst

@@ -0,0 +1,4 @@
+Deactivation now unsets ``PKG_CONFIG_PATH`` when it was not set before activation, instead of leaving the environment's
+``lib/pkgconfig`` behind for good. The bash, fish and PowerShell scripts could not tell "nothing saved" apart from "was
+unset", so they restored nothing. Activation also no longer leaves a trailing separator in the batch, fish, nushell and
+PowerShell scripts when there was nothing to prepend to - by :user:`darrenhuai`.

src/virtualenv/activation/bash/activate.sh

@@ -32,9 +32,13 @@ deactivate () {
         export TK_LIBRARY
         unset _OLD_VIRTUAL_TK_LIBRARY
     fi
-    if [ -n "${_OLD_PKG_CONFIG_PATH:-}" ]; then
-        PKG_CONFIG_PATH="$_OLD_PKG_CONFIG_PATH"
-        export PKG_CONFIG_PATH
+    if [ -n "${_OLD_PKG_CONFIG_PATH+x}" ]; then
+        if [ -n "$_OLD_PKG_CONFIG_PATH" ]; then
+            PKG_CONFIG_PATH="$_OLD_PKG_CONFIG_PATH"
+            export PKG_CONFIG_PATH
+        else
+            unset PKG_CONFIG_PATH
+        fi
         unset _OLD_PKG_CONFIG_PATH
     fi
 

src/virtualenv/activation/batch/activate.bat

@@ -40,7 +40,8 @@
 @if NOT "__TK_LIBRARY__"=="" @set "TK_LIBRARY=__TK_LIBRARY__"
 
 @if defined PKG_CONFIG_PATH @set "_OLD_PKG_CONFIG_PATH=%PKG_CONFIG_PATH%"
-@set "PKG_CONFIG_PATH=%VIRTUAL_ENV%\lib\pkgconfig;%PKG_CONFIG_PATH%"
+@if defined PKG_CONFIG_PATH @set "PKG_CONFIG_PATH=%VIRTUAL_ENV%\lib\pkgconfig;%PKG_CONFIG_PATH%"
+@if not defined PKG_CONFIG_PATH @set "PKG_CONFIG_PATH=%VIRTUAL_ENV%\lib\pkgconfig"
 
 @REM if defined _OLD_VIRTUAL_PATH (
 @if not defined _OLD_VIRTUAL_PATH @goto ENDIFVPATH1

src/virtualenv/activation/fish/activate.fish

@@ -25,8 +25,12 @@ function deactivate -d 'Exit virtualenv mode and return to the normal environmen
       end
     end
 
-    if test -n "$_OLD_PKG_CONFIG_PATH"
-        set -gx PKG_CONFIG_PATH "$_OLD_PKG_CONFIG_PATH"
+    if set -q _OLD_PKG_CONFIG_PATH
+        if test -n "$_OLD_PKG_CONFIG_PATH"
+            set -gx PKG_CONFIG_PATH "$_OLD_PKG_CONFIG_PATH"
+        else
+            set -e PKG_CONFIG_PATH
+        end
         set -e _OLD_PKG_CONFIG_PATH
     end
 
@@ -66,7 +70,11 @@ if string match -qr 'CYGWIN|MSYS|MINGW' (uname)
 end
 
 set -gx _OLD_PKG_CONFIG_PATH "$PKG_CONFIG_PATH"
-set -gx PKG_CONFIG_PATH "$VIRTUAL_ENV/lib/pkgconfig:$PKG_CONFIG_PATH"
+if test -n "$PKG_CONFIG_PATH"
+    set -gx PKG_CONFIG_PATH "$VIRTUAL_ENV/lib/pkgconfig:$PKG_CONFIG_PATH"
+else
+    set -gx PKG_CONFIG_PATH "$VIRTUAL_ENV/lib/pkgconfig"
+end
 
 set -gx _OLD_VIRTUAL_PATH $PATH
 set -gx PATH "$VIRTUAL_ENV"'/'__BIN_NAME__ $PATH

src/virtualenv/activation/nushell/activate.nu

@@ -59,7 +59,11 @@ export-env {
         __VIRTUAL_PROMPT__
     }
     let old_pkg_config_path = if (has-env 'PKG_CONFIG_PATH') { $env.PKG_CONFIG_PATH } else { '' }
-    let new_pkg_config_path = $'($virtual_env)/lib/pkgconfig:($old_pkg_config_path)'
+    let new_pkg_config_path = if ($old_pkg_config_path | is-empty) {
+        $'($virtual_env)/lib/pkgconfig'
+    } else {
+        $'($virtual_env)/lib/pkgconfig:($old_pkg_config_path)'
+    }
     let new_env = { $path_name: $new_path VIRTUAL_ENV: $virtual_env VIRTUAL_ENV_PROMPT: $virtual_env_prompt PKG_CONFIG_PATH: $new_pkg_config_path }
     if (has-env 'TCL_LIBRARY')  {
         let $new_env = $new_env | insert TCL_LIBRARY __TCL_LIBRARY__

src/virtualenv/activation/powershell/activate.ps1

@@ -194,8 +194,11 @@ New-Variable -Scope global -Name _OLD_VIRTUAL_PATH -Value $env:PATH
 
 if (Test-Path env:PKG_CONFIG_PATH) {
     New-Variable -Scope global -Name _OLD_PKG_CONFIG_PATH -Value $env:PKG_CONFIG_PATH
+    $env:PKG_CONFIG_PATH = "$env:VIRTUAL_ENV\lib\pkgconfig;$env:PKG_CONFIG_PATH"
+} else {
+    New-Variable -Scope global -Name _OLD_PKG_CONFIG_PATH -Value $null
+    $env:PKG_CONFIG_PATH = "$env:VIRTUAL_ENV\lib\pkgconfig"
 }
-$env:PKG_CONFIG_PATH = "$env:VIRTUAL_ENV\lib\pkgconfig;$env:PKG_CONFIG_PATH"
 
 $env:PATH = "$env:VIRTUAL_ENV/" + __BIN_NAME__ + __PATH_SEP__ + $env:PATH
 

tests/unit/activation/test_bash.py

@@ -5,6 +5,7 @@
 import subprocess
 import sys
 from argparse import Namespace
+from pathlib import Path
 
 import pytest
 
@@ -132,6 +133,37 @@ def test_bash_activate_does_not_export_ps1(tmp_path, current_fastest) -> None:
     assert result.stdout.splitlines() == ["None", "None"]
 
 
+@pytest.mark.skipif(IS_WIN, reason="Github Actions ships with WSL bash")
+def test_bash_deactivate_unsets_pkg_config_path_that_was_not_set(tmp_path, current_fastest) -> None:
+    dest = tmp_path / "env"
+    cli_run([
+        "--without-pip",
+        str(dest),
+        "--creator",
+        current_fastest,
+        "--no-periodic-update",
+        "--activators",
+        "bash",
+    ])
+    activate_script = dest / "bin" / "activate"
+    print_var = f"{shlex.quote(sys.executable)} -c 'import os; print(os.environ.get(\"PKG_CONFIG_PATH\"))'"
+    result = subprocess.run(
+        [
+            "bash",
+            "-c",
+            (f'unset PKG_CONFIG_PATH; source "{activate_script}" && {print_var} && deactivate && {print_var}'),
+        ],
+        capture_output=True,
+        encoding="utf-8",
+        text=True,
+    )
+    assert result.returncode == 0, result.stderr
+    activated, deactivated = result.stdout.splitlines()
+    # no trailing separator when there was nothing to prepend to, and gone again afterwards
+    assert Path(activated) == dest / "lib" / "pkgconfig", result.stdout
+    assert deactivated == "None", result.stdout
+
+
 @pytest.mark.skipif(IS_WIN, reason="Github Actions ships with WSL bash")
 @pytest.mark.parametrize("hashing_enabled", [True, False])
 def test_bash(raise_on_non_source_class, hashing_enabled, activation_tester) -> None:

tests/unit/activation/test_batch.py

@@ -1,10 +1,44 @@
 from __future__ import annotations
 
+import subprocess
+import sys
 from argparse import Namespace
+from pathlib import Path
 
 import pytest
 
 from virtualenv.activation import BatchActivator
+from virtualenv.info import IS_WIN
+from virtualenv.run import cli_run
+
+
+@pytest.mark.skipif(not IS_WIN, reason="batch is Windows only")
+def test_batch_deactivate_unsets_pkg_config_path_that_was_not_set(tmp_path, current_fastest) -> None:
+    dest = tmp_path / "env"
+    cli_run([
+        "--without-pip",
+        str(dest),
+        "--creator",
+        current_fastest,
+        "--no-periodic-update",
+        "--activators",
+        "batch",
+    ])
+    scripts = dest / "Scripts"
+    print_var = f'"{sys.executable}" -c "import os; print(os.environ.get(\'PKG_CONFIG_PATH\'))"'
+    driver = tmp_path / "driver.bat"
+    driver.write_text(
+        f'@echo off\nset "PKG_CONFIG_PATH="\ncall "{scripts / "activate.bat"}"\n'
+        f'{print_var}\ncall "{scripts / "deactivate.bat"}"\n{print_var}\n',
+        encoding="utf-8",
+    )
+    out = subprocess.run(
+        ["cmd", "/c", str(driver)], capture_output=True, text=True, encoding="utf-8", timeout=60, check=True
+    ).stdout
+    activated, deactivated = out.splitlines()
+    # no trailing separator when there was nothing to prepend to, and gone again afterwards
+    assert Path(activated) == dest / "lib" / "pkgconfig", out
+    assert deactivated == "None", out
 
 
 def test_batch_pydoc_bat_quoting(tmp_path) -> None:

tests/unit/activation/test_fish.py

@@ -5,11 +5,13 @@
 import subprocess
 import sys
 from argparse import Namespace
+from pathlib import Path
 
 import pytest
 
 from virtualenv.activation import FishActivator
 from virtualenv.info import IS_WIN
+from virtualenv.run import cli_run
 
 FISH = shutil.which("fish")
 
@@ -83,6 +85,34 @@ def test_fish_prompt_survives_shadowed_source(activation_python, tmp_path) -> No
     assert f"PWD={start}\n" in out, out
 
 
+@pytest.mark.skipif(IS_WIN, reason="fish is not available on Windows")
+@pytest.mark.skipif(FISH is None, reason="fish is not installed")
+def test_fish_deactivate_unsets_pkg_config_path_that_was_not_set(tmp_path, current_fastest) -> None:
+    dest = tmp_path / "env"
+    cli_run([
+        "--without-pip",
+        str(dest),
+        "--creator",
+        current_fastest,
+        "--no-periodic-update",
+        "--activators",
+        "fish",
+    ])
+    print_var = f"'{sys.executable}' -c 'import os; print(os.environ.get(\"PKG_CONFIG_PATH\"))'"
+    driver = tmp_path / "driver.fish"
+    driver.write_text(
+        f"set -e PKG_CONFIG_PATH\nsource '{dest / 'bin' / 'activate.fish'}'\n{print_var}\ndeactivate\n{print_var}\n",
+        encoding="utf-8",
+    )
+    out = subprocess.run(
+        [FISH, str(driver)], capture_output=True, text=True, encoding="utf-8", timeout=60, check=True
+    ).stdout
+    activated, deactivated = out.splitlines()
+    # no trailing separator when there was nothing to prepend to, and gone again afterwards
+    assert Path(activated) == dest / "lib" / "pkgconfig", out
+    assert deactivated == "None", out
+
+
 @pytest.mark.skipif(IS_WIN, reason="we have not setup fish in CI yet")
 def test_fish(activation_tester_class, activation_tester, monkeypatch, tmp_path) -> None:
     monkeypatch.setenv("HOME", str(tmp_path))

tests/unit/activation/test_powershell.py

@@ -1,12 +1,15 @@
 from __future__ import annotations
 
 import shutil
+import subprocess
 import sys
 from argparse import Namespace
+from pathlib import Path
 
 import pytest
 
 from virtualenv.activation import PowerShellActivator
+from virtualenv.run import cli_run
 
 
 def test_powershell_pydoc_call_operator(tmp_path) -> None:
@@ -96,6 +99,43 @@ def __init__(self, dest) -> None:
         assert "$env:TCL_LIBRARY = ''" in content
 
 
+POWERSHELL = shutil.which("pwsh") or (shutil.which("powershell.exe") if sys.platform == "win32" else None)
+
+
+@pytest.mark.skipif(POWERSHELL is None, reason="powershell is not installed")
+def test_powershell_deactivate_unsets_pkg_config_path_that_was_not_set(tmp_path, current_fastest) -> None:
+    dest = tmp_path / "env"
+    cli_run([
+        "--without-pip",
+        str(dest),
+        "--creator",
+        current_fastest,
+        "--no-periodic-update",
+        "--activators",
+        "powershell",
+    ])
+    activate_script = dest / ("Scripts" if sys.platform == "win32" else "bin") / "activate.ps1"
+    print_var = f'& "{sys.executable}" -c "import os; print(os.environ.get(\'PKG_CONFIG_PATH\'))"'
+    driver = tmp_path / "driver.ps1"
+    driver.write_text(
+        f'Remove-Item env:PKG_CONFIG_PATH -ErrorAction SilentlyContinue\n. "{activate_script}"\n'
+        f"{print_var}\ndeactivate\n{print_var}\n",
+        encoding="utf-8-sig",
+    )
+    out = subprocess.run(
+        [POWERSHELL, "-NonInteractive", "-NoProfile", "-ExecutionPolicy", "ByPass", "-File", str(driver)],
+        capture_output=True,
+        text=True,
+        encoding="utf-8",
+        timeout=60,
+        check=True,
+    ).stdout
+    activated, deactivated = out.splitlines()
+    # no trailing separator when there was nothing to prepend to, and gone again afterwards
+    assert Path(activated) == dest / "lib" / "pkgconfig", out
+    assert deactivated == "None", out
+
+
 @pytest.mark.slow
 def test_powershell(activation_tester_class, activation_tester, monkeypatch) -> None:
     monkeypatch.setenv("TERM", "xterm")