Skip to content

Commit

Permalink
feat: fail if any npm_translate_lock patches are not used (#730)
Browse files Browse the repository at this point in the history 
Check that all patches files specified were used; this is a defense-in-depth since it is too easy to make a type in the patches keys or for a dep to change both of with could result in a patch file being silently ignored.
  • Loading branch information
@gregmagolan
gregmagolan committed Dec 20, 2022
1 parent 5dfce6d commit 3bbb4c0
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 15 deletions.
5 changes: 3 additions & 2 deletions npm/extensions.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ load("//npm:npm_import.bzl", "npm_import", "npm_translate_lock")
load("//npm/private:transitive_closure.bzl", "translate_to_transitive_closure")

def _extension_impl(module_ctx):
npm_translate_lock_name = "npm"
for mod in module_ctx.modules:
for attr in mod.tags.npm_translate_lock:
# npm_translate_lock MUST run before parse_pnpm_lock below since it may update
# the pnpm-lock.yaml file when update_pnpm_lock is True.
npm_translate_lock(
name = "npm",
name = npm_translate_lock_name,
pnpm_lock = attr.pnpm_lock,
# TODO: get this working with bzlmod
# update_pnpm_lock = attr.update_pnpm_lock,
Expand All @@ -27,7 +28,7 @@ def _extension_impl(module_ctx):
registries = {}
lock_importers, lock_packages = utils.parse_pnpm_lock(module_ctx.read(attr.pnpm_lock))
importers, packages = translate_to_transitive_closure(lock_importers, lock_packages, attr.prod, attr.dev, attr.no_optional)
imports = npm_translate_lock_helpers.gen_npm_imports(importers, packages, attr.pnpm_lock.package, attr, registries, utils.default_registry())
imports = npm_translate_lock_helpers.gen_npm_imports(importers, packages, attr.pnpm_lock.package, npm_translate_lock_name, attr, registries, utils.default_registry())
for i in imports:
npm_import(
name = i.name,
Expand Down
49 changes: 36 additions & 13 deletions npm/private/npm_translate_lock_generate.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,11 @@ def _is_url(url):

################################################################################
def _gather_values_from_matching_names(additive, keyed_lists, *names):
keys = []
result = []
for name in names:
if name and name in keyed_lists:
keys.append(name)
v = keyed_lists[name]
if additive:
if type(v) == "list":
Expand All @@ -115,7 +117,7 @@ def _gather_values_from_matching_names(additive, keyed_lists, *names):
result = [v]
else:
fail("expected value to be list or string")
return result
return (result, keys)

################################################################################
def _get_npm_auth(npmrc, npmrc_path, environ):
Expand Down Expand Up @@ -245,7 +247,7 @@ def _get_npm_auth(npmrc, npmrc_path, environ):
return (registries, auth)

################################################################################
def _gen_npm_imports(importers, packages, root_package, attr, registries, default_registry):
def _gen_npm_imports(importers, packages, root_package, rctx_name, attr, registries, default_registry):
"Converts packages from the lockfile to a struct of attributes for npm_import"

if attr.prod and attr.dev:
Expand Down Expand Up @@ -278,6 +280,7 @@ def _gen_npm_imports(importers, packages, root_package, attr, registries, defaul
links["packages"] = linked_packages
importer_links[import_path] = links

patches_used = []
result = []
for package, package_info in packages.items():
name = package_info.get("name")
Expand Down Expand Up @@ -334,11 +337,12 @@ def _gen_npm_imports(importers, packages, root_package, attr, registries, defaul
unfriendly_name = None

# gather patches & patch args
patches = _gather_values_from_matching_names(True, attr.patches, name, friendly_name, unfriendly_name)
patch_args = _gather_values_from_matching_names(False, attr.patch_args, "*", name, friendly_name, unfriendly_name)
patches, patches_keys = _gather_values_from_matching_names(True, attr.patches, name, friendly_name, unfriendly_name)
patch_args, _ = _gather_values_from_matching_names(False, attr.patch_args, "*", name, friendly_name, unfriendly_name)
patches_used.extend(patches_keys)

# gather custom postinstalls
custom_postinstalls = _gather_values_from_matching_names(True, attr.custom_postinstalls, name, friendly_name, unfriendly_name)
custom_postinstalls, _ = _gather_values_from_matching_names(True, attr.custom_postinstalls, name, friendly_name, unfriendly_name)
custom_postinstall = " && ".join([c for c in custom_postinstalls if c])

repo_name = "{}__{}".format(attr.name, utils.bazel_name(name, version))
Expand All @@ -354,20 +358,21 @@ def _gen_npm_imports(importers, packages, root_package, attr, registries, defaul
link_packages[links["link_package"]] = link_names

# check if this package should be hoisted via public_hoist_packages
public_hoist_packages = _gather_values_from_matching_names(True, attr.public_hoist_packages, name, friendly_name, unfriendly_name)
public_hoist_packages, _ = _gather_values_from_matching_names(True, attr.public_hoist_packages, name, friendly_name, unfriendly_name)
for public_hoist_package in public_hoist_packages:
if public_hoist_package not in link_packages:
link_packages[public_hoist_package] = [name]
elif name not in link_packages[public_hoist_package]:
link_packages[public_hoist_package].append(name)

lifecycle_hooks = _gather_values_from_matching_names(False, attr.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, attr.lifecycle_hooks_execution_requirements, "*", name, friendly_name, unfriendly_name)
lifecycle_hooks, _ = _gather_values_from_matching_names(False, attr.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, attr.lifecycle_hooks_execution_requirements, "*", name, friendly_name, unfriendly_name)
run_lifecycle_hooks = requires_build and lifecycle_hooks

bins = {}
for bin in _gather_values_from_matching_names(False, attr.bins, "*", name, friendly_name, unfriendly_name):
matching_bins, _ = _gather_values_from_matching_names(False, attr.bins, "*", name, friendly_name, unfriendly_name)
for bin in matching_bins:
key_value = bin.split("=", 1)
if len(key_value) == 2:
bins[key_value[0]] = key_value[1]
Expand Down Expand Up @@ -417,6 +422,23 @@ def _gen_npm_imports(importers, packages, root_package, attr, registries, defaul
package_info = package_info,
))

# Check that all patches files specified were used; this is a defense-in-depth since it is too
# easy to make a type in the patches keys or for a dep to change both of with could result
# in a patch file being silently ignored.
for key in attr.patches.keys():
if key not in patches_used:
msg = """
ERROR: Patch file key `{key}` does not match any npm packages in `npm_translate_lock(name = "{repo}").
Either remove this patch file if it is no longer needed or change its key to match an existing npm package.
""".format(
key = key,
repo = rctx_name,
)
fail(msg)

return result

################################################################################
Expand Down Expand Up @@ -497,7 +519,7 @@ def _verify_node_modules_ignored(rctx, importers, root_package):
if rctx.attr.verify_node_modules_ignored != None:
missing_ignores = _find_missing_bazel_ignores(root_package, importers.keys(), rctx.read(rctx.path(rctx.attr.verify_node_modules_ignored)))
if missing_ignores:
fail("""
msg = """
ERROR: in verify_node_modules_ignored:
pnpm install will create nested node_modules, but not all of them are ignored by Bazel.
Expand All @@ -513,7 +535,8 @@ or disable this check by setting `verify_node_modules_ignored = None` in `npm_tr
fixes = "\n".join(missing_ignores),
bazelignore = rctx.attr.verify_node_modules_ignored,
repo = rctx.name,
))
)
fail(msg)

################################################################################
def _generate_repository_files(rctx, pnpm_lock_label, importers, packages, root_package, default_registry, npm_registries, npm_auth, link_workspace):
Expand All @@ -534,7 +557,7 @@ def _generate_repository_files(rctx, pnpm_lock_label, importers, packages, root_
defs_bzl_header = ["""# buildifier: disable=bzl-visibility
load("@aspect_rules_js//js:defs.bzl", _js_library = "js_library")"""]

npm_imports = _gen_npm_imports(importers, packages, root_package, rctx.attr, npm_registries, default_registry)
npm_imports = _gen_npm_imports(importers, packages, root_package, rctx.name, rctx.attr, npm_registries, default_registry)

fp_links = {}
rctx_files = {
Expand Down

0 comments on commit 3bbb4c0

Please sign in to comment.