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.

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

Absolutize all of the execute_pex_args in the venv script. #12727

Merged
merged 1 commit into from
Sep 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 4 additions & 2 deletions src/python/pants/backend/python/util_rules/pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ def _create_venv_script(
target_venv_executable = shlex.quote(str(venv_executable))
venv_dir = shlex.quote(str(self.venv_dir))
execute_pex_args = " ".join(
shlex.quote(arg)
f"$(ensure_absolute {shlex.quote(arg)})"
for arg in self.complete_pex_env.create_argv(self.pex.name, python=self.pex.python)
)

Expand Down Expand Up @@ -799,7 +799,7 @@ def _create_venv_script(
export {" ".join(env_vars)}
export PEX_ROOT="$(ensure_absolute ${{PEX_ROOT}})"

execute_pex_args="$(ensure_absolute {execute_pex_args})"
execute_pex_args="{execute_pex_args}"
target_venv_executable="$(ensure_absolute {target_venv_executable})"
venv_dir="$(ensure_absolute {venv_dir})"

Expand Down Expand Up @@ -852,6 +852,7 @@ class VenvPex:
pex: Script
python: Script
bin: FrozenDict[str, Script]
venv_rel_dir: str
Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably good to tack on ~ # for tests to be friendly to future refactorers.



@frozen_after_init
Expand Down Expand Up @@ -943,6 +944,7 @@ async def create_venv_pex(
pex=pex.script,
python=python.script,
bin=FrozenDict((bin_name, venv_script.script) for bin_name, venv_script in scripts.items()),
venv_rel_dir=venv_rel_dir.as_posix(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Folks have a habit of doing this, but str(venv_rel_dir) will be Windows-proof and is actually correct today too.

)


Expand Down
26 changes: 25 additions & 1 deletion src/python/pants/backend/python/util_rules/pex_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@
from pants.backend.python.util_rules.pex_cli import PexPEX
from pants.engine.fs import EMPTY_DIGEST, CreateDigest, Digest, Directory, FileContent
from pants.engine.internals.scheduler import ExecutionError
from pants.engine.process import Process, ProcessResult
from pants.engine.process import Process, ProcessCacheScope, ProcessResult
from pants.python.python_setup import InvalidLockfileBehavior
from pants.testutil.rule_runner import QueryRule, RuleRunner
from pants.util.dirutil import safe_rmtree


@dataclass(frozen=True)
Expand Down Expand Up @@ -337,9 +338,32 @@ def test_pex_working_directory(rule_runner: RuleRunner, pex_type: type[Pex | Ven
description="Run the pex and check its cwd",
working_directory=working_dir,
input_digest=runtime_files,
# We skip the process cache for this PEX to ensure that it re-runs.
cache_scope=ProcessCacheScope.PER_SESSION,
)
],
)

# For VenvPexes, run the PEX twice while clearing the venv dir in between. This emulates
# situations where a PEX creation hits the process cache, while venv seeding misses the PEX
# cache.
if isinstance(pex, VenvPex):
# Request once to ensure that the directory is seeded, and then start a new session so that
# the second run happens as well.
_ = rule_runner.request(ProcessResult, [process])
rule_runner.new_session("re-run-for-venv-pex")
rule_runner.set_options(
["--backend-packages=pants.backend.python"],
env_inherit={"PATH", "PYENV_ROOT", "HOME"},
)
# Clear the cache.
named_caches_dir = (
rule_runner.options_bootstrapper.bootstrap_options.for_global_scope().named_caches_dir
)
venv_dir = os.path.join(named_caches_dir, "pex_root", pex.venv_rel_dir)
assert os.path.isdir(venv_dir)
safe_rmtree(venv_dir)

result = rule_runner.request(ProcessResult, [process])
output_str = result.stdout.decode()
mo = re.search(r"CWD: (.*)\n", output_str)
Expand Down