Skip to content

Commit 5f16090

Browse files
authored
refactor/javascript: Move package manager decisions to one place (#21135)
Future extensions could be exposing the `PackageManager` fields as configurable overrides in `pants.toml` to attempt easier integration with future versions of package managers.
1 parent 8adcb47 commit 5f16090

File tree

7 files changed

+141
-87
lines changed

7 files changed

+141
-87
lines changed

src/python/pants/backend/javascript/install_node_package.py

+5
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
NodePackageVersionField,
1919
PackageJsonSourceField,
2020
)
21+
from pants.backend.javascript.package_manager import PackageManager
2122
from pants.backend.javascript.subsystems import nodejs
2223
from pants.backend.javascript.target_types import JSSourceField
2324
from pants.build_graph.address import Address
@@ -52,6 +53,10 @@ def join_relative_workspace_directory(self, path: str) -> str:
5253
def target(self) -> Target:
5354
return self.project_env.ensure_target()
5455

56+
@property
57+
def package_manager(self) -> PackageManager:
58+
return self.project_env.project.package_manager
59+
5560

5661
@dataclass(frozen=True)
5762
class InstalledNodePackageWithSource(InstalledNodePackage):

src/python/pants/backend/javascript/nodejs_project.py

+16-60
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,14 @@
88
from pathlib import PurePath
99
from typing import Iterable
1010

11-
import nodesemver
12-
1311
from pants.backend.javascript import package_json
1412
from pants.backend.javascript.package_json import (
1513
AllPackageJson,
1614
PackageJson,
1715
PnpmWorkspaceGlobs,
1816
PnpmWorkspaces,
1917
)
18+
from pants.backend.javascript.package_manager import PackageManager
2019
from pants.backend.javascript.subsystems import nodejs
2120
from pants.backend.javascript.subsystems.nodejs import NodeJS, UserChosenNodeJSResolveAliases
2221
from pants.core.util_rules import stripped_source_files
@@ -56,80 +55,38 @@ class NodeJSProject:
5655
root_dir: str
5756
workspaces: FrozenOrderedSet[PackageJson]
5857
default_resolve_name: str
59-
package_manager: str
60-
package_manager_version: str | None = None
58+
package_manager: PackageManager
6159
pnpm_workspace: PnpmWorkspaceGlobs | None = None
6260

6361
@property
6462
def lockfile_name(self) -> str:
65-
if self.package_manager == "pnpm":
66-
return "pnpm-lock.yaml"
67-
elif self.package_manager == "yarn":
68-
return "yarn.lock"
69-
return "package-lock.json"
63+
return self.package_manager.lockfile_name
7064

7165
@property
7266
def generate_lockfile_args(self) -> tuple[str, ...]:
73-
if self.package_manager == "pnpm":
74-
return ("install", "--lockfile-only")
75-
elif self.package_manager == "yarn":
76-
return ("install",) # yarn does not provide a lockfile only mode.
77-
return ("install", "--package-lock-only")
67+
return self.package_manager.generate_lockfile_args
7868

7969
@property
8070
def immutable_install_args(self) -> tuple[str, ...]:
81-
if self.package_manager == "npm":
82-
return ("clean-install",)
83-
if self.package_manager == "pnpm":
84-
return ("install", "--frozen-lockfile")
85-
if self.package_manager == "yarn":
86-
if nodesemver.satisfies(self.package_manager_version, "1.x"):
87-
return ("install", "--frozen-lockfile")
88-
return ("install", "--immutable")
89-
raise ValueError(f"Unsupported package manager: {self.package_manager}")
71+
return self.package_manager.immutable_install_args
9072

9173
@property
9274
def workspace_specifier_arg(self) -> str:
93-
if self.package_manager == "pnpm":
94-
return "--filter"
95-
elif self.package_manager == "yarn":
96-
return "workspace"
97-
return "--workspace"
75+
return self.package_manager.workspace_specifier_arg
9876

9977
@property
10078
def args_separator(self) -> tuple[str, ...]:
101-
# pnpm 7 changed the arguments to the `run` command - all other package managers
102-
# accept an args separator --, but pnpm does not in versions 7 and above.
103-
# > When using pnpm run <script>, all command line arguments after the script
104-
# > name are now passed to the script's argv, even --.
105-
if self.package_manager == "pnpm" and (
106-
self.package_manager_version is None
107-
or nodesemver.satisfies(self.package_manager_version, ">=7")
108-
):
109-
return ()
110-
111-
return ("--",)
79+
return self.package_manager.run_arg_separator
11280

11381
def extra_env(self) -> dict[str, str]:
114-
if self.package_manager == "pnpm":
115-
return {"PNPM_HOME": "{chroot}/._pnpm_home"}
116-
elif self.package_manager == "yarn":
117-
return {"YARN_CACHE_FOLDER": "{chroot}/._yarn_cache"}
118-
return {}
82+
return dict(self.package_manager.extra_env)
11983

12084
@property
12185
def pack_archive_format(self) -> str:
122-
if self.package_manager == "yarn":
123-
return "{}-v{}.tgz"
124-
else:
125-
return "{}-{}.tgz"
86+
return self.package_manager.pack_archive_format
12687

12788
def extra_caches(self) -> dict[str, str]:
128-
if self.package_manager == "pnpm":
129-
return {"pnpm_home": "._pnpm_home"}
130-
elif self.package_manager == "yarn":
131-
return {"yarn_cache": "._yarn_cache"}
132-
return {}
89+
return dict(self.package_manager.extra_caches)
13390

13491
def get_project_digest(self) -> MergeDigests:
13592
return MergeDigests(
@@ -182,15 +139,12 @@ def from_tentative(
182139
"""
183140
)
184141
)
185-
package_manager_command, *maybe_version = package_manager.split("@")
186-
package_manager_version = maybe_version[0] if maybe_version else None
187142

188143
return NodeJSProject(
189144
root_dir=project.root_dir,
190145
workspaces=project.workspaces,
191146
default_resolve_name=project.default_resolve_name or "nodejs-default",
192-
package_manager=package_manager_command,
193-
package_manager_version=package_manager_version,
147+
package_manager=PackageManager.from_string(package_manager),
194148
pnpm_workspace=pnpm_workspaces.for_root(project.root_dir),
195149
)
196150

@@ -239,9 +193,11 @@ async def find_node_js_projects(
239193
# Note: If pnpm_workspace.yaml is present for an npm-managed project, it will override the package.json["workspaces"] setting, which is not intuitive
240194
# pnpm_workspace.yaml should only be used for pnpm projects - see https://github.com/pantsbuild/pants/issues/21134
241195
project_paths = (
242-
ProjectPaths(pkg.root_dir, ["", *pkg.workspaces])
243-
if pkg not in pnpm_workspaces
244-
else ProjectPaths(pkg.root_dir, ["", *pnpm_workspaces[pkg].packages])
196+
(
197+
ProjectPaths(pkg.root_dir, ["", *pkg.workspaces])
198+
if pkg not in pnpm_workspaces
199+
else ProjectPaths(pkg.root_dir, ["", *pnpm_workspaces[pkg].packages])
200+
)
245201
for pkg in package_workspaces
246202
)
247203

src/python/pants/backend/javascript/nodejs_project_environment.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,8 @@ async def setup_nodejs_project_environment_process(req: NodeJsProjectEnvironment
142142
Process,
143143
NodeJSToolProcess,
144144
NodeJSToolProcess(
145-
tool=req.env.project.package_manager,
146-
tool_version=req.env.project.package_manager_version,
145+
tool=req.env.project.package_manager.name,
146+
tool_version=req.env.project.package_manager.version,
147147
args=args,
148148
description=req.description,
149149
level=req.level,

src/python/pants/backend/javascript/nodejs_project_test.py

+3-4
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,9 @@ def test_immutable_install_args_property_with_unsupported_package_manager(
110110
"src/js/foo/package.json": given_package("foo", "0.0.1", package_manager="[email protected]"),
111111
}
112112
)
113-
projects = rule_runner.request(AllNodeJSProjects, [])
114113
expected_error = "Unsupported package manager: bar"
115-
with pytest.raises(ValueError, match=expected_error):
116-
{project.immutable_install_args for project in projects}
114+
with engine_error(ValueError, contains=expected_error):
115+
rule_runner.request(AllNodeJSProjects, [])
117116

118117

119118
def test_root_package_json_is_supported(rule_runner: RuleRunner) -> None:
@@ -144,7 +143,7 @@ def test_parses_project_with_workspaces(rule_runner: RuleRunner) -> None:
144143
[project] = rule_runner.request(AllNodeJSProjects, [])
145144
assert project.root_dir == "src/js"
146145
assert {workspace.name for workspace in project.workspaces} == {"egg", "ham", "spam"}
147-
assert project.package_manager == "npm"
146+
assert project.package_manager.name == "npm"
148147

149148

150149
def test_parses_project_with_nested_workspaces(rule_runner: RuleRunner) -> None:
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
# Copyright 2024 Pants project contributors (see CONTRIBUTORS.md).
2+
# Licensed under the Apache License, Version 2.0 (see LICENSE).
3+
from __future__ import annotations
4+
5+
from dataclasses import dataclass
6+
7+
import nodesemver
8+
9+
from pants.util.frozendict import FrozenDict
10+
11+
12+
@dataclass(frozen=True)
13+
class PackageManager:
14+
name: str
15+
version: str | None
16+
lockfile_name: str
17+
generate_lockfile_args: tuple[str, ...]
18+
immutable_install_args: tuple[str, ...]
19+
workspace_specifier_arg: str
20+
run_arg_separator: tuple[str, ...]
21+
download_and_execute_args: tuple[str, ...]
22+
execute_args: tuple[str, ...]
23+
current_directory_args: tuple[str, ...]
24+
25+
extra_env: FrozenDict[str, str]
26+
pack_archive_format: str
27+
extra_caches: FrozenDict[str, str]
28+
29+
@classmethod
30+
def from_string(cls, string: str) -> PackageManager:
31+
package_manager_command, *maybe_version = string.split("@")
32+
package_manager_version = maybe_version[0] if maybe_version else None
33+
if package_manager_command == "npm":
34+
return cls.npm(package_manager_version)
35+
if package_manager_command == "yarn":
36+
return cls.yarn(package_manager_version)
37+
if package_manager_command == "pnpm":
38+
return cls.pnpm(package_manager_version)
39+
raise ValueError(f"Unsupported package manager: {package_manager_command}.")
40+
41+
@classmethod
42+
def pnpm(cls, version: str | None) -> PackageManager:
43+
return PackageManager(
44+
name="pnpm",
45+
version=version,
46+
lockfile_name="pnpm-lock.yaml",
47+
generate_lockfile_args=("install", "--lockfile-only"),
48+
immutable_install_args=("install", "--frozen-lockfile"),
49+
workspace_specifier_arg="--filter",
50+
run_arg_separator=(
51+
() if version is None or nodesemver.satisfies(version, ">=7") else ("--",)
52+
),
53+
download_and_execute_args=("dlx",),
54+
execute_args=("exec",),
55+
current_directory_args=("--prefix",),
56+
extra_env=FrozenDict({"PNPM_HOME": "{chroot}/._pnpm_home"}),
57+
pack_archive_format="{}-{}.tgz",
58+
extra_caches=FrozenDict({"pnpm_home": "._pnpm_home"}),
59+
)
60+
61+
@classmethod
62+
def yarn(cls, version: str | None) -> PackageManager:
63+
return PackageManager(
64+
name="yarn",
65+
version=version,
66+
lockfile_name="yarn.lock",
67+
generate_lockfile_args=("install",),
68+
immutable_install_args=(
69+
("install", "--frozen-lockfile")
70+
if version is None or nodesemver.satisfies(version, "1.x")
71+
else ("install", "--immutable")
72+
),
73+
workspace_specifier_arg="workspace",
74+
run_arg_separator=("--",),
75+
download_and_execute_args=("dlx", "--quiet"),
76+
execute_args=("--silent", "exec", "--"),
77+
current_directory_args=("--cwd",),
78+
extra_env=FrozenDict({"YARN_CACHE_FOLDER": "{chroot}/._yarn_cache"}),
79+
pack_archive_format="{}-v{}.tgz",
80+
extra_caches=FrozenDict({"yarn_cache": "._yarn_cache"}),
81+
)
82+
83+
@classmethod
84+
def npm(cls, version: str | None) -> PackageManager:
85+
return PackageManager(
86+
name="npm",
87+
version=version,
88+
lockfile_name="package-lock.json",
89+
generate_lockfile_args=("install", "--package-lock-only"),
90+
immutable_install_args=("clean-install",),
91+
workspace_specifier_arg="--workspace",
92+
run_arg_separator=("--",),
93+
download_and_execute_args=("exec", "--yes", "--"),
94+
execute_args=("exec", "--no", "--"),
95+
current_directory_args=("--prefix",),
96+
extra_env=FrozenDict(),
97+
pack_archive_format="{}-{}.tgz",
98+
extra_caches=FrozenDict(),
99+
)

src/python/pants/backend/javascript/run/rules.py

+6-5
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,16 @@ async def run_node_build_script(
4646
EnvironmentVars, EnvironmentVarsRequest(field_set.extra_env_vars.value or ())
4747
)
4848

49-
prefix_arg = "--prefix"
50-
if installation.project_env.project.package_manager == "yarn":
51-
prefix_arg = "--cwd"
52-
5349
process = await Get(
5450
Process,
5551
NodeJsProjectEnvironmentProcess(
5652
installation.project_env,
57-
args=(prefix_arg, "{chroot}", "run", str(field_set.entry_point.value)),
53+
args=(
54+
*installation.package_manager.current_directory_args,
55+
"{chroot}",
56+
"run",
57+
str(field_set.entry_point.value),
58+
),
5859
description=f"Running {str(field_set.entry_point.value)}.",
5960
input_digest=installation.digest,
6061
extra_env=target_env_vars,

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

+10-16
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
InstalledNodePackageRequest,
1313
)
1414
from pants.backend.javascript.nodejs_project_environment import NodeJsProjectEnvironmentProcess
15+
from pants.backend.javascript.package_manager import PackageManager
1516
from pants.backend.javascript.resolve import FirstPartyNodePackageResolves, NodeJSProjectResolves
1617
from pants.backend.javascript.subsystems.nodejs import NodeJS, NodeJSToolProcess
1718
from pants.engine.internals.native_engine import Digest, MergeDigests
@@ -97,14 +98,10 @@ class NodeJSToolRequest:
9798

9899
async def _run_tool_without_resolve(request: NodeJSToolRequest) -> Process:
99100
nodejs = await Get(NodeJS)
100-
dl_and_execute_args = {
101-
"npm": ("exec", "--yes", "--"),
102-
"pnpm": ("dlx",),
103-
"yarn": ("dlx", "--quiet"),
104-
}
105101

106102
pkg_manager_version = nodejs.package_managers.get(nodejs.package_manager)
107-
if pkg_manager_version is None:
103+
pkg_manager_and_version = nodejs.default_package_manager
104+
if pkg_manager_version is None or pkg_manager_and_version is None:
108105
# Occurs when a user configures a custom package manager but without a resolve.
109106
# Corepack requires a package.json to make a decision on a "good known release".
110107
raise ValueError(
@@ -116,19 +113,21 @@ async def _run_tool_without_resolve(request: NodeJSToolRequest) -> Process:
116113
"""
117114
)
118115
)
116+
pkg_manager = PackageManager.from_string(pkg_manager_and_version)
117+
119118
return await Get(
120119
Process,
121120
NodeJSToolProcess(
122-
nodejs.package_manager,
123-
pkg_manager_version,
124-
args=(*dl_and_execute_args[nodejs.package_manager], request.tool, *request.args),
121+
pkg_manager.name,
122+
pkg_manager.version,
123+
args=(*pkg_manager.download_and_execute_args, request.tool, *request.args),
125124
description=request.description,
126125
input_digest=request.input_digest,
127126
output_files=request.output_files,
128127
output_directories=request.output_directories,
129128
append_only_caches=request.append_only_caches,
130129
timeout_seconds=request.timeout_seconds,
131-
extra_env=FrozenDict({"PNPM_HOME": "{chroot}/._pnpm_home", **request.extra_env}),
130+
extra_env=FrozenDict({**pkg_manager.extra_env, **request.extra_env}),
132131
),
133132
)
134133

@@ -150,18 +149,13 @@ async def _run_tool_with_resolve(request: NodeJSToolRequest, resolve: str) -> Pr
150149
installed = await Get(
151150
InstalledNodePackage, InstalledNodePackageRequest(package_for_resolve.address)
152151
)
153-
execute_args = {
154-
"npm": ("exec", "--no", "--"),
155-
"pnpm": ("exec",),
156-
"yarn": ("--silent", "exec", "--"),
157-
}
158152
request_tool_without_version = request.tool.partition("@")[0]
159153
return await Get(
160154
Process,
161155
NodeJsProjectEnvironmentProcess(
162156
env=installed.project_env,
163157
args=(
164-
*execute_args[project.package_manager],
158+
*project.package_manager.execute_args,
165159
request_tool_without_version,
166160
*request.args,
167161
),

0 commit comments

Comments
 (0)