Skip to content

Commit

Permalink
fix: fail lockfile translation for pnpm pnpm v9+ when no onlyBuildDep…
Browse files Browse the repository at this point in the history
…endencies specified (aspect-build#1734)
  • Loading branch information
jbedard committed May 16, 2024
1 parent bdc47f4 commit 3fbc509
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 2 deletions.
2 changes: 2 additions & 0 deletions npm/private/npm_translate_lock.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
26 changes: 26 additions & 0 deletions npm/private/npm_translate_lock_helpers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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,
)

Expand Down
8 changes: 7 additions & 1 deletion npm/private/npm_translate_lock_state.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,7 @@ def _load_lockfile(priv, rctx, _, label_store):
importers = {}
packages = {}
patched_dependencies = {}
lock_version = None
lock_parse_err = None

yq_args = [
Expand All @@ -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
Expand Down Expand Up @@ -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"]

Expand Down Expand Up @@ -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),
Expand Down
2 changes: 2 additions & 0 deletions npm/private/test/parse_pnpm_lock_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ def _parse_lockfile_v5_test_impl(ctx):
},
},
{},
5.4,
None,
)

Expand Down Expand Up @@ -130,6 +131,7 @@ def _parse_lockfile_v6_test_impl(ctx):
},
},
{},
6.0,
None,
)

Expand Down
2 changes: 1 addition & 1 deletion npm/private/utils.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit 3fbc509

Please sign in to comment.