From 31118a6ed7bbdafcfc3a77ec60206bba68102d08 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Mon, 27 May 2024 22:27:15 -0700 Subject: [PATCH] refactor: move npm_registry_url to utils (#1758) --- e2e/pnpm_lockfiles/.bazelignore | 1 + e2e/pnpm_lockfiles/MODULE.bazel | 23 +++++++++++++++++++ e2e/pnpm_lockfiles/README.md | 6 +++++ e2e/pnpm_lockfiles/WORKSPACE | 19 +++++++++++++++ e2e/pnpm_lockfiles/cases/BUILD.bazel | 14 +++++++++++ e2e/pnpm_lockfiles/cases/package.json | 1 + e2e/pnpm_lockfiles/cases/pnpm-workspace.yaml | 2 ++ .../cases/tarball-no-url-v54.yaml | 19 +++++++++++++++ npm/private/test/utils_tests.bzl | 12 +++++----- npm/private/utils.bzl | 2 +- 10 files changed, 92 insertions(+), 7 deletions(-) create mode 100644 e2e/pnpm_lockfiles/cases/BUILD.bazel create mode 100644 e2e/pnpm_lockfiles/cases/package.json create mode 100644 e2e/pnpm_lockfiles/cases/pnpm-workspace.yaml create mode 100644 e2e/pnpm_lockfiles/cases/tarball-no-url-v54.yaml diff --git a/e2e/pnpm_lockfiles/.bazelignore b/e2e/pnpm_lockfiles/.bazelignore index 4b16cac4a..d45f3e6b8 100644 --- a/e2e/pnpm_lockfiles/.bazelignore +++ b/e2e/pnpm_lockfiles/.bazelignore @@ -1,4 +1,5 @@ node_modules/ +cases/node_modules projects/a/node_modules projects/b/node_modules projects/c/node_modules diff --git a/e2e/pnpm_lockfiles/MODULE.bazel b/e2e/pnpm_lockfiles/MODULE.bazel index 84e4bd949..4c5e7d4c9 100644 --- a/e2e/pnpm_lockfiles/MODULE.bazel +++ b/e2e/pnpm_lockfiles/MODULE.bazel @@ -4,6 +4,7 @@ local_path_override( path = "../..", ) +# The same primary lockfile across all versions PNPM_LOCK_VERSIONS = [ "v54", "v60", @@ -11,6 +12,11 @@ PNPM_LOCK_VERSIONS = [ "v90", ] +# Lockfiles with unique test cases +PNPM_LOCK_TEST_CASES = [ + "tarball-no-url-v54.yaml", +] + bazel_dep(name = "aspect_bazel_lib", version = "2.7.6") bazel_dep(name = "bazel_skylib", version = "1.5.0", dev_dependency = True) @@ -62,3 +68,20 @@ npm = use_extension( ) for version in PNPM_LOCK_VERSIONS ] + +[ + npm.npm_translate_lock( + name = lockfile.replace(".yaml", ""), + pnpm_lock = "//cases:%s" % lockfile, + verify_node_modules_ignored = "//:.bazelignore", + ) + for lockfile in PNPM_LOCK_TEST_CASES +] + +[ + use_repo( + npm, + lockfile.replace(".yaml", ""), + ) + for lockfile in PNPM_LOCK_TEST_CASES +] diff --git a/e2e/pnpm_lockfiles/README.md b/e2e/pnpm_lockfiles/README.md index 1848438e1..3d9276e1c 100644 --- a/e2e/pnpm_lockfiles/README.md +++ b/e2e/pnpm_lockfiles/README.md @@ -15,3 +15,9 @@ TODO: No :node_modules/\* targets are generated for aliases to npm packages. Note: _sometimes_ fails to install with pnpm9 + +## pnpm lockfile edge cases + +Unique test cases hard to cover with normal pnpm workspaces + package.json. Each +test case is a pnpm-lock.yaml with a unique filename, see cases/BUILD for how the test +cases run on each of those lockfiles. diff --git a/e2e/pnpm_lockfiles/WORKSPACE b/e2e/pnpm_lockfiles/WORKSPACE index a5dbb520b..309e979b4 100644 --- a/e2e/pnpm_lockfiles/WORKSPACE +++ b/e2e/pnpm_lockfiles/WORKSPACE @@ -3,6 +3,7 @@ local_repository( path = "../..", ) +# The same primary lockfile across all versions PNPM_LOCK_VERSIONS = [ "v54", "v60", @@ -10,6 +11,11 @@ PNPM_LOCK_VERSIONS = [ "v90", ] +# Lockfiles with unique test cases +PNPM_LOCK_TEST_CASES = [ + "tarball-no-url-v54.yaml", +] + load("@aspect_rules_js//js:repositories.bzl", "rules_js_dependencies") rules_js_dependencies() @@ -45,3 +51,16 @@ npm_repositories_v60() npm_repositories_v61() npm_repositories_v90() + +[ + npm_translate_lock( + name = lockfile.replace(".yaml", ""), + pnpm_lock = "//cases:%s" % lockfile, + verify_node_modules_ignored = "//:.bazelignore", + ) + for lockfile in PNPM_LOCK_TEST_CASES +] + +load("@tarball-no-url-v54//:repositories.bzl", npm_repositories_tarball_no_url_v54 = "npm_repositories") + +npm_repositories_tarball_no_url_v54() diff --git a/e2e/pnpm_lockfiles/cases/BUILD.bazel b/e2e/pnpm_lockfiles/cases/BUILD.bazel new file mode 100644 index 000000000..582b1c5ce --- /dev/null +++ b/e2e/pnpm_lockfiles/cases/BUILD.bazel @@ -0,0 +1,14 @@ +load("@bazel_skylib//rules:build_test.bzl", "build_test") +load("@tarball-no-url-v54//:defs.bzl", tarball_no_url_link_all = "npm_link_all_packages") + +tarball_no_url_link_all(name = "tarball-no-url-v54-modules") + +build_test( + name = "tarball-no-url-v54", + targets = [ + ":tarball-no-url-v54-modules", + ":tarball-no-url-v54-modules/@aspect-build/a", + ], +) + +exports_files(glob(["*.yaml"])) diff --git a/e2e/pnpm_lockfiles/cases/package.json b/e2e/pnpm_lockfiles/cases/package.json new file mode 100644 index 000000000..0967ef424 --- /dev/null +++ b/e2e/pnpm_lockfiles/cases/package.json @@ -0,0 +1 @@ +{} diff --git a/e2e/pnpm_lockfiles/cases/pnpm-workspace.yaml b/e2e/pnpm_lockfiles/cases/pnpm-workspace.yaml new file mode 100644 index 000000000..2cce0eb74 --- /dev/null +++ b/e2e/pnpm_lockfiles/cases/pnpm-workspace.yaml @@ -0,0 +1,2 @@ +packages: + - '.' diff --git a/e2e/pnpm_lockfiles/cases/tarball-no-url-v54.yaml b/e2e/pnpm_lockfiles/cases/tarball-no-url-v54.yaml new file mode 100644 index 000000000..e81fcbb86 --- /dev/null +++ b/e2e/pnpm_lockfiles/cases/tarball-no-url-v54.yaml @@ -0,0 +1,19 @@ +lockfileVersion: 5.4 + +importers: + .: + specifiers: + '@aspect-build/a': 1.0.0 + dependencies: + '@aspect-build/a': 1.0.0 + +packages: + # This was found in https://github.com/aspect-build/rules_js/blob/795138561e878c09f0b62a25572358d64960d732/e2e/npm_translate_lock_auth/pnpm-lock.yaml + /@aspect-build/a/1.0.0: + resolution: + { + integrity: sha512-MYeL/yqAPYJXOnnSEOAdtQSu/8tiifFtKN6Jg/rgpKRqxKL8NVsQXrX9H2dlJ4mS23pu7VS0+i9mZNiRoCUYwg==, + tarball: '@aspect-test/a/-/a-1.0.0.tgz', + } + hasBin: true + dev: false diff --git a/npm/private/test/utils_tests.bzl b/npm/private/test/utils_tests.bzl index 423f0643b..a04f7a57a 100644 --- a/npm/private/test/utils_tests.bzl +++ b/npm/private/test/utils_tests.bzl @@ -92,32 +92,32 @@ def test_npm_registry_url(ctx): asserts.equals( env, "https://default", - utils_test.npm_registry_url("a", {}, "https://default"), + utils.npm_registry_url("a", {}, "https://default"), ) asserts.equals( env, "http://default", - utils_test.npm_registry_url("a", {}, "http://default"), + utils.npm_registry_url("a", {}, "http://default"), ) asserts.equals( env, "//default", - utils_test.npm_registry_url("a", {}, "//default"), + utils.npm_registry_url("a", {}, "//default"), ) asserts.equals( env, "https://default", - utils_test.npm_registry_url("@a/b", {}, "https://default"), + utils.npm_registry_url("@a/b", {}, "https://default"), ) asserts.equals( env, "https://default", - utils_test.npm_registry_url("@a/b", {"@ab": "not me"}, "https://default"), + utils.npm_registry_url("@a/b", {"@ab": "not me"}, "https://default"), ) asserts.equals( env, "https://scoped-registry", - utils_test.npm_registry_url("@a/b", {"@a": "https://scoped-registry"}, "https://default"), + utils.npm_registry_url("@a/b", {"@a": "https://scoped-registry"}, "https://default"), ) return unittest.end(env) diff --git a/npm/private/utils.bzl b/npm/private/utils.bzl index b517e9e30..1ef6821d6 100644 --- a/npm/private/utils.bzl +++ b/npm/private/utils.bzl @@ -741,6 +741,7 @@ utils = struct( links_repo_suffix = "__links", # Output group name for the package directory of a linked npm package package_directory_output_group = "package_directory", + npm_registry_url = _npm_registry_url, npm_registry_download_url = _npm_registry_download_url, is_git_repository_url = _is_git_repository_url, to_registry_url = _to_registry_url, @@ -756,7 +757,6 @@ utils = struct( # Exported only to be tested utils_test = struct( - npm_registry_url = _npm_registry_url, parse_package_name = _parse_package_name, strip_v5_peer_dep_or_patched_version = _strip_v5_peer_dep_or_patched_version, )