From 4db6bd05f9bb0fe69b9b9d3ac4cc42f79f56e1a8 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Tue, 14 May 2024 14:23:08 -0700 Subject: [PATCH] feat: support pnpm.onlyBuiltDependencies (#1723) --- docs/pnpm.md | 14 +++++++-- e2e/pnpm_lockfiles/.bazelignore | 12 ++------ e2e/pnpm_lockfiles/BUILD.bazel | 2 +- e2e/pnpm_lockfiles/MODULE.bazel | 5 +--- e2e/pnpm_lockfiles/WORKSPACE | 5 +--- e2e/pnpm_lockfiles/{ => base}/.npmrc | 0 e2e/pnpm_lockfiles/{ => base}/package.json | 0 .../{ => base}/patched-dependencies-test.js | 0 .../patches/meaning-of-life@1.0.0-pnpm.patch | 0 e2e/pnpm_lockfiles/base/pnpm-workspace.yaml | 3 ++ e2e/pnpm_lockfiles/lockfile-test.bzl | 30 +++++++++---------- e2e/pnpm_lockfiles/pnpm-workspace.yaml | 3 -- e2e/pnpm_lockfiles/setup.sh | 6 ++-- e2e/pnpm_lockfiles/v54/package.json | 1 + e2e/pnpm_lockfiles/v54/patches | 2 +- e2e/pnpm_lockfiles/v54/pnpm-lock.yaml | 6 ++-- e2e/pnpm_lockfiles/v60/package.json | 1 + e2e/pnpm_lockfiles/v60/patches | 2 +- e2e/pnpm_lockfiles/v60/pnpm-lock.yaml | 6 ++-- e2e/pnpm_lockfiles/v61/package.json | 1 + e2e/pnpm_lockfiles/v61/patches | 2 +- e2e/pnpm_lockfiles/v61/pnpm-lock.yaml | 6 ++-- npm/extensions.bzl | 1 + npm/private/npm_translate_lock.bzl | 1 + npm/private/npm_translate_lock_generate.bzl | 4 +-- npm/private/npm_translate_lock_helpers.bzl | 5 ++-- npm/private/npm_translate_lock_state.bzl | 13 ++++++-- 27 files changed, 71 insertions(+), 60 deletions(-) rename e2e/pnpm_lockfiles/{ => base}/.npmrc (100%) rename e2e/pnpm_lockfiles/{ => base}/package.json (100%) rename e2e/pnpm_lockfiles/{ => base}/patched-dependencies-test.js (100%) rename e2e/pnpm_lockfiles/{ => base}/patches/meaning-of-life@1.0.0-pnpm.patch (100%) create mode 100644 e2e/pnpm_lockfiles/base/pnpm-workspace.yaml delete mode 100644 e2e/pnpm_lockfiles/pnpm-workspace.yaml create mode 120000 e2e/pnpm_lockfiles/v54/package.json create mode 120000 e2e/pnpm_lockfiles/v60/package.json create mode 120000 e2e/pnpm_lockfiles/v61/package.json diff --git a/docs/pnpm.md b/docs/pnpm.md index 7abb2f73d..1baba851f 100644 --- a/docs/pnpm.md +++ b/docs/pnpm.md @@ -179,11 +179,13 @@ See our [blog post](https://blog.aspect.dev/easier-merges-on-lockfiles) for a lo First, mark the `npm_translate_lock_` file (with `` replaced with the hash generated in your workspace) to use a custom custom merge driver, in this example named `ours`: + ``` .aspect/rules/external_repository_action_cache/npm_translate_lock_= merge=ours ``` Second, developers must define the `ours` custom merge driver in their git configuration to always accept local change: + ``` git config --global merge.ours.driver true ``` @@ -258,8 +260,16 @@ npm packages have "lifecycle scripts" such as `postinstall` which are documented We refer to these as "lifecycle hooks". -> You can disable this feature completely by setting all packages to have no hooks, using -> `lifecycle_hooks = { "*": [] }` in `npm_translate_lock`. +The lifecycle hooks of a package are determined by the `package.json` [`pnpm.onlyBuiltDependencies` attribute](https://pnpm.io/package_json#pnpmonlybuiltdependencies). + +If `pnpm.onlyBuiltDependencies` is unspecified `npm_translate_lock` will fallback to the legacy pnpm lockfile `requiresBuild` attribute. +This attribute is only available in pnpm _before_ v9, see [pnpm #7707](https://github.com/pnpm/pnpm/issues/7707) for reasons why this attribute was removed. + +When a package has lifecycle hooks the `lifecycle_*` attributes are applied to filter which hooks are run and how they are run. + +For example, you can restrict lifecycle hooks across all packages to only run `postinstall`: + +> `lifecycle_hooks = { "*": ["postinstall"] }` in `npm_translate_lock`. Because rules_js models the execution of these hooks as build actions, rather than repository rules, the result can be stored in the remote cache and shared between developers. diff --git a/e2e/pnpm_lockfiles/.bazelignore b/e2e/pnpm_lockfiles/.bazelignore index 677efa370..12f533568 100644 --- a/e2e/pnpm_lockfiles/.bazelignore +++ b/e2e/pnpm_lockfiles/.bazelignore @@ -1,13 +1,7 @@ node_modules/ +projects/a/node_modules +projects/b/node_modules +projects/c/node_modules v54/node_modules/ v60/node_modules/ v61/node_modules/ -v54/projects/a/node_modules -v54/projects/b/node_modules -v54/projects/c/node_modules -v60/projects/a/node_modules -v60/projects/b/node_modules -v60/projects/c/node_modules -v61/projects/a/node_modules -v61/projects/b/node_modules -v61/projects/c/node_modules diff --git a/e2e/pnpm_lockfiles/BUILD.bazel b/e2e/pnpm_lockfiles/BUILD.bazel index ccaeaa9f0..fa2c989fe 100644 --- a/e2e/pnpm_lockfiles/BUILD.bazel +++ b/e2e/pnpm_lockfiles/BUILD.bazel @@ -1 +1 @@ -exports_files(["patched-dependencies-test.js"]) +exports_files(["base/patched-dependencies-test.js"]) diff --git a/e2e/pnpm_lockfiles/MODULE.bazel b/e2e/pnpm_lockfiles/MODULE.bazel index 86d29aa8d..2dce334fc 100644 --- a/e2e/pnpm_lockfiles/MODULE.bazel +++ b/e2e/pnpm_lockfiles/MODULE.bazel @@ -30,12 +30,9 @@ npm = use_extension( npm.npm_translate_lock( name = "lock-%s" % version, data = [ - "//:package.json", + "//%s:package.json" % version, "//%s:patches/meaning-of-life@1.0.0-pnpm.patch" % version, - "//:pnpm-workspace.yaml", ], - npmrc = "//:.npmrc", - patch_args = {"*": ["-p1"]}, pnpm_lock = "//%s:pnpm-lock.yaml" % version, verify_node_modules_ignored = "//:.bazelignore", ) diff --git a/e2e/pnpm_lockfiles/WORKSPACE b/e2e/pnpm_lockfiles/WORKSPACE index e69fe4278..366876927 100644 --- a/e2e/pnpm_lockfiles/WORKSPACE +++ b/e2e/pnpm_lockfiles/WORKSPACE @@ -17,12 +17,9 @@ load("@aspect_rules_js//npm:repositories.bzl", "npm_translate_lock") npm_translate_lock( name = "lock-%s" % version, data = [ - "//:package.json", + "//%s:package.json" % version, "//%s:patches/meaning-of-life@1.0.0-pnpm.patch" % version, - "//:pnpm-workspace.yaml", ], - npmrc = "//:.npmrc", - patch_args = {"*": ["-p1"]}, pnpm_lock = "//%s:pnpm-lock.yaml" % version, verify_node_modules_ignored = "//:.bazelignore", ) diff --git a/e2e/pnpm_lockfiles/.npmrc b/e2e/pnpm_lockfiles/base/.npmrc similarity index 100% rename from e2e/pnpm_lockfiles/.npmrc rename to e2e/pnpm_lockfiles/base/.npmrc diff --git a/e2e/pnpm_lockfiles/package.json b/e2e/pnpm_lockfiles/base/package.json similarity index 100% rename from e2e/pnpm_lockfiles/package.json rename to e2e/pnpm_lockfiles/base/package.json diff --git a/e2e/pnpm_lockfiles/patched-dependencies-test.js b/e2e/pnpm_lockfiles/base/patched-dependencies-test.js similarity index 100% rename from e2e/pnpm_lockfiles/patched-dependencies-test.js rename to e2e/pnpm_lockfiles/base/patched-dependencies-test.js diff --git a/e2e/pnpm_lockfiles/patches/meaning-of-life@1.0.0-pnpm.patch b/e2e/pnpm_lockfiles/base/patches/meaning-of-life@1.0.0-pnpm.patch similarity index 100% rename from e2e/pnpm_lockfiles/patches/meaning-of-life@1.0.0-pnpm.patch rename to e2e/pnpm_lockfiles/base/patches/meaning-of-life@1.0.0-pnpm.patch diff --git a/e2e/pnpm_lockfiles/base/pnpm-workspace.yaml b/e2e/pnpm_lockfiles/base/pnpm-workspace.yaml new file mode 100644 index 000000000..07c5608e3 --- /dev/null +++ b/e2e/pnpm_lockfiles/base/pnpm-workspace.yaml @@ -0,0 +1,3 @@ +packages: + - '.' + - '../projects/*' diff --git a/e2e/pnpm_lockfiles/lockfile-test.bzl b/e2e/pnpm_lockfiles/lockfile-test.bzl index deb47fbcb..a87282940 100644 --- a/e2e/pnpm_lockfiles/lockfile-test.bzl +++ b/e2e/pnpm_lockfiles/lockfile-test.bzl @@ -2,9 +2,9 @@ Test utils for lockfiles """ -load("@bazel_skylib//rules:build_test.bzl", "build_test") -load("@aspect_rules_js//js:defs.bzl", "js_test") load("@aspect_bazel_lib//lib:copy_file.bzl", "copy_file") +load("@aspect_rules_js//js:defs.bzl", "js_test") +load("@bazel_skylib//rules:build_test.bzl", "build_test") def lockfile_test(name = "lockfile", node_modules = "node_modules"): """ @@ -15,21 +15,19 @@ def lockfile_test(name = "lockfile", node_modules = "node_modules"): node_modules: name of the tested 'node_modules' directory """ - # TODO: lockfile v53 does not support patching? - if native.package_name() != "v53": - copy_file( - name = "copy-tests", - src = "//:patched-dependencies-test.js", - out = "patched-dependencies-test.js", - ) + copy_file( + name = "copy-tests", + src = "//:base/patched-dependencies-test.js", + out = "patched-dependencies-test.js", + ) - js_test( - name = "patch-test", - data = [ - ":%s/meaning-of-life" % node_modules, - ], - entry_point = "patched-dependencies-test.js", - ) + js_test( + name = "patch-test", + data = [ + ":%s/meaning-of-life" % node_modules, + ], + entry_point = "patched-dependencies-test.js", + ) build_test( name = name, diff --git a/e2e/pnpm_lockfiles/pnpm-workspace.yaml b/e2e/pnpm_lockfiles/pnpm-workspace.yaml deleted file mode 100644 index ed1fda246..000000000 --- a/e2e/pnpm_lockfiles/pnpm-workspace.yaml +++ /dev/null @@ -1,3 +0,0 @@ -packages: - - '.' - - 'projects/*' diff --git a/e2e/pnpm_lockfiles/setup.sh b/e2e/pnpm_lockfiles/setup.sh index f87fccbb0..3e25fa947 100755 --- a/e2e/pnpm_lockfiles/setup.sh +++ b/e2e/pnpm_lockfiles/setup.sh @@ -4,9 +4,9 @@ # 6.0 - pnpm v8.0.0 bumped the lockfile version to 6.0; this included breaking changes # 6.1 - pnpm v8.6.0 bumped the lockfile version to 6.1 -mv v54/pnpm-lock.yaml . && npx pnpm@^7.0 install --lockfile-only && mv pnpm-lock.yaml v54/ +mv v54/pnpm-lock.yaml base && pushd base && npx pnpm@^7.0 install --lockfile-only && mv pnpm-lock.yaml ../v54/ && popd # pnpm v8.0.0 bumped the lockfile version to 6.0, 8.6.0 bumped it to 6.1 which was then reverted to 6.0 # while still presenting minor differences from <8.6.0. -mv v60/pnpm-lock.yaml . && npx pnpm@8.5.1 install --lockfile-only && mv pnpm-lock.yaml v60/ -mv v61/pnpm-lock.yaml . && npx pnpm@8.6.0 install --lockfile-only && mv pnpm-lock.yaml v61/ +mv v60/pnpm-lock.yaml base && pushd base && npx pnpm@8.5.1 install --lockfile-only && mv pnpm-lock.yaml ../v60/ && popd +mv v61/pnpm-lock.yaml base && pushd base && npx pnpm@8.6.0 install --lockfile-only && mv pnpm-lock.yaml ../v61/ && popd diff --git a/e2e/pnpm_lockfiles/v54/package.json b/e2e/pnpm_lockfiles/v54/package.json new file mode 120000 index 000000000..9846ac29f --- /dev/null +++ b/e2e/pnpm_lockfiles/v54/package.json @@ -0,0 +1 @@ +../base/package.json \ No newline at end of file diff --git a/e2e/pnpm_lockfiles/v54/patches b/e2e/pnpm_lockfiles/v54/patches index b263e782b..143935d1f 120000 --- a/e2e/pnpm_lockfiles/v54/patches +++ b/e2e/pnpm_lockfiles/v54/patches @@ -1 +1 @@ -../patches \ No newline at end of file +../base/patches \ No newline at end of file diff --git a/e2e/pnpm_lockfiles/v54/pnpm-lock.yaml b/e2e/pnpm_lockfiles/v54/pnpm-lock.yaml index 3f461e5fb..fa1d2d22f 100644 --- a/e2e/pnpm_lockfiles/v54/pnpm-lock.yaml +++ b/e2e/pnpm_lockfiles/v54/pnpm-lock.yaml @@ -27,16 +27,16 @@ importers: '@aspect-test/b': 5.0.2 '@types/node': registry.npmjs.org/@types/node/16.18.11 - projects/a: + ../projects/a: specifiers: {} - projects/b: + ../projects/b: specifiers: '@scoped/a': workspace:* dependencies: '@scoped/a': link:../a - projects/c: + ../projects/c: specifiers: '@scoped/a': link:../a dependencies: diff --git a/e2e/pnpm_lockfiles/v60/package.json b/e2e/pnpm_lockfiles/v60/package.json new file mode 120000 index 000000000..9846ac29f --- /dev/null +++ b/e2e/pnpm_lockfiles/v60/package.json @@ -0,0 +1 @@ +../base/package.json \ No newline at end of file diff --git a/e2e/pnpm_lockfiles/v60/patches b/e2e/pnpm_lockfiles/v60/patches index b263e782b..143935d1f 120000 --- a/e2e/pnpm_lockfiles/v60/patches +++ b/e2e/pnpm_lockfiles/v60/patches @@ -1 +1 @@ -../patches \ No newline at end of file +../base/patches \ No newline at end of file diff --git a/e2e/pnpm_lockfiles/v60/pnpm-lock.yaml b/e2e/pnpm_lockfiles/v60/pnpm-lock.yaml index 5d4b1224a..15b032282 100644 --- a/e2e/pnpm_lockfiles/v60/pnpm-lock.yaml +++ b/e2e/pnpm_lockfiles/v60/pnpm-lock.yaml @@ -36,15 +36,15 @@ importers: specifier: ~16.18.11 version: registry.npmjs.org/@types/node@16.18.11 - projects/a: {} + ../projects/a: {} - projects/b: + ../projects/b: dependencies: '@scoped/a': specifier: workspace:* version: link:../a - projects/c: + ../projects/c: dependencies: '@scoped/a': specifier: link:../a diff --git a/e2e/pnpm_lockfiles/v61/package.json b/e2e/pnpm_lockfiles/v61/package.json new file mode 120000 index 000000000..9846ac29f --- /dev/null +++ b/e2e/pnpm_lockfiles/v61/package.json @@ -0,0 +1 @@ +../base/package.json \ No newline at end of file diff --git a/e2e/pnpm_lockfiles/v61/patches b/e2e/pnpm_lockfiles/v61/patches index b263e782b..143935d1f 120000 --- a/e2e/pnpm_lockfiles/v61/patches +++ b/e2e/pnpm_lockfiles/v61/patches @@ -1 +1 @@ -../patches \ No newline at end of file +../base/patches \ No newline at end of file diff --git a/e2e/pnpm_lockfiles/v61/pnpm-lock.yaml b/e2e/pnpm_lockfiles/v61/pnpm-lock.yaml index 677e77723..9e21764c9 100644 --- a/e2e/pnpm_lockfiles/v61/pnpm-lock.yaml +++ b/e2e/pnpm_lockfiles/v61/pnpm-lock.yaml @@ -40,15 +40,15 @@ importers: specifier: ~16.18.11 version: registry.npmjs.org/@types/node@16.18.11 - projects/a: {} + ../projects/a: {} - projects/b: + ../projects/b: dependencies: '@scoped/a': specifier: workspace:* version: link:../a - projects/c: + ../projects/c: dependencies: '@scoped/a': specifier: link:../a diff --git a/npm/extensions.bzl b/npm/extensions.bzl index 925c02613..2b15f526a 100644 --- a/npm/extensions.bzl +++ b/npm/extensions.bzl @@ -119,6 +119,7 @@ WARNING: Cannot determine home directory in order to load home `.npmrc` file in importers = importers, packages = packages, patched_dependencies = state.patched_dependencies(), + only_built_dependencies = state.only_built_dependencies(), root_package = attr.pnpm_lock.package, rctx_name = attr.name, attr = attr, diff --git a/npm/private/npm_translate_lock.bzl b/npm/private/npm_translate_lock.bzl index f58df9b9d..d6d584351 100644 --- a/npm/private/npm_translate_lock.bzl +++ b/npm/private/npm_translate_lock.bzl @@ -132,6 +132,7 @@ See https://github.com/aspect-build/rules_js/issues/1445 importers, packages, state.patched_dependencies(), + state.only_built_dependencies(), state.root_package(), state.default_registry(), state.npm_registries(), diff --git a/npm/private/npm_translate_lock_generate.bzl b/npm/private/npm_translate_lock_generate.bzl index a889a5eeb..d6050931b 100644 --- a/npm/private/npm_translate_lock_generate.bzl +++ b/npm/private/npm_translate_lock_generate.bzl @@ -80,11 +80,11 @@ _PACKAGE_JSON_BZL_FILENAME = "package_json.bzl" _RESOLVED_JSON_FILENAME = "resolved.json" # buildifier: disable=function-docstring -def generate_repository_files(rctx, pnpm_lock_label, importers, packages, patched_dependencies, root_package, default_registry, npm_registries, npm_auth, link_workspace): +def generate_repository_files(rctx, pnpm_lock_label, importers, packages, patched_dependencies, only_built_dependencies, root_package, default_registry, npm_registries, npm_auth, link_workspace): # empty line after bzl docstring since buildifier expects this if this file is vendored in generated_by_prefix = "\"\"\"@generated by npm_translate_lock(name = \"{}\", pnpm_lock = \"{}\")\"\"\"\n".format(helpers.to_apparent_repo_name(rctx.name), str(pnpm_lock_label)) - npm_imports = helpers.get_npm_imports(importers, packages, patched_dependencies, root_package, rctx.name, rctx.attr, rctx.attr.lifecycle_hooks, rctx.attr.lifecycle_hooks_execution_requirements, rctx.attr.lifecycle_hooks_use_default_shell_env, npm_registries, default_registry, npm_auth) + npm_imports = helpers.get_npm_imports(importers, packages, patched_dependencies, only_built_dependencies, root_package, rctx.name, rctx.attr, rctx.attr.lifecycle_hooks, rctx.attr.lifecycle_hooks_execution_requirements, rctx.attr.lifecycle_hooks_use_default_shell_env, npm_registries, default_registry, npm_auth) link_packages = [helpers.link_package(root_package, import_path) for import_path in importers.keys()] diff --git a/npm/private/npm_translate_lock_helpers.bzl b/npm/private/npm_translate_lock_helpers.bzl index f2265cb94..3d0ebaee1 100644 --- a/npm/private/npm_translate_lock_helpers.bzl +++ b/npm/private/npm_translate_lock_helpers.bzl @@ -236,7 +236,7 @@ def _select_npm_auth(url, npm_auth): return npm_auth_bearer, npm_auth_basic, npm_auth_username, npm_auth_password ################################################################################ -def _get_npm_imports(importers, packages, patched_dependencies, root_package, rctx_name, attr, all_lifecycle_hooks, all_lifecycle_hooks_execution_requirements, all_lifecycle_hooks_use_default_shell_env, registries, default_registry, npm_auth): +def _get_npm_imports(importers, packages, patched_dependencies, only_built_dependencies, root_package, rctx_name, attr, all_lifecycle_hooks, all_lifecycle_hooks_execution_requirements, all_lifecycle_hooks_use_default_shell_env, registries, default_registry, npm_auth): "Converts packages from the lockfile to a struct of attributes for npm_import" if attr.prod and attr.dev: fail("prod and dev attributes cannot both be set to true") @@ -393,7 +393,8 @@ ERROR: can not apply both `pnpm.patchedDependencies` and `npm_translate_lock(pat elif name not in link_packages[public_hoist_package]: link_packages[public_hoist_package].append(name) - if requires_build: + run_lifecycle_hooks = name in only_built_dependencies if only_built_dependencies != None else requires_build + if run_lifecycle_hooks: lifecycle_hooks, _ = _gather_values_from_matching_names(False, all_lifecycle_hooks, "*", name, friendly_name, unfriendly_name) lifecycle_hooks_env, _ = _gather_values_from_matching_names(True, attr.lifecycle_hooks_envs, "*", name, friendly_name, unfriendly_name) lifecycle_hooks_execution_requirements, _ = _gather_values_from_matching_names(False, all_lifecycle_hooks_execution_requirements, "*", name, friendly_name, unfriendly_name) diff --git a/npm/private/npm_translate_lock_state.bzl b/npm/private/npm_translate_lock_state.bzl index d3a2d5b00..c2776a899 100644 --- a/npm/private/npm_translate_lock_state.bzl +++ b/npm/private/npm_translate_lock_state.bzl @@ -37,8 +37,6 @@ WARNING: `update_pnpm_lock` attribute in `npm_translate_lock(name = "{rctx_name} _init_patches_labels(priv, rctx, attr, label_store) - _init_root_package(priv, rctx, attr, label_store) - if _should_update_pnpm_lock(priv) or not attr.pnpm_lock: # labels only needed when updating or bootstrapping the pnpm lock file _init_pnpm_labels(priv, rctx, attr, label_store) @@ -56,6 +54,9 @@ WARNING: `update_pnpm_lock` attribute in `npm_translate_lock(name = "{rctx_name} _load_lockfile(priv, rctx, attr, label_store) _init_patched_dependencies_labels(priv, rctx, attr, label_store) + # May depend on lockfile state + _init_root_package(priv, rctx, attr, label_store) + if _should_update_pnpm_lock(priv): _init_importer_labels(priv, label_store) @@ -531,6 +532,9 @@ def _packages(priv): def _patched_dependencies(priv): return priv["patched_dependencies"] +def _only_built_dependencies(priv): + return _root_package_json(priv).get("pnpm", {}).get("onlyBuildDependencies", None) + def _num_patches(priv): return priv["num_patches"] @@ -543,6 +547,9 @@ def _npm_auth(priv): def _root_package(priv): return priv["root_package"] +def _root_package_json(priv): + return priv["root_package_json"] + ################################################################################ def _new(rctx_name, rctx, attr, bzlmod): label_store = repository_label_store.new(rctx.path) @@ -564,6 +571,7 @@ def _new(rctx_name, rctx, attr, bzlmod): "npm_registries": {}, "packages": {}, "root_package": None, + "root_package_json": {}, "patched_dependencies": {}, "should_update_pnpm_lock": should_update_pnpm_lock, } @@ -578,6 +586,7 @@ def _new(rctx_name, rctx, attr, bzlmod): importers = lambda: _importers(priv), packages = lambda: _packages(priv), patched_dependencies = lambda: _patched_dependencies(priv), + only_built_dependencies = lambda: _only_built_dependencies(priv), npm_registries = lambda: _npm_registries(priv), npm_auth = lambda: _npm_auth(priv), num_patches = lambda: _num_patches(priv),