diff --git a/docs/changelog/3233.bugfix.rst b/docs/changelog/3233.bugfix.rst new file mode 100644 index 000000000..5cb26c729 --- /dev/null +++ b/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`. diff --git a/src/virtualenv/activation/bash/activate.sh b/src/virtualenv/activation/bash/activate.sh index 9380bfef8..f54c539c5 100644 --- a/src/virtualenv/activation/bash/activate.sh +++ b/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 diff --git a/src/virtualenv/activation/batch/activate.bat b/src/virtualenv/activation/batch/activate.bat index 06192921f..e95eb39c0 100644 --- a/src/virtualenv/activation/batch/activate.bat +++ b/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 diff --git a/src/virtualenv/activation/fish/activate.fish b/src/virtualenv/activation/fish/activate.fish index 625d8c669..3f707ce13 100644 --- a/src/virtualenv/activation/fish/activate.fish +++ b/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 diff --git a/src/virtualenv/activation/nushell/activate.nu b/src/virtualenv/activation/nushell/activate.nu index 559872d23..11b4d290c 100644 --- a/src/virtualenv/activation/nushell/activate.nu +++ b/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__ diff --git a/src/virtualenv/activation/powershell/activate.ps1 b/src/virtualenv/activation/powershell/activate.ps1 index e56633a5b..ff38a3a21 100644 --- a/src/virtualenv/activation/powershell/activate.ps1 +++ b/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 diff --git a/tests/unit/activation/test_bash.py b/tests/unit/activation/test_bash.py index f6193d4c4..e0d9bdaa3 100644 --- a/tests/unit/activation/test_bash.py +++ b/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: diff --git a/tests/unit/activation/test_batch.py b/tests/unit/activation/test_batch.py index 9ae5be688..6c43cfea4 100644 --- a/tests/unit/activation/test_batch.py +++ b/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: diff --git a/tests/unit/activation/test_fish.py b/tests/unit/activation/test_fish.py index 692eefb77..f1d511c18 100644 --- a/tests/unit/activation/test_fish.py +++ b/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)) diff --git a/tests/unit/activation/test_powershell.py b/tests/unit/activation/test_powershell.py index 297030e3b..a9e937348 100644 --- a/tests/unit/activation/test_powershell.py +++ b/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")