Skip to content

Commit

Permalink
Write adhoc_tool(stdout/stderr="...") relative to workdir, support ab…
Browse files Browse the repository at this point in the history
…solute paths (Cherry-pick of #18814) (#18839)

This fixes #18810 by ensuring `adhoc_tool(stdout="...", stderr="...")`
are written relative to the working directory, and before the
`root_output_directory` filtering happens.

This also fixes a second bug I noticed while working on this: if the
path is absolute, e.g. `stdout="/out.txt"`, pants crashes: ```panic at
'called `Result::unwrap()` on an `Err` value: "Absolute paths are not
allowed: \"/out.txt\""', src/intrinsics.rs:420```. After this PR, they
are supported, and taken to start at the build root.
  • Loading branch information
huonw authored Apr 27, 2023
1 parent eece2ed commit 4940005
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 42 deletions.
22 changes: 4 additions & 18 deletions src/python/pants/backend/adhoc/adhoc_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
from pants.core.util_rules.environments import EnvironmentNameRequest
from pants.engine.addresses import Addresses
from pants.engine.environment import EnvironmentName
from pants.engine.fs import CreateDigest, Digest, FileContent, MergeDigests, Snapshot
from pants.engine.fs import Digest, MergeDigests, Snapshot
from pants.engine.internals.native_engine import EMPTY_DIGEST
from pants.engine.rules import Get, collect_rules, rule
from pants.engine.target import (
Expand Down Expand Up @@ -167,6 +167,8 @@ async def run_in_sandbox_request(
supplied_env_var_values=FrozenDict(extra_env),
log_on_process_errors=None,
log_output=target[AdhocToolLogOutputField].value,
capture_stderr_file=target[AdhocToolStderrFilenameField].value,
capture_stdout_file=target[AdhocToolStdoutFilenameField].value,
)

adhoc_result = await Get(
Expand All @@ -177,23 +179,7 @@ async def run_in_sandbox_request(
},
)

result = adhoc_result.process_result
adjusted = adhoc_result.adjusted_digest

extras = (
(target[AdhocToolStdoutFilenameField].value, result.stdout),
(target[AdhocToolStderrFilenameField].value, result.stderr),
)
extra_contents = {i: j for i, j in extras if i}

if extra_contents:
extra_digest = await Get(
Digest,
CreateDigest(FileContent(name, content) for name, content in extra_contents.items()),
)
adjusted = await Get(Digest, MergeDigests((adjusted, extra_digest)))

output = await Get(Snapshot, Digest, adjusted)
output = await Get(Snapshot, Digest, adhoc_result.adjusted_digest)
return GeneratedSources(output)


Expand Down
51 changes: 32 additions & 19 deletions src/python/pants/backend/adhoc/adhoc_tool_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,29 +141,42 @@ def test_adhoc_tool_with_workdir(rule_runner: RuleRunner) -> None:
)


def test_adhoc_tool_capture_stdout_err(rule_runner: RuleRunner) -> None:
@pytest.mark.parametrize(
("write_dir", "workdir", "root_output_directory", "expected_dir"),
[
# various relative paths:
("", None, None, "src/"),
("dir/", None, None, "src/dir/"),
("../", None, None, ""),
# absolute path
("/", None, None, ""),
# interaction with workdir and root_output_directory:
("", "/", None, ""),
("dir/", None, ".", "dir/"),
("3/", "1/2", "1", "2/3/"),
],
)
def test_adhoc_tool_capture_stdout_err(
rule_runner: RuleRunner,
write_dir: str,
workdir: None | str,
root_output_directory: None | str,
expected_dir: str,
) -> None:
rule_runner.write_files(
{
"src/fruitcake.py": dedent(
"""\
import sys
print("fruitcake")
print("inconceivable", file=sys.stderr)
"""
),
"src/BUILD": dedent(
"""\
python_source(
source="fruitcake.py",
name="fruitcake",
)
f"""\
system_binary(name="bash", binary_name="bash")
adhoc_tool(
name="run_fruitcake",
runnable=":fruitcake",
stdout="stdout",
stderr="stderr",
root_output_directory=".",
runnable=":bash",
args=["-c", "echo fruitcake; echo inconceivable >&2"],
stdout="{write_dir}stdout",
stderr="{write_dir}stderr",
workdir={workdir!r},
root_output_directory={root_output_directory!r},
)
"""
),
Expand All @@ -174,8 +187,8 @@ def test_adhoc_tool_capture_stdout_err(rule_runner: RuleRunner) -> None:
rule_runner,
Address("src", target_name="run_fruitcake"),
expected_contents={
"stderr": "inconceivable\n",
"stdout": "fruitcake\n",
f"{expected_dir}stderr": "inconceivable\n",
f"{expected_dir}stdout": "fruitcake\n",
},
)

Expand Down
10 changes: 6 additions & 4 deletions src/python/pants/backend/adhoc/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,9 @@ class AdhocToolStdoutFilenameField(StringField):
default = None
help = help_text(
lambda: f"""
A filename to capture the contents of `stdout` to, relative to the value of
`{AdhocToolWorkdirField.alias}`.
A filename to capture the contents of `stdout` to. Relative paths are
relative to the value of `{AdhocToolWorkdirField.alias}`, absolute paths
start at the build root.
"""
)

Expand All @@ -165,8 +166,9 @@ class AdhocToolStderrFilenameField(StringField):
default = None
help = help_text(
lambda: f"""
A filename to capture the contents of `stderr` to, relative to the value of
`{AdhocToolWorkdirField.alias}`
A filename to capture the contents of `stderr` to. Relative paths are
relative to the value of `{AdhocToolWorkdirField.alias}`, absolute paths
start at the build root.
"""
)

Expand Down
2 changes: 2 additions & 0 deletions src/python/pants/backend/shell/util_rules/shell_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ async def _prepare_process_request_from_target(
immutable_input_digests=FrozenDict.frozen(merged_extras.immutable_input_digests),
log_on_process_errors=_LOG_ON_PROCESS_ERRORS,
log_output=shell_command[ShellCommandLogOutputField].value,
capture_stdout_file=None,
capture_stderr_file=None,
)


Expand Down
32 changes: 31 additions & 1 deletion src/python/pants/core/util_rules/adhoc_process_support.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ class AdhocProcessRequest:
supplied_env_var_values: FrozenDict[str, str] | None
log_on_process_errors: FrozenDict[int, str] | None
log_output: bool
capture_stdout_file: str | None
capture_stderr_file: str | None


@dataclass(frozen=True)
Expand Down Expand Up @@ -410,7 +412,25 @@ async def run_adhoc_process(
request.root_output_directory, working_directory
)

adjusted = await Get(Digest, RemovePrefix(result.output_digest, root_output_directory))
extras = (
(request.capture_stdout_file, result.stdout),
(request.capture_stderr_file, result.stderr),
)
extra_contents = {i: j for i, j in extras if i}

output_digest = result.output_digest

if extra_contents:
extra_digest = await Get(
Digest,
CreateDigest(
FileContent(_parse_relative_file(name, working_directory), content)
for name, content in extra_contents.items()
),
)
output_digest = await Get(Digest, MergeDigests((output_digest, extra_digest)))

adjusted = await Get(Digest, RemovePrefix(output_digest, root_output_directory))

return AdhocProcessResult(result, adjusted)

Expand Down Expand Up @@ -508,5 +528,15 @@ def _parse_relative_directory(workdir_in: str, relative_to: Union[Address, str])
return workdir_in


def _parse_relative_file(file_in: str, relative_to: str) -> str:
"""Convert the `capture_std..._file` fields into something that can be understood by
`Process`."""

if file_in.startswith("/"):
return file_in[1:]

return os.path.join(relative_to, file_in)


def rules():
return collect_rules()

0 comments on commit 4940005

Please sign in to comment.