From d6e6ef8fe278c5b4b75a01d57cb1415d6d5e4a0d Mon Sep 17 00:00:00 2001 From: Sven Date: Fri, 18 Oct 2024 07:58:33 +0200 Subject: [PATCH 1/4] Use corepack to run package managers so its possible to use interactive processes --- .../javascript/subsystems/nodejs_tool.py | 7 ++++--- .../javascript/subsystems/nodejs_tool_test.py | 18 +++++++++++++----- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs_tool.py b/src/python/pants/backend/javascript/subsystems/nodejs_tool.py index b984f721bb2..f6a10eb51c1 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs_tool.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs_tool.py @@ -14,7 +14,7 @@ from pants.backend.javascript.nodejs_project_environment import NodeJsProjectEnvironmentProcess from pants.backend.javascript.package_manager import PackageManager from pants.backend.javascript.resolve import FirstPartyNodePackageResolves, NodeJSProjectResolves -from pants.backend.javascript.subsystems.nodejs import NodeJS, NodeJSToolProcess +from pants.backend.javascript.subsystems.nodejs import NodeJS, NodeJSProcessEnvironment, NodeJSToolProcess from pants.engine.internals.native_engine import Digest, MergeDigests from pants.engine.internals.selectors import Get from pants.engine.process import Process @@ -98,6 +98,7 @@ class NodeJSToolRequest: async def _run_tool_without_resolve(request: NodeJSToolRequest) -> Process: nodejs = await Get(NodeJS) + env = await Get(NodeJSProcessEnvironment) pkg_manager_version = nodejs.package_managers.get(nodejs.package_manager) pkg_manager_and_version = nodejs.default_package_manager @@ -118,9 +119,9 @@ async def _run_tool_without_resolve(request: NodeJSToolRequest) -> Process: return await Get( Process, NodeJSToolProcess( - pkg_manager.name, + env.binaries.binary_dir + "/corepack", pkg_manager.version, - args=(*pkg_manager.download_and_execute_args, request.tool, *request.args), + args=(pkg_manager.name, *pkg_manager.download_and_execute_args, request.tool, *request.args), description=request.description, input_digest=request.input_digest, output_files=request.output_files, diff --git a/src/python/pants/backend/javascript/subsystems/nodejs_tool_test.py b/src/python/pants/backend/javascript/subsystems/nodejs_tool_test.py index 7d48994925f..403439de538 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs_tool_test.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs_tool_test.py @@ -13,8 +13,8 @@ from pants.backend.javascript.subsystems import nodejs_tool from pants.backend.javascript.subsystems.nodejs_tool import NodeJSToolBase, NodeJSToolRequest from pants.engine.internals.native_engine import EMPTY_DIGEST -from pants.engine.process import Process, ProcessResult -from pants.testutil.rule_runner import QueryRule, RuleRunner +from pants.engine.process import InteractiveProcess, InteractiveProcessResult, Process, ProcessResult +from pants.testutil.rule_runner import QueryRule, RuleRunner, mock_console from pants.util.logging import LogLevel @@ -74,7 +74,15 @@ def test_execute_process_with_package_manager( to_run = rule_runner.request(Process, [request]) - assert to_run.argv == expected_argv + ("cowsay@1.6.0", "--version") + ip = InteractiveProcess.from_process(to_run) + with mock_console(rule_runner.options_bootstrapper) as mocked_console: + interactive_result = rule_runner.run_interactive_process(ip) + assert interactive_result.exit_code == 0, mocked_console[ + 1 + ].get_stderr() + + # Remove the corepack binary path from argv. + assert to_run.argv[1:] == expected_argv + ("cowsay@1.6.0", "--version") result = rule_runner.request(ProcessResult, [request]) @@ -85,8 +93,8 @@ def test_execute_process_with_package_manager( "package_manager, version", [ pytest.param("yarn", "1.22.22", id="yarn"), - pytest.param("npm", "10.8.2", id="npm"), - pytest.param("pnpm", "9.5.0", id="pnpm"), + pytest.param("npm", "10.9.0", id="npm"), + pytest.param("pnpm", "9.12.1", id="pnpm"), ], ) def test_execute_process_with_package_manager_version_from_configuration( From feb3f918ba9eed29b47c72085fb9a1f8f2be6e25 Mon Sep 17 00:00:00 2001 From: Sven Date: Wed, 6 Nov 2024 11:35:08 +0100 Subject: [PATCH 2/4] Fix fmt --- .../backend/javascript/subsystems/nodejs_tool.py | 13 +++++++++++-- .../javascript/subsystems/nodejs_tool_test.py | 6 ++---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs_tool.py b/src/python/pants/backend/javascript/subsystems/nodejs_tool.py index f6a10eb51c1..2f0fbeed07b 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs_tool.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs_tool.py @@ -14,7 +14,11 @@ from pants.backend.javascript.nodejs_project_environment import NodeJsProjectEnvironmentProcess from pants.backend.javascript.package_manager import PackageManager from pants.backend.javascript.resolve import FirstPartyNodePackageResolves, NodeJSProjectResolves -from pants.backend.javascript.subsystems.nodejs import NodeJS, NodeJSProcessEnvironment, NodeJSToolProcess +from pants.backend.javascript.subsystems.nodejs import ( + NodeJS, + NodeJSProcessEnvironment, + NodeJSToolProcess, +) from pants.engine.internals.native_engine import Digest, MergeDigests from pants.engine.internals.selectors import Get from pants.engine.process import Process @@ -121,7 +125,12 @@ async def _run_tool_without_resolve(request: NodeJSToolRequest) -> Process: NodeJSToolProcess( env.binaries.binary_dir + "/corepack", pkg_manager.version, - args=(pkg_manager.name, *pkg_manager.download_and_execute_args, request.tool, *request.args), + args=( + pkg_manager.name, + *pkg_manager.download_and_execute_args, + request.tool, + *request.args, + ), description=request.description, input_digest=request.input_digest, output_files=request.output_files, diff --git a/src/python/pants/backend/javascript/subsystems/nodejs_tool_test.py b/src/python/pants/backend/javascript/subsystems/nodejs_tool_test.py index 403439de538..159dfd69f48 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs_tool_test.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs_tool_test.py @@ -13,7 +13,7 @@ from pants.backend.javascript.subsystems import nodejs_tool from pants.backend.javascript.subsystems.nodejs_tool import NodeJSToolBase, NodeJSToolRequest from pants.engine.internals.native_engine import EMPTY_DIGEST -from pants.engine.process import InteractiveProcess, InteractiveProcessResult, Process, ProcessResult +from pants.engine.process import InteractiveProcess, Process, ProcessResult from pants.testutil.rule_runner import QueryRule, RuleRunner, mock_console from pants.util.logging import LogLevel @@ -77,9 +77,7 @@ def test_execute_process_with_package_manager( ip = InteractiveProcess.from_process(to_run) with mock_console(rule_runner.options_bootstrapper) as mocked_console: interactive_result = rule_runner.run_interactive_process(ip) - assert interactive_result.exit_code == 0, mocked_console[ - 1 - ].get_stderr() + assert interactive_result.exit_code == 0, mocked_console[1].get_stderr() # Remove the corepack binary path from argv. assert to_run.argv[1:] == expected_argv + ("cowsay@1.6.0", "--version") From ca355d953f2facf06017c0a25878d4082896b600 Mon Sep 17 00:00:00 2001 From: Sven Date: Wed, 6 Nov 2024 11:48:16 +0100 Subject: [PATCH 3/4] Fix pnpm tests --- .../pants/backend/javascript/subsystems/nodejs_tool_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs_tool_test.py b/src/python/pants/backend/javascript/subsystems/nodejs_tool_test.py index 159dfd69f48..bd88491c2e2 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs_tool_test.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs_tool_test.py @@ -92,7 +92,7 @@ def test_execute_process_with_package_manager( [ pytest.param("yarn", "1.22.22", id="yarn"), pytest.param("npm", "10.9.0", id="npm"), - pytest.param("pnpm", "9.12.1", id="pnpm"), + pytest.param("pnpm", "9.12.3", id="pnpm"), ], ) def test_execute_process_with_package_manager_version_from_configuration( From f161f350d5390dc96c90f2b603ebbc92d08ae147 Mon Sep 17 00:00:00 2001 From: Sven Date: Mon, 25 Nov 2024 11:11:13 +0100 Subject: [PATCH 4/4] Use specific versions for all package managers except yarn --- .../pants/backend/javascript/package_manager.py | 3 +++ .../backend/javascript/subsystems/nodejs_tool.py | 6 +++++- .../javascript/subsystems/nodejs_tool_test.py | 14 +++++++++----- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/python/pants/backend/javascript/package_manager.py b/src/python/pants/backend/javascript/package_manager.py index cae04338faf..2d0c12a00bf 100644 --- a/src/python/pants/backend/javascript/package_manager.py +++ b/src/python/pants/backend/javascript/package_manager.py @@ -97,3 +97,6 @@ def npm(cls, version: str | None) -> PackageManager: pack_archive_format="{}-{}.tgz", extra_caches=FrozenDict(), ) + + def spec(self) -> str: + return self.name if self.version is None else f"{self.name}@{self.version}" diff --git a/src/python/pants/backend/javascript/subsystems/nodejs_tool.py b/src/python/pants/backend/javascript/subsystems/nodejs_tool.py index 2f0fbeed07b..9b691af47c4 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs_tool.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs_tool.py @@ -119,6 +119,10 @@ async def _run_tool_without_resolve(request: NodeJSToolRequest) -> Process: ) ) pkg_manager = PackageManager.from_string(pkg_manager_and_version) + if pkg_manager.name == PackageManager.yarn.__name__: + cmd = pkg_manager.name + else: + cmd = pkg_manager.spec() return await Get( Process, @@ -126,7 +130,7 @@ async def _run_tool_without_resolve(request: NodeJSToolRequest) -> Process: env.binaries.binary_dir + "/corepack", pkg_manager.version, args=( - pkg_manager.name, + cmd, *pkg_manager.download_and_execute_args, request.tool, *request.args, diff --git a/src/python/pants/backend/javascript/subsystems/nodejs_tool_test.py b/src/python/pants/backend/javascript/subsystems/nodejs_tool_test.py index bd88491c2e2..4f38e1f6bbd 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs_tool_test.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs_tool_test.py @@ -52,8 +52,8 @@ def test_version_option_overrides_default(rule_runner: RuleRunner): "package_manager, expected_argv", [ pytest.param("yarn", ("yarn", "dlx", "--quiet"), id="yarn"), - pytest.param("npm", ("npm", "exec", "--yes", "--"), id="npm"), - pytest.param("pnpm", ("pnpm", "dlx"), id="pnpm"), + pytest.param("npm", ("npm@10.8.2", "exec", "--yes", "--"), id="npm"), + pytest.param("pnpm", ("pnpm@9.5.0", "dlx"), id="pnpm"), ], ) def test_execute_process_with_package_manager( @@ -91,8 +91,8 @@ def test_execute_process_with_package_manager( "package_manager, version", [ pytest.param("yarn", "1.22.22", id="yarn"), - pytest.param("npm", "10.9.0", id="npm"), - pytest.param("pnpm", "9.12.3", id="pnpm"), + pytest.param("npm", "10.8.2", id="npm"), + pytest.param("pnpm", "9.5.0", id="pnpm"), ], ) def test_execute_process_with_package_manager_version_from_configuration( @@ -199,8 +199,12 @@ def request_package_manager_version_for_tool( ) -> str: request = tool.request((), EMPTY_DIGEST, "Inspect package manager version", LogLevel.DEBUG) process = rule_runner.request(Process, [request]) + if process.argv[0].find("corepack") != -1: + args = process.argv[:2] + ("--version",) + else: + args = (package_manager, "--version") result = rule_runner.request( ProcessResult, - [dataclasses.replace(process, argv=(package_manager, "--version"))], + [dataclasses.replace(process, argv=args)], ) return result.stdout.decode().strip()