Do not choke on . in tool names for experimental_shell_commands. (#…

…13293)

This addresses an issue with the naming of the tool env vars, which resulted in syntax errors in the `__run.sh` script in the processes execution sandbox for tools with a period on them, for instance.
Also make the `__run.sh` script idempotent wrgt the symlinks it sets up in the `.bin` folder to avoid spurious error output when re-running manually.

src/python/pants/backend/shell/shell_command.py

@@ -5,6 +5,7 @@
 
 import logging
 import os
+import re
 import shlex
 from dataclasses import dataclass
 from textwrap import dedent
@@ -95,6 +96,11 @@ async def run_shell_command(
     return GeneratedSources(output)
 
 
+def _shell_tool_safe_env_name(tool_name: str) -> str:
+    """Replace any characters not suitable in an environment variable name with `_`."""
+    return re.sub(r"\W", "_", tool_name)
+
+
 @rule
 async def prepare_shell_command_process(
     request: ShellCommandProcessRequest, shell_setup: ShellSetup, bash: BashBinary
@@ -140,12 +146,16 @@ async def prepare_shell_command_process(
         )
 
         command_env = {
-            "TOOLS": " ".join(shlex.quote(tool.binary_name) for tool in tool_requests),
+            "TOOLS": " ".join(
+                _shell_tool_safe_env_name(tool.binary_name) for tool in tool_requests
+            ),
         }
 
         for binary, tool_request in zip(tool_paths, tool_requests):
             if binary.first_path:
-                command_env[tool_request.binary_name] = binary.first_path.path
+                command_env[
+                    _shell_tool_safe_env_name(tool_request.binary_name)
+                ] = binary.first_path.path
             else:
                 raise BinaryNotFoundError.from_request(
                     tool_request,
@@ -194,13 +204,14 @@ async def prepare_shell_command_process(
         )
         boot_script = f"cd {shlex.quote(relpath)}; " if relpath != "." else ""
     else:
-        # Setup bin_relpath dir with symlinks to all requested tools, so that we can use PATH.
+        # Setup bin_relpath dir with symlinks to all requested tools, so that we can use PATH, force
+        # symlinks to avoid issues with repeat runs using the __run.sh script in the sandbox.
         bin_relpath = ".bin"
         boot_script = ";".join(
             dedent(
                 f"""\
                 $mkdir -p {bin_relpath}
-                for tool in $TOOLS; do $ln -s ${{!tool}} {bin_relpath}; done
+                for tool in $TOOLS; do $ln -sf ${{!tool}} {bin_relpath}; done
                 export PATH="$PWD/{bin_relpath}"
                 """
             ).split("\n")

src/python/pants/backend/shell/shell_command_test.py

@@ -8,7 +8,11 @@
 
 import pytest
 
-from pants.backend.shell.shell_command import GenerateFilesFromShellCommandRequest, RunShellCommand
+from pants.backend.shell.shell_command import (
+    GenerateFilesFromShellCommandRequest,
+    RunShellCommand,
+    ShellCommandProcessRequest,
+)
 from pants.backend.shell.shell_command import rules as shell_command_rules
 from pants.backend.shell.target_types import (
     ShellCommand,
@@ -23,6 +27,7 @@
 from pants.core.util_rules.source_files import rules as source_files_rules
 from pants.engine.addresses import Address
 from pants.engine.fs import EMPTY_SNAPSHOT, DigestContents
+from pants.engine.process import Process
 from pants.engine.target import (
     GeneratedSources,
     MultipleSourcesField,
@@ -41,6 +46,7 @@ def rule_runner() -> RuleRunner:
             *source_files_rules(),
             *core_target_type_rules(),
             QueryRule(GeneratedSources, [GenerateFilesFromShellCommandRequest]),
+            QueryRule(Process, [ShellCommandProcessRequest]),
             QueryRule(RunRequest, [RunShellCommand]),
             QueryRule(SourceFiles, [SourceFilesRequest]),
             QueryRule(TransitiveTargets, [TransitiveTargetsRequest]),
@@ -363,3 +369,39 @@ def assert_run_args(target: str, args: tuple[str, ...]) -> None:
 
     assert_run_args("test", ("bash", "-c", "some cmd string"))
     assert_run_args("cd-test", ("bash", "-c", "cd 'src/with space'\"'\"'n quote'; some cmd string"))
+
+
+def test_shell_command_boot_script(rule_runner: RuleRunner) -> None:
+    rule_runner.write_files(
+        {
+            "src/BUILD": dedent(
+                """\
+                experimental_shell_command(
+                  name="boot-script-test",
+                  tools=[
+                    "python3.8",
+                  ],
+                  command="./command.script",
+                )
+                """
+            ),
+        }
+    )
+
+    tgt = rule_runner.get_target(Address("src", target_name="boot-script-test"))
+    res = rule_runner.request(Process, [ShellCommandProcessRequest(tgt)])
+    assert "bash" in res.argv[0]
+    assert res.argv[1:] == (
+        "-c",
+        (
+            "$mkdir -p .bin;"
+            "for tool in $TOOLS; do $ln -sf ${!tool} .bin; done;"
+            'export PATH="$PWD/.bin";'
+            "./command.script"
+        ),
+    )
+
+    tools = sorted({"python3_8", "mkdir", "ln"})
+    assert sorted(res.env["TOOLS"].split()) == tools
+    for tool in tools:
+        assert res.env[tool].endswith(f"/{tool.replace('_', '.')}")