Merge 98dd578 into ee98ef6

src/python/pants/engine/process.py

@@ -30,6 +30,9 @@
 logger = logging.getLogger(__name__)
 
 
+BASH_SEARCH_PATHS = ("/usr/bin", "/bin", "/usr/local/bin")
+
+
 @dataclass(frozen=True)
 class ProductDescription:
     value: str
@@ -482,22 +485,33 @@ def __init__(
 
 @rule(desc="Find binary path", level=LogLevel.DEBUG)
 async def find_binary(request: BinaryPathRequest) -> BinaryPaths:
-    # TODO(John Sirois): Replace this script with a statically linked native binary so we don't
+    # If we are not already locating bash, recurse to locate bash to use it as an absolute path in
+    # our shebang. This avoids mixing locations that we would search for bash into the search paths
+    # of the request we are servicing.
+    # TODO(#10769): Replace this script with a statically linked native binary so we don't
     #  depend on either /bin/bash being available on the Process host.
+    if request.binary_name == "bash":
+        shebang = "#!/usr/bin/env bash"
+    else:
+        bash_request = BinaryPathRequest(binary_name="bash", search_path=BASH_SEARCH_PATHS)
+        bash_paths = await Get(BinaryPaths, BinaryPathRequest, bash_request)
+        if not bash_paths.first_path:
+            raise BinaryNotFoundError(bash_request, rationale="use it to locate other executables")
+        shebang = f"#!{bash_paths.first_path.path}"
 
     # Note: the backslash after the """ marker ensures that the shebang is at the start of the
     # script file. Many OSs will not see the shebang if there is intervening whitespace.
-    script_path = "./script.sh"
+    script_path = "./find_binary.sh"
     script_content = dedent(
-        """\
-        #!/usr/bin/env bash
+        f"""\
+        {shebang}
 
-        set -euo pipefail
+        set -euox pipefail
 
         if command -v which > /dev/null; then
-            command which -a $1
+            command which -a $1 || true
         else
-            command -v $1
+            command -v $1 || true
         fi
         """
     )
@@ -508,7 +522,7 @@ async def find_binary(request: BinaryPathRequest) -> BinaryPaths:
 
     search_path = create_path_env_var(request.search_path)
     result = await Get(
-        FallibleProcessResult,
+        ProcessResult,
         # We use a volatile process to force re-run since any binary found on the host system today
         # could be gone tomorrow. Ideally we'd only do this for local processes since all known
         # remoting configurations include a static container image as part of their cache key which
@@ -526,9 +540,6 @@ async def find_binary(request: BinaryPathRequest) -> BinaryPaths:
     )
 
     binary_paths = BinaryPaths(binary_name=request.binary_name)
-    if result.exit_code != 0:
-        return binary_paths
-
     found_paths = result.stdout.decode().splitlines()
     if not request.test:
         return dataclasses.replace(binary_paths, paths=[BinaryPath(path) for path in found_paths])

src/python/pants/engine/process_test.py

@@ -10,16 +10,28 @@
 from pants.engine.fs import CreateDigest, Digest, DigestContents, FileContent, PathGlobs, Snapshot
 from pants.engine.internals.scheduler import ExecutionError
 from pants.engine.process import (
+    BinaryPathRequest,
+    BinaryPaths,
     FallibleProcessResult,
     InteractiveProcess,
     Process,
     ProcessExecutionFailure,
     ProcessResult,
 )
 from pants.engine.rules import Get, rule
-from pants.testutil.rule_runner import QueryRule
+from pants.testutil.rule_runner import QueryRule, RuleRunner
 from pants.testutil.test_base import TestBase
 from pants.util.contextutil import temporary_dir
+from pants.util.dirutil import safe_mkdir, touch
+
+
+@pytest.fixture
+def rule_runner() -> RuleRunner:
+    return RuleRunner(
+        rules=[
+            QueryRule(BinaryPaths, [BinaryPathRequest]),
+        ],
+    )
 
 
 @dataclass(frozen=True)
@@ -442,3 +454,32 @@ def test_running_interactive_process_in_workspace_cannot_have_input_files() -> N
     mock_digest = Digest("fake", 1)
     with pytest.raises(ValueError):
         InteractiveProcess(argv=["/bin/echo"], input_digest=mock_digest, run_in_workspace=True)
+
+
+def test_find_binary_non_existent(rule_runner: RuleRunner) -> None:
+    with temporary_dir() as tmpdir:
+        search_path = [tmpdir]
+        binary_paths = rule_runner.request(
+            BinaryPaths, [BinaryPathRequest(binary_name="anybin", search_path=search_path)]
+        )
+        assert binary_paths.first_path is None
+
+
+def test_find_binary_on_path_without_bash(rule_runner: RuleRunner) -> None:
+    # Test that locating a binary on a PATH which does not include bash works (by recursing to
+    # locate bash first).
+    binary_name = "mybin"
+    binary_dir = "bin"
+    with temporary_dir() as tmpdir:
+        binary_dir_abs = os.path.join(tmpdir, binary_dir)
+        binary_path_abs = os.path.join(binary_dir_abs, binary_name)
+        safe_mkdir(binary_dir_abs)
+        touch(binary_path_abs)
+
+        search_path = [binary_dir_abs]
+        binary_paths = rule_runner.request(
+            BinaryPaths, [BinaryPathRequest(binary_name=binary_name, search_path=search_path)]
+        )
+        assert os.path.exists(os.path.join(binary_dir_abs, binary_name))
+        assert binary_paths.first_path is not None
+        assert binary_paths.first_path.path == binary_path_abs