additional tools during nodejs processes invocation (#21833)

# Description

During nodejs processes execution, such as `yarn install`, some
additional tools might need to be present on the PATH, e.g. make, cp,
c++, etc, specially when the npm dependencies uses `node-gyp` under the
hood, so these additional tools are necessary to compile binaries. The
current implementation doesn’t allow us to specify more tools than the
default.

This adds `tools` and `optional_tools` as options to the `nodejs`
subsystem so that these additional tools can be added to the sandbox
during nodejs processes execution.

There might be cases where macos specific tools might be needed - mostly
when `node-gyp` is used - such as `xcrun` and `xcodebuild`, which are
necessary to compile native stuff, and these tools are not available on
linux based os, so I've added the `optional_tools` where these specific
tools can be set.

Fixes #21638.

docs/notes/2.25.x.md

@@ -181,6 +181,13 @@ def inject_my_custom_fields(request: MyCustomInjectFieldsRequest) -> InjectedNfp
 
 The dependency inference now considers `.ts` and `.tsx` file extensions.
 
+The NodeJS subsystem now supports configuring additional tools that should be available in the NodeJS process execution. These tools can be configured via two options:
+
+- [`[nodejs].tools`](https://www.pantsbuild.org/2.25/reference/subsystems/nodejs#tools): Specify additional executables required by nodejs processes.
+- [`[nodejs].optional_tools`](https://www.pantsbuild.org/2.25/reference/subsystems/nodejs#optional_tools): Additional tools that may be needed but aren't required. Unlike `tools`, the build won't fail if these aren't found. Useful for tools that only exist in some environments.
+
+The paths to these tools will be included in the PATH used in the execution sandbox, so that they may be used by NodeJS processes during execution.
+
 ### Plugin API changes
 
 The version of Python used by Pants itself is now [3.11](https://docs.python.org/3/whatsnew/3.11.html) (up from 3.9).

src/python/pants/backend/javascript/subsystems/nodejs.py

@@ -159,6 +159,38 @@ def default_package_manager(self) -> str | None:
             return f"{self.package_manager}@{self.package_managers[self.package_manager]}"
         return self.package_manager
 
+    _tools = StrListOption(
+        default=[],
+        help=softwrap(
+            """
+            List any additional executable tools required for node processes to work. The paths to
+            these tools will be included in the PATH used in the execution sandbox, so that
+            they may be used by nodejs processes execution.
+            """
+        ),
+        advanced=True,
+    )
+
+    _optional_tools = StrListOption(
+        default=[],
+        help=softwrap(
+            """
+            List any additional executable which are not mandatory for node processes to work, but
+            which should be included if available. The paths to these tools will be included in the
+            PATH used in the execution sandbox, so that they may be used by nodejs processes execution.
+            """
+        ),
+        advanced=True,
+    )
+
+    @property
+    def tools(self) -> tuple[str, ...]:
+        return tuple(sorted(set(self._tools)))
+
+    @property
+    def optional_tools(self) -> tuple[str, ...]:
+        return tuple(sorted(set(self._optional_tools)))
+
     class EnvironmentAware(ExecutableSearchPathsOptionMixin, Subsystem.EnvironmentAware):
         search_path = StrListOption(
             default=["<PATH>"],
@@ -348,24 +380,66 @@ async def add_corepack_shims_to_digest(
     return await Get(Digest, MergeDigests((binary_digest, enable_corepack_result.output_digest)))
 
 
+async def get_nodejs_process_tools_shims(
+    *,
+    tools: Sequence[str],
+    optional_tools: Sequence[str],
+    search_path: Sequence[str],
+    rationale: str,
+) -> BinaryShims:
+    requests = [
+        BinaryPathRequest(binary_name=binary_name, search_path=search_path)
+        for binary_name in (*tools, *optional_tools)
+    ]
+    paths = await MultiGet(Get(BinaryPaths, BinaryPathRequest, request) for request in requests)
+    required_tools_paths = [
+        path.first_path_or_raise(request, rationale=rationale)
+        for request, path in zip(requests, paths)
+        if request.binary_name in tools
+    ]
+    optional_tools_paths = [
+        path.first_path
+        for request, path in zip(requests, paths)
+        if request.binary_name in optional_tools and path.first_path
+    ]
+
+    tools_shims = await Get(
+        BinaryShims,
+        BinaryShimsRequest,
+        BinaryShimsRequest.for_paths(
+            *required_tools_paths,
+            *optional_tools_paths,
+            rationale=rationale,
+        ),
+    )
+
+    return tools_shims
+
+
 @rule(level=LogLevel.DEBUG)
 async def node_process_environment(
-    binaries: NodeJSBinaries, nodejs: NodeJS.EnvironmentAware
+    binaries: NodeJSBinaries,
+    nodejs: NodeJS,
+    nodejs_environment: NodeJS.EnvironmentAware,
 ) -> NodeJSProcessEnvironment:
     default_required_tools = ["sh", "bash"]
     tools_used_by_setup_scripts = ["mkdir", "rm", "touch", "which"]
     pnpm_shim_tools = ["sed", "dirname"]
-    binary_shims = await Get(
-        BinaryShims,
-        BinaryShimsRequest.for_binaries(
+
+    binary_shims = await get_nodejs_process_tools_shims(
+        tools=[
             *default_required_tools,
             *tools_used_by_setup_scripts,
             *pnpm_shim_tools,
-            rationale="execute a nodejs process",
-            search_path=nodejs.executable_search_path,
-        ),
+            *nodejs.tools,
+        ],
+        optional_tools=nodejs.optional_tools,
+        search_path=nodejs_environment.executable_search_path,
+        rationale="execute a nodejs process",
+    )
+    corepack_env_vars = await Get(
+        EnvironmentVars, EnvironmentVarsRequest(nodejs_environment.corepack_env_vars)
     )
-    corepack_env_vars = await Get(EnvironmentVars, EnvironmentVarsRequest(nodejs.corepack_env_vars))
     binary_digest_with_shims = await add_corepack_shims_to_digest(
         binaries, binary_shims, corepack_env_vars
     )

src/python/pants/backend/javascript/subsystems/nodejs_test.py

@@ -22,6 +22,7 @@
     _BinaryPathsPerVersion,
     _get_nvm_root,
     determine_nodejs_binaries,
+    node_process_environment,
 )
 from pants.backend.javascript.target_types import JSSourcesGeneratorTarget
 from pants.backend.python import target_types_rules
@@ -35,11 +36,25 @@
     VersionManagerSearchPaths,
     VersionManagerSearchPathsRequest,
 )
-from pants.core.util_rules.system_binaries import BinaryNotFoundError, BinaryPath, BinaryShims
+from pants.core.util_rules.system_binaries import (
+    BinaryNotFoundError,
+    BinaryPath,
+    BinaryPathRequest,
+    BinaryPaths,
+    BinaryShims,
+    BinaryShimsRequest,
+)
 from pants.engine.env_vars import EnvironmentVars, EnvironmentVarsRequest
-from pants.engine.internals.native_engine import EMPTY_DIGEST, Digest, Snapshot
+from pants.engine.fs import CreateDigest, Digest, MergeDigests, Snapshot
+from pants.engine.internals.native_engine import EMPTY_DIGEST, EMPTY_FILE_DIGEST
 from pants.engine.platform import Platform
-from pants.engine.process import ProcessResult
+from pants.engine.process import (
+    Process,
+    ProcessExecutionEnvironment,
+    ProcessResult,
+    ProcessResultMetadata,
+)
+from pants.testutil.option_util import create_subsystem
 from pants.testutil.rule_runner import MockGet, QueryRule, RuleRunner, run_rule_with_mocks
 from pants.util.contextutil import temporary_dir
 
@@ -381,7 +396,7 @@ def mock_environment_vars(_req: EnvironmentVarsRequest) -> EnvironmentVars:
         ),
     ],
 )
-def test_process_environment_variables_are_merged(
+def test_node_process_environment_variables_are_merged(
     extra_environment: dict[str, str] | None, expected: dict[str, str]
 ) -> None:
     environment = NodeJSProcessEnvironment(
@@ -394,3 +409,106 @@ def test_process_environment_variables_are_merged(
     )
 
     assert environment.to_env_dict(extra_environment) == expected
+
+
+def test_node_process_environment_with_tools(rule_runner: RuleRunner) -> None:
+    def mock_get_binary_path(request: BinaryPathRequest) -> BinaryPaths:
+        # These are the tools that are required by default for any nodejs process to work.
+        default_required_tools = ["sh", "bash", "mkdir", "rm", "touch", "which", "sed", "dirname"]
+        if request.binary_name in default_required_tools:
+            return BinaryPaths(
+                request.binary_name, paths=[BinaryPath(f"/bin/{request.binary_name}")]
+            )
+
+        if request.binary_name == "real-tool":
+            return BinaryPaths("real-tool", paths=[BinaryPath("/bin/a-real-tool")])
+
+        return BinaryPaths(request.binary_name, ())
+
+    def mock_get_binary_shims(request: BinaryShimsRequest) -> BinaryShims:
+        return BinaryShims(EMPTY_DIGEST, "cache_name")
+
+    def mock_environment_vars(request: EnvironmentVarsRequest) -> EnvironmentVars:
+        return EnvironmentVars({})
+
+    def mock_create_digest(request: CreateDigest) -> Digest:
+        return EMPTY_DIGEST
+
+    def mock_merge_digests(request: MergeDigests) -> Digest:
+        return EMPTY_DIGEST
+
+    def mock_enable_corepack_process_result(request: Process) -> ProcessResult:
+        return ProcessResult(
+            stdout=b"",
+            stdout_digest=EMPTY_FILE_DIGEST,
+            stderr=b"",
+            stderr_digest=EMPTY_FILE_DIGEST,
+            output_digest=EMPTY_DIGEST,
+            metadata=ProcessResultMetadata(
+                0,
+                ProcessExecutionEnvironment(
+                    environment_name=None,
+                    platform=Platform.create_for_localhost().value,
+                    docker_image=None,
+                    remote_execution=False,
+                    remote_execution_extra_platform_properties=[],
+                    execute_in_workspace=False,
+                ),
+                "ran_locally",
+                0,
+            ),
+        )
+
+    def run(tools: list[str], optional_tools: list[str]) -> None:
+        nodejs_binaries = NodeJSBinaries("__node/v18")
+        nodejs_options = create_subsystem(
+            NodeJS,
+            tools=tools,
+            optional_tools=optional_tools,
+        )
+        nodejs_options_env_aware = MagicMock(spec=NodeJS.EnvironmentAware)
+
+        run_rule_with_mocks(
+            node_process_environment,
+            rule_args=[nodejs_binaries, nodejs_options, nodejs_options_env_aware],
+            mock_gets=[
+                MockGet(
+                    output_type=BinaryPaths,
+                    input_types=(BinaryPathRequest,),
+                    mock=mock_get_binary_path,
+                ),
+                MockGet(
+                    output_type=BinaryShims,
+                    input_types=(BinaryShimsRequest,),
+                    mock=mock_get_binary_shims,
+                ),
+                MockGet(
+                    output_type=EnvironmentVars,
+                    input_types=(EnvironmentVarsRequest,),
+                    mock=mock_environment_vars,
+                ),
+                MockGet(
+                    output_type=Digest,
+                    input_types=(CreateDigest,),
+                    mock=mock_create_digest,
+                ),
+                MockGet(
+                    output_type=Digest,
+                    input_types=(MergeDigests,),
+                    mock=mock_merge_digests,
+                ),
+                MockGet(
+                    output_type=ProcessResult,
+                    input_types=(Process,),
+                    mock=mock_enable_corepack_process_result,
+                ),
+            ],
+        )
+
+    run(tools=["real-tool"], optional_tools=[])
+
+    with pytest.raises(BinaryNotFoundError, match="Cannot find `nonexistent-tool`"):
+        run(tools=["real-tool", "nonexistent-tool"], optional_tools=[])
+
+    # Optional non-existent tool should still succeed.
+    run(tools=[], optional_tools=["real-tool", "nonexistent-tool"])