From d41c3a84787e45048af6aed2f515f356cbc39a57 Mon Sep 17 00:00:00 2001 From: Tobias Nilsson Date: Sun, 5 Mar 2023 12:54:47 +0100 Subject: [PATCH 01/17] refactor: Extract rule for finding nodejs binaries --- .../backend/javascript/subsystems/nodejs.py | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs.py b/src/python/pants/backend/javascript/subsystems/nodejs.py index 2986dca532a..d93477484c1 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs.py @@ -177,19 +177,24 @@ async def node_process_environment(nodejs: NodeJS, platform: Platform) -> NodeJS return NodeJSProcessEnvironment(binary_directory=nodejs_bin_dir, npm_config_cache="._npm") -@rule(level=LogLevel.DEBUG) -async def setup_node_tool_process( - request: NodeJSToolProcess, - nodejs: NodeJS, - platform: Platform, - environment: NodeJSProcessEnvironment, -) -> Process: - # Ensure nodejs is installed - downloaded_nodejs = await Get( +@dataclass(frozen=True) +class NodejsBinaries: + digest: Digest + + +@rule(level=LogLevel.INFO, desc="Determining nodejs binaries.") +async def determine_nodejs_binaries(nodejs: NodeJS, platform: Platform) -> NodejsBinaries: + downloaded = await Get( DownloadedExternalTool, ExternalToolRequest, nodejs.get_request(platform) ) + return NodejsBinaries(downloaded.digest) + - immutable_input_digests = {environment.base_bin_dir: downloaded_nodejs.digest} +@rule(level=LogLevel.DEBUG) +async def setup_node_tool_process( + request: NodeJSToolProcess, binaries: NodejsBinaries, environment: NodeJSProcessEnvironment +) -> Process: + immutable_input_digests = {environment.base_bin_dir: binaries.digest} return Process( argv=filter(None, request.args), From eab475d077c688ab9725402e7816f04c3bf6dd0d Mon Sep 17 00:00:00 2001 From: Tobias Nilsson Date: Sun, 5 Mar 2023 23:10:50 +0100 Subject: [PATCH 02/17] chore: Add node-semver --- 3rdparty/python/BUILD | 1 + 3rdparty/python/requirements.txt | 1 + 3rdparty/python/user_reqs.lock | 22 ++++++++++++++++++++++ 3 files changed, 24 insertions(+) diff --git a/3rdparty/python/BUILD b/3rdparty/python/BUILD index 5dfc6149d63..2bfcd6f3b70 100644 --- a/3rdparty/python/BUILD +++ b/3rdparty/python/BUILD @@ -6,6 +6,7 @@ python_requirements( "strawberry-graphql": ["strawberry"], "beautifulsoup4": ["bs4"], "python-gnupg": ["gnupg"], + "node-semver": ["nodesemver"], }, overrides={ "humbug": {"dependencies": ["#setuptools"]}, diff --git a/3rdparty/python/requirements.txt b/3rdparty/python/requirements.txt index 4800864cba0..7099c5e399b 100644 --- a/3rdparty/python/requirements.txt +++ b/3rdparty/python/requirements.txt @@ -38,6 +38,7 @@ types-setuptools==62.6.1 types-toml==0.10.8 typing-extensions==4.3.0 mypy-typing-asserts==0.1.1 +node-semver==0.9.0 # This dependency is only for debugging Pants itself, and should never be imported diff --git a/3rdparty/python/user_reqs.lock b/3rdparty/python/user_reqs.lock index 1fd7bf49dae..3040fbd48ae 100644 --- a/3rdparty/python/user_reqs.lock +++ b/3rdparty/python/user_reqs.lock @@ -21,6 +21,7 @@ // "ijson==3.1.4", // "importlib_resources==5.0.*", // "mypy-typing-asserts==0.1.1", +// "node-semver==0.9.0", // "packaging==21.3", // "pex==2.1.129", // "psutil==5.9.0", @@ -1048,6 +1049,26 @@ "requires_python": "<4.0,>=3.7", "version": "0.1.1" }, + { + "artifacts": [ + { + "algorithm": "sha256", + "hash": "8153270903772b1e59500ced6f0aca0f7bdb021651c27584e9283b7077b4916b", + "url": "https://files.pythonhosted.org/packages/1a/4b/180481021692a76dc91f46fa6a49cdef4c3e630c77a83b7fda3f4eb7aa04/node_semver-0.9.0-py3-none-any.whl" + }, + { + "algorithm": "sha256", + "hash": "04aa0b0016dbc06748d6378c42d8cf82a343415bd9fca6284f488041d08b33bb", + "url": "https://files.pythonhosted.org/packages/eb/c5/e823658f716b17ab1c52d68ed13a0e09c0130af052401a26b5738e4290cc/node-semver-0.9.0.tar.gz" + } + ], + "project_name": "node-semver", + "requires_dists": [ + "pytest; extra == \"testing\"" + ], + "requires_python": null, + "version": "0.9.0" + }, { "artifacts": [ { @@ -2787,6 +2808,7 @@ "ijson==3.1.4", "importlib_resources==5.0.*", "mypy-typing-asserts==0.1.1", + "node-semver==0.9.0", "packaging==21.3", "pex==2.1.129", "psutil==5.9.0", From 1609ad7a34e6dfef0991892429f2e90baf792725 Mon Sep 17 00:00:00 2001 From: Tobias Nilsson Date: Mon, 6 Mar 2023 22:00:45 +0100 Subject: [PATCH 03/17] refactor: Extract mixins for options for reuse --- .../pants/core/util_rules/external_tool.py | 127 ++++++++++-------- 1 file changed, 68 insertions(+), 59 deletions(-) diff --git a/src/python/pants/core/util_rules/external_tool.py b/src/python/pants/core/util_rules/external_tool.py index fde9dbb94f8..c52760b9efa 100644 --- a/src/python/pants/core/util_rules/external_tool.py +++ b/src/python/pants/core/util_rules/external_tool.py @@ -75,51 +75,8 @@ def decode(cls, version_str: str) -> ExternalToolVersion: return cls(version, platform, sha256, int(filesize)) -class ExternalTool(Subsystem, metaclass=ABCMeta): - """Configuration for an invocable tool that we download from an external source. - - Subclass this to configure a specific tool. - - - Idiomatic use: - - class MyExternalTool(ExternalTool): - options_scope = "my-external-tool" - default_version = "1.2.3" - default_known_versions = [ - "1.2.3|linux_arm64 |feed6789feed6789feed6789feed6789feed6789feed6789feed6789feed6789|112233", - "1.2.3|linux_x86_64|cafebabacafebabacafebabacafebabacafebabacafebabacafebabacafebaba|878986", - "1.2.3|macos_arm64 |deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef|222222", - "1.2.3|macos_x86_64|1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd|333333", - ] - - version_constraints = ">=1.2.3, <2.0" - - def generate_url(self, plat: Platform) -> str: - ... - - def generate_exe(self, plat: Platform) -> str: - return "./path-to/binary - - @rule - def my_rule(my_external_tool: MyExternalTool, platform: Platform) -> Foo: - downloaded_tool = await Get( - DownloadedExternalTool, - ExternalToolRequest, - my_external_tool.get_request(platform) - ) - ... - """ - - # The default values for --version and --known-versions, and the supported versions. - # Subclasses must set appropriately. - default_version: str - default_known_versions: list[str] - version_constraints: str | None = None - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.check_version_constraints() +class ExternalToolOptionsMixin: + """Common options for implementing subsystem providing an `ExternalToolRequest`.""" @classproperty def name(cls): @@ -129,6 +86,12 @@ def name(cls): """ return cls.__name__.lower() + # The default values for --version and --known-versions, and the supported versions. + # Subclasses must set appropriately. + default_version: str + default_known_versions: list[str] + version_constraints: str | None = None + version = StrOption( default=lambda cls: cls.default_version, advanced=True, @@ -165,6 +128,47 @@ def name(cls): ), ) + +class ExternalTool(Subsystem, ExternalToolOptionsMixin, metaclass=ABCMeta): + """Configuration for an invocable tool that we download from an external source. + + Subclass this to configure a specific tool. + + + Idiomatic use: + + class MyExternalTool(ExternalTool): + options_scope = "my-external-tool" + default_version = "1.2.3" + default_known_versions = [ + "1.2.3|linux_arm64 |feed6789feed6789feed6789feed6789feed6789feed6789feed6789feed6789|112233", + "1.2.3|linux_x86_64|cafebabacafebabacafebabacafebabacafebabacafebabacafebabacafebaba|878986", + "1.2.3|macos_arm64 |deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef|222222", + "1.2.3|macos_x86_64|1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd|333333", + ] + + version_constraints = ">=1.2.3, <2.0" + + def generate_url(self, plat: Platform) -> str: + ... + + def generate_exe(self, plat: Platform) -> str: + return "./path-to/binary + + @rule + def my_rule(my_external_tool: MyExternalTool, platform: Platform) -> Foo: + downloaded_tool = await Get( + DownloadedExternalTool, + ExternalToolRequest, + my_external_tool.get_request(platform) + ) + ... + """ + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.check_version_constraints() + use_unsupported_version = EnumOption( advanced=True, help=lambda cls: textwrap.dedent( @@ -275,20 +279,9 @@ def check_version_constraints(self) -> None: raise UnsupportedVersion(" ".join(msg)) -class TemplatedExternalTool(ExternalTool): - """Extends the ExternalTool to allow url templating for custom/self-hosted source. - - In addition to ExternalTool functionalities, it is needed to set, e.g.: - - default_url_template = "https://tool.url/{version}/{platform}-mytool.zip" - default_url_platform_mapping = { - "macos_x86_64": "osx_intel", - "macos_arm64": "osx_arm", - "linux_x86_64": "linux", - } - - The platform mapping dict is optional. - """ +class TemplatedExternalToolOptionsMixin(ExternalToolOptionsMixin): + """Common options for implementing a subsystem providing an `ExternalToolRequest` via a URL + template.""" default_url_template: str default_url_platform_mapping: dict[str, str] | None = None @@ -331,6 +324,22 @@ class TemplatedExternalTool(ExternalTool): ), ) + +class TemplatedExternalTool(ExternalTool, TemplatedExternalToolOptionsMixin): + """Extends the ExternalTool to allow url templating for custom/self-hosted source. + + In addition to ExternalTool functionalities, it is needed to set, e.g.: + + default_url_template = "https://tool.url/{version}/{platform}-mytool.zip" + default_url_platform_mapping = { + "macos_x86_64": "osx_intel", + "macos_arm64": "osx_arm", + "linux_x86_64": "linux", + } + + The platform mapping dict is optional. + """ + def generate_url(self, plat: Platform): platform = self.url_platform_mapping.get(plat.value, "") return self.url_template.format(version=self.version, platform=platform) From b32992ba4d5c46240b7d379fd33e0023b9bbe7fb Mon Sep 17 00:00:00 2001 From: Tobias Nilsson Date: Mon, 6 Mar 2023 21:20:05 +0100 Subject: [PATCH 04/17] feat: Support semver version spec in nodejs subsystem --- .../backend/javascript/subsystems/nodejs.py | 116 +++++++++++++----- .../javascript/subsystems/nodejs_test.py | 69 ++++++++++- 2 files changed, 153 insertions(+), 32 deletions(-) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs.py b/src/python/pants/backend/javascript/subsystems/nodejs.py index d93477484c1..e2d4cf33c5c 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs.py @@ -5,27 +5,34 @@ import os.path from dataclasses import dataclass, field +from itertools import groupby from typing import ClassVar, Iterable, Mapping +from nodesemver import min_satisfying + from pants.core.util_rules.external_tool import ( DownloadedExternalTool, ExternalToolRequest, - TemplatedExternalTool, + ExternalToolVersion, + TemplatedExternalToolOptionsMixin, ) from pants.core.util_rules.external_tool import rules as external_tool_rules -from pants.engine.fs import EMPTY_DIGEST, Digest +from pants.core.util_rules.system_binaries import BinaryNotFoundError +from pants.engine.fs import EMPTY_DIGEST, Digest, DownloadFile +from pants.engine.internals.native_engine import FileDigest from pants.engine.platform import Platform from pants.engine.process import Process from pants.engine.rules import Get, Rule, collect_rules, rule from pants.engine.unions import UnionRule from pants.option.option_types import DictOption +from pants.option.subsystem import Subsystem from pants.util.docutil import bin_name from pants.util.frozendict import FrozenDict from pants.util.logging import LogLevel from pants.util.strutil import softwrap -class NodeJS(TemplatedExternalTool): +class NodeJS(Subsystem, TemplatedExternalToolOptionsMixin): options_scope = "nodejs" help = "The NodeJS Javascript runtime (including npm and npx)." @@ -68,16 +75,25 @@ class NodeJS(TemplatedExternalTool): advanced=True, ) - def generate_url(self, plat: Platform) -> str: + def generate_url(self, version: str, plat: Platform) -> str: """NodeJS binaries are compressed as .gz for Mac, .xz for Linux.""" - url = super().generate_url(plat) + platform = self.url_platform_mapping.get(plat.value, "") + url = self.url_template.format(version=version, platform=platform) extension = "gz" if plat.is_macos else "xz" return f"{url}.{extension}" - def generate_exe(self, plat: Platform) -> str: + def generate_exe(self, version: str, plat: Platform) -> str: assert self.default_url_platform_mapping is not None plat_str = self.default_url_platform_mapping[plat.value] - return f"./node-{self.version}-{plat_str}/bin/node" + return f"./node-{version}-{plat_str}/bin/node" + + async def download_known_version( + self, known_version: ExternalToolVersion, platform: Platform + ) -> DownloadedExternalTool: + exe = self.generate_exe(known_version.version, platform) + url = self.generate_url(known_version.version, platform) + download_file = DownloadFile(url, FileDigest(known_version.sha256, known_version.filesize)) + return await Get(DownloadedExternalTool, ExternalToolRequest(download_file, exe)) class UserChosenNodeJSResolveAliases(FrozenDict[str, str]): @@ -144,9 +160,15 @@ def npx( ) +@dataclass(frozen=True) +class NodejsBinaries: + binary_dir: str + digest: Digest | None = None + + @dataclass(frozen=True) class NodeJSProcessEnvironment: - binary_directory: str + binaries: NodejsBinaries npm_config_cache: str base_bin_dir: ClassVar[str] = "__node" @@ -161,46 +183,78 @@ def to_env_dict(self) -> dict[str, str]: def append_only_caches(self) -> Mapping[str, str]: return {"npm": self.npm_config_cache} + @property + def binary_directory(self) -> str: + return self.binaries.binary_dir -@rule(level=LogLevel.DEBUG) -async def node_process_environment(nodejs: NodeJS, platform: Platform) -> NodeJSProcessEnvironment: - # Get reference to tool - assert nodejs.default_url_platform_mapping is not None - plat_str = nodejs.default_url_platform_mapping[platform.value] - nodejs_bin_dir = os.path.join( - "{chroot}", - NodeJSProcessEnvironment.base_bin_dir, - f"node-{nodejs.version}-{plat_str}", - "bin", - ) + def immutable_digest(self) -> dict[str, Digest]: + return {self.base_bin_dir: self.binaries.digest} if self.binaries.digest else {} - return NodeJSProcessEnvironment(binary_directory=nodejs_bin_dir, npm_config_cache="._npm") + +@rule(level=LogLevel.DEBUG) +async def node_process_environment(binaries: NodejsBinaries) -> NodeJSProcessEnvironment: + return NodeJSProcessEnvironment(binaries=binaries, npm_config_cache="._npm") @dataclass(frozen=True) -class NodejsBinaries: - digest: Digest +class NodeJsBootstrap: + nodejs_search_paths: tuple[str, ...] -@rule(level=LogLevel.INFO, desc="Determining nodejs binaries.") -async def determine_nodejs_binaries(nodejs: NodeJS, platform: Platform) -> NodejsBinaries: - downloaded = await Get( - DownloadedExternalTool, ExternalToolRequest, nodejs.get_request(platform) +@rule +async def nodejs_bootstrap() -> NodeJsBootstrap: + return NodeJsBootstrap(nodejs_search_paths=()) + + +@rule(level=LogLevel.INFO, desc="Finding Node.js distribution binaries.") +async def determine_nodejs_binaries( + nodejs: NodeJS, bootstrap: NodeJsBootstrap, platform: Platform +) -> NodejsBinaries: + decoded_versions = groupby( + (ExternalToolVersion.decode(unparsed) for unparsed in nodejs.known_versions), + lambda version: version.version, + ) + + decoded_per_version = { + version: tuple( + known_version + for known_version in known_versions + if known_version.platform == platform.value + ) + for version, known_versions in decoded_versions + } + + satisfying_version = min_satisfying(decoded_per_version.keys(), nodejs.version) + if not satisfying_version: + raise BinaryNotFoundError( + softwrap( + f""" + Cannot find any `node` binaries satisfying the range '{nodejs.version}'. + + To fix, add a `[nodejs].url_platform_mapping` version that satisfies the range. + """ + ) + ) + + known_version = decoded_per_version[satisfying_version][0] + downloaded = await nodejs.download_known_version(known_version, platform) + nodejs_bin_dir = os.path.join( + "{chroot}", + NodeJSProcessEnvironment.base_bin_dir, + os.path.dirname(downloaded.exe), ) - return NodejsBinaries(downloaded.digest) + return NodejsBinaries(nodejs_bin_dir, downloaded.digest) @rule(level=LogLevel.DEBUG) async def setup_node_tool_process( - request: NodeJSToolProcess, binaries: NodejsBinaries, environment: NodeJSProcessEnvironment + request: NodeJSToolProcess, environment: NodeJSProcessEnvironment ) -> Process: - immutable_input_digests = {environment.base_bin_dir: binaries.digest} - return Process( argv=filter(None, request.args), input_digest=request.input_digest, output_files=request.output_files, - immutable_input_digests=immutable_input_digests, + immutable_input_digests=environment.immutable_digest(), output_directories=request.output_directories, description=request.description, level=request.level, diff --git a/src/python/pants/backend/javascript/subsystems/nodejs_test.py b/src/python/pants/backend/javascript/subsystems/nodejs_test.py index 758fbca7a01..079244d6971 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs_test.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs_test.py @@ -3,14 +3,28 @@ from __future__ import annotations +from unittest.mock import Mock + import pytest from pants.backend.javascript.subsystems import nodejs +from pants.backend.javascript.subsystems.nodejs import ( + NodeJS, + NodeJsBootstrap, + determine_nodejs_binaries, +) from pants.backend.javascript.target_types import JSSourcesGeneratorTarget from pants.backend.python import target_types_rules from pants.core.util_rules import config_files, source_files +from pants.core.util_rules.external_tool import ( + DownloadedExternalTool, + ExternalToolRequest, + ExternalToolVersion, +) +from pants.engine.internals.native_engine import EMPTY_DIGEST +from pants.engine.platform import Platform from pants.engine.process import ProcessResult -from pants.testutil.rule_runner import QueryRule, RuleRunner +from pants.testutil.rule_runner import MockGet, QueryRule, RuleRunner, run_rule_with_mocks @pytest.fixture @@ -54,3 +68,56 @@ def test_npm_process(rule_runner: RuleRunner): ) assert result.stdout.strip() == b"8.5.5" + + +def given_known_version(version: str) -> str: + return f"{version}|linux_x86_64|1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd|333333" + + +_SEMVER_1_1_0 = given_known_version("1.1.0") +_SEMVER_2_1_0 = given_known_version("2.1.0") +_SEMVER_2_2_0 = given_known_version("2.2.0") +_SEMVER_2_2_2 = given_known_version("2.2.2") +_SEMVER_3_0_0 = given_known_version("3.0.0") + + +@pytest.mark.parametrize( + ("semver_range", "expected"), + [ + pytest.param("1.x", _SEMVER_1_1_0, id="x_range"), + pytest.param("2.0 - 3.0", _SEMVER_2_1_0, id="hyphen"), + pytest.param(">2.2.0", _SEMVER_2_2_2, id="gt"), + pytest.param("2.2.x", _SEMVER_2_2_0, id="x_range_patch"), + pytest.param("~2.2.0", _SEMVER_2_2_0, id="thilde"), + pytest.param("^2.2.0", _SEMVER_2_2_0, id="caret"), + pytest.param("3.0.0", _SEMVER_3_0_0, id="exact"), + pytest.param("=3.0.0", _SEMVER_3_0_0, id="exact_equals"), + pytest.param("<3.0.0 >2.1", _SEMVER_2_2_0, id="and_range"), + pytest.param(">2.1 || <2.1", _SEMVER_1_1_0, id="or_range"), + ], +) +def test_node_version_from_semver(semver_range: str, expected: str): + nodejs_subsystem = Mock(spec_set=NodeJS) + nodejs_subsystem.version = semver_range + nodejs_subsystem.known_versions = [ + _SEMVER_1_1_0, + _SEMVER_2_1_0, + _SEMVER_2_2_0, + _SEMVER_2_2_2, + _SEMVER_3_0_0, + ] + run_rule_with_mocks( + determine_nodejs_binaries, + rule_args=(nodejs_subsystem, NodeJsBootstrap(()), Platform.linux_x86_64), + mock_gets=[ + MockGet( + DownloadedExternalTool, + (ExternalToolRequest,), + mock=lambda *_: DownloadedExternalTool(EMPTY_DIGEST, "myexe"), + ) + ], + ) + + nodejs_subsystem.download_known_version.assert_called_once_with( + ExternalToolVersion.decode(expected), Platform.linux_x86_64 + ) From aa30b17789da2af39de690a357becd711055163c Mon Sep 17 00:00:00 2001 From: Tobias Nilsson Date: Mon, 6 Mar 2023 22:37:01 +0100 Subject: [PATCH 05/17] chore: Ignore node-semver having no stubs --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 5074859c561..3dc896c1d63 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -65,6 +65,7 @@ module = [ "hdrh", "hdrh.histogram", "ijson.*", + "nodesemver", "pex.*", "psutil", ] From 9bc21599ce93b6cf4e6c1e7f052e2120d74f33b1 Mon Sep 17 00:00:00 2001 From: Tobias Nilsson Date: Mon, 6 Mar 2023 23:48:12 +0100 Subject: [PATCH 06/17] feat: Support finding nodejs Either on PATH or via asdf installations. --- .../backend/javascript/subsystems/nodejs.py | 131 ++++++++++++++++-- .../javascript/subsystems/nodejs_test.py | 110 ++++++++++++++- 2 files changed, 228 insertions(+), 13 deletions(-) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs.py b/src/python/pants/backend/javascript/subsystems/nodejs.py index e2d4cf33c5c..10ce1377c42 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs.py @@ -6,10 +6,12 @@ import os.path from dataclasses import dataclass, field from itertools import groupby -from typing import ClassVar, Iterable, Mapping +from typing import ClassVar, Iterable, Mapping, Collection from nodesemver import min_satisfying +from pants.core.util_rules import asdf, search_paths, system_binaries +from pants.core.util_rules.asdf import AsdfPathString, AsdfToolPathsResult from pants.core.util_rules.external_tool import ( DownloadedExternalTool, ExternalToolRequest, @@ -17,24 +19,33 @@ TemplatedExternalToolOptionsMixin, ) from pants.core.util_rules.external_tool import rules as external_tool_rules -from pants.core.util_rules.system_binaries import BinaryNotFoundError +from pants.core.util_rules.search_paths import ValidatedSearchPaths, ValidateSearchPathsRequest +from pants.core.util_rules.system_binaries import ( + BinaryNotFoundError, + BinaryPath, + BinaryPathRequest, + BinaryPaths, + BinaryPathTest, +) +from pants.engine.env_vars import PathEnvironmentVariable from pants.engine.fs import EMPTY_DIGEST, Digest, DownloadFile from pants.engine.internals.native_engine import FileDigest from pants.engine.platform import Platform from pants.engine.process import Process from pants.engine.rules import Get, Rule, collect_rules, rule from pants.engine.unions import UnionRule -from pants.option.option_types import DictOption +from pants.option.option_types import DictOption, StrListOption from pants.option.subsystem import Subsystem from pants.util.docutil import bin_name from pants.util.frozendict import FrozenDict from pants.util.logging import LogLevel +from pants.util.ordered_set import FrozenOrderedSet from pants.util.strutil import softwrap class NodeJS(Subsystem, TemplatedExternalToolOptionsMixin): options_scope = "nodejs" - help = "The NodeJS Javascript runtime (including npm and npx)." + help = "The Node.js Javascript runtime (including npm and npx)." default_version = "v16.15.0" default_known_versions = [ @@ -95,6 +106,39 @@ async def download_known_version( download_file = DownloadFile(url, FileDigest(known_version.sha256, known_version.filesize)) return await Get(DownloadedExternalTool, ExternalToolRequest(download_file, exe)) + class EnvironmentAware(Subsystem.EnvironmentAware): + env_vars_used_by_options = ("PATH",) + + search_path = StrListOption( + default=[""], + help=softwrap( + f""" + A list of paths to search for Node.js distributions. + + This option take precedence over the templated url download, + if a version matching the configured version range is found + in these paths. + + You can specify absolute paths to binaries + and/or to directories containing binaries. The order of entries does + not matter. + + The following special strings are supported: + + For all runtime environment types: + + * ``, the contents of the PATH env var + + When the environment is a `local_environment` target: + + * `{AsdfPathString.STANDARD}`, {AsdfPathString.STANDARD.description("Node.js")} + * `{AsdfPathString.LOCAL}`, {AsdfPathString.LOCAL.description("binaries")} + """ + ), + advanced=True, + metavar="", + ) + class UserChosenNodeJSResolveAliases(FrozenDict[str, str]): pass @@ -201,18 +245,83 @@ class NodeJsBootstrap: nodejs_search_paths: tuple[str, ...] +async def _nodejs_search_paths(env_tgt: EnvironmentTarget, paths: Collection[str]) -> tuple[str, ...]: + asdf_result = await AsdfToolPathsResult.get_un_cachable_search_paths( + paths, + env_tgt=env_tgt, + tool_name="nodejs", + tool_description="Node.js distribution", + paths_option_name="[nodejs].search_path", + ) + asdf_standard_tool_paths = asdf_result.standard_tool_paths + asdf_local_tool_paths = asdf_result.local_tool_paths + special_strings: dict[str, Iterable[str]] = { + AsdfPathString.STANDARD: asdf_standard_tool_paths, + AsdfPathString.LOCAL: asdf_local_tool_paths, + } + + expanded: list[str] = [] + for s in paths: + if s == "": + expanded.extend(await Get(PathEnvironmentVariable, {})) # noqa: PNT30: Linear search + elif s in special_strings: + expanded.extend(special_strings[s]) + else: + expanded.append(s) + return tuple(expanded) + + @rule -async def nodejs_bootstrap() -> NodeJsBootstrap: - return NodeJsBootstrap(nodejs_search_paths=()) +async def nodejs_bootstrap(nodejs_env_aware: NodeJS.EnvironmentAware) -> NodeJsBootstrap: + search_paths = await Get( + ValidatedSearchPaths, + ValidateSearchPathsRequest( + env_tgt=nodejs_env_aware.env_tgt, + search_paths=tuple(nodejs_env_aware.search_path), + option_origin=f"[{NodeJS.options_scope}].search_path", + environment_key="nodejs_search_path", + is_default=nodejs_env_aware._is_default("search_path"), + local_only=FrozenOrderedSet((AsdfPathString.STANDARD, AsdfPathString.LOCAL)), + ), + ) + + expanded_paths = await _nodejs_search_paths(nodejs_env_aware.env_tgt, search_paths) + + return NodeJsBootstrap(nodejs_search_paths=expanded_paths) + + +class _BinaryPathsPerVersion(FrozenDict[str, tuple[BinaryPath, ...]]): + pass + + +@rule(level=LogLevel.DEBUG, desc="Testing for Node.js binaries.") +async def get_valid_nodejs_paths_by_version(bootstrap: NodeJsBootstrap) -> _BinaryPathsPerVersion: + paths = await Get( + BinaryPaths, + BinaryPathRequest( + search_path=bootstrap.nodejs_search_paths, + binary_name="node", + test=BinaryPathTest( + ["--version"], fingerprint_stdout=False + ), # Hack to retain version info + ), + ) + + group_by_version = groupby((path for path in paths.paths), key=lambda path: path.fingerprint) + return _BinaryPathsPerVersion({version: tuple(paths) for version, paths in group_by_version}) @rule(level=LogLevel.INFO, desc="Finding Node.js distribution binaries.") async def determine_nodejs_binaries( - nodejs: NodeJS, bootstrap: NodeJsBootstrap, platform: Platform + nodejs: NodeJS, platform: Platform, paths_per_version: _BinaryPathsPerVersion ) -> NodejsBinaries: + satisfying_version = min_satisfying(paths_per_version.keys(), nodejs.version) + if satisfying_version: + return NodejsBinaries(os.path.dirname(paths_per_version[satisfying_version][0].path)) + decoded_versions = groupby( (ExternalToolVersion.decode(unparsed) for unparsed in nodejs.known_versions), - lambda version: version.version, + lambda v: v.version, ) decoded_per_version = { @@ -231,7 +340,8 @@ async def determine_nodejs_binaries( f""" Cannot find any `node` binaries satisfying the range '{nodejs.version}'. - To fix, add a `[nodejs].url_platform_mapping` version that satisfies the range. + To fix, either list a `[{NodeJS.options_scope}].url_platform_mapping` version that satisfies the range, + or ensure `[{NodeJS.options_scope}].search_path` contains a path to binaries that satisfy the range. """ ) ) @@ -268,4 +378,7 @@ def rules() -> Iterable[Rule | UnionRule]: return ( *collect_rules(), *external_tool_rules(), + *asdf.rules(), + *system_binaries.rules(), + *search_paths.rules(), ) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs_test.py b/src/python/pants/backend/javascript/subsystems/nodejs_test.py index 079244d6971..5d8206d941c 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs_test.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs_test.py @@ -3,6 +3,10 @@ from __future__ import annotations +import stat +from pathlib import Path +from textwrap import dedent +from typing import NoReturn from unittest.mock import Mock import pytest @@ -10,7 +14,8 @@ from pants.backend.javascript.subsystems import nodejs from pants.backend.javascript.subsystems.nodejs import ( NodeJS, - NodeJsBootstrap, + NodejsBinaries, + _BinaryPathsPerVersion, determine_nodejs_binaries, ) from pants.backend.javascript.target_types import JSSourcesGeneratorTarget @@ -21,10 +26,12 @@ ExternalToolRequest, ExternalToolVersion, ) +from pants.core.util_rules.system_binaries import BinaryNotFoundError, BinaryPath from pants.engine.internals.native_engine import EMPTY_DIGEST from pants.engine.platform import Platform from pants.engine.process import ProcessResult from pants.testutil.rule_runner import MockGet, QueryRule, RuleRunner, run_rule_with_mocks +from pants.util.contextutil import temporary_dir @pytest.fixture @@ -36,6 +43,7 @@ def rule_runner() -> RuleRunner: *config_files.rules(), *target_types_rules.rules(), QueryRule(ProcessResult, [nodejs.NodeJSToolProcess]), + QueryRule(NodejsBinaries, ()), ], target_types=[JSSourcesGeneratorTarget], ) @@ -96,7 +104,7 @@ def given_known_version(version: str) -> str: pytest.param(">2.1 || <2.1", _SEMVER_1_1_0, id="or_range"), ], ) -def test_node_version_from_semver(semver_range: str, expected: str): +def test_node_version_from_semver_download(semver_range: str, expected: str) -> None: nodejs_subsystem = Mock(spec_set=NodeJS) nodejs_subsystem.version = semver_range nodejs_subsystem.known_versions = [ @@ -108,16 +116,110 @@ def test_node_version_from_semver(semver_range: str, expected: str): ] run_rule_with_mocks( determine_nodejs_binaries, - rule_args=(nodejs_subsystem, NodeJsBootstrap(()), Platform.linux_x86_64), + rule_args=(nodejs_subsystem, Platform.linux_x86_64, _BinaryPathsPerVersion()), mock_gets=[ MockGet( DownloadedExternalTool, (ExternalToolRequest,), mock=lambda *_: DownloadedExternalTool(EMPTY_DIGEST, "myexe"), - ) + ), ], ) nodejs_subsystem.download_known_version.assert_called_once_with( ExternalToolVersion.decode(expected), Platform.linux_x86_64 ) + + +@pytest.mark.parametrize( + ("semver_range", "expected_path"), + [ + pytest.param("1.x", "1/1/0", id="x_range"), + pytest.param("2.0 - 3.0", "2/1/0", id="hyphen"), + pytest.param(">2.2.0", "2/2/2", id="gt"), + pytest.param("2.2.x", "2/2/0", id="x_range_patch"), + pytest.param("~2.2.0", "2/2/0", id="thilde"), + pytest.param("^2.2.0", "2/2/0", id="caret"), + pytest.param("3.0.0", "3/0/0", id="exact"), + pytest.param("=3.0.0", "3/0/0", id="exact_equals"), + pytest.param("<3.0.0 >2.1", "2/2/0", id="and_range"), + pytest.param(">2.1 || <2.1", "1/1/0", id="or_range"), + ], +) +def test_node_version_from_semver_bootstrap(semver_range: str, expected_path: str) -> None: + nodejs_subsystem = Mock(spec_set=NodeJS) + nodejs_subsystem.version = semver_range + discoverable_versions = _BinaryPathsPerVersion( + { + "1.1.0": (BinaryPath("1/1/0/node"),), + "2.1.0": (BinaryPath("2/1/0/node"),), + "2.2.0": (BinaryPath("2/2/0/node"),), + "2.2.2": (BinaryPath("2/2/2/node"),), + "3.0.0": (BinaryPath("3/0/0/node"),), + } + ) + + def mock_download(*_) -> NoReturn: + raise AssertionError("Should not run.") + + result = run_rule_with_mocks( + determine_nodejs_binaries, + rule_args=(nodejs_subsystem, Platform.linux_x86_64, discoverable_versions), + mock_gets=[ + MockGet(DownloadedExternalTool, (ExternalToolRequest,), mock=mock_download), + ], + ) + + assert result.binary_dir == expected_path + + +def test_finding_no_node_version_is_an_error() -> None: + nodejs_subsystem = Mock(spec_set=NodeJS) + nodejs_subsystem.version = "*" + nodejs_subsystem.known_versions = [] + discoverable_versions = _BinaryPathsPerVersion() + + def mock_download(*_) -> DownloadedExternalTool: + return DownloadedExternalTool(EMPTY_DIGEST, "myexe") + + with pytest.raises(BinaryNotFoundError): + run_rule_with_mocks( + determine_nodejs_binaries, + rule_args=(nodejs_subsystem, Platform.linux_x86_64, discoverable_versions), + mock_gets=[ + MockGet(DownloadedExternalTool, (ExternalToolRequest,), mock=mock_download), + ], + ) + + +def mock_nodejs(version: str) -> str: + """Return a bash script that emulates `node --version`.""" + return dedent( + f"""\ + #!/bin/bash + + if [[ "$1" == '--version' ]]; then + echo '{version}' + fi + """ + ) + + +def test_find_valid_binary(rule_runner: RuleRunner) -> None: + mock_binary = mock_nodejs("v3.0.0") + with temporary_dir() as tmpdir: + binary_dir = Path(tmpdir) / "bin" + binary_dir.mkdir() + binary_path = binary_dir / "node" + binary_path.write_text(mock_binary) + binary_path.chmod(binary_path.stat().st_mode | stat.S_IEXEC) + + rule_runner.set_options( + [ + f"--nodejs-search-path=['{binary_dir}']", + "--nodejs-version=>2", + ], + env_inherit={"PATH"}, + ) + result = rule_runner.request(NodejsBinaries, ()) + assert result.binary_dir == str(binary_dir) From bdb7df12e158082f0df90d67fddbc300519ec7e9 Mon Sep 17 00:00:00 2001 From: Tobias Nilsson Date: Thu, 16 Mar 2023 22:21:21 +0100 Subject: [PATCH 07/17] feat: support nvm for binary discovery --- .../backend/javascript/subsystems/nodejs.py | 95 ++++++++++++++++++- .../javascript/subsystems/nodejs_test.py | 77 ++++++++++++++- 2 files changed, 167 insertions(+), 5 deletions(-) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs.py b/src/python/pants/backend/javascript/subsystems/nodejs.py index 10ce1377c42..808e3d9f211 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs.py @@ -3,15 +3,20 @@ from __future__ import annotations +import itertools +import logging import os.path from dataclasses import dataclass, field from itertools import groupby -from typing import ClassVar, Iterable, Mapping, Collection +from pathlib import Path +from typing import ClassVar, Collection, Iterable, Mapping from nodesemver import min_satisfying +from pants.base.build_environment import get_buildroot from pants.core.util_rules import asdf, search_paths, system_binaries from pants.core.util_rules.asdf import AsdfPathString, AsdfToolPathsResult +from pants.core.util_rules.environments import EnvironmentTarget, LocalEnvironmentTarget from pants.core.util_rules.external_tool import ( DownloadedExternalTool, ExternalToolRequest, @@ -27,12 +32,14 @@ BinaryPaths, BinaryPathTest, ) -from pants.engine.env_vars import PathEnvironmentVariable +from pants.engine.collection import DeduplicatedCollection +from pants.engine.env_vars import EnvironmentVars, EnvironmentVarsRequest, PathEnvironmentVariable from pants.engine.fs import EMPTY_DIGEST, Digest, DownloadFile from pants.engine.internals.native_engine import FileDigest +from pants.engine.internals.selectors import MultiGet from pants.engine.platform import Platform from pants.engine.process import Process -from pants.engine.rules import Get, Rule, collect_rules, rule +from pants.engine.rules import Get, Rule, _uncacheable_rule, collect_rules, rule from pants.engine.unions import UnionRule from pants.option.option_types import DictOption, StrListOption from pants.option.subsystem import Subsystem @@ -42,6 +49,8 @@ from pants.util.ordered_set import FrozenOrderedSet from pants.util.strutil import softwrap +_logger = logging.getLogger(__name__) + class NodeJS(Subsystem, TemplatedExternalToolOptionsMixin): options_scope = "nodejs" @@ -133,6 +142,8 @@ class EnvironmentAware(Subsystem.EnvironmentAware): * `{AsdfPathString.STANDARD}`, {AsdfPathString.STANDARD.description("Node.js")} * `{AsdfPathString.LOCAL}`, {AsdfPathString.LOCAL.description("binaries")} + * ``, all NodeJS versions under $NVM_DIR/versions/node + * ``, the nvm installation with the version in BUILD_ROOT/.nvmrc """ ), advanced=True, @@ -245,7 +256,74 @@ class NodeJsBootstrap: nodejs_search_paths: tuple[str, ...] -async def _nodejs_search_paths(env_tgt: EnvironmentTarget, paths: Collection[str]) -> tuple[str, ...]: +@dataclass(frozen=True) +class _NvmPathsRequest: + env_tgt: EnvironmentTarget + local: bool + + +async def _get_nvm_root() -> str | None: + """See https://github.com/nvm-sh/nvm#installing-and-updating.""" + + env = await Get(EnvironmentVars, EnvironmentVarsRequest(("NVM_DIR", "XDG_CONFIG_HOME", "HOME"))) + nvm_dir = env.get("NVM_DIR") + default_dir = env.get("XDG_CONFIG_HOME", env.get("HOME")) + if nvm_dir: + return nvm_dir + elif default_dir: + return os.path.join(default_dir, ".nvm") + return None + + +class _NvmSearchPaths(DeduplicatedCollection[str]): + pass + + +@_uncacheable_rule +async def _get_nvm_paths(request: _NvmPathsRequest) -> _NvmSearchPaths: + if not (request.env_tgt.val is None or isinstance(request.env_tgt.val, LocalEnvironmentTarget)): + return _NvmSearchPaths() + + nvm_local = request.local + nvm_dir = await _get_nvm_root() + if not nvm_dir: + return _NvmSearchPaths() + nvm_path = Path(nvm_dir) + if not nvm_path.exists(): + return _NvmSearchPaths() + + node_versions_path = nvm_path / "versions" / "node" + if not node_versions_path.is_dir(): + return _NvmSearchPaths() + + if nvm_local: + local_version_file = Path(get_buildroot(), ".nvmrc") + if not local_version_file.exists(): + _logger.warning( + softwrap( + f""" + No `.nvmrc` file found in the build root, + but was set in `[{NodeJS.options_scope}].search_path`. + """ + ) + ) + return _NvmSearchPaths() + + local_version = local_version_file.read_text().strip() + path = Path(node_versions_path, local_version, "bin") + if path.is_dir(): + return _NvmSearchPaths([str(path)]) + return _NvmSearchPaths() + + versions_in_dir = ( + node_versions_path / version / "bin" for version in sorted(node_versions_path.iterdir()) + ) + return _NvmSearchPaths(str(version) for version in versions_in_dir if version.is_dir()) + + +async def _nodejs_search_paths( + env_tgt: EnvironmentTarget, paths: Collection[str] +) -> tuple[str, ...]: asdf_result = await AsdfToolPathsResult.get_un_cachable_search_paths( paths, env_tgt=env_tgt, @@ -261,11 +339,20 @@ async def _nodejs_search_paths(env_tgt: EnvironmentTarget, paths: Collection[str } expanded: list[str] = [] + nvm_path_results = await MultiGet( + Get(_NvmSearchPaths, _NvmPathsRequest(env_tgt, s == "")) + for s in paths + if s == "" or s == "" + ) + for nvm_path in FrozenOrderedSet(itertools.chain.from_iterable(nvm_path_results)): + expanded.append(nvm_path) for s in paths: if s == "": expanded.extend(await Get(PathEnvironmentVariable, {})) # noqa: PNT30: Linear search elif s in special_strings: expanded.extend(special_strings[s]) + elif s == "" or s == "": + continue else: expanded.append(s) return tuple(expanded) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs_test.py b/src/python/pants/backend/javascript/subsystems/nodejs_test.py index 5d8206d941c..ad9db55a9d9 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs_test.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs_test.py @@ -3,10 +3,12 @@ from __future__ import annotations +import os import stat +from contextlib import contextmanager from pathlib import Path from textwrap import dedent -from typing import NoReturn +from typing import Generator, NoReturn from unittest.mock import Mock import pytest @@ -16,17 +18,23 @@ NodeJS, NodejsBinaries, _BinaryPathsPerVersion, + _get_nvm_root, + _NvmPathsRequest, + _NvmSearchPaths, determine_nodejs_binaries, ) from pants.backend.javascript.target_types import JSSourcesGeneratorTarget from pants.backend.python import target_types_rules +from pants.build_graph.address import Address from pants.core.util_rules import config_files, source_files +from pants.core.util_rules.environments import EnvironmentTarget, LocalEnvironmentTarget from pants.core.util_rules.external_tool import ( DownloadedExternalTool, ExternalToolRequest, ExternalToolVersion, ) from pants.core.util_rules.system_binaries import BinaryNotFoundError, BinaryPath +from pants.engine.env_vars import CompleteEnvironmentVars, EnvironmentVars, EnvironmentVarsRequest from pants.engine.internals.native_engine import EMPTY_DIGEST from pants.engine.platform import Platform from pants.engine.process import ProcessResult @@ -44,6 +52,7 @@ def rule_runner() -> RuleRunner: *target_types_rules.rules(), QueryRule(ProcessResult, [nodejs.NodeJSToolProcess]), QueryRule(NodejsBinaries, ()), + QueryRule(_NvmSearchPaths, (_NvmPathsRequest,)), ], target_types=[JSSourcesGeneratorTarget], ) @@ -223,3 +232,69 @@ def test_find_valid_binary(rule_runner: RuleRunner) -> None: ) result = rule_runner.request(NodejsBinaries, ()) assert result.binary_dir == str(binary_dir) + + +@pytest.mark.parametrize( + "env, expected_directory", + [ + pytest.param({"NVM_DIR": "/somewhere/.nvm"}, "/somewhere/.nvm", id="explicit_nvm_dir"), + pytest.param( + {"HOME": "/somewhere-else", "XDG_CONFIG_HOME": "/somewhere"}, + "/somewhere/.nvm", + id="xdg_config_home_set", + ), + pytest.param({"HOME": "/somewhere-else"}, "/somewhere-else/.nvm", id="home_dir_set"), + pytest.param({}, None, id="no_dirs_set"), + ], +) +def test_get_nvm_root(env: dict[str, str], expected_directory: str | None) -> None: + def mock_environment_vars(_req: EnvironmentVarsRequest) -> EnvironmentVars: + return EnvironmentVars(env) + + result = run_rule_with_mocks( + _get_nvm_root, + mock_gets=[MockGet(EnvironmentVars, (EnvironmentVarsRequest,), mock_environment_vars)], + ) + assert result == expected_directory + + +@contextmanager +def fake_nvm_root( + fake_versions: list[str], fake_local_version: str +) -> Generator[tuple[str, tuple[str, ...], tuple[str]], None, None]: + with temporary_dir() as nvm_root: + fake_version_dirs = tuple( + os.path.join(nvm_root, "versions", "node", v, "bin") for v in fake_versions + ) + for d in fake_version_dirs: + os.makedirs(d) + fake_local_version_dirs = ( + os.path.join(nvm_root, "versions", "node", fake_local_version, "bin"), + ) + yield nvm_root, fake_version_dirs, fake_local_version_dirs + + +def test_get_local_nvm_paths(rule_runner: RuleRunner) -> None: + local_nvm_version = "3.5.5" + all_nvm_versions = ["2.7.14", local_nvm_version] + rule_runner.write_files({".nvmrc": f"{local_nvm_version}\n"}) + with fake_nvm_root(all_nvm_versions, local_nvm_version) as ( + nvm_root, + expected_paths, + expected_local_paths, + ): + rule_runner.set_session_values( + {CompleteEnvironmentVars: CompleteEnvironmentVars({"NVM_DIR": nvm_root})} + ) + env_name = "name" + tgt = EnvironmentTarget(env_name, LocalEnvironmentTarget({}, Address("flem"))) + paths = rule_runner.request( + _NvmSearchPaths, + [_NvmPathsRequest(tgt, False)], + ) + local_paths = rule_runner.request( + _NvmSearchPaths, + [_NvmPathsRequest(tgt, True)], + ) + assert set(expected_paths) == set(paths) + assert set(expected_local_paths) == set(local_paths) From b0038bde19902655474b8994fe8a367d4ed32813 Mon Sep 17 00:00:00 2001 From: Tobias Nilsson Date: Thu, 16 Mar 2023 23:59:50 +0100 Subject: [PATCH 08/17] feat: Generalize searching version manager installations --- .../backend/javascript/subsystems/nodejs.py | 82 ++++------------ .../javascript/subsystems/nodejs_test.py | 29 ++++-- .../pants/core/subsystems/python_bootstrap.py | 97 ++++++------------- .../core/subsystems/python_bootstrap_test.py | 59 ++++++++--- .../pants/core/util_rules/search_paths.py | 76 ++++++++++++++- 5 files changed, 192 insertions(+), 151 deletions(-) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs.py b/src/python/pants/backend/javascript/subsystems/nodejs.py index 808e3d9f211..3f58b772a35 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs.py @@ -8,15 +8,13 @@ import os.path from dataclasses import dataclass, field from itertools import groupby -from pathlib import Path from typing import ClassVar, Collection, Iterable, Mapping from nodesemver import min_satisfying -from pants.base.build_environment import get_buildroot from pants.core.util_rules import asdf, search_paths, system_binaries from pants.core.util_rules.asdf import AsdfPathString, AsdfToolPathsResult -from pants.core.util_rules.environments import EnvironmentTarget, LocalEnvironmentTarget +from pants.core.util_rules.environments import EnvironmentTarget from pants.core.util_rules.external_tool import ( DownloadedExternalTool, ExternalToolRequest, @@ -24,7 +22,12 @@ TemplatedExternalToolOptionsMixin, ) from pants.core.util_rules.external_tool import rules as external_tool_rules -from pants.core.util_rules.search_paths import ValidatedSearchPaths, ValidateSearchPathsRequest +from pants.core.util_rules.search_paths import ( + ValidatedSearchPaths, + ValidateSearchPathsRequest, + VersionManagerSearchPaths, + VersionManagerSearchPathsRequest, +) from pants.core.util_rules.system_binaries import ( BinaryNotFoundError, BinaryPath, @@ -32,14 +35,13 @@ BinaryPaths, BinaryPathTest, ) -from pants.engine.collection import DeduplicatedCollection from pants.engine.env_vars import EnvironmentVars, EnvironmentVarsRequest, PathEnvironmentVariable from pants.engine.fs import EMPTY_DIGEST, Digest, DownloadFile from pants.engine.internals.native_engine import FileDigest from pants.engine.internals.selectors import MultiGet from pants.engine.platform import Platform from pants.engine.process import Process -from pants.engine.rules import Get, Rule, _uncacheable_rule, collect_rules, rule +from pants.engine.rules import Get, Rule, collect_rules, rule from pants.engine.unions import UnionRule from pants.option.option_types import DictOption, StrListOption from pants.option.subsystem import Subsystem @@ -256,12 +258,6 @@ class NodeJsBootstrap: nodejs_search_paths: tuple[str, ...] -@dataclass(frozen=True) -class _NvmPathsRequest: - env_tgt: EnvironmentTarget - local: bool - - async def _get_nvm_root() -> str | None: """See https://github.com/nvm-sh/nvm#installing-and-updating.""" @@ -275,52 +271,6 @@ async def _get_nvm_root() -> str | None: return None -class _NvmSearchPaths(DeduplicatedCollection[str]): - pass - - -@_uncacheable_rule -async def _get_nvm_paths(request: _NvmPathsRequest) -> _NvmSearchPaths: - if not (request.env_tgt.val is None or isinstance(request.env_tgt.val, LocalEnvironmentTarget)): - return _NvmSearchPaths() - - nvm_local = request.local - nvm_dir = await _get_nvm_root() - if not nvm_dir: - return _NvmSearchPaths() - nvm_path = Path(nvm_dir) - if not nvm_path.exists(): - return _NvmSearchPaths() - - node_versions_path = nvm_path / "versions" / "node" - if not node_versions_path.is_dir(): - return _NvmSearchPaths() - - if nvm_local: - local_version_file = Path(get_buildroot(), ".nvmrc") - if not local_version_file.exists(): - _logger.warning( - softwrap( - f""" - No `.nvmrc` file found in the build root, - but was set in `[{NodeJS.options_scope}].search_path`. - """ - ) - ) - return _NvmSearchPaths() - - local_version = local_version_file.read_text().strip() - path = Path(node_versions_path, local_version, "bin") - if path.is_dir(): - return _NvmSearchPaths([str(path)]) - return _NvmSearchPaths() - - versions_in_dir = ( - node_versions_path / version / "bin" for version in sorted(node_versions_path.iterdir()) - ) - return _NvmSearchPaths(str(version) for version in versions_in_dir if version.is_dir()) - - async def _nodejs_search_paths( env_tgt: EnvironmentTarget, paths: Collection[str] ) -> tuple[str, ...]: @@ -329,7 +279,7 @@ async def _nodejs_search_paths( env_tgt=env_tgt, tool_name="nodejs", tool_description="Node.js distribution", - paths_option_name="[nodejs].search_path", + paths_option_name=f"[{NodeJS.options_scope}].search_path", ) asdf_standard_tool_paths = asdf_result.standard_tool_paths asdf_local_tool_paths = asdf_result.local_tool_paths @@ -337,10 +287,20 @@ async def _nodejs_search_paths( AsdfPathString.STANDARD: asdf_standard_tool_paths, AsdfPathString.LOCAL: asdf_local_tool_paths, } - + nvm_dir = await _get_nvm_root() expanded: list[str] = [] nvm_path_results = await MultiGet( - Get(_NvmSearchPaths, _NvmPathsRequest(env_tgt, s == "")) + Get( + VersionManagerSearchPaths, + VersionManagerSearchPathsRequest( + env_tgt, + nvm_dir, + "versions/node", + f"[{NodeJS.options_scope}].search_path", + (".nvmrc",), + s if s == "" else None, + ), + ) for s in paths if s == "" or s == "" ) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs_test.py b/src/python/pants/backend/javascript/subsystems/nodejs_test.py index ad9db55a9d9..aa6472abe12 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs_test.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs_test.py @@ -19,8 +19,6 @@ NodejsBinaries, _BinaryPathsPerVersion, _get_nvm_root, - _NvmPathsRequest, - _NvmSearchPaths, determine_nodejs_binaries, ) from pants.backend.javascript.target_types import JSSourcesGeneratorTarget @@ -33,6 +31,10 @@ ExternalToolRequest, ExternalToolVersion, ) +from pants.core.util_rules.search_paths import ( + VersionManagerSearchPaths, + VersionManagerSearchPathsRequest, +) from pants.core.util_rules.system_binaries import BinaryNotFoundError, BinaryPath from pants.engine.env_vars import CompleteEnvironmentVars, EnvironmentVars, EnvironmentVarsRequest from pants.engine.internals.native_engine import EMPTY_DIGEST @@ -52,7 +54,7 @@ def rule_runner() -> RuleRunner: *target_types_rules.rules(), QueryRule(ProcessResult, [nodejs.NodeJSToolProcess]), QueryRule(NodejsBinaries, ()), - QueryRule(_NvmSearchPaths, (_NvmPathsRequest,)), + QueryRule(VersionManagerSearchPaths, (VersionManagerSearchPathsRequest,)), ], target_types=[JSSourcesGeneratorTarget], ) @@ -289,12 +291,25 @@ def test_get_local_nvm_paths(rule_runner: RuleRunner) -> None: env_name = "name" tgt = EnvironmentTarget(env_name, LocalEnvironmentTarget({}, Address("flem"))) paths = rule_runner.request( - _NvmSearchPaths, - [_NvmPathsRequest(tgt, False)], + VersionManagerSearchPaths, + [ + VersionManagerSearchPathsRequest( + tgt, nvm_root, "versions/node", "[nodejs].search_path", (".nvmrc",), None + ) + ], ) local_paths = rule_runner.request( - _NvmSearchPaths, - [_NvmPathsRequest(tgt, True)], + VersionManagerSearchPaths, + [ + VersionManagerSearchPathsRequest( + tgt, + nvm_root, + "versions/node", + "[nodejs].search_path", + (".nvmrc",), + "", + ) + ], ) assert set(expected_paths) == set(paths) assert set(expected_local_paths) == set(local_paths) diff --git a/src/python/pants/core/subsystems/python_bootstrap.py b/src/python/pants/core/subsystems/python_bootstrap.py index 7fa2cffde7b..f94aa9763bf 100644 --- a/src/python/pants/core/subsystems/python_bootstrap.py +++ b/src/python/pants/core/subsystems/python_bootstrap.py @@ -3,21 +3,26 @@ from __future__ import annotations +import itertools import logging import os from dataclasses import dataclass -from pathlib import Path from typing import Collection from pex.variables import Variables -from pants.base.build_environment import get_buildroot from pants.core.util_rules import asdf, search_paths from pants.core.util_rules.asdf import AsdfPathString, AsdfToolPathsResult -from pants.core.util_rules.environments import EnvironmentTarget, LocalEnvironmentTarget -from pants.core.util_rules.search_paths import ValidatedSearchPaths, ValidateSearchPathsRequest +from pants.core.util_rules.environments import EnvironmentTarget +from pants.core.util_rules.search_paths import ( + ValidatedSearchPaths, + ValidateSearchPathsRequest, + VersionManagerSearchPaths, + VersionManagerSearchPathsRequest, +) from pants.engine.env_vars import EnvironmentVars, EnvironmentVarsRequest, PathEnvironmentVariable -from pants.engine.rules import Get, _uncacheable_rule, collect_rules, rule +from pants.engine.internals.selectors import MultiGet +from pants.engine.rules import Get, collect_rules, rule from pants.option.option_types import DictOption, StrListOption from pants.option.subsystem import Subsystem from pants.util.ordered_set import FrozenOrderedSet @@ -143,12 +148,6 @@ class _ExpandInterpreterSearchPathsRequest: env_tgt: EnvironmentTarget -@dataclass(frozen=True) -class _PyEnvPathsRequest: - env_tgt: EnvironmentTarget - pyenv_local: bool = False - - @dataclass(frozen=False) class _SearchPaths: paths: tuple[str, ...] @@ -182,6 +181,26 @@ async def _expand_interpreter_search_paths( expanded: list[str] = [] from_pexrc = None + + pyenv_env = await Get(EnvironmentVars, EnvironmentVarsRequest(("PYENV_ROOT", "HOME"))) + pyenv_root = _get_pyenv_root(pyenv_env) + pyenv_path_results = await MultiGet( + Get( + VersionManagerSearchPaths, + VersionManagerSearchPathsRequest( + env_tgt, + pyenv_root, + "versions", + f"[{PythonBootstrapSubsystem.options_scope}].search_path", + (".python-version",), + s if s == "" else None, + ), + ) + for s in interpreter_search_paths + if s == "" or s == "" + ) + for pyenv_path in FrozenOrderedSet(itertools.chain.from_iterable(pyenv_path_results)): + expanded.append(pyenv_path) for s in interpreter_search_paths: if s in special_strings: special_paths = special_strings[s]() @@ -189,10 +208,7 @@ async def _expand_interpreter_search_paths( from_pexrc = special_paths expanded.extend(special_paths) elif s == "" or s == "": - paths = await Get( # noqa: PNT30: requires triage - _SearchPaths, _PyEnvPathsRequest(env_tgt, s == "") - ) - expanded.extend(paths.paths) + continue else: expanded.append(s) # Some special-case logging to avoid misunderstandings. @@ -223,57 +239,6 @@ def _get_pex_python_paths(): return [] -@_uncacheable_rule -async def _get_pyenv_paths(request: _PyEnvPathsRequest) -> _SearchPaths: - """Returns a tuple of paths to Python interpreters managed by pyenv. - - :param `request.env_tgt`: The environment target -- if not referring to a local/no environment, - this will return an empty path. - :param bool pyenv_local: If True, only use the interpreter specified by - '.python-version' file under `build_root`. - """ - - if not (request.env_tgt.val is None or isinstance(request.env_tgt.val, LocalEnvironmentTarget)): - return _SearchPaths(()) - - pyenv_local = request.pyenv_local - env = await Get(EnvironmentVars, EnvironmentVarsRequest(("PYENV_ROOT", "HOME"))) - - pyenv_root = _get_pyenv_root(env) - if not pyenv_root: - return _SearchPaths(()) - - versions_dir = Path(pyenv_root, "versions") - if not versions_dir.is_dir(): - return _SearchPaths(()) - - if pyenv_local: - local_version_file = Path(get_buildroot(), ".python-version") - if not local_version_file.exists(): - logger.warning( - softwrap( - """ - No `.python-version` file found in the build root, - but was set in `[python-bootstrap].search_path`. - """ - ) - ) - return _SearchPaths(()) - - local_version = local_version_file.read_text().strip() - path = Path(versions_dir, local_version, "bin") - if path.is_dir(): - return _SearchPaths((str(path),)) - return _SearchPaths(()) - - paths = [] - for version in sorted(versions_dir.iterdir()): - path = Path(versions_dir, version, "bin") - if path.is_dir(): - paths.append(str(path)) - return _SearchPaths(tuple(paths)) - - def _get_pyenv_root(env: EnvironmentVars) -> str | None: """See https://github.com/pyenv/pyenv#environment-variables.""" from_env = env.get("PYENV_ROOT") diff --git a/src/python/pants/core/subsystems/python_bootstrap_test.py b/src/python/pants/core/subsystems/python_bootstrap_test.py index d044e8e9d8a..2154a4e7dd9 100644 --- a/src/python/pants/core/subsystems/python_bootstrap_test.py +++ b/src/python/pants/core/subsystems/python_bootstrap_test.py @@ -14,7 +14,6 @@ _ExpandInterpreterSearchPathsRequest, _get_pex_python_paths, _get_pyenv_root, - _PyEnvPathsRequest, _SearchPaths, ) from pants.core.subsystems.python_bootstrap import rules as python_bootstrap_rules @@ -22,6 +21,10 @@ from pants.core.util_rules.asdf import AsdfToolPathsRequest, AsdfToolPathsResult from pants.core.util_rules.asdf_test import fake_asdf_root from pants.core.util_rules.environments import EnvironmentTarget, LocalEnvironmentTarget +from pants.core.util_rules.search_paths import ( + VersionManagerSearchPaths, + VersionManagerSearchPathsRequest, +) from pants.engine.addresses import Address from pants.engine.env_vars import CompleteEnvironmentVars, EnvironmentVars from pants.engine.rules import QueryRule @@ -39,8 +42,8 @@ def rule_runner() -> RuleRunner: *python_bootstrap_rules(), *asdf.rules(), QueryRule(AsdfToolPathsResult, (AsdfToolPathsRequest,)), - QueryRule(_SearchPaths, [_PyEnvPathsRequest]), QueryRule(_SearchPaths, [_ExpandInterpreterSearchPathsRequest]), + QueryRule(VersionManagerSearchPaths, (VersionManagerSearchPathsRequest,)), ], target_types=[], ) @@ -93,14 +96,20 @@ def test_get_pex_python_paths() -> None: assert ["foo/bar", "baz", "/qux/quux"] == paths -def test_get_pyenv_root() -> None: - home = "/♡" - default_root = f"{home}/.pyenv" - explicit_root = f"{home}/explicit" +_HOME = "/♡" + - assert explicit_root == _get_pyenv_root(EnvironmentVars({"PYENV_ROOT": explicit_root})) - assert default_root == _get_pyenv_root(EnvironmentVars({"HOME": home})) - assert _get_pyenv_root(EnvironmentVars({})) is None +@pytest.mark.parametrize( + "env, expected", + [ + pytest.param({"PYENV_ROOT": f"{_HOME}/explicit"}, f"{_HOME}/explicit", id="explicit_root"), + pytest.param({"HOME": _HOME}, f"{_HOME}/.pyenv", id="default_root"), + pytest.param({}, None, id="no_env"), + ], +) +def test_get_pyenv_root(env: dict[str, str], expected: str | None) -> None: + result = _get_pyenv_root(EnvironmentVars(env)) + assert result == expected def test_get_pyenv_paths(rule_runner: RuleRunner) -> None: @@ -118,15 +127,33 @@ def test_get_pyenv_paths(rule_runner: RuleRunner) -> None: env_name = "name" tgt = EnvironmentTarget(env_name, LocalEnvironmentTarget({}, Address("flem"))) paths = rule_runner.request( - _SearchPaths, - [_PyEnvPathsRequest(tgt, False)], + VersionManagerSearchPaths, + [ + VersionManagerSearchPathsRequest( + tgt, + pyenv_root, + "versions", + "[python-bootstrap].search_path", + (".python-version",), + None, + ) + ], ) local_paths = rule_runner.request( - _SearchPaths, - [_PyEnvPathsRequest(tgt, True)], + VersionManagerSearchPaths, + [ + VersionManagerSearchPathsRequest( + tgt, + pyenv_root, + "versions", + "[python-bootstrap].search_path", + (".python-version",), + "", + ) + ], ) - assert expected_paths == paths.paths - assert expected_local_paths == local_paths.paths + assert set(expected_paths) == set(paths) + assert set(expected_local_paths) == set(local_paths) def test_expand_interpreter_search_paths(rule_runner: RuleRunner) -> None: @@ -216,4 +243,4 @@ def test_expand_interpreter_search_paths(rule_runner: RuleRunner) -> None: *expected_pyenv_local_paths, "/qux", ) - assert expected == expanded_paths.paths + assert set(expected) == set(expanded_paths.paths) diff --git a/src/python/pants/core/util_rules/search_paths.py b/src/python/pants/core/util_rules/search_paths.py index e6eacd88e41..c90f9ad15f2 100644 --- a/src/python/pants/core/util_rules/search_paths.py +++ b/src/python/pants/core/util_rules/search_paths.py @@ -2,15 +2,21 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). from __future__ import annotations +import logging from dataclasses import dataclass +from pathlib import Path from typing import Iterable +from pants.base.build_environment import get_buildroot from pants.core.util_rules.environments import EnvironmentTarget, LocalEnvironmentTarget -from pants.engine.rules import Rule, collect_rules, rule +from pants.engine.collection import DeduplicatedCollection +from pants.engine.rules import Rule, _uncacheable_rule, collect_rules, rule from pants.util.logging import LogLevel from pants.util.ordered_set import FrozenOrderedSet from pants.util.strutil import softwrap +_logger = logging.getLogger(__name__) + @dataclass(frozen=True) class ValidateSearchPathsRequest: @@ -22,6 +28,74 @@ class ValidateSearchPathsRequest: local_only: FrozenOrderedSet[str] +@dataclass(frozen=True) +class VersionManagerSearchPathsRequest: + env_tgt: EnvironmentTarget + root_dir: str | None + tool_path: str + option: str + version_files: tuple[str, ...] = tuple() + local_token: str | None = None + + +class VersionManagerSearchPaths(DeduplicatedCollection[str]): + pass + + +@_uncacheable_rule +async def get_un_cachable_version_manager_paths( + request: VersionManagerSearchPathsRequest, +) -> VersionManagerSearchPaths: + """Inspects the directory of a version manager tool to find installations.""" + if not (request.env_tgt.val is None or isinstance(request.env_tgt.val, LocalEnvironmentTarget)): + return VersionManagerSearchPaths() + + manager_root_dir = request.root_dir + if not manager_root_dir: + return VersionManagerSearchPaths() + root_path = Path(manager_root_dir) + if not root_path.exists(): + return VersionManagerSearchPaths() + + tool_versions_path = root_path / request.tool_path + if not tool_versions_path.is_dir(): + return VersionManagerSearchPaths() + + if request.local_token and request.version_files: + local_version_files = [Path(get_buildroot(), file) for file in request.version_files] + first_version_file = next((file for file in local_version_files if file.exists()), None) + if not first_version_file: + file_string = ", ".join(f"`{file}`" for file in local_version_files) + no_file = ( + f"No {file_string}" if len(local_version_files) == 1 else f"None of {file_string}" + ) + _logger.warning( + softwrap( + f""" + {no_file} found in the build root, + but {request.local_token} was set in `{request.option}`. + """ + ) + ) + return VersionManagerSearchPaths() + + _logger.info( + f"Reading {first_version_file} to determine desired version for {request.option}." + ) + local_version = first_version_file.read_text().strip() + path = Path(tool_versions_path, local_version, "bin") + if path.is_dir(): + return VersionManagerSearchPaths([str(path)]) + return VersionManagerSearchPaths() + + versions_in_dir = ( + tool_versions_path / version / "bin" for version in sorted(tool_versions_path.iterdir()) + ) + return VersionManagerSearchPaths( + str(version) for version in versions_in_dir if version.is_dir() + ) + + class ValidatedSearchPaths(FrozenOrderedSet): """Search paths that are valid for the current target environment.""" From 3872ac83bf417535f0cadd69d7ccf4215b9279c9 Mon Sep 17 00:00:00 2001 From: Tobias Nilsson Date: Fri, 17 Mar 2023 00:14:50 +0100 Subject: [PATCH 09/17] test: Cover specific cases in a general testcase --- .../javascript/subsystems/nodejs_test.py | 63 +-------------- .../core/subsystems/python_bootstrap_test.py | 44 ----------- .../core/util_rules/search_paths_test.py | 76 ++++++++++++++++++- 3 files changed, 76 insertions(+), 107 deletions(-) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs_test.py b/src/python/pants/backend/javascript/subsystems/nodejs_test.py index aa6472abe12..27ee251987a 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs_test.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs_test.py @@ -3,12 +3,10 @@ from __future__ import annotations -import os import stat -from contextlib import contextmanager from pathlib import Path from textwrap import dedent -from typing import Generator, NoReturn +from typing import NoReturn from unittest.mock import Mock import pytest @@ -23,9 +21,7 @@ ) from pants.backend.javascript.target_types import JSSourcesGeneratorTarget from pants.backend.python import target_types_rules -from pants.build_graph.address import Address from pants.core.util_rules import config_files, source_files -from pants.core.util_rules.environments import EnvironmentTarget, LocalEnvironmentTarget from pants.core.util_rules.external_tool import ( DownloadedExternalTool, ExternalToolRequest, @@ -36,7 +32,7 @@ VersionManagerSearchPathsRequest, ) from pants.core.util_rules.system_binaries import BinaryNotFoundError, BinaryPath -from pants.engine.env_vars import CompleteEnvironmentVars, EnvironmentVars, EnvironmentVarsRequest +from pants.engine.env_vars import EnvironmentVars, EnvironmentVarsRequest from pants.engine.internals.native_engine import EMPTY_DIGEST from pants.engine.platform import Platform from pants.engine.process import ProcessResult @@ -258,58 +254,3 @@ def mock_environment_vars(_req: EnvironmentVarsRequest) -> EnvironmentVars: mock_gets=[MockGet(EnvironmentVars, (EnvironmentVarsRequest,), mock_environment_vars)], ) assert result == expected_directory - - -@contextmanager -def fake_nvm_root( - fake_versions: list[str], fake_local_version: str -) -> Generator[tuple[str, tuple[str, ...], tuple[str]], None, None]: - with temporary_dir() as nvm_root: - fake_version_dirs = tuple( - os.path.join(nvm_root, "versions", "node", v, "bin") for v in fake_versions - ) - for d in fake_version_dirs: - os.makedirs(d) - fake_local_version_dirs = ( - os.path.join(nvm_root, "versions", "node", fake_local_version, "bin"), - ) - yield nvm_root, fake_version_dirs, fake_local_version_dirs - - -def test_get_local_nvm_paths(rule_runner: RuleRunner) -> None: - local_nvm_version = "3.5.5" - all_nvm_versions = ["2.7.14", local_nvm_version] - rule_runner.write_files({".nvmrc": f"{local_nvm_version}\n"}) - with fake_nvm_root(all_nvm_versions, local_nvm_version) as ( - nvm_root, - expected_paths, - expected_local_paths, - ): - rule_runner.set_session_values( - {CompleteEnvironmentVars: CompleteEnvironmentVars({"NVM_DIR": nvm_root})} - ) - env_name = "name" - tgt = EnvironmentTarget(env_name, LocalEnvironmentTarget({}, Address("flem"))) - paths = rule_runner.request( - VersionManagerSearchPaths, - [ - VersionManagerSearchPathsRequest( - tgt, nvm_root, "versions/node", "[nodejs].search_path", (".nvmrc",), None - ) - ], - ) - local_paths = rule_runner.request( - VersionManagerSearchPaths, - [ - VersionManagerSearchPathsRequest( - tgt, - nvm_root, - "versions/node", - "[nodejs].search_path", - (".nvmrc",), - "", - ) - ], - ) - assert set(expected_paths) == set(paths) - assert set(expected_local_paths) == set(local_paths) diff --git a/src/python/pants/core/subsystems/python_bootstrap_test.py b/src/python/pants/core/subsystems/python_bootstrap_test.py index 2154a4e7dd9..8cf12b31e2b 100644 --- a/src/python/pants/core/subsystems/python_bootstrap_test.py +++ b/src/python/pants/core/subsystems/python_bootstrap_test.py @@ -112,50 +112,6 @@ def test_get_pyenv_root(env: dict[str, str], expected: str | None) -> None: assert result == expected -def test_get_pyenv_paths(rule_runner: RuleRunner) -> None: - local_pyenv_version = "3.5.5" - all_pyenv_versions = ["2.7.14", local_pyenv_version] - rule_runner.write_files({".python-version": f"{local_pyenv_version}\n"}) - with fake_pyenv_root(all_pyenv_versions, local_pyenv_version) as ( - pyenv_root, - expected_paths, - expected_local_paths, - ): - rule_runner.set_session_values( - {CompleteEnvironmentVars: CompleteEnvironmentVars({"PYENV_ROOT": pyenv_root})} - ) - env_name = "name" - tgt = EnvironmentTarget(env_name, LocalEnvironmentTarget({}, Address("flem"))) - paths = rule_runner.request( - VersionManagerSearchPaths, - [ - VersionManagerSearchPathsRequest( - tgt, - pyenv_root, - "versions", - "[python-bootstrap].search_path", - (".python-version",), - None, - ) - ], - ) - local_paths = rule_runner.request( - VersionManagerSearchPaths, - [ - VersionManagerSearchPathsRequest( - tgt, - pyenv_root, - "versions", - "[python-bootstrap].search_path", - (".python-version",), - "", - ) - ], - ) - assert set(expected_paths) == set(paths) - assert set(expected_local_paths) == set(local_paths) - - def test_expand_interpreter_search_paths(rule_runner: RuleRunner) -> None: local_pyenv_version = "3.5.5" all_python_versions = ["2.7.14", local_pyenv_version, "3.7.10", "3.9.4", "3.9.5"] diff --git a/src/python/pants/core/util_rules/search_paths_test.py b/src/python/pants/core/util_rules/search_paths_test.py index aa4dbdfd8a4..8fcdde7ac91 100644 --- a/src/python/pants/core/util_rules/search_paths_test.py +++ b/src/python/pants/core/util_rules/search_paths_test.py @@ -2,9 +2,14 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). from __future__ import annotations +import os +from contextlib import contextmanager +from typing import Generator + import pytest from pants.build_graph.address import Address +from pants.core.util_rules import search_paths from pants.core.util_rules.asdf import AsdfPathString from pants.core.util_rules.environments import ( DockerEnvironmentTarget, @@ -13,11 +18,28 @@ LocalEnvironmentTarget, RemoteEnvironmentTarget, ) -from pants.core.util_rules.search_paths import ValidateSearchPathsRequest, validate_search_paths -from pants.testutil.rule_runner import run_rule_with_mocks +from pants.core.util_rules.search_paths import ( + ValidateSearchPathsRequest, + VersionManagerSearchPaths, + VersionManagerSearchPathsRequest, + validate_search_paths, +) +from pants.engine.rules import QueryRule +from pants.testutil.rule_runner import RuleRunner, run_rule_with_mocks +from pants.util.contextutil import temporary_dir from pants.util.ordered_set import FrozenOrderedSet +@pytest.fixture +def rule_runner() -> RuleRunner: + return RuleRunner( + rules=[ + *search_paths.rules(), + QueryRule(VersionManagerSearchPaths, (VersionManagerSearchPathsRequest,)), + ], + ) + + @pytest.mark.parametrize( ("env_tgt_type", "search_paths", "is_default", "expected"), ( @@ -115,3 +137,53 @@ def test_validated_search_paths( ) ], ) + + +@contextmanager +def fake_tool_root( + fake_versions: list[str], fake_local_version: str +) -> Generator[tuple[str, tuple[str, ...], tuple[str]], None, None]: + with temporary_dir() as tool_root: + fake_version_dirs = tuple( + os.path.join(tool_root, "versions", v, "bin") for v in fake_versions + ) + for d in fake_version_dirs: + os.makedirs(d) + fake_local_version_dirs = (os.path.join(tool_root, "versions", fake_local_version, "bin"),) + yield tool_root, fake_version_dirs, fake_local_version_dirs + + +def test_get_local_tool_paths(rule_runner: RuleRunner) -> None: + local_version = "3.5.5" + all_versions = ["2.7.14", local_version] + rule_runner.write_files({".version-file": f"{local_version}\n"}) + with fake_tool_root(all_versions, local_version) as ( + tool_root, + expected_paths, + expected_local_paths, + ): + env_name = "name" + tgt = EnvironmentTarget(env_name, LocalEnvironmentTarget({}, Address("flem"))) + paths = rule_runner.request( + VersionManagerSearchPaths, + [ + VersionManagerSearchPathsRequest( + tgt, tool_root, "versions", "[mock].search_path", (".version-file",), None + ) + ], + ) + local_paths = rule_runner.request( + VersionManagerSearchPaths, + [ + VersionManagerSearchPathsRequest( + tgt, + tool_root, + "versions", + "[mock].search_path", + (".version-file",), + "", + ) + ], + ) + assert set(expected_paths) == set(paths) + assert set(expected_local_paths) == set(local_paths) From f17a4b979b7484c07a9cc1e45bce7970d2c39d76 Mon Sep 17 00:00:00 2001 From: Tobias Nilsson Date: Fri, 17 Mar 2023 00:43:42 +0100 Subject: [PATCH 10/17] fix: Use Sequence type --- src/python/pants/backend/javascript/subsystems/nodejs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs.py b/src/python/pants/backend/javascript/subsystems/nodejs.py index 3f58b772a35..40f393286a1 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs.py @@ -8,7 +8,7 @@ import os.path from dataclasses import dataclass, field from itertools import groupby -from typing import ClassVar, Collection, Iterable, Mapping +from typing import ClassVar, Collection, Iterable, Mapping, Sequence from nodesemver import min_satisfying @@ -337,7 +337,7 @@ async def nodejs_bootstrap(nodejs_env_aware: NodeJS.EnvironmentAware) -> NodeJsB return NodeJsBootstrap(nodejs_search_paths=expanded_paths) -class _BinaryPathsPerVersion(FrozenDict[str, tuple[BinaryPath, ...]]): +class _BinaryPathsPerVersion(FrozenDict[str, Sequence[BinaryPath]]): pass From 24a34fc7dfd087831939c1cffd53f3ba8b25b193 Mon Sep 17 00:00:00 2001 From: Tobias Nilsson Date: Fri, 17 Mar 2023 00:57:34 +0100 Subject: [PATCH 11/17] doc: Specify format restriction --- src/python/pants/backend/javascript/subsystems/nodejs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs.py b/src/python/pants/backend/javascript/subsystems/nodejs.py index 40f393286a1..9ee5b211a33 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs.py @@ -146,6 +146,7 @@ class EnvironmentAware(Subsystem.EnvironmentAware): * `{AsdfPathString.LOCAL}`, {AsdfPathString.LOCAL.description("binaries")} * ``, all NodeJS versions under $NVM_DIR/versions/node * ``, the nvm installation with the version in BUILD_ROOT/.nvmrc + Note that the version in the .nvmrc file has to be on the form "vX.Y.Z". """ ), advanced=True, From 16bec9e5a4fb48375626b5be839da1f9a45b2bfb Mon Sep 17 00:00:00 2001 From: Tobias Nilsson Date: Fri, 17 Mar 2023 00:58:07 +0100 Subject: [PATCH 12/17] fix: Do not allow NVM usage in non-local environments --- src/python/pants/backend/javascript/subsystems/nodejs.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs.py b/src/python/pants/backend/javascript/subsystems/nodejs.py index 9ee5b211a33..0d945725671 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs.py @@ -329,7 +329,9 @@ async def nodejs_bootstrap(nodejs_env_aware: NodeJS.EnvironmentAware) -> NodeJsB option_origin=f"[{NodeJS.options_scope}].search_path", environment_key="nodejs_search_path", is_default=nodejs_env_aware._is_default("search_path"), - local_only=FrozenOrderedSet((AsdfPathString.STANDARD, AsdfPathString.LOCAL)), + local_only=FrozenOrderedSet( + (AsdfPathString.STANDARD, AsdfPathString.LOCAL, "", "") + ), ), ) From 88ebd5dfb03a8585096dd534a942bff4f11af46e Mon Sep 17 00:00:00 2001 From: Tobias Nilsson Date: Fri, 17 Mar 2023 20:05:40 +0100 Subject: [PATCH 13/17] test: Mock async method the 3.7 way --- .../javascript/subsystems/nodejs_test.py | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs_test.py b/src/python/pants/backend/javascript/subsystems/nodejs_test.py index 27ee251987a..2cc69496a4a 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs_test.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs_test.py @@ -4,10 +4,11 @@ from __future__ import annotations import stat +from asyncio import Future from pathlib import Path from textwrap import dedent from typing import NoReturn -from unittest.mock import Mock +from unittest.mock import MagicMock, Mock import pytest @@ -89,6 +90,15 @@ def given_known_version(version: str) -> str: return f"{version}|linux_x86_64|1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd|333333" +@pytest.fixture +def mock_nodejs_subsystem() -> Mock: + nodejs_subsystem = Mock(spec=NodeJS) + future: Future[DownloadedExternalTool] = Future() + future.set_result(DownloadedExternalTool(EMPTY_DIGEST, "")) + nodejs_subsystem.download_known_version = MagicMock(return_value=future) + return nodejs_subsystem + + _SEMVER_1_1_0 = given_known_version("1.1.0") _SEMVER_2_1_0 = given_known_version("2.1.0") _SEMVER_2_2_0 = given_known_version("2.2.0") @@ -111,8 +121,10 @@ def given_known_version(version: str) -> str: pytest.param(">2.1 || <2.1", _SEMVER_1_1_0, id="or_range"), ], ) -def test_node_version_from_semver_download(semver_range: str, expected: str) -> None: - nodejs_subsystem = Mock(spec_set=NodeJS) +def test_node_version_from_semver_download( + mock_nodejs_subsystem: Mock, semver_range: str, expected: str +) -> None: + nodejs_subsystem = mock_nodejs_subsystem nodejs_subsystem.version = semver_range nodejs_subsystem.known_versions = [ _SEMVER_1_1_0, @@ -153,8 +165,10 @@ def test_node_version_from_semver_download(semver_range: str, expected: str) -> pytest.param(">2.1 || <2.1", "1/1/0", id="or_range"), ], ) -def test_node_version_from_semver_bootstrap(semver_range: str, expected_path: str) -> None: - nodejs_subsystem = Mock(spec_set=NodeJS) +def test_node_version_from_semver_bootstrap( + mock_nodejs_subsystem: Mock, semver_range: str, expected_path: str +) -> None: + nodejs_subsystem = mock_nodejs_subsystem nodejs_subsystem.version = semver_range discoverable_versions = _BinaryPathsPerVersion( { @@ -180,8 +194,8 @@ def mock_download(*_) -> NoReturn: assert result.binary_dir == expected_path -def test_finding_no_node_version_is_an_error() -> None: - nodejs_subsystem = Mock(spec_set=NodeJS) +def test_finding_no_node_version_is_an_error(mock_nodejs_subsystem: Mock) -> None: + nodejs_subsystem = mock_nodejs_subsystem nodejs_subsystem.version = "*" nodejs_subsystem.known_versions = [] discoverable_versions = _BinaryPathsPerVersion() From 8427275bac0408e2db9130407b9dbc0e4819b342 Mon Sep 17 00:00:00 2001 From: Tobias Nilsson Date: Mon, 20 Mar 2023 20:20:28 +0100 Subject: [PATCH 14/17] refactor: Consistent capitalization of NodeJS --- .../backend/javascript/subsystems/nodejs.py | 22 +++++++++---------- .../javascript/subsystems/nodejs_test.py | 6 ++--- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs.py b/src/python/pants/backend/javascript/subsystems/nodejs.py index 0d945725671..27a8493a043 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs.py @@ -165,7 +165,7 @@ async def user_chosen_resolve_aliases(nodejs: NodeJS) -> UserChosenNodeJSResolve @dataclass(frozen=True) class NodeJSToolProcess: - """A request for a tool installed with NodeJs.""" + """A request for a tool installed with NodeJS.""" args: tuple[str, ...] description: str @@ -219,14 +219,14 @@ def npx( @dataclass(frozen=True) -class NodejsBinaries: +class NodeJSBinaries: binary_dir: str digest: Digest | None = None @dataclass(frozen=True) class NodeJSProcessEnvironment: - binaries: NodejsBinaries + binaries: NodeJSBinaries npm_config_cache: str base_bin_dir: ClassVar[str] = "__node" @@ -250,12 +250,12 @@ def immutable_digest(self) -> dict[str, Digest]: @rule(level=LogLevel.DEBUG) -async def node_process_environment(binaries: NodejsBinaries) -> NodeJSProcessEnvironment: +async def node_process_environment(binaries: NodeJSBinaries) -> NodeJSProcessEnvironment: return NodeJSProcessEnvironment(binaries=binaries, npm_config_cache="._npm") @dataclass(frozen=True) -class NodeJsBootstrap: +class NodeJSBootstrap: nodejs_search_paths: tuple[str, ...] @@ -320,7 +320,7 @@ async def _nodejs_search_paths( @rule -async def nodejs_bootstrap(nodejs_env_aware: NodeJS.EnvironmentAware) -> NodeJsBootstrap: +async def nodejs_bootstrap(nodejs_env_aware: NodeJS.EnvironmentAware) -> NodeJSBootstrap: search_paths = await Get( ValidatedSearchPaths, ValidateSearchPathsRequest( @@ -337,7 +337,7 @@ async def nodejs_bootstrap(nodejs_env_aware: NodeJS.EnvironmentAware) -> NodeJsB expanded_paths = await _nodejs_search_paths(nodejs_env_aware.env_tgt, search_paths) - return NodeJsBootstrap(nodejs_search_paths=expanded_paths) + return NodeJSBootstrap(nodejs_search_paths=expanded_paths) class _BinaryPathsPerVersion(FrozenDict[str, Sequence[BinaryPath]]): @@ -345,7 +345,7 @@ class _BinaryPathsPerVersion(FrozenDict[str, Sequence[BinaryPath]]): @rule(level=LogLevel.DEBUG, desc="Testing for Node.js binaries.") -async def get_valid_nodejs_paths_by_version(bootstrap: NodeJsBootstrap) -> _BinaryPathsPerVersion: +async def get_valid_nodejs_paths_by_version(bootstrap: NodeJSBootstrap) -> _BinaryPathsPerVersion: paths = await Get( BinaryPaths, BinaryPathRequest( @@ -364,10 +364,10 @@ async def get_valid_nodejs_paths_by_version(bootstrap: NodeJsBootstrap) -> _Bina @rule(level=LogLevel.INFO, desc="Finding Node.js distribution binaries.") async def determine_nodejs_binaries( nodejs: NodeJS, platform: Platform, paths_per_version: _BinaryPathsPerVersion -) -> NodejsBinaries: +) -> NodeJSBinaries: satisfying_version = min_satisfying(paths_per_version.keys(), nodejs.version) if satisfying_version: - return NodejsBinaries(os.path.dirname(paths_per_version[satisfying_version][0].path)) + return NodeJSBinaries(os.path.dirname(paths_per_version[satisfying_version][0].path)) decoded_versions = groupby( (ExternalToolVersion.decode(unparsed) for unparsed in nodejs.known_versions), @@ -403,7 +403,7 @@ async def determine_nodejs_binaries( NodeJSProcessEnvironment.base_bin_dir, os.path.dirname(downloaded.exe), ) - return NodejsBinaries(nodejs_bin_dir, downloaded.digest) + return NodeJSBinaries(nodejs_bin_dir, downloaded.digest) @rule(level=LogLevel.DEBUG) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs_test.py b/src/python/pants/backend/javascript/subsystems/nodejs_test.py index 2cc69496a4a..a8ebd5ed73c 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs_test.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs_test.py @@ -15,7 +15,7 @@ from pants.backend.javascript.subsystems import nodejs from pants.backend.javascript.subsystems.nodejs import ( NodeJS, - NodejsBinaries, + NodeJSBinaries, _BinaryPathsPerVersion, _get_nvm_root, determine_nodejs_binaries, @@ -50,7 +50,7 @@ def rule_runner() -> RuleRunner: *config_files.rules(), *target_types_rules.rules(), QueryRule(ProcessResult, [nodejs.NodeJSToolProcess]), - QueryRule(NodejsBinaries, ()), + QueryRule(NodeJSBinaries, ()), QueryRule(VersionManagerSearchPaths, (VersionManagerSearchPathsRequest,)), ], target_types=[JSSourcesGeneratorTarget], @@ -242,7 +242,7 @@ def test_find_valid_binary(rule_runner: RuleRunner) -> None: ], env_inherit={"PATH"}, ) - result = rule_runner.request(NodejsBinaries, ()) + result = rule_runner.request(NodeJSBinaries, ()) assert result.binary_dir == str(binary_dir) From 5bcbcafe9a8302a9b21bd77d77d0f93df665c009 Mon Sep 17 00:00:00 2001 From: Tobias Nilsson Date: Mon, 20 Mar 2023 20:21:22 +0100 Subject: [PATCH 15/17] chore: Lower log level to debug --- src/python/pants/backend/javascript/subsystems/nodejs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs.py b/src/python/pants/backend/javascript/subsystems/nodejs.py index 27a8493a043..122f9586a4e 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs.py @@ -361,7 +361,7 @@ async def get_valid_nodejs_paths_by_version(bootstrap: NodeJSBootstrap) -> _Bina return _BinaryPathsPerVersion({version: tuple(paths) for version, paths in group_by_version}) -@rule(level=LogLevel.INFO, desc="Finding Node.js distribution binaries.") +@rule(level=LogLevel.DEBUG, desc="Finding Node.js distribution binaries.") async def determine_nodejs_binaries( nodejs: NodeJS, platform: Platform, paths_per_version: _BinaryPathsPerVersion ) -> NodeJSBinaries: From 1f0eb9ebaaa50f92599e936fd3503ed760e28242 Mon Sep 17 00:00:00 2001 From: Tobias Nilsson Date: Mon, 20 Mar 2023 20:22:43 +0100 Subject: [PATCH 16/17] doc: Add examples of version manager tools --- src/python/pants/core/util_rules/search_paths.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/core/util_rules/search_paths.py b/src/python/pants/core/util_rules/search_paths.py index c90f9ad15f2..c82255289e7 100644 --- a/src/python/pants/core/util_rules/search_paths.py +++ b/src/python/pants/core/util_rules/search_paths.py @@ -46,7 +46,7 @@ class VersionManagerSearchPaths(DeduplicatedCollection[str]): async def get_un_cachable_version_manager_paths( request: VersionManagerSearchPathsRequest, ) -> VersionManagerSearchPaths: - """Inspects the directory of a version manager tool to find installations.""" + """Inspects the directory of a version manager tool like pyenv or nvm to find installations.""" if not (request.env_tgt.val is None or isinstance(request.env_tgt.val, LocalEnvironmentTarget)): return VersionManagerSearchPaths() From 38f1ec28164fa86b2b4bed5a77d28f394ea08bec Mon Sep 17 00:00:00 2001 From: Tobias Nilsson Date: Mon, 20 Mar 2023 22:05:27 +0100 Subject: [PATCH 17/17] feat: Prefer the downloaded nodejs binaries over search paths --- .../backend/javascript/subsystems/nodejs.py | 39 ++++++++++--------- .../javascript/subsystems/nodejs_test.py | 2 + 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs.py b/src/python/pants/backend/javascript/subsystems/nodejs.py index 122f9586a4e..52a5688639b 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs.py @@ -49,7 +49,7 @@ from pants.util.frozendict import FrozenDict from pants.util.logging import LogLevel from pants.util.ordered_set import FrozenOrderedSet -from pants.util.strutil import softwrap +from pants.util.strutil import help_text, softwrap _logger = logging.getLogger(__name__) @@ -122,13 +122,14 @@ class EnvironmentAware(Subsystem.EnvironmentAware): search_path = StrListOption( default=[""], - help=softwrap( + help=lambda cls: help_text( f""" A list of paths to search for Node.js distributions. - This option take precedence over the templated url download, - if a version matching the configured version range is found - in these paths. + This option is only used if a templated url download + specified via [{cls.subsystem.options_scope}].known_versions + does not contain a version matching the configured + [{cls.subsystem.options_scope}].version range. You can specify absolute paths to binaries and/or to directories containing binaries. The order of entries does @@ -365,10 +366,6 @@ async def get_valid_nodejs_paths_by_version(bootstrap: NodeJSBootstrap) -> _Bina async def determine_nodejs_binaries( nodejs: NodeJS, platform: Platform, paths_per_version: _BinaryPathsPerVersion ) -> NodeJSBinaries: - satisfying_version = min_satisfying(paths_per_version.keys(), nodejs.version) - if satisfying_version: - return NodeJSBinaries(os.path.dirname(paths_per_version[satisfying_version][0].path)) - decoded_versions = groupby( (ExternalToolVersion.decode(unparsed) for unparsed in nodejs.known_versions), lambda v: v.version, @@ -384,26 +381,30 @@ async def determine_nodejs_binaries( } satisfying_version = min_satisfying(decoded_per_version.keys(), nodejs.version) + if satisfying_version: + known_version = decoded_per_version[satisfying_version][0] + downloaded = await nodejs.download_known_version(known_version, platform) + nodejs_bin_dir = os.path.join( + "{chroot}", + NodeJSProcessEnvironment.base_bin_dir, + os.path.dirname(downloaded.exe), + ) + + return NodeJSBinaries(nodejs_bin_dir, downloaded.digest) + + satisfying_version = min_satisfying(paths_per_version.keys(), nodejs.version) if not satisfying_version: raise BinaryNotFoundError( softwrap( f""" Cannot find any `node` binaries satisfying the range '{nodejs.version}'. - To fix, either list a `[{NodeJS.options_scope}].url_platform_mapping` version that satisfies the range, + To fix, either list a `[{NodeJS.options_scope}].known_versions` version that satisfies the range, or ensure `[{NodeJS.options_scope}].search_path` contains a path to binaries that satisfy the range. """ ) ) - - known_version = decoded_per_version[satisfying_version][0] - downloaded = await nodejs.download_known_version(known_version, platform) - nodejs_bin_dir = os.path.join( - "{chroot}", - NodeJSProcessEnvironment.base_bin_dir, - os.path.dirname(downloaded.exe), - ) - return NodeJSBinaries(nodejs_bin_dir, downloaded.digest) + return NodeJSBinaries(os.path.dirname(paths_per_version[satisfying_version][0].path)) @rule(level=LogLevel.DEBUG) diff --git a/src/python/pants/backend/javascript/subsystems/nodejs_test.py b/src/python/pants/backend/javascript/subsystems/nodejs_test.py index a8ebd5ed73c..bea81da15d4 100644 --- a/src/python/pants/backend/javascript/subsystems/nodejs_test.py +++ b/src/python/pants/backend/javascript/subsystems/nodejs_test.py @@ -170,6 +170,7 @@ def test_node_version_from_semver_bootstrap( ) -> None: nodejs_subsystem = mock_nodejs_subsystem nodejs_subsystem.version = semver_range + nodejs_subsystem.known_versions = [] discoverable_versions = _BinaryPathsPerVersion( { "1.1.0": (BinaryPath("1/1/0/node"),), @@ -238,6 +239,7 @@ def test_find_valid_binary(rule_runner: RuleRunner) -> None: rule_runner.set_options( [ f"--nodejs-search-path=['{binary_dir}']", + "--nodejs-known-versions=[]", "--nodejs-version=>2", ], env_inherit={"PATH"},