Skip to content

Commit

Permalink
Handle workdir="."/default properly in run_shell_command (Cherry-pick…
Browse files Browse the repository at this point in the history
… of #18840) (#18850)
  • Loading branch information
huonw authored Apr 29, 2023
1 parent 4940005 commit c12f37e
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 38 deletions.
10 changes: 2 additions & 8 deletions src/python/pants/backend/shell/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
9 changes: 3 additions & 6 deletions src/python/pants/backend/shell/util_rules/shell_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
from __future__ import annotations

import logging
import os
import shlex
from dataclasses import dataclass

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
)


Expand Down
44 changes: 24 additions & 20 deletions src/python/pants/backend/shell/util_rules/shell_command_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
8 changes: 4 additions & 4 deletions src/python/pants/core/util_rules/adhoc_process_support.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit c12f37e

Please sign in to comment.