From 28e8e9804363406cb41ee5c34c290713f46f7a99 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Wed, 15 May 2024 11:23:31 -0700 Subject: [PATCH] fix: fail lockfile translation for pnpm pnpm v9+ when no onlyBuildDependencies specified (#1734) --- npm/private/npm_translate_lock.bzl | 2 ++ npm/private/npm_translate_lock_helpers.bzl | 26 ++++++++++++++++++++++ npm/private/npm_translate_lock_state.bzl | 8 ++++++- npm/private/test/parse_pnpm_lock_tests.bzl | 2 ++ npm/private/utils.bzl | 2 +- 5 files changed, 38 insertions(+), 2 deletions(-) diff --git a/npm/private/npm_translate_lock.bzl b/npm/private/npm_translate_lock.bzl index b7a07c348..84b1a71fc 100644 --- a/npm/private/npm_translate_lock.bzl +++ b/npm/private/npm_translate_lock.bzl @@ -114,6 +114,8 @@ See https://github.com/aspect-build/rules_js/issues/1445 helpers.verify_patches(rctx, state) + helpers.verify_lifecycle_hooks_specified(rctx, state) + rctx.report_progress("Translating {}".format(state.label_store.relative_path("pnpm_lock"))) importers, packages = translate_to_transitive_closure( diff --git a/npm/private/npm_translate_lock_helpers.bzl b/npm/private/npm_translate_lock_helpers.bzl index 2e83d4bda..fe78ee7fd 100644 --- a/npm/private/npm_translate_lock_helpers.bzl +++ b/npm/private/npm_translate_lock_helpers.bzl @@ -564,6 +564,31 @@ def _normalize_bazelignore(lines): result.append(line.rstrip(strip_trailing_chars)) return result +################################################################################ +def _verify_lifecycle_hooks_specified(_, state): + # lockfiles <9.0 specify the `requiresBuild` flag in the lockfile. + # + # lockfiles >=9.0 no longer specify if packages have lifecycle hooks, + # and declaration of hooks must be done manually in the `pnpm.onlyBuiltDependencies`. + if state.lockfile_version() < 9.0: + return + + if state.only_built_dependencies() == None: + fail("""\ +ERROR: pnpm.onlyBuiltDependencies required in pnpm workspace root package.json when using pnpm v9 or later + +As of pnpm v9, the lockfile no longer specifies if packages have lifecycle hooks. + +Packages that rules_js should generate lifecycle hook actions for must now be declared in +pnpm.onlyBuiltDependencies in the pnpm workspace root package.json. See +https://pnpm.io/package_json#pnpmonlybuiltdependencies for more information. + +Prior to pnpm v9, rules_js keyed off of the requiresBuild attribute in the pnpm lock +file to determine if a lifecycle hook action should be generated for an npm package. +See [pnpm #7707](https://github.com/pnpm/pnpm/issues/7707) for the reasons that pnpm +removed the requiredBuild attribute from the lockfile in v9. +""") + ################################################################################ def _verify_patches(rctx, state): if rctx.attr.verify_patches and rctx.attr.patches != None: @@ -598,6 +623,7 @@ helpers = struct( link_package = _link_package, to_apparent_repo_name = _to_apparent_repo_name, verify_node_modules_ignored = _verify_node_modules_ignored, + verify_lifecycle_hooks_specified = _verify_lifecycle_hooks_specified, verify_patches = _verify_patches, ) diff --git a/npm/private/npm_translate_lock_state.bzl b/npm/private/npm_translate_lock_state.bzl index c2776a899..28e941567 100644 --- a/npm/private/npm_translate_lock_state.bzl +++ b/npm/private/npm_translate_lock_state.bzl @@ -478,6 +478,7 @@ def _load_lockfile(priv, rctx, _, label_store): importers = {} packages = {} patched_dependencies = {} + lock_version = None lock_parse_err = None yq_args = [ @@ -489,8 +490,9 @@ def _load_lockfile(priv, rctx, _, label_store): if result.return_code: lock_parse_err = "failed to parse pnpm lock file with yq. '{}' exited with {}: \nSTDOUT:\n{}\nSTDERR:\n{}".format(" ".join(yq_args), result.return_code, result.stdout, result.stderr) else: - importers, packages, patched_dependencies, lock_parse_err = utils.parse_pnpm_lock_json(result.stdout if result.stdout != "null" else None) # NB: yq will return the string "null" if the yaml file is empty + importers, packages, patched_dependencies, lock_version, lock_parse_err = utils.parse_pnpm_lock_json(result.stdout if result.stdout != "null" else None) # NB: yq will return the string "null" if the yaml file is empty + priv["lock_version"] = lock_version priv["importers"] = importers priv["packages"] = packages priv["patched_dependencies"] = patched_dependencies @@ -523,6 +525,9 @@ def _default_registry(priv): def _link_workspace(priv): return priv["link_workspace"] +def _lockfile_version(priv): + return priv["lock_version"] + def _importers(priv): return priv["importers"] @@ -583,6 +588,7 @@ def _new(rctx_name, rctx, attr, bzlmod): should_update_pnpm_lock = lambda: _should_update_pnpm_lock(priv), default_registry = lambda: _default_registry(priv), link_workspace = lambda: _link_workspace(priv), + lockfile_version = lambda: _lockfile_version(priv), importers = lambda: _importers(priv), packages = lambda: _packages(priv), patched_dependencies = lambda: _patched_dependencies(priv), diff --git a/npm/private/test/parse_pnpm_lock_tests.bzl b/npm/private/test/parse_pnpm_lock_tests.bzl index 9221d3181..e35a89c62 100644 --- a/npm/private/test/parse_pnpm_lock_tests.bzl +++ b/npm/private/test/parse_pnpm_lock_tests.bzl @@ -69,6 +69,7 @@ def _parse_lockfile_v5_test_impl(ctx): }, }, {}, + 5.4, None, ) @@ -130,6 +131,7 @@ def _parse_lockfile_v6_test_impl(ctx): }, }, {}, + 6.0, None, ) diff --git a/npm/private/utils.bzl b/npm/private/utils.bzl index 1f34f3b75..f310c14df 100644 --- a/npm/private/utils.bzl +++ b/npm/private/utils.bzl @@ -291,7 +291,7 @@ def _parse_pnpm_lock_common(parsed, err): patched_dependencies = parsed.get("patchedDependencies", {}) - return importers, packages, patched_dependencies, None + return importers, packages, patched_dependencies, lockfile_version, None def _assert_lockfile_version(version, testonly = False): if type(version) != type(1.0):