Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relocate environment variable injection to before the interpreter is run #2260

Merged
merged 10 commits into from
Nov 7, 2023
Merged

Relocate environment variable injection to before the interpreter is run #2260

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b1a0046
relocate environment variable injection to before the interpreter is run
Oct 2, 2023
e71813f
Merge branch 'pantsbuild:main' into relocate-environment-injection
erikwilliamson Nov 3, 2023
d4530fd
Merge branch 'pantsbuild:main' into relocate-environment-injection
erikwilliamson Nov 6, 2023
fdbdf08
Update tests to reflect relocation of where we're injecting ENV vars
Nov 6, 2023
632b592
relocating env var injection
erikwilliamson Nov 6, 2023
b5c1e23
move argument injection back to where it came from
erikwilliamson Nov 6, 2023
0096b2c
updated to work with relocation of env var injection
erikwilliamson Nov 6, 2023
29a7925
missed uncommenting test_inject_env_invalid()
erikwilliamson Nov 6, 2023
0c62995
fix typo & remove commented out code
erikwilliamson Nov 7, 2023
840d601
move environment variable injection up more for cases where PEX_SCRIP…
erikwilliamson Nov 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions pex/pex.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,9 @@ 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)

if force_interpreter:
TRACER.log("PEX_INTERPRETER specified, dropping into interpreter")
return self.execute_interpreter()
Expand Down Expand Up @@ -596,8 +599,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:
Expand Down
7 changes: 4 additions & 3 deletions pex/venv/installer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}:
jsirois marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -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(":")
Expand Down
11 changes: 6 additions & 5 deletions tests/integration/test_inject_env_and_args.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,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 (
"<not set>"
"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()
Expand Down Expand Up @@ -240,8 +240,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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the typo at the end of the sentence. Should be: ... not the env.

assert {"args": ["foo", "bar"], "MESSAGE": "Hello, world!"} == json.loads(
# assert {"MESSAGE": "Hello, world!"} == json.loads(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kill this commented out line.

subprocess.check_output(args=[pex, "foo", "bar"], env=make_env(PEX_MODULE="example"))
)

Expand Down