diff --git a/MODULE.bazel b/MODULE.bazel index e36d067c0..2c4238b8e 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -187,12 +187,6 @@ npm.npm_translate_lock( "unused": ["//visibility:private"], "@mycorp/pkg-a": ["//examples:__subpackages__"], }, - patch_args = { - "*": ["-p1"], - }, - patches = { - "meaning-of-life@1.0.0": ["//examples/npm_deps:patches/meaning-of-life@1.0.0-after_pnpm.patch"], - }, pnpm_lock = "//:pnpm-lock.yaml", public_hoist_packages = { # Instructs the linker to hoist the ms@2.1.3 npm package to `node_modules/ms` in the `examples/npm_deps` package. diff --git a/WORKSPACE b/WORKSPACE index 5ad219b82..73754a9fa 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -141,12 +141,6 @@ npm_translate_lock( "unused": ["//visibility:private"], "@mycorp/pkg-a": ["//examples:__subpackages__"], }, - patch_args = { - "*": ["-p1"], - }, - patches = { - "meaning-of-life@1.0.0": ["//examples/npm_deps:patches/meaning-of-life@1.0.0-after_pnpm.patch"], - }, pnpm_lock = "//:pnpm-lock.yaml", # Use a version that's not vendored into rules_js by providing a (version, integrity) tuple. # curl --silent https://registry.npmjs.org/pnpm | jq '.versions["8.6.11"].dist.integrity' diff --git a/examples/npm_deps/patched-dependencies-test.js b/examples/npm_deps/patched-dependencies-test.js index 834530261..cf9d08bea 100644 --- a/examples/npm_deps/patched-dependencies-test.js +++ b/examples/npm_deps/patched-dependencies-test.js @@ -1,19 +1,12 @@ const meaningOfLife = require('meaning-of-life') -// meaning-of-life should have been patched twice: -// -// First, by the `pnpm.patchedDependencies` patch: +// Verify meaning-of-life was patched with // examples/npm_deps/patches/meaning-of-life@1.0.0-pnpm.patch -// 42 => "forty two" -// -// Then by the the following patch in the `patches` attr: -// examples/npm_deps/patches/meaning-of-life@1.0.0-after_pnpm.patch -// "forty two" => 32 if (meaningOfLife === 42) { throw new Error('Patches were not applied!') -} else if (meaningOfLife === 'forty two') { - throw new Error('Only `pnpm.patchedDependencies` patch was applied!') -} else if (meaningOfLife !== 32) { - throw new Error('Patch in `patches` was not applied!') +} + +if (meaningOfLife !== 'forty two') { + throw new Error('`pnpm.patchedDependencies` was not applied!') } diff --git a/examples/npm_deps/patches/meaning-of-life@1.0.0-after_pnpm.patch b/examples/npm_deps/patches/meaning-of-life@1.0.0-after_pnpm.patch deleted file mode 100644 index 5498723e7..000000000 --- a/examples/npm_deps/patches/meaning-of-life@1.0.0-after_pnpm.patch +++ /dev/null @@ -1,7 +0,0 @@ -diff --git a/index.js b/index.js -index a8653a9c9264ca1ac9fd2acb6c523a321912ab33..ae967de93c7c12074a686e1e8437b7b2344108be 100644 ---- a/index.js -+++ b/index.js -@@ -1 +1 @@ --module.exports = "forty two" -+module.exports = 32 diff --git a/examples/npm_deps/patches/patches b/examples/npm_deps/patches/patches index 5f42322f4..eb2b06868 100644 --- a/examples/npm_deps/patches/patches +++ b/examples/npm_deps/patches/patches @@ -1,2 +1 @@ -examples/npm_deps/patches/meaning-of-life@1.0.0-after_pnpm.patch examples/npm_deps/patches/meaning-of-life@1.0.0-pnpm.patch diff --git a/npm/private/npm_translate_lock_helpers.bzl b/npm/private/npm_translate_lock_helpers.bzl index 85a665aa2..3a995dd31 100644 --- a/npm/private/npm_translate_lock_helpers.bzl +++ b/npm/private/npm_translate_lock_helpers.bzl @@ -326,12 +326,30 @@ def _get_npm_imports(importers, packages, patched_dependencies, root_package, rc unfriendly_name = None # gather patches & patch args - patches, patches_keys = _gather_values_from_matching_names(True, attr.patches, name, friendly_name, unfriendly_name) + patches = [] + patch_args = [] + + translate_patches, patches_keys = _gather_values_from_matching_names(True, attr.patches, name, friendly_name, unfriendly_name) + + if len(translate_patches) > 0 and pnpm_patched: + msg = """\ +ERROR: can not apply both `pnpm.patchedDependencies` and `npm_translate_lock(patches)` to the same package {pkg}. +""".format( + pkg = name, + ) + fail(msg) # Apply patch from `pnpm.patchedDependencies` first if pnpm_patched: patch_path = "//%s:%s" % (attr.pnpm_lock.package, patched_dependencies.get(friendly_name).get("path")) - patches.insert(0, patch_path) + patches.append(patch_path) + + # pnpm patches are always applied with -p1 + patch_args = ["-p1"] + elif len(translate_patches) > 0: + patches = translate_patches + patch_args, _ = _gather_values_from_matching_names(False, attr.patch_args, "*", name, friendly_name, unfriendly_name) + patches_used.extend(patches_keys) # Resolve string patch labels relative to the root respository rather than relative to rules_js. # https://docs.google.com/document/d/1N81qfCa8oskCk5LqTW-LNthy6EBrDot7bdUsjz6JFC4/ @@ -341,25 +359,6 @@ def _get_npm_imports(importers, packages, patched_dependencies, root_package, rc # that checked in repositories.bzl files don't fail diff tests when run under multiple versions of Bazel. patches = [("@" if patch.startswith("//") else "") + patch for patch in patches] - patch_args, _ = _gather_values_from_matching_names(False, attr.patch_args, "*", name, friendly_name, unfriendly_name) - - # Patches in `pnpm.patchedDependencies` must have the -p1 format. Therefore any - # patches applied via `patches` must also use -p1 since we don't support different - # patch args for different patches. - if pnpm_patched and not _has_strip_prefix_arg(patch_args, 1): - if _has_strip_prefix_arg(patch_args): - msg = """\ -ERROR: patch_args for package {package} contains a strip prefix that is incompatible with a patch applied via `pnpm.patchedDependencies`. - -`pnpm.patchedDependencies` requires a strip prefix of `-p1`. All applied patches must use the same strip prefix. - -""".format(package = friendly_name) - fail(msg) - patch_args = patch_args[:] - patch_args.append("-p1") - - patches_used.extend(patches_keys) - # gather replace packages replace_packages, _ = _gather_values_from_matching_names(True, attr.replace_packages, name, friendly_name, unfriendly_name) if len(replace_packages) > 1: @@ -500,15 +499,6 @@ def _link_package(root_package, import_path, rel_path = "."): def _is_url(url): return url.find("://") != -1 -################################################################################ -def _has_strip_prefix_arg(patch_args, strip_num = None): - if strip_num != None: - return "-p%d" % strip_num in patch_args or "--strip=%d" % strip_num in patch_args - for arg in patch_args: - if arg.startswith("-p") or arg.startswith("--strip="): - return True - return False - ################################################################################ def _to_apparent_repo_name(canonical_name): return canonical_name[canonical_name.rfind("~") + 1:] diff --git a/npm/private/test/snapshots/wksp/repositories.bzl b/npm/private/test/snapshots/wksp/repositories.bzl index 23ce22659..e580b3669 100644 --- a/npm/private/test/snapshots/wksp/repositories.bzl +++ b/npm/private/test/snapshots/wksp/repositories.bzl @@ -14534,7 +14534,7 @@ def npm_repositories(): transitive_closure = { "meaning-of-life": ["1.0.0_-1287509853"], }, - patches = ["@//:examples/npm_deps/patches/meaning-of-life@1.0.0-pnpm.patch", "@//examples/npm_deps:patches/meaning-of-life@1.0.0-after_pnpm.patch"], + patches = ["@//:examples/npm_deps/patches/meaning-of-life@1.0.0-pnpm.patch"], patch_args = ["-p1"], )