From 226ddbaa15f8cf065008c5048df9973258facfe8 Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Fri, 4 Oct 2024 09:44:11 +0200 Subject: [PATCH 1/2] test: unittest pnpm extension version resolution This is in preparation for #1907: As dependency resolution gets more complex, we should test it. Note: No updates to to bzl_library targets, because it seems `extensions.bzl` isn't in a bzl_library. --- npm/extensions.bzl | 45 +++--------- npm/private/pnpm_extension.bzl | 59 +++++++++++++++ npm/private/test/BUILD.bazel | 3 + npm/private/test/pnpm_test.bzl | 126 +++++++++++++++++++++++++++++++++ 4 files changed, 196 insertions(+), 37 deletions(-) create mode 100644 npm/private/pnpm_extension.bzl create mode 100644 npm/private/test/pnpm_test.bzl diff --git a/npm/extensions.bzl b/npm/extensions.bzl index 76bb756fc..2f2e17706 100644 --- a/npm/extensions.bzl +++ b/npm/extensions.bzl @@ -11,6 +11,7 @@ load("//npm/private:npm_translate_lock.bzl", "npm_translate_lock_lib", "npm_tran load("//npm/private:npm_translate_lock_helpers.bzl", npm_translate_lock_helpers = "helpers") load("//npm/private:npm_translate_lock_macro_helpers.bzl", macro_helpers = "helpers") load("//npm/private:npm_translate_lock_state.bzl", "npm_translate_lock_state") +load("//npm/private:pnpm_extension.bzl", "DEFAULT_PNPM_REPO_NAME", "resolve_pnpm_repositories") load("//npm/private:npmrc.bzl", "parse_npmrc") load("//npm/private:tar.bzl", "detect_system_tar") load("//npm/private:transitive_closure.bzl", "translate_to_transitive_closure") @@ -18,8 +19,6 @@ load("//npm/private:transitive_closure.bzl", "translate_to_transitive_closure") DEFAULT_PNPM_VERSION = _DEFAULT_PNPM_VERSION LATEST_PNPM_VERSION = _LATEST_PNPM_VERSION -_DEFAULT_PNPM_REPO_NAME = "pnpm" - def _npm_extension_impl(module_ctx): if not bazel_lib_utils.is_bazel_6_or_greater(): # ctx.actions.declare_symlink was added in Bazel 6 @@ -240,45 +239,17 @@ npm = module_extension( }, ) -# copied from https://github.com/bazelbuild/bazel-skylib/blob/b459822483e05da514b539578f81eeb8a705d600/lib/versions.bzl#L60 -# to avoid taking a dependency on skylib here -def _parse_version(version): - return tuple([int(n) for n in version.split(".")]) - def _pnpm_extension_impl(module_ctx): - registrations = {} - integrity = {} - for mod in module_ctx.modules: - for attr in mod.tags.pnpm: - if attr.name != _DEFAULT_PNPM_REPO_NAME and not mod.is_root: - fail("""\ - Only the root module may override the default name for the pnpm repository. - This prevents conflicting registrations in the global namespace of external repos. - """) - if attr.name not in registrations.keys(): - registrations[attr.name] = [] - registrations[attr.name].append(attr.pnpm_version) - if attr.pnpm_version_integrity: - integrity[attr.pnpm_version] = attr.pnpm_version_integrity - for name, versions in registrations.items(): - # Use "Minimal Version Selection" like bzlmod does for resolving module conflicts - # Note, the 'sorted(list)' function in starlark doesn't allow us to provide a custom comparator - if len(versions) > 1: - selected = versions[0] - selected_tuple = _parse_version(selected) - for idx in range(1, len(versions)): - if _parse_version(versions[idx]) > selected_tuple: - selected = versions[idx] - selected_tuple = _parse_version(selected) + resolved = resolve_pnpm_repositories(module_ctx.modules) - # buildifier: disable=print - print("NOTE: repo '{}' has multiple versions {}; selected {}".format(name, versions, selected)) - else: - selected = versions[0] + for note in resolved.notes: + # buildifier: disable=print + print(note) + for name, pnpm_version in resolved.repositories.items(): pnpm_repository( name = name, - pnpm_version = (selected, integrity[selected]) if selected in integrity.keys() else selected, + pnpm_version = pnpm_version, ) pnpm = module_extension( @@ -289,7 +260,7 @@ pnpm = module_extension( "name": attr.string( doc = """Name of the generated repository, allowing more than one pnpm version to be registered. Overriding the default is only permitted in the root module.""", - default = _DEFAULT_PNPM_REPO_NAME, + default = DEFAULT_PNPM_REPO_NAME, ), "pnpm_version": attr.string(default = DEFAULT_PNPM_VERSION), "pnpm_version_integrity": attr.string(), diff --git a/npm/private/pnpm_extension.bzl b/npm/private/pnpm_extension.bzl new file mode 100644 index 000000000..44ebccb44 --- /dev/null +++ b/npm/private/pnpm_extension.bzl @@ -0,0 +1,59 @@ +"""pnpm extension logic (the extension itself is in npm/extensions.bzl).""" + +DEFAULT_PNPM_REPO_NAME = "pnpm" + +# copied from https://github.com/bazelbuild/bazel-skylib/blob/b459822483e05da514b539578f81eeb8a705d600/lib/versions.bzl#L60 +# to avoid taking a dependency on skylib here +def _parse_version(version): + return tuple([int(n) for n in version.split(".")]) + +def resolve_pnpm_repositories(modules): + """Resolves pnpm tags in all `modules` + + Args: + modules: module_ctx.modules + + Returns: + A struct with the following fields: + - `repositories`: dict (name -> pnpm_version) to invoke `pnpm_repository` with. + - `notes`: list of notes to print to the user. + """ + + registrations = {} + integrity = {} + + result = struct( + notes = [], + repositories = {}, + ) + + for mod in modules: + for attr in mod.tags.pnpm: + if attr.name != DEFAULT_PNPM_REPO_NAME and not mod.is_root: + fail("""\ + Only the root module may override the default name for the pnpm repository. + This prevents conflicting registrations in the global namespace of external repos. + """) + if attr.name not in registrations.keys(): + registrations[attr.name] = [] + registrations[attr.name].append(attr.pnpm_version) + if attr.pnpm_version_integrity: + integrity[attr.pnpm_version] = attr.pnpm_version_integrity + for name, versions in registrations.items(): + # Use "Minimal Version Selection" like bzlmod does for resolving module conflicts + # Note, the 'sorted(list)' function in starlark doesn't allow us to provide a custom comparator + if len(versions) > 1: + selected = versions[0] + selected_tuple = _parse_version(selected) + for idx in range(1, len(versions)): + if _parse_version(versions[idx]) > selected_tuple: + selected = versions[idx] + selected_tuple = _parse_version(selected) + + result.notes.append("NOTE: repo '{}' has multiple versions {}; selected {}".format(name, versions, selected)) + else: + selected = versions[0] + + result.repositories[name] = (selected, integrity[selected]) if selected in integrity.keys() else selected + + return result diff --git a/npm/private/test/BUILD.bazel b/npm/private/test/BUILD.bazel index 05ac282e7..4464ba0b0 100644 --- a/npm/private/test/BUILD.bazel +++ b/npm/private/test/BUILD.bazel @@ -4,6 +4,7 @@ load("@npm//:defs.bzl", "npm_link_all_packages") load(":generated_pkg_json_test.bzl", "generated_pkg_json_test") load(":npm_auth_test.bzl", "npm_auth_test_suite") load(":npmrc_test.bzl", "npmrc_tests") +load(":pnpm_test.bzl", "pnpm_tests") load(":parse_pnpm_lock_tests.bzl", "parse_pnpm_lock_tests") load(":transitive_closure_tests.bzl", "transitive_closure_tests") load(":translate_lock_helpers_tests.bzl", "translate_lock_helpers_tests") @@ -18,6 +19,8 @@ utils_tests(name = "test_utils") npmrc_tests(name = "test_npmrc") +pnpm_tests(name = "test_pnpm") + transitive_closure_tests(name = "test_transitive_closure") translate_lock_helpers_tests(name = "test_translate_lock") diff --git a/npm/private/test/pnpm_test.bzl b/npm/private/test/pnpm_test.bzl new file mode 100644 index 000000000..d6fe47b43 --- /dev/null +++ b/npm/private/test/pnpm_test.bzl @@ -0,0 +1,126 @@ +"""Test for pnpm extension version resolution.""" + +load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest") +load("//npm/private:pnpm_extension.bzl", "DEFAULT_PNPM_REPO_NAME", "resolve_pnpm_repositories") + +def _fake_pnpm_tag(version, name = DEFAULT_PNPM_REPO_NAME, integrity = None): + return struct( + name = name, + pnpm_version = version, + pnpm_version_integrity = integrity, + ) + +def _fake_mod(is_root, *pnpm_tags): + return struct( + is_root = is_root, + tags = struct(pnpm = pnpm_tags), + ) + +def _resolve_test(ctx, repositories = [], notes = [], modules = []): + env = unittest.begin(ctx) + + expected = struct( + repositories = repositories, + notes = notes, + ) + + result = resolve_pnpm_repositories(modules) + + asserts.equals(env, expected, result) + return unittest.end(env) + +def _basic(ctx): + # Essentially what happens without any user configuration. + # - Root module doesn't have any pnpm tag. + # - rules_js sets a default. + return _resolve_test( + ctx, + repositories = {"pnpm": ("8.6.7", "8.6.7-integrity")}, + modules = [ + _fake_mod(True), + _fake_mod( + False, + _fake_pnpm_tag(version = "8.6.7", integrity = "8.6.7-integrity"), + ), + ], + ) + +def _override(ctx): + # What happens when the root overrides the pnpm version. + return _resolve_test( + ctx, + repositories = {"pnpm": "9.1.0"}, + notes = [ + """NOTE: repo 'pnpm' has multiple versions ["9.1.0", "8.6.7"]; selected 9.1.0""", + ], + modules = [ + _fake_mod( + True, + _fake_pnpm_tag(version = "9.1.0"), + ), + _fake_mod( + False, + _fake_pnpm_tag(version = "8.6.7", integrity = "8.6.7-integrity"), + ), + ], + ) + +def _custom_name(ctx): + return _resolve_test( + ctx, + repositories = { + "my-pnpm": "9.1.0", + "pnpm": ("8.6.7", "8.6.7-integrity"), + }, + modules = [ + _fake_mod( + True, + _fake_pnpm_tag(name = "my-pnpm", version = "9.1.0"), + ), + _fake_mod( + False, + _fake_pnpm_tag(version = "8.6.7", integrity = "8.6.7-integrity"), + ), + ], + ) + +def _integrity_conflict(ctx): + # What happens if two modules define the same version with conflicting integrity parameters. + # @gzm0, 2024-10-04: The behavior here is probably not intended and merely an implementation artifact. + # I've added a test anyways to capture the existing behavior. + + return _resolve_test( + ctx, + repositories = { + "pnpm": ("8.6.7", "dep-integrity"), + }, + notes = [ + """NOTE: repo 'pnpm' has multiple versions ["8.6.7", "8.6.7"]; selected 8.6.7""", + ], + # Modules are *BFS* from root: + # https://bazel.build/rules/lib/builtins/module_ctx#modules + modules = [ + _fake_mod( + True, + _fake_pnpm_tag(version = "8.6.7", integrity = "root-integrity"), + ), + _fake_mod( + False, + _fake_pnpm_tag(version = "8.6.7", integrity = "dep-integrity"), + ), + ], + ) + +basic_test = unittest.make(_basic) +override_test = unittest.make(_override) +custom_name_test = unittest.make(_custom_name) +integrity_conflict_test = unittest.make(_integrity_conflict) + +def pnpm_tests(name): + unittest.suite( + name, + basic_test, + override_test, + custom_name_test, + integrity_conflict_test, + ) From caa3916e31af9a07ba9fd58ff764d894c8a7ad9e Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Fri, 4 Oct 2024 10:57:48 +0200 Subject: [PATCH 2/2] feat(pnpm): allow `latest` as version to get the latest version Fixes #1907. --- npm/extensions.bzl | 5 ++++- npm/private/pnpm_extension.bzl | 9 ++++++++- npm/private/test/pnpm_test.bzl | 22 ++++++++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/npm/extensions.bzl b/npm/extensions.bzl index 2f2e17706..d2ad37e7f 100644 --- a/npm/extensions.bzl +++ b/npm/extensions.bzl @@ -262,7 +262,10 @@ pnpm = module_extension( Overriding the default is only permitted in the root module.""", default = DEFAULT_PNPM_REPO_NAME, ), - "pnpm_version": attr.string(default = DEFAULT_PNPM_VERSION), + "pnpm_version": attr.string( + doc = "pnpm version to use. The string `latest` will be resolved to LATEST_PNPM_VERSION.", + default = DEFAULT_PNPM_VERSION, + ), "pnpm_version_integrity": attr.string(), }, ), diff --git a/npm/private/pnpm_extension.bzl b/npm/private/pnpm_extension.bzl index 44ebccb44..a7e5b27a7 100644 --- a/npm/private/pnpm_extension.bzl +++ b/npm/private/pnpm_extension.bzl @@ -1,5 +1,7 @@ """pnpm extension logic (the extension itself is in npm/extensions.bzl).""" +load(":pnpm_repository.bzl", "LATEST_PNPM_VERSION") + DEFAULT_PNPM_REPO_NAME = "pnpm" # copied from https://github.com/bazelbuild/bazel-skylib/blob/b459822483e05da514b539578f81eeb8a705d600/lib/versions.bzl#L60 @@ -36,7 +38,12 @@ def resolve_pnpm_repositories(modules): """) if attr.name not in registrations.keys(): registrations[attr.name] = [] - registrations[attr.name].append(attr.pnpm_version) + + v = attr.pnpm_version + if v == "latest": + v = LATEST_PNPM_VERSION + + registrations[attr.name].append(v) if attr.pnpm_version_integrity: integrity[attr.pnpm_version] = attr.pnpm_version_integrity for name, versions in registrations.items(): diff --git a/npm/private/test/pnpm_test.bzl b/npm/private/test/pnpm_test.bzl index d6fe47b43..1903c85ae 100644 --- a/npm/private/test/pnpm_test.bzl +++ b/npm/private/test/pnpm_test.bzl @@ -2,6 +2,7 @@ load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest") load("//npm/private:pnpm_extension.bzl", "DEFAULT_PNPM_REPO_NAME", "resolve_pnpm_repositories") +load("//npm/private:pnpm_repository.bzl", "LATEST_PNPM_VERSION") def _fake_pnpm_tag(version, name = DEFAULT_PNPM_REPO_NAME, integrity = None): return struct( @@ -65,6 +66,25 @@ def _override(ctx): ], ) +def _latest(ctx): + # Test the "latest" magic version, + # + # The test case is not entirely realistic: In reality, we'd have at least two tags: + # - The one of the root module (present in the test) + # - The one from rules_js (omitted in the test). + # + # We do this, to avoid `notes` that are dependent on `LATEST_PNPM_VERSION`. + # Otherwise we'd have to either: + # - Use regexes to check notes. + # - Accept a brittle test. + return _resolve_test( + ctx, + repositories = {"pnpm": LATEST_PNPM_VERSION}, + modules = [ + _fake_mod(True, _fake_pnpm_tag(version = "latest")), + ], + ) + def _custom_name(ctx): return _resolve_test( ctx, @@ -113,6 +133,7 @@ def _integrity_conflict(ctx): basic_test = unittest.make(_basic) override_test = unittest.make(_override) +latest_test = unittest.make(_latest) custom_name_test = unittest.make(_custom_name) integrity_conflict_test = unittest.make(_integrity_conflict) @@ -121,6 +142,7 @@ def pnpm_tests(name): name, basic_test, override_test, + latest_test, custom_name_test, integrity_conflict_test, )