From b1a00465837d1b5f9697fd952921eecdfee3e433 Mon Sep 17 00:00:00 2001 From: Erik Williamson <60066165+erik-at-techsanity@users.noreply.github.com> Date: Mon, 2 Oct 2023 09:09:17 -0400 Subject: [PATCH 1/8] relocate environment variable injection to before the interpreter is run --- pex/pex.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pex/pex.py b/pex/pex.py index e59d126a4..1ebe1fda8 100644 --- a/pex/pex.py +++ b/pex/pex.py @@ -565,6 +565,10 @@ def _execute(self): self._clean_environment(strip_pex_env=self._pex_info.strip_pex_env) + for name, value in self._pex_info.inject_env.items(): + os.environ.setdefault(name, value) + sys.argv[1:1] = list(self._pex_info.inject_args) + if force_interpreter: TRACER.log("PEX_INTERPRETER specified, dropping into interpreter") return self.execute_interpreter() @@ -593,10 +597,6 @@ def _execute(self): EntryPoint.parse("run = {}".format(self._pex_info_overrides.entry_point)) ) - for name, value in self._pex_info.inject_env.items(): - os.environ.setdefault(name, value) - sys.argv[1:1] = list(self._pex_info.inject_args) - if self._pex_info.script: return self.execute_script(self._pex_info.script) else: From fdbdf08e839b41f35eb896dd200236f4f5dcc4d1 Mon Sep 17 00:00:00 2001 From: Erik Williamson <60066165+erik-at-techsanity@users.noreply.github.com> Date: Mon, 6 Nov 2023 16:19:07 -0500 Subject: [PATCH 2/8] Update tests to reflect relocation of where we're injecting ENV vars --- tests/integration/test_inject_env_and_args.py | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/integration/test_inject_env_and_args.py b/tests/integration/test_inject_env_and_args.py index aa16857dd..be5ce94fe 100644 --- a/tests/integration/test_inject_env_and_args.py +++ b/tests/integration/test_inject_env_and_args.py @@ -22,14 +22,14 @@ from typing import Any, Iterable, List, Optional -def test_inject_env_invalid(): - # type: () -> None - result = run_pex_command(args=["--inject-env", "FOO"]) - result.assert_failure() - assert "--inject-env" in result.error - assert ( - "Environment variable values must be of the form `name=value`. Given: FOO" in result.error - ) +# def test_inject_env_invalid(): +# # type: () -> None +# result = run_pex_command(args=["--inject-env", "FOO"]) +# result.assert_failure() +# assert "--inject-env" in result.error +# assert ( +# "Environment variable values must be of the form `name=value`. Given: FOO" in result.error +# ) parametrize_execution_mode_args = pytest.mark.parametrize( @@ -40,7 +40,6 @@ def test_inject_env_invalid(): ], ) - @parametrize_execution_mode_args def test_inject_env( tmpdir, # type: Any @@ -72,11 +71,11 @@ def assert_FOO( assert_FOO(expected_env_value="baz", runtime_env_value="baz") assert_FOO(expected_env_value="", runtime_env_value="") - # Switching away from the built-in entrypoint should disable the injected env. + # Switching away from the built-in entrypoint should retain the injected env. assert ( - "" + "bar" == subprocess.check_output( - args=[pex, "-c", print_FOO_env_code], env=make_env(PEX_INTERPRETER=1) + args=[pex, "-c", print_FOO_env_code], env=make_env(PEX_INTERPRETER=1, FOO="bar") ) .decode("utf-8") .strip() @@ -240,8 +239,9 @@ def assert_message( assert_message(b"Hello, world!") assert_message(b"42", MESSAGE="42") - # Switching away from the built-in entrypoint should disable injected args and env. - assert {"args": ["foo", "bar"], "MESSAGE": None} == json.loads( + # Switching away from the built-in entrypoint should disable injected args but not the en env. + assert {"args": ["foo", "bar"], "MESSAGE": "Hello, world!"} == json.loads( + # assert {"MESSAGE": "Hello, world!"} == json.loads( subprocess.check_output(args=[pex, "foo", "bar"], env=make_env(PEX_MODULE="example")) ) From 632b592b807d8139d4c397c3a141776ed53fef44 Mon Sep 17 00:00:00 2001 From: erikwilliamson Date: Mon, 6 Nov 2023 16:26:38 -0500 Subject: [PATCH 3/8] relocating env var injection --- pex/venv/installer.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pex/venv/installer.py b/pex/venv/installer.py index a25b13e4d..e487194f4 100644 --- a/pex/venv/installer.py +++ b/pex/venv/installer.py @@ -829,7 +829,10 @@ def sys_executable_paths(): if pex_interpreter else pex_overrides.get("PEX_MODULE", {entry_point!r} or PEX_INTERPRETER_ENTRYPOINT) ) - + + for name, value in {inject_env!r}: + os.environ.setdefault(name, value) + if entry_point == PEX_INTERPRETER_ENTRYPOINT: # A Python interpreter always inserts the CWD at the head of the sys.path. # See https://docs.python.org/3/library/sys.html#sys.path @@ -917,8 +920,6 @@ def sys_executable_paths(): sys.exit(0) if not is_exec_override: - for name, value in {inject_env!r}: - os.environ.setdefault(name, value) sys.argv[1:1] = {inject_args!r} module_name, _, function = entry_point.partition(":") From b5c1e2335e8f1136268abf9e37e01e53ea51d1af Mon Sep 17 00:00:00 2001 From: erikwilliamson Date: Mon, 6 Nov 2023 16:27:03 -0500 Subject: [PATCH 4/8] move argument injection back to where it came from --- pex/pex.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pex/pex.py b/pex/pex.py index 4b9fd7391..05e944141 100644 --- a/pex/pex.py +++ b/pex/pex.py @@ -570,7 +570,6 @@ def _execute(self): for name, value in self._pex_info.inject_env.items(): os.environ.setdefault(name, value) - sys.argv[1:1] = list(self._pex_info.inject_args) if force_interpreter: TRACER.log("PEX_INTERPRETER specified, dropping into interpreter") @@ -600,6 +599,8 @@ def _execute(self): EntryPoint.parse("run = {}".format(self._pex_info_overrides.entry_point)) ) + sys.argv[1:1] = list(self._pex_info.inject_args) + if self._pex_info.script: return self.execute_script(self._pex_info.script) else: From 0096b2c9f44cfc22d66bbee68c0f6905a4704fb4 Mon Sep 17 00:00:00 2001 From: erikwilliamson Date: Mon, 6 Nov 2023 18:43:01 -0500 Subject: [PATCH 5/8] updated to work with relocation of env var injection --- tests/integration/test_inject_env_and_args.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_inject_env_and_args.py b/tests/integration/test_inject_env_and_args.py index be5ce94fe..f1feefe72 100644 --- a/tests/integration/test_inject_env_and_args.py +++ b/tests/integration/test_inject_env_and_args.py @@ -40,6 +40,7 @@ ], ) + @parametrize_execution_mode_args def test_inject_env( tmpdir, # type: Any @@ -241,7 +242,7 @@ def assert_message( # Switching away from the built-in entrypoint should disable injected args but not the en env. assert {"args": ["foo", "bar"], "MESSAGE": "Hello, world!"} == json.loads( - # assert {"MESSAGE": "Hello, world!"} == json.loads( + # assert {"MESSAGE": "Hello, world!"} == json.loads( subprocess.check_output(args=[pex, "foo", "bar"], env=make_env(PEX_MODULE="example")) ) From 29a7925fbd2cce7e76062fd4f539597f626ac3b1 Mon Sep 17 00:00:00 2001 From: erikwilliamson Date: Mon, 6 Nov 2023 18:47:18 -0500 Subject: [PATCH 6/8] missed uncommenting test_inject_env_invalid() --- tests/integration/test_inject_env_and_args.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/integration/test_inject_env_and_args.py b/tests/integration/test_inject_env_and_args.py index f1feefe72..3e1506d12 100644 --- a/tests/integration/test_inject_env_and_args.py +++ b/tests/integration/test_inject_env_and_args.py @@ -22,14 +22,14 @@ from typing import Any, Iterable, List, Optional -# def test_inject_env_invalid(): -# # type: () -> None -# result = run_pex_command(args=["--inject-env", "FOO"]) -# result.assert_failure() -# assert "--inject-env" in result.error -# assert ( -# "Environment variable values must be of the form `name=value`. Given: FOO" in result.error -# ) +def test_inject_env_invalid(): + # type: () -> None + result = run_pex_command(args=["--inject-env", "FOO"]) + result.assert_failure() + assert "--inject-env" in result.error + assert ( + "Environment variable values must be of the form `name=value`. Given: FOO" in result.error + ) parametrize_execution_mode_args = pytest.mark.parametrize( From 0c629950ac9e8a73c8cf4911c6889f875cbcb160 Mon Sep 17 00:00:00 2001 From: erikwilliamson Date: Tue, 7 Nov 2023 08:00:15 -0500 Subject: [PATCH 7/8] fix typo & remove commented out code --- tests/integration/test_inject_env_and_args.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/integration/test_inject_env_and_args.py b/tests/integration/test_inject_env_and_args.py index 3e1506d12..26cb2229e 100644 --- a/tests/integration/test_inject_env_and_args.py +++ b/tests/integration/test_inject_env_and_args.py @@ -240,9 +240,8 @@ def assert_message( assert_message(b"Hello, world!") assert_message(b"42", MESSAGE="42") - # Switching away from the built-in entrypoint should disable injected args but not the en env. + # Switching away from the built-in entrypoint should disable injected args but not the env. assert {"args": ["foo", "bar"], "MESSAGE": "Hello, world!"} == json.loads( - # assert {"MESSAGE": "Hello, world!"} == json.loads( subprocess.check_output(args=[pex, "foo", "bar"], env=make_env(PEX_MODULE="example")) ) From 840d601bd6ebf64337ef6abb92e529890cbe1f35 Mon Sep 17 00:00:00 2001 From: erikwilliamson Date: Tue, 7 Nov 2023 11:26:04 -0500 Subject: [PATCH 8/8] move environment variable injection up more for cases where PEX_SCRIPT is defined and run --- pex/venv/installer.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pex/venv/installer.py b/pex/venv/installer.py index e487194f4..58b38a30f 100644 --- a/pex/venv/installer.py +++ b/pex/venv/installer.py @@ -816,7 +816,10 @@ def sys_executable_paths(): "PEX_EXTRA_SYS_PATH" ) del os.environ[key] - + + for name, value in {inject_env!r}: + os.environ.setdefault(name, value) + pex_script = pex_overrides.get("PEX_SCRIPT") if pex_overrides else {script!r} if pex_script: script_path = os.path.join(venv_bin_dir, pex_script) @@ -829,9 +832,6 @@ def sys_executable_paths(): if pex_interpreter else pex_overrides.get("PEX_MODULE", {entry_point!r} or PEX_INTERPRETER_ENTRYPOINT) ) - - for name, value in {inject_env!r}: - os.environ.setdefault(name, value) if entry_point == PEX_INTERPRETER_ENTRYPOINT: # A Python interpreter always inserts the CWD at the head of the sys.path.