Skip to content

Commit

Permalink
javascript: Assign a default name to root nodejs resolves (pantsbuild…
Browse files Browse the repository at this point in the history
…#19245)

Also validate the resolve names, ensuring they are 1:1 with project
roots.

Fixes pantsbuild#18924
  • Loading branch information
tobni authored Aug 14, 2023
1 parent 3bbf36f commit 62956b9
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 21 deletions.
3 changes: 2 additions & 1 deletion src/python/pants/backend/javascript/goals/lockfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
NodeJsProjectEnvironmentProcess,
)
from pants.backend.javascript.package_json import PackageJsonTarget
from pants.backend.javascript.resolve import NodeJSProjectResolves, UserChosenNodeJSResolveAliases
from pants.backend.javascript.resolve import NodeJSProjectResolves
from pants.backend.javascript.subsystems.nodejs import UserChosenNodeJSResolveAliases
from pants.core.goals.generate_lockfiles import (
GenerateLockfile,
GenerateLockfileResult,
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/javascript/goals/lockfile_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
PackageJsonForGlobs,
PackageJsonTarget,
)
from pants.backend.javascript.resolve import UserChosenNodeJSResolveAliases
from pants.backend.javascript.subsystems.nodejs import UserChosenNodeJSResolveAliases
from pants.core.goals.generate_lockfiles import (
GenerateLockfileResult,
KnownUserResolveNames,
Expand Down
50 changes: 44 additions & 6 deletions src/python/pants/backend/javascript/nodejs_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
PnpmWorkspaces,
)
from pants.backend.javascript.subsystems import nodejs
from pants.backend.javascript.subsystems.nodejs import NodeJS
from pants.backend.javascript.subsystems.nodejs import NodeJS, UserChosenNodeJSResolveAliases
from pants.core.util_rules import stripped_source_files
from pants.core.util_rules.stripped_source_files import StrippedFileName, StrippedFileNameRequest
from pants.engine.collection import Collection
Expand All @@ -25,7 +25,7 @@
from pants.engine.rules import Rule, collect_rules, rule
from pants.engine.unions import UnionRule
from pants.util.ordered_set import FrozenOrderedSet
from pants.util.strutil import softwrap
from pants.util.strutil import bullet_list, softwrap


@dataclass(frozen=True)
Expand Down Expand Up @@ -123,7 +123,10 @@ def single_workspace(self) -> bool:

@classmethod
def from_tentative(
cls, project: _TentativeProject, nodejs: NodeJS, pnpm_workspaces: PnpmWorkspaces
cls,
project: _TentativeProject,
nodejs: NodeJS,
pnpm_workspaces: PnpmWorkspaces,
) -> NodeJSProject:
root_ws = project.root_workspace()
package_manager: str | None = None
Expand Down Expand Up @@ -163,7 +166,7 @@ def from_tentative(
return NodeJSProject(
root_dir=project.root_dir,
workspaces=project.workspaces,
default_resolve_name=project.default_resolve_name,
default_resolve_name=project.default_resolve_name or "nodejs-default",
package_manager=package_manager_command,
package_manager_version=package_manager_version,
pnpm_workspace=pnpm_workspaces.for_root(project.root_dir),
Expand Down Expand Up @@ -206,7 +209,10 @@ async def _get_default_resolve_name(path: str) -> str:

@rule
async def find_node_js_projects(
package_workspaces: AllPackageJson, pnpm_workspaces: PnpmWorkspaces, nodejs: NodeJS
package_workspaces: AllPackageJson,
pnpm_workspaces: PnpmWorkspaces,
nodejs: NodeJS,
resolve_names: UserChosenNodeJSResolveAliases,
) -> AllNodeJSProjects:
project_paths = (
ProjectPaths(pkg.root_dir, ["", *pkg.workspaces])
Expand All @@ -224,9 +230,41 @@ async def find_node_js_projects(
for paths in project_paths
}
merged_projects = _merge_workspaces(node_js_projects)
return AllNodeJSProjects(
all_projects = AllNodeJSProjects(
NodeJSProject.from_tentative(p, nodejs, pnpm_workspaces) for p in merged_projects
)
_ensure_resolve_names_are_unique(all_projects, resolve_names)

return all_projects


_AMBIGUOUS_RESOLVE_SOLUTIONS = [
f"Configure [{NodeJS.options_scope}].resolves to grant the package.json directories different names.",
"Make one package a workspace of the other.",
"Re-configure your source root(s).",
]


def _ensure_resolve_names_are_unique(
all_projects: AllNodeJSProjects, resolve_names: UserChosenNodeJSResolveAliases
) -> None:
seen: dict[str, NodeJSProject] = {}
for project in all_projects:
resolve_name = resolve_names.get(project.root_dir, project.default_resolve_name)
seen_project = seen.get(resolve_name)
if seen_project:
raise ValueError(
softwrap(
f"""
Projects with root directories '{project.root_dir}' and '{seen_project.root_dir}'
have the same resolve name {resolve_name}. This will cause ambiguities.
To disambiguate, either:\n\n
{bullet_list(_AMBIGUOUS_RESOLVE_SOLUTIONS)}
"""
)
)
seen[resolve_name] = project


def _project_to_parents(
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/javascript/nodejs_project_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ def test_root_package_json_is_supported(rule_runner: RuleRunner) -> None:
)
projects = rule_runner.request(AllNodeJSProjects, [])
assert {project.root_dir for project in projects} == {"", "src/js/bar"}
assert {project.default_resolve_name for project in projects} == {"nodejs-default", "js.bar"}


def test_parses_project_with_workspaces(rule_runner: RuleRunner) -> None:
Expand Down
12 changes: 1 addition & 11 deletions src/python/pants/backend/javascript/resolve.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,14 @@
OwningNodePackageRequest,
PackageJsonSourceField,
)
from pants.backend.javascript.subsystems.nodejs import NodeJS
from pants.backend.javascript.subsystems.nodejs import UserChosenNodeJSResolveAliases
from pants.build_graph.address import Address
from pants.engine.fs import PathGlobs
from pants.engine.internals.selectors import Get
from pants.engine.rules import Rule, collect_rules, rule
from pants.engine.target import Target, WrappedTarget, WrappedTargetRequest
from pants.engine.unions import UnionRule
from pants.util.frozendict import FrozenDict
from pants.util.logging import LogLevel


@dataclass(frozen=True)
Expand Down Expand Up @@ -76,10 +75,6 @@ class NodeJSProjectResolves(FrozenDict[str, NodeJSProject]):
pass


class UserChosenNodeJSResolveAliases(FrozenDict[str, str]):
pass


@rule
async def resolve_to_projects(
all_projects: AllNodeJSProjects, user_chosen_resolves: UserChosenNodeJSResolveAliases
Expand All @@ -106,10 +101,5 @@ async def resolve_to_first_party_node_package(
)


@rule(level=LogLevel.DEBUG)
async def user_chosen_resolve_aliases(nodejs: NodeJS) -> UserChosenNodeJSResolveAliases:
return UserChosenNodeJSResolveAliases((value, key) for key, value in nodejs.resolves.items())


def rules() -> Iterable[Rule | UnionRule]:
return [*collect_rules(), *nodejs_project.rules()]
9 changes: 9 additions & 0 deletions src/python/pants/backend/javascript/subsystems/nodejs.py
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,15 @@ async def setup_node_tool_process(
)


class UserChosenNodeJSResolveAliases(FrozenDict[str, str]):
pass


@rule(level=LogLevel.DEBUG)
async def user_chosen_resolve_aliases(nodejs: NodeJS) -> UserChosenNodeJSResolveAliases:
return UserChosenNodeJSResolveAliases((value, key) for key, value in nodejs.resolves.items())


def rules() -> Iterable[Rule | UnionRule]:
return (
*collect_rules(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,10 @@ def test_resolve_dictates_version(
}
)
rule_runner.set_options(
["--cowsay-install-from-resolve=", f"--nodejs-package-manager={package_manager}"],
[
"--cowsay-install-from-resolve=nodejs-default",
f"--nodejs-package-manager={package_manager}",
],
env_inherit={"PATH"},
)
tool = rule_runner.request(CowsayTool, [])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ def test_config_file(
},
(
f"--source-root-patterns=['{LIB_1_PACKAGE}', '{LIB_2_PACKAGE}', '/']",
"--pyright-install-from-resolve=", # See: https://github.com/pantsbuild/pants/issues/18924
"--pyright-install-from-resolve=nodejs-default",
),
id="from_resolve_at_root",
),
Expand Down

0 comments on commit 62956b9

Please sign in to comment.