Skip to content

Commit

Permalink
Makes tools optional for experimental_shell_command (pantsbuild#1…
Browse files Browse the repository at this point in the history
…7704)

This change looks for the `bash` exit code 127, which indicates that a command could not be found. An error log is raised at that point.

As a side effect, we no longer need to nag the user to set `tools`, which makes `experimental_shell_commands` that depend only on `bash` internal commands a bit more concise.

Note that this would _not_ raise a warning if the error is not one that bash would ignore -- e.g. running a command in backticks, without `set -e` enabled.
  • Loading branch information
Christopher Neugebauer authored Dec 2, 2022
1 parent 991878e commit 3bf2200
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 8 deletions.
2 changes: 1 addition & 1 deletion src/python/pants/backend/shell/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ class ShellCommandTimeoutField(IntField):

class ShellCommandToolsField(StringSequenceField):
alias = "tools"
required = True
default = ()
help = softwrap(
"""
Specify required executable tools that might be used.
Expand Down
29 changes: 22 additions & 7 deletions src/python/pants/backend/shell/util_rules/shell_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
MergeDigests,
Snapshot,
)
from pants.engine.process import Process, ProcessResult
from pants.engine.process import FallibleProcessResult, Process, ProcessResult, ProductDescription
from pants.engine.rules import Get, MultiGet, collect_rules, rule, rule_helper
from pants.engine.target import (
FieldSetsPerTarget,
Expand Down Expand Up @@ -137,7 +137,7 @@ async def _prepare_process_request_from_target(shell_command: Target) -> ShellCo
output_directories = tuple(d for d in outputs if d.endswith("/"))

return ShellCommandProcessRequest(
description="the `{alias}` at {address}",
description=f"the `{shell_command.alias}` at `{shell_command.address}`",
interactive=interactive,
working_directory=working_directory,
command=command,
Expand Down Expand Up @@ -173,8 +173,9 @@ async def run_shell_command(
environment_name = await Get(
EnvironmentName, EnvironmentNameRequest, EnvironmentNameRequest.from_target(shell_command)
)
result = await Get(
ProcessResult,

fallible_result = await Get(
FallibleProcessResult,
{
environment_name: EnvironmentName,
ShellCommandProcessFromTargetRequest(
Expand All @@ -183,6 +184,23 @@ async def run_shell_command(
},
)

if fallible_result.exit_code == 127:
logger.error(
f"`{shell_command.alias}` requires the names of any external commands used by this "
f"shell command to be specified in the `{ShellCommandToolsField.alias}` field. If "
f"`bash` cannot find a tool, add it to the `{ShellCommandToolsField.alias}` field."
)

result = await Get(
ProcessResult,
{
fallible_result: FallibleProcessResult,
ProductDescription(
f"the `{shell_command.alias}` at `{shell_command.address}`"
): ProductDescription,
},
)

if shell_command[ShellCommandLogOutputField].value:
if result.stdout:
logger.info(result.stdout.decode())
Expand Down Expand Up @@ -253,9 +271,6 @@ async def prepare_shell_command_process(
"CHROOT": "{chroot}",
}
else:
if not tools:
raise ValueError(f"Must provide any `tools` used by the {description}.")

resolved_tools = await _shell_command_tools(shell_setup, tools, f"execute {description}")
tools = tuple(tool for tool in sorted(resolved_tools))

Expand Down

0 comments on commit 3bf2200

Please sign in to comment.