From 5bf0a6b8ab63322c384326789be777a7ad002136 Mon Sep 17 00:00:00 2001 From: Derek Cormier Date: Tue, 31 Jan 2023 17:41:35 -0800 Subject: [PATCH] feat: support pnpm.patchedDependencies --- .../npm_translate_lock_LTE4Nzc1MDcwNjU= | 9 +-- WORKSPACE | 2 + docs/npm_import.md | 2 +- docs/pnpm.md | 36 +++++++++-- examples/npm_deps/BUILD.bazel | 10 +++ examples/npm_deps/package.json | 1 + .../npm_deps/patched-dependencies-test.js | 19 ++++++ .../meaning-of-life@1.0.0-after_pnpm.patch | 7 +++ .../patches/meaning-of-life@1.0.0-pnpm.patch | 7 +++ examples/npm_deps/patches/patches | 2 + npm/extensions.bzl | 4 +- npm/npm_import.bzl | 2 + npm/private/npm_translate_lock.bzl | 3 +- npm/private/npm_translate_lock_generate.bzl | 36 ++++++++++- npm/private/npm_translate_lock_helpers.bzl | 19 +++--- npm/private/npm_translate_lock_state.bzl | 62 +++++++++++++++++-- npm/private/test/repositories_checked.bzl | 20 ++++++ npm/private/test/utils_tests.bzl | 9 +-- npm/private/transitive_closure.bzl | 3 +- npm/private/utils.bzl | 13 ++-- package.json | 3 + pnpm-lock.yaml | 12 ++++ 22 files changed, 241 insertions(+), 40 deletions(-) create mode 100644 examples/npm_deps/patched-dependencies-test.js create mode 100644 examples/npm_deps/patches/meaning-of-life@1.0.0-after_pnpm.patch create mode 100644 examples/npm_deps/patches/meaning-of-life@1.0.0-pnpm.patch diff --git a/.aspect/rules/external_repository_action_cache/npm_translate_lock_LTE4Nzc1MDcwNjU= b/.aspect/rules/external_repository_action_cache/npm_translate_lock_LTE4Nzc1MDcwNjU= index 045aa54a8..32b90dbc2 100755 --- a/.aspect/rules/external_repository_action_cache/npm_translate_lock_LTE4Nzc1MDcwNjU= +++ b/.aspect/rules/external_repository_action_cache/npm_translate_lock_LTE4Nzc1MDcwNjU= @@ -1,12 +1,13 @@ # Input hashes for repository rule npm_translate_lock(name = "npm", pnpm_lock = "//:pnpm-lock.yaml"). # This file should be checked into version control along with the pnpm-lock.yaml file. .npmrc=-2065072158 -pnpm-lock.yaml=-824957789 -package.json=-2091148127 +pnpm-lock.yaml=-194549367 +package.json=325733595 pnpm-workspace.yaml=-1988748742 +examples/npm_deps/patches/meaning-of-life@1.0.0-pnpm.patch=-442666336 examples/js_binary/package.json=-41174383 examples/macro/package.json=-696073599 -examples/npm_deps/package.json=-462506805 +examples/npm_deps/package.json=283109008 examples/npm_package/libs/lib_a/package.json=-1377103079 examples/npm_package/packages/pkg_a/package.json=-1053875011 examples/npm_package/packages/pkg_b/package.json=-994654274 @@ -14,7 +15,7 @@ examples/webpack_cli/package.json=1775843160 js/private/coverage/bundle/package.json=-975746706 js/private/worker/src/package.json=-1936685546 npm/private/test/package.json=1727477721 -npm/private/test/npm_package/package.json=-1377103079 npm/private/test/vendored/lodash-4.17.21.tgz=-1206623349 +npm/private/test/npm_package/package.json=-1377103079 npm/private/test/vendored/is-odd/package.json=1041695223 npm/private/test/vendored/semver-max/package.json=578664053 diff --git a/WORKSPACE b/WORKSPACE index 12c5ca315..e46c9f71e 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -74,6 +74,7 @@ npm_translate_lock( "@aspect-test/c@2.0.2": "echo mooo >> cow.txt", }, data = [ + "//:examples/npm_deps/patches/meaning-of-life@1.0.0-pnpm.patch", "//:package.json", "//:pnpm-workspace.yaml", "//examples/js_binary:package.json", @@ -141,6 +142,7 @@ npm_translate_lock( "@gregmagolan/test-a": ["//examples/npm_deps:patches/test-a.patch"], "@gregmagolan/test-a@0.0.1": ["//examples/npm_deps:patches/test-a@0.0.1.patch"], "@gregmagolan/test-b": ["//examples/npm_deps:patches/test-b.patch"], + "meaning-of-life@1.0.0": ["//examples/npm_deps:patches/meaning-of-life@1.0.0-after_pnpm.patch"], }, pnpm_lock = "//:pnpm-lock.yaml", pnpm_version = "7.25.0", diff --git a/docs/npm_import.md b/docs/npm_import.md index 646dc0a6e..6be585902 100644 --- a/docs/npm_import.md +++ b/docs/npm_import.md @@ -246,7 +246,7 @@ For more about how to use npm_translate_lock, read [pnpm and rules_js](/docs/pnp | npmrc | The .npmrc file, if any, to use.

When set, the .npmrc file specified is parsed and npm auth tokens and basic authentication configuration specified in the file are passed to the Bazel downloader for authentication with private npm registries.

In a future release, pnpm settings such as public-hoist-patterns will be used. | None | | use_home_npmrc | Use the $HOME/.npmrc file (or $USERPROFILE/.npmrc when on Windows) if it exists.

Settings from home .npmrc are merged with settings loaded from the .npmrc file specified in the npmrc attribute, if any. Where there are conflicting settings, the home .npmrc values will take precedence.

WARNING: The repository rule will not be invalidated by changes to the home .npmrc file since there is no way to specify this file as an input to the repository rule. If changes are made to the home .npmrc you can force the repository rule to re-run and pick up the changes by running: bazel run @{name}//:sync where name is the name of the npm_translate_lock you want to re-run.

Because of the repository rule invalidation issue, using the home .npmrc is not recommended. .npmrc settings should generally go in the npmrc in your repository so they are shared by all developers. The home .npmrc should be reserved for authentication settings for private npm repositories. | None | | data | Data files required by this repository rule when auto-updating the pnpm lock file.

Only needed when update_pnpm_lock is True. Read more: [using update_pnpm_lock](/docs/pnpm.md#update_pnpm_lock) | [] | -| patches | A map of package names or package names with their version (e.g., "my-package" or "my-package@v1.2.3") to a label list of patches to apply to the downloaded npm package. Multiple matches are additive.

Read more: [patching](/docs/pnpm.md#patching) | {} | +| patches | A map of package names or package names with their version (e.g., "my-package" or "my-package@v1.2.3") to a label list of patches to apply to the downloaded npm package. Multiple matches are additive.

These patches are applied after any patches in [pnpm.patchedDependencies](https://pnpm.io/next/package_json#pnpmpatcheddependencies).

Read more: [patching](/docs/pnpm.md#patching) | {} | | patch_args | A map of package names or package names with their version (e.g., "my-package" or "my-package@v1.2.3") to a label list arguments to pass to the patch tool. The most specific match wins.

Read more: [patching](/docs/pnpm.md#patching) | {"*": ["-p0"]} | | custom_postinstalls | A map of package names or package names with their version (e.g., "my-package" or "my-package@v1.2.3") to a custom postinstall script to apply to the downloaded npm package after its lifecycle scripts runs. If the version is left out of the package name, the script will run on every version of the npm package. If a custom postinstall scripts exists for a package as well as for a specific version, the script for the versioned package will be appended with && to the non-versioned package script.

For example,

 custom_postinstalls = {     "@foo/bar": "echo something > somewhere.txt",     "fum@0.0.1": "echo something_else > somewhere_else.txt", },


Custom postinstalls are additive and joined with && when there are multiple matches for a package. More specific matches are appended to previous matches. | {} | | prod | If True, only install dependencies but not devDependencies. | False | diff --git a/docs/pnpm.md b/docs/pnpm.md index 2aa427d74..56875e815 100644 --- a/docs/pnpm.md +++ b/docs/pnpm.md @@ -132,10 +132,36 @@ It is recommended to set this environment variable on CI when `update_pnpm_lock` ## Working with packages -### Patching +### Patching via pnpm.patchedDependencies + +Patches included in [pnpm.patchedDependencies](https://pnpm.io/next/package_json#pnpmpatcheddependencies) are automatically applied. These patches must be included in the `data` attribute of `npm_translate_lock`, for example: + +```json +{ + ... + "pnpm": { + "patchedDependencies": { + "fum@0.0.1": "patches/fum@0.0.1.patch" + } + } +} +``` + +```starlark +npm_translate_lock( + ... + data = [ + "//:patches/fum@0.0.1.patch", + ], +) +``` + +### Patching via `patches` attribute + +We recommend patching via [pnpm.patchedDependencies](#patching-via-pnpmpatcheddependencies) as above, but if you are importing +a yarn or npm lockfile and do not have this field in your package.json, you can apply additional +patches using the `patches` and `patch_args` attributes of `npm_translate_lock`. -You can apply patches to packages you fetch remotely such as from npm. -Use the `patches` and `patch_args` attributes of `npm_translate_lock`. These are designed to be similar to the same-named attributes of [http_archive](https://bazel.build/rules/lib/repo/http#http_archive-patch_args). @@ -148,10 +174,12 @@ In case multiple entries in `patches` match, the list of patches are additive. (More specific matches are appended to previous matches.) However if multiple entries in `patch_args` match, then the more specific name matches take precedence. +Patches in `patches` are applied after any patches included in `pnpm.patchedDependencies`. + For example, ```starlark -npm_translate_lock ( +npm_translate_lock( ... patches = { "@foo/bar": ["//:patches/foo+bar.patch"], diff --git a/examples/npm_deps/BUILD.bazel b/examples/npm_deps/BUILD.bazel index 0cd25cd36..07b348f77 100644 --- a/examples/npm_deps/BUILD.bazel +++ b/examples/npm_deps/BUILD.bazel @@ -252,3 +252,13 @@ js_test( ], entry_point = "case7.js", ) + +####################################### +# Case 8: patch applied via `pnpm.patchesDependencies` then by `patches` +js_test( + name = "test8", + data = [ + ":node_modules/meaning-of-life", + ], + entry_point = "patched-dependencies-test.js", +) diff --git a/examples/npm_deps/package.json b/examples/npm_deps/package.json index 9abfc6b5d..ad1e900d1 100644 --- a/examples/npm_deps/package.json +++ b/examples/npm_deps/package.json @@ -7,6 +7,7 @@ "@mycorp/pkg-a": "workspace:*", "@rollup/plugin-commonjs": "21.1.0", "debug": "3.2.7", + "meaning-of-life": "1.0.0", "mobx-react": "7.3.0", "mobx": "6.3.0", "react": "17.0.2", diff --git a/examples/npm_deps/patched-dependencies-test.js b/examples/npm_deps/patched-dependencies-test.js new file mode 100644 index 000000000..834530261 --- /dev/null +++ b/examples/npm_deps/patched-dependencies-test.js @@ -0,0 +1,19 @@ +const meaningOfLife = require('meaning-of-life') + +// meaning-of-life should have been patched twice: +// +// First, by the `pnpm.patchedDependencies` patch: +// 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!') +} 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 new file mode 100644 index 000000000..5498723e7 --- /dev/null +++ b/examples/npm_deps/patches/meaning-of-life@1.0.0-after_pnpm.patch @@ -0,0 +1,7 @@ +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/meaning-of-life@1.0.0-pnpm.patch b/examples/npm_deps/patches/meaning-of-life@1.0.0-pnpm.patch new file mode 100644 index 000000000..b0a2e5585 --- /dev/null +++ b/examples/npm_deps/patches/meaning-of-life@1.0.0-pnpm.patch @@ -0,0 +1,7 @@ +diff --git a/index.js b/index.js +index a8653a9c9264ca1ac9fd2acb6c523a321912ab33..ae967de93c7c12074a686e1e8437b7b2344108be 100644 +--- a/index.js ++++ b/index.js +@@ -1 +1 @@ +-module.exports = 42 ++module.exports = "forty two" diff --git a/examples/npm_deps/patches/patches b/examples/npm_deps/patches/patches index 67737e48b..a1e367639 100644 --- a/examples/npm_deps/patches/patches +++ b/examples/npm_deps/patches/patches @@ -1,3 +1,5 @@ +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 examples/npm_deps/patches/test-a.patch examples/npm_deps/patches/test-a@0.0.1.patch examples/npm_deps/patches/test-b.patch diff --git a/npm/extensions.bzl b/npm/extensions.bzl index 4cbfb1ab2..6ac4d2ed8 100644 --- a/npm/extensions.bzl +++ b/npm/extensions.bzl @@ -44,14 +44,14 @@ def _extension_impl(module_ctx): if not attr.pnpm_lock: continue - lock_importers, lock_packages = utils.parse_pnpm_lock(module_ctx.read(attr.pnpm_lock)) + lock_importers, lock_packages, lock_patched_dependencies = 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) registries = {} npm_auth = {} if attr.npmrc: npmrc = parse_npmrc(module_ctx.read(attr.npmrc)) (registries, npm_auth) = npm_translate_lock_helpers.get_npm_auth(npmrc, module_ctx.path(attr.npmrc), module_ctx.os.environ) - imports = npm_translate_lock_helpers.gen_npm_imports(importers, packages, attr.pnpm_lock.package, attr.name, attr, registries, utils.default_registry(), npm_auth) + imports = npm_translate_lock_helpers.gen_npm_imports(importers, packages, lock_patched_dependencies, attr.pnpm_lock.package, attr.name, attr, registries, utils.default_registry(), npm_auth) for i in imports: npm_import( name = i.name, diff --git a/npm/npm_import.bzl b/npm/npm_import.bzl index 8bac45d13..107aa1e80 100644 --- a/npm/npm_import.bzl +++ b/npm/npm_import.bzl @@ -180,6 +180,8 @@ def npm_translate_lock( patches: A map of package names or package names with their version (e.g., "my-package" or "my-package@v1.2.3") to a label list of patches to apply to the downloaded npm package. Multiple matches are additive. + These patches are applied after any patches in [pnpm.patchedDependencies](https://pnpm.io/next/package_json#pnpmpatcheddependencies). + Read more: [patching](/docs/pnpm.md#patching) patch_args: A map of package names or package names with their version (e.g., "my-package" or "my-package@v1.2.3") diff --git a/npm/private/npm_translate_lock.bzl b/npm/private/npm_translate_lock.bzl index 846607cc5..e4ba9b71f 100644 --- a/npm/private/npm_translate_lock.bzl +++ b/npm/private/npm_translate_lock.bzl @@ -69,7 +69,7 @@ def _impl(rctx): gen_helpers.verify_node_modules_ignored(rctx, state.importers(), state.root_package()) - helpers.verify_patches(rctx, state.label_store) + helpers.verify_patches(rctx, state) rctx.report_progress("Translating {}".format(state.label_store.relative_path("pnpm_lock"))) @@ -88,6 +88,7 @@ def _impl(rctx): state.label_store.label("pnpm_lock"), importers, packages, + state.patched_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 02cd5d882..b90d7e5e4 100644 --- a/npm/private/npm_translate_lock_generate.bzl +++ b/npm/private/npm_translate_lock_generate.bzl @@ -252,7 +252,7 @@ def _get_npm_auth(npmrc, npmrc_path, environ): return (registries, auth) ################################################################################ -def _gen_npm_imports(importers, packages, root_package, rctx_name, attr, registries, default_registry, npm_auth): +def _gen_npm_imports(importers, packages, patched_dependencies, root_package, rctx_name, attr, registries, default_registry, npm_auth): "Converts packages from the lockfile to a struct of attributes for npm_import" if attr.prod and attr.dev: @@ -295,6 +295,7 @@ def _gen_npm_imports(importers, packages, root_package, rctx_name, attr, registr optional_deps = package_info.get("optional_dependencies") dev = package_info.get("dev") optional = package_info.get("optional") + pnpm_patched = package_info.get("patched") requires_build = package_info.get("requires_build") transitive_closure = package_info.get("transitive_closure") resolution = package_info.get("resolution") @@ -344,6 +345,11 @@ def _gen_npm_imports(importers, packages, root_package, rctx_name, attr, registr # gather patches & patch args patches, patches_keys = _gather_values_from_matching_names(True, attr.patches, name, friendly_name, unfriendly_name) + # Apply patch from `pnpm.patchedDependencies` first + if pnpm_patched: + patch_path = "//:%s" % patched_dependencies.get(friendly_name).get("path") + patches.insert(0, patch_path) + # Prevent the patch string labels from going through further repo mapping: # https://docs.google.com/document/d/1N81qfCa8oskCk5LqTW-LNthy6EBrDot7bdUsjz6JFC4/ # Further, prepend the optional '@' for earlier versions of Bazel so that checked in @@ -351,6 +357,22 @@ def _gen_npm_imports(importers, packages, root_package, rctx_name, attr, registr patches = [("" if utils.bzlmod_supported else "@") + str(attr.pnpm_lock.relative(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 custom postinstalls @@ -576,7 +598,7 @@ or disable this check by setting `verify_node_modules_ignored = None` in `npm_tr fail(msg) ################################################################################ -def _generate_repository_files(rctx, pnpm_lock_label, importers, packages, root_package, default_registry, npm_registries, npm_auth, link_workspace): +def _generate_repository_files(rctx, pnpm_lock_label, importers, packages, patched_dependencies, root_package, default_registry, npm_registries, npm_auth, link_workspace): generated_by_lines = [ "\"\"\"@generated by npm_translate_lock(name = \"{}\", pnpm_lock = \"{}\")\"\"\"".format(_to_apparent_repo_name(rctx.name), utils.consistent_label_str(pnpm_lock_label)), "", # empty line after bzl docstring since buildifier expects this if this file is vendored in @@ -594,7 +616,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.name, rctx.attr, npm_registries, default_registry, npm_auth) + npm_imports = _gen_npm_imports(importers, packages, patched_dependencies, root_package, rctx.name, rctx.attr, npm_registries, default_registry, npm_auth) fp_links = {} rctx_files = { @@ -1004,6 +1026,14 @@ load("@aspect_rules_js//npm/private:npm_package_store.bzl", _npm_package_store = for filename, contents in rctx_files.items(): rctx.file(filename, "\n".join(generated_by_lines + contents)) +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 + helpers = struct( # TODO(cleanup): move non-generation helpers out of this file in a follow-up PR to_apparent_repo_name = _to_apparent_repo_name, diff --git a/npm/private/npm_translate_lock_helpers.bzl b/npm/private/npm_translate_lock_helpers.bzl index 5adbb4abe..c67ef0f71 100644 --- a/npm/private/npm_translate_lock_helpers.bzl +++ b/npm/private/npm_translate_lock_helpers.bzl @@ -2,19 +2,16 @@ load("@bazel_skylib//lib:new_sets.bzl", "sets") -def _verify_patches(rctx, label_store): +def _verify_patches(rctx, state): if rctx.attr.verify_patches and rctx.attr.patches != None: - rctx.report_progress("Verifying patches in {}".format(label_store.relative_path("verify_patches"))) + rctx.report_progress("Verifying patches in {}".format(state.label_store.relative_path("verify_patches"))) # Patches in the patch list verification file - verify_patches_content = rctx.read(label_store.label("verify_patches")).strip(" \t\n\r") + verify_patches_content = rctx.read(state.label_store.label("verify_patches")).strip(" \t\n\r") verify_patches = sets.make(verify_patches_content.split("\n")) - # Patches declared in the `patches` attr - declared_patch_count = 0 - for pkg_patches in rctx.attr.patches.values(): - declared_patch_count += len(pkg_patches) - declared_patches = sets.make([label_store.relative_path("patches_%d" % i) for i in range(declared_patch_count)]) + # Patches in `patches` or `pnpm.patchedDependencies` + declared_patches = sets.make([state.label_store.relative_path("patches_%d" % i) for i in range(state.num_patches())]) if not sets.is_subset(verify_patches, declared_patches): missing_patches = sets.to_list(sets.difference(verify_patches, declared_patches)) @@ -22,14 +19,14 @@ def _verify_patches(rctx, label_store): fail(""" ERROR: in verify_patches: -The following patches were found in {patches_list} but were not listed -in the `patches` attribute of `npm_translate_lock`. +The following patches were found in {patches_list} but were not listed in the +`patches` attribute of `npm_translate_lock` or in `pnpm.patchedDependencies`. {missing_patches} To disable this check, remove the `verify_patches` attribute from `npm_translate_lock`. -""".format(patches_list = label_store.relative_path("verify_patches"), missing_patches = missing_patches_formatted)) +""".format(patches_list = state.label_store.relative_path("verify_patches"), missing_patches = missing_patches_formatted)) helpers = struct( verify_patches = _verify_patches, diff --git a/npm/private/npm_translate_lock_state.bzl b/npm/private/npm_translate_lock_state.bzl index feb2b38bf..a0c29c016 100644 --- a/npm/private/npm_translate_lock_state.bzl +++ b/npm/private/npm_translate_lock_state.bzl @@ -40,8 +40,6 @@ WARNING: `update_pnpm_lock` attribute in `npm_translate_lock(name = "{rctx_name} # labels only needed when updating the pnpm lock file _init_update_labels(rctx, label_store) - _init_patch_labels(rctx, label_store) - _init_link_workspace(priv, rctx, label_store) # parse the pnpm lock file incase since we need the importers list for additional init @@ -53,6 +51,8 @@ WARNING: `update_pnpm_lock` attribute in `npm_translate_lock(name = "{rctx_name} if _should_update_pnpm_lock(priv): _init_importer_labels(priv, label_store) + _init_patch_labels(priv, rctx, label_store) + _init_root_package(priv, rctx, label_store) _init_npmrc(priv, rctx, label_store) @@ -106,6 +106,9 @@ def _init_common_labels(rctx, label_store): # pnpm-workspace.yaml file label_store.add_sibling("lock", "pnpm_workspace", PNPM_WORKSPACE_FILENAME) + # root package.json file + label_store.add_sibling("lock", "package_json_root", PACKAGE_JSON_FILENAME) + ################################################################################ def _init_pnpm_labels(label_store, rctx): # Note that we must reference the node binary under the platform-specific node @@ -137,11 +140,18 @@ def _init_update_labels(rctx, label_store): label_store.add("yarn_lock", attr.yarn_lock, seed_root = True) ################################################################################ -def _init_patch_labels(rctx, label_store): +def _init_patch_labels(priv, rctx, label_store): if rctx.attr.verify_patches: label_store.add("verify_patches", rctx.attr.verify_patches) patches = [] + + # Add patches from `pnpm.patchedDependencies` + root_package_json = _read_root_package_json(priv, rctx, label_store) + for patch in root_package_json.get("pnpm", {}).get("patchedDependencies", {}).values(): + patches.append("//:%s" % patch) + + # Add patches in `patches` attribute for pkg_patches in rctx.attr.patches.values(): patches.extend(pkg_patches) @@ -151,6 +161,8 @@ def _init_patch_labels(rctx, label_store): for i, d in enumerate(patches): label_store.add("patches_{}".format(i), d) + priv["num_patches"] = len(patches) + ################################################################################ def _init_importer_labels(priv, label_store): for i, p in enumerate(priv["importers"].keys()): @@ -281,6 +293,32 @@ WARNING: Implicitly using package.json file `{package_json}` since the `{pnpm_lo fail(msg) _copy_input_file(priv, rctx, label_store, package_json_key) + # pnpm.patchedDependencies patch files + root_package_json = _read_root_package_json(priv, rctx, label_store) + pnpm_patches = root_package_json.get("pnpm", {}).get("patchedDependencies", {}).values() + root_package_json_label = label_store.label("package_json_root") + + for i, _ in enumerate(pnpm_patches): + # The key for pnpm.patchesDependencies patches are indexed before other patches and start at 0 + patch_key = "patches_{}".format(i) + if not _has_input_hash(priv, label_store.relative_path(patch_key)): + # buildifier: disable=print + print(""" +WARNING: Implicitly using patch file `{patch}` since the `{package_json}` file contains this patch in `pnpm.patchedDependencies`. + Add '{patch}' to the 'data' attribute of `npm_translate_lock(name = "{rctx_name}")` to suppress this warning. +""".format( + package_json = root_package_json_label, + patch = label_store.label(patch_key), + rctx_name = rctx.name, + )) + if not utils.exists(rctx, label_store.path(patch_key)): + msg = "ERROR: expected {path} to exist since the `{package_json}` file contains this patch in `pnpm.patchedDependencies`.".format( + path = label_store.path(patch_key), + package_json = root_package_json_label, + ) + fail(msg) + _copy_input_file(priv, rctx, label_store, patch_key) + ################################################################################ def _has_input_hash(priv, path): return path in priv["input_hashes"] @@ -374,15 +412,23 @@ WARNING: Cannot determine home directory in order to load home `.npmrc` file in ################################################################################ def _load_lockfile(priv, rctx, label_store): - importers, packages = utils.parse_pnpm_lock(rctx.read(label_store.path("pnpm_lock"))) + importers, packages, patched_dependencies = utils.parse_pnpm_lock(rctx.read(label_store.path("pnpm_lock"))) priv["importers"] = importers priv["packages"] = packages + priv["patched_dependencies"] = patched_dependencies ################################################################################ def _has_workspaces(priv): importer_paths = priv["importers"].keys() return importer_paths and (len(importer_paths) > 1 or importer_paths[0] != ".") +################################################################################ +def _read_root_package_json(priv, rctx, label_store): + if "root_package_json" not in priv: + root_package_json_path = label_store.path("package_json_root") + priv["root_package_json"] = json.decode(rctx.read(root_package_json_path)) + return priv["root_package_json"] + ################################################################################ def _should_update_pnpm_lock(priv): return priv["should_update_pnpm_lock"] @@ -399,6 +445,12 @@ def _importers(priv): def _packages(priv): return priv["packages"] +def _patched_dependencies(priv): + return priv["patched_dependencies"] + +def _num_patches(priv): + return priv["num_patches"] + def _npm_registries(priv): return priv["npm_registries"] @@ -433,8 +485,10 @@ def _new(rctx): link_workspace = lambda: _link_workspace(priv), importers = lambda: _importers(priv), packages = lambda: _packages(priv), + patched_dependencies = lambda: _patched_dependencies(priv), npm_registries = lambda: _npm_registries(priv), npm_auth = lambda: _npm_auth(priv), + num_patches = lambda: _num_patches(priv), root_package = lambda: _root_package(priv), set_input_hash = lambda label, value: _set_input_hash(priv, label, value), action_cache_miss = lambda: _action_cache_miss(priv, rctx, label_store), diff --git a/npm/private/test/repositories_checked.bzl b/npm/private/test/repositories_checked.bzl index 78a261f19..80926256c 100644 --- a/npm/private/test/repositories_checked.bzl +++ b/npm/private/test/repositories_checked.bzl @@ -12718,6 +12718,26 @@ def npm_repositories(): }, ) + npm_import( + name = "npm__meaning-of-life__1.0.0__o3deharooos255qt5xdujc3cuq", + root_package = "", + link_workspace = "", + link_packages = { + "examples/npm_deps": ["meaning-of-life"], + }, + package = "meaning-of-life", + version = "1.0.0_o3deharooos255qt5xdujc3cuq", + url = "https://registry.npmjs.org/meaning-of-life/-/meaning-of-life-1.0.0.tgz", + npm_translate_lock_repo = "npm", + generate_bzl_library_targets = True, + integrity = "sha512-fVA4xSydqtK9owabGcYw1r4EKEsMOVVeYQLeCXPu77Z+8Y2j2B2I16UqZlKIOHnYkJ4RSvpJ00ywy9IWjmuxYw==", + transitive_closure = { + "meaning-of-life": ["1.0.0_o3deharooos255qt5xdujc3cuq"], + }, + 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"], + patch_args = ["-p1"], + ) + npm_import( name = "npm__media-query-parser__2.0.2", root_package = "", diff --git a/npm/private/test/utils_tests.bzl b/npm/private/test/utils_tests.bzl index 18e7d0392..e5b2f9167 100644 --- a/npm/private/test/utils_tests.bzl +++ b/npm/private/test/utils_tests.bzl @@ -5,14 +5,15 @@ See https://docs.bazel.build/versions/main/skylark/testing.html#for-testing-star load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest") load("//npm/private:utils.bzl", "utils") -def test_strip_peer_dep_version(ctx): +def test_strip_peer_dep_or_patched_version(ctx): env = unittest.begin(ctx) asserts.equals( env, "21.1.0", - utils.strip_peer_dep_version("21.1.0_rollup@2.70.2_x@1.1.1"), + utils.strip_peer_dep_or_patched_version("21.1.0_rollup@2.70.2_x@1.1.1"), ) - asserts.equals(env, "21.1.0", utils.strip_peer_dep_version("21.1.0")) + asserts.equals(env, "1.0.0", utils.strip_peer_dep_or_patched_version("1.0.0_o3deharooos255qt5xdujc3cuq")) + asserts.equals(env, "21.1.0", utils.strip_peer_dep_or_patched_version("21.1.0")) return unittest.end(env) def test_bazel_name(ctx): @@ -135,7 +136,7 @@ def test_npm_registry_download_url(ctx): ) return unittest.end(env) -t0_test = unittest.make(test_strip_peer_dep_version) +t0_test = unittest.make(test_strip_peer_dep_or_patched_version) t1_test = unittest.make(test_bazel_name) t2_test = unittest.make(test_pnpm_name) t3_test = unittest.make(test_friendly_name) diff --git a/npm/private/transitive_closure.bzl b/npm/private/transitive_closure.bzl index 589167c39..16bac3457 100644 --- a/npm/private/transitive_closure.bzl +++ b/npm/private/transitive_closure.bzl @@ -53,7 +53,7 @@ def _gather_package_info(package_path, package_snapshot): # an aliased dependency package = package_path[1:] name, version = utils.parse_pnpm_name(package) - friendly_version = utils.strip_peer_dep_version(version) + friendly_version = utils.strip_peer_dep_or_patched_version(version) package_key = package elif package_path.startswith("file:") and utils.is_vendored_tarfile(package_snapshot): if "name" not in package_snapshot: @@ -103,6 +103,7 @@ def _gather_package_info(package_path, package_snapshot): "optional_dependencies": package_snapshot.get("optionalDependencies", {}), "dev": "dev" in package_snapshot.keys(), "optional": "optional" in package_snapshot.keys(), + "patched": package_snapshot.get("patched", False), "has_bin": "hasBin" in package_snapshot.keys(), "requires_build": "requiresBuild" in package_snapshot.keys(), } diff --git a/npm/private/utils.bzl b/npm/private/utils.bzl index 14f3c4b1e..7a582bb2e 100644 --- a/npm/private/utils.bzl +++ b/npm/private/utils.bzl @@ -34,10 +34,11 @@ def _bazel_name(name, version = None): escaped_version = "%s__%s" % (escaped_version, _sanitize_string(peer_version)) return "%s__%s" % (escaped_name, escaped_version) -def _strip_peer_dep_version(version): - "Remove peer dependency syntax from version string" +def _strip_peer_dep_or_patched_version(version): + "Remove peer dependency or patched syntax from version string" # 21.1.0_rollup@2.70.2 becomes 21.1.0 + # 1.0.0_o3deharooos255qt5xdujc3cuq becomes 1.0.0 index = version.find("_") if index != -1: return version[:index] @@ -84,7 +85,9 @@ def _parse_pnpm_lock(content): packages = parsed.get("packages", {}) - return importers, packages + patched_dependencies = parsed.get("patchedDependencies", {}) + + return importers, packages, patched_dependencies def _assert_lockfile_version(version, testonly = False): if type(version) != type(1.0): @@ -176,7 +179,7 @@ def _npm_registry_download_url(package, version, registries, default_registry): registry.removesuffix("/"), package, package_name_no_scope, - _strip_peer_dep_version(version), + _strip_peer_dep_or_patched_version(version), ) def _is_git_repository_url(url): @@ -317,7 +320,7 @@ utils = struct( parse_pnpm_lock = _parse_pnpm_lock, friendly_name = _friendly_name, virtual_store_name = _virtual_store_name, - strip_peer_dep_version = _strip_peer_dep_version, + strip_peer_dep_or_patched_version = _strip_peer_dep_or_patched_version, make_symlink = _make_symlink, # Symlinked node_modules structure virtual store path under node_modules virtual_store_root = ".aspect_rules_js", diff --git a/package.json b/package.json index 4f27f81aa..9874d7873 100644 --- a/package.json +++ b/package.json @@ -36,6 +36,9 @@ "jsonify": "https://github.com/aspect-build/test-packages/releases/download/0.0.0/@foo-jsonify-0.0.0.tgz", "semver-max": "file:./npm/private/test/vendored/semver-max", "is-odd": "file:./npm/private/test/vendored/is-odd" + }, + "patchedDependencies": { + "meaning-of-life@1.0.0": "examples/npm_deps/patches/meaning-of-life@1.0.0-pnpm.patch" } } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 029377412..aeeddc0bb 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -7,6 +7,11 @@ overrides: packageExtensionsChecksum: 60d08949101fc004c7564f98037b7764 +patchedDependencies: + meaning-of-life@1.0.0: + hash: o3deharooos255qt5xdujc3cuq + path: examples/npm_deps/patches/meaning-of-life@1.0.0-pnpm.patch + importers: .: @@ -45,6 +50,7 @@ importers: '@mycorp/pkg-a': workspace:* '@rollup/plugin-commonjs': 21.1.0 debug: 3.2.7 + meaning-of-life: 1.0.0 mobx: 6.3.0 mobx-react: 7.3.0 react: 17.0.2 @@ -57,6 +63,7 @@ importers: '@mycorp/pkg-a': link:../npm_package/packages/pkg_a '@rollup/plugin-commonjs': 21.1.0_rollup@2.70.2 debug: 3.2.7 + meaning-of-life: 1.0.0_o3deharooos255qt5xdujc3cuq mobx: 6.3.0 mobx-react: 7.3.0_mobx@6.3.0+react@17.0.2 react: 17.0.2 @@ -3782,6 +3789,11 @@ packages: is-buffer: 1.1.6 dev: true + /meaning-of-life/1.0.0_o3deharooos255qt5xdujc3cuq: + resolution: {integrity: sha512-fVA4xSydqtK9owabGcYw1r4EKEsMOVVeYQLeCXPu77Z+8Y2j2B2I16UqZlKIOHnYkJ4RSvpJ00ywy9IWjmuxYw==} + dev: true + patched: true + /media-query-parser/2.0.2: resolution: {integrity: sha512-1N4qp+jE0pL5Xv4uEcwVUhIkwdUO3S/9gML90nqKA7v7FcOS5vUtatfzok9S9U1EJU8dHWlcv95WLnKmmxZI9w==} dependencies: