From c12f37e7ef64c1243ad0283816d35caba005677e Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Sat, 29 Apr 2023 21:36:27 +1000 Subject: [PATCH] Handle workdir="."/default properly in run_shell_command (Cherry-pick of #18840) (#18850) --- .../pants/backend/shell/target_types.py | 10 +---- .../backend/shell/util_rules/shell_command.py | 9 ++-- .../shell/util_rules/shell_command_test.py | 44 ++++++++++--------- .../core/util_rules/adhoc_process_support.py | 8 ++-- 4 files changed, 33 insertions(+), 38 deletions(-) diff --git a/src/python/pants/backend/shell/target_types.py b/src/python/pants/backend/shell/target_types.py index e57067fa5bd..596eead0600 100644 --- a/src/python/pants/backend/shell/target_types.py +++ b/src/python/pants/backend/shell/target_types.py @@ -342,14 +342,8 @@ class ShellCommandWorkdirField(AdhocToolWorkdirField): pass -class RunShellCommandWorkdirField(StringField): - alias = "workdir" - default = "." - help = help_text( - "Sets the current working directory of the command that is `run`. Values that begin with " - "`.` are relative to the directory you are running Pants from. Values that begin with `/` " - "are from your project root." - ) +class RunShellCommandWorkdirField(AdhocToolWorkdirField): + pass class ShellCommandOutputRootDirField(AdhocToolOutputRootDirField): diff --git a/src/python/pants/backend/shell/util_rules/shell_command.py b/src/python/pants/backend/shell/util_rules/shell_command.py index 1fd05a556fd..65f229ca1a0 100644 --- a/src/python/pants/backend/shell/util_rules/shell_command.py +++ b/src/python/pants/backend/shell/util_rules/shell_command.py @@ -4,7 +4,6 @@ from __future__ import annotations import logging -import os import shlex from dataclasses import dataclass @@ -37,6 +36,7 @@ MergeExtraSandboxContents, ResolvedExecutionDependencies, ResolveExecutionDependenciesRequest, + parse_relative_directory, ) from pants.core.util_rules.adhoc_process_support import rules as adhoc_process_support_rules from pants.core.util_rules.environments import EnvironmentNameRequest @@ -262,17 +262,14 @@ async def _interactive_shell_command( ) dependencies_digest = execution_environment.digest - relpath = os.path.relpath( - working_directory, start="/" if os.path.isabs(working_directory) else "." - ) - boot_script = f"cd {shlex.quote(relpath)}; " if relpath != "." else "" + relpath = parse_relative_directory(working_directory, shell_command.address) + boot_script = f"cd {shlex.quote(relpath)}; " if relpath != "" else "" return Process( argv=(bash.path, "-c", boot_script + command, shell_name), description=f"Running {description}", env=command_env, input_digest=dependencies_digest, - working_directory=working_directory, ) diff --git a/src/python/pants/backend/shell/util_rules/shell_command_test.py b/src/python/pants/backend/shell/util_rules/shell_command_test.py index cb31297df66..a459f90b0b2 100644 --- a/src/python/pants/backend/shell/util_rules/shell_command_test.py +++ b/src/python/pants/backend/shell/util_rules/shell_command_test.py @@ -542,39 +542,43 @@ def test_old_style_dependencies(caplog, rule_runner: RuleRunner) -> None: ) -def test_run_shell_command_request(rule_runner: RuleRunner) -> None: +@pytest.mark.parametrize( + ("workdir", "expected_boot"), + [ + (None, "cd src; "), + (".", "cd src; "), + ("/", ""), + ("src/with space'n quote", """cd 'src/with space'\"'\"'n quote'; """), + ("./with space'n quote", """cd 'src/with space'\"'\"'n quote'; """), + ], +) +def test_run_shell_command_request( + rule_runner: RuleRunner, workdir: None | str, expected_boot: str +) -> None: rule_runner.write_files( { "src/BUILD": dedent( - """\ + f"""\ run_shell_command( name="test", command="some cmd string", - ) - - run_shell_command( - name="cd-test", - command="some cmd string", - workdir="src/with space'n quote", + workdir={workdir!r}, ) """ ), } ) - def assert_run_args(target: str, args: tuple[str, ...]) -> None: - tgt = rule_runner.get_target(Address("src", target_name=target)) - run = RunShellCommand.create(tgt) - request = rule_runner.request(RunRequest, [run]) - assert len(args) == len(request.args) - for arg, request_arg in zip(args, request.args): - arg in request_arg + args = ("bash", "-c", expected_boot + "some cmd string", "src:test") - assert_run_args("test", ("bash", "-c", "some cmd string", "src:test")) - assert_run_args( - "cd-test", - ("bash", "-c", "cd 'src/with space'\"'\"'n quote'; some cmd string", "src:cd-test"), - ) + tgt = rule_runner.get_target(Address("src", target_name="test")) + run = RunShellCommand.create(tgt) + request = rule_runner.request(RunRequest, [run]) + assert len(args) == len(request.args) + # handle the binary name specially, because the path may differ + assert args[0] in request.args[0] + for arg, request_arg in zip(args[1:], request.args[1:]): + assert arg == request_arg @pytest.mark.parametrize( diff --git a/src/python/pants/core/util_rules/adhoc_process_support.py b/src/python/pants/core/util_rules/adhoc_process_support.py index 2b0fa13be32..6d64967c4cf 100644 --- a/src/python/pants/core/util_rules/adhoc_process_support.py +++ b/src/python/pants/core/util_rules/adhoc_process_support.py @@ -407,8 +407,8 @@ async def run_adhoc_process( if result.stderr: logger.warning(result.stderr.decode()) - working_directory = _parse_relative_directory(request.working_directory, request.address) - root_output_directory = _parse_relative_directory( + working_directory = parse_relative_directory(request.working_directory, request.address) + root_output_directory = parse_relative_directory( request.root_output_directory, working_directory ) @@ -444,7 +444,7 @@ async def prepare_adhoc_process( description = request.description address = request.address - working_directory = _parse_relative_directory(request.working_directory or "", address) + working_directory = parse_relative_directory(request.working_directory or "", address) argv = request.argv timeout: int | None = request.timeout output_files = request.output_files @@ -510,7 +510,7 @@ def _output_at_build_root(process: Process, bash: BashBinary) -> Process: ) -def _parse_relative_directory(workdir_in: str, relative_to: Union[Address, str]) -> str: +def parse_relative_directory(workdir_in: str, relative_to: Union[Address, str]) -> str: """Convert the `workdir` field into something that can be understood by `Process`.""" if isinstance(relative_to, Address):