From c7d53825f39c0dbf6fc106b65f6160265a98e62f Mon Sep 17 00:00:00 2001 From: Nevada Perry Date: Tue, 31 Dec 2024 00:56:31 +0000 Subject: [PATCH 1/6] Complete support for patches --- npm/workspace.bzl | 14 ++++++++++++++ npm/yarn-resolve/src/bzl.ts | 14 ++++++++++++-- npm/yarn-resolve/src/resolve.ts | 16 ++++++++++++++++ util/starlark/src/index.ts | 2 +- 4 files changed, 43 insertions(+), 3 deletions(-) diff --git a/npm/workspace.bzl b/npm/workspace.bzl index 15b56c6f..75f1de64 100644 --- a/npm/workspace.bzl +++ b/npm/workspace.bzl @@ -19,6 +19,15 @@ def _npm_import_external_impl(ctx, plugins): ctx.execute(["rm", "-r", "tmp"]) + patch_args = "--directory=npm --strip=1 --forward --reject-file=-" + for patch in ctx.attr.patches: + patch_result = ctx.execute([ + "sh", "-c", "patch %s < %s" % (patch_args, ctx.path(patch)), + ]) + # Ignore return code 2, which signals the patch has already been applied + if patch_result.return_code != 0 and patch_result.return_code != 2: + fail("Could not apply patch %s: %s" % (patch, patch_result.stderr)) + files_result = ctx.execute(["find", "npm", "-type", "f"]) if files_result.return_code: fail("Could not list files") @@ -63,6 +72,10 @@ def npm_import_external_rule(plugins): doc = "Package name.", mandatory = True, ), + "patches": attr.label_list( + allow_files = True, + mandatory = True, + ), }, ) @@ -155,6 +168,7 @@ def npm(name, packages, roots, plugins = DEFAULT_PLUGINS, auth_patterns = None, name = repo_name, package = file, package_name = package["name"], + patches = package.get("patches", []), deps = [json.encode({"id": package_repo_name(name, dep["id"]), "name": dep.get("name")}) for dep in package["deps"]], extra_deps = extra_deps, ) diff --git a/npm/yarn-resolve/src/bzl.ts b/npm/yarn-resolve/src/bzl.ts index ffc0451c..485efc3f 100644 --- a/npm/yarn-resolve/src/bzl.ts +++ b/npm/yarn-resolve/src/bzl.ts @@ -14,6 +14,7 @@ export interface BzlPackage { integrity: string; name: string; url: string; + patches: string[]; } export namespace BzlPackage { @@ -26,13 +27,22 @@ export namespace BzlPackage { extraDepsEntries.push([new StarlarkString(id), BzlDeps.toStarlark(deps)]); } - return new StarlarkDict([ + const elements: [StarlarkValue, StarlarkValue][] = [ [new StarlarkString("deps"), BzlDeps.toStarlark(value.deps)], [new StarlarkString("extra_deps"), new StarlarkDict(extraDepsEntries)], [new StarlarkString("integrity"), new StarlarkString(value.integrity)], [new StarlarkString("name"), new StarlarkString(value.name)], [new StarlarkString("url"), new StarlarkString(value.url)], - ]); + ]; + if (value.patches.length > 0) { + elements.push([ + new StarlarkString("patches"), + new StarlarkArray( + value.patches.map((patchPath) => new StarlarkString(patchPath)), + ), + ]); + } + return new StarlarkDict(elements); } } diff --git a/npm/yarn-resolve/src/resolve.ts b/npm/yarn-resolve/src/resolve.ts index 09b425fa..eac28107 100644 --- a/npm/yarn-resolve/src/resolve.ts +++ b/npm/yarn-resolve/src/resolve.ts @@ -84,6 +84,7 @@ export async function resolvePackages( integrity: npmPackage.contentIntegrity, name: structUtils.stringifyIdent(yarnPackage.locator), url: npmPackage.contentUrl, + patches: extractPatchPaths(yarnPackage.locator), }); finished++; } else if (yarnPackage.locator.reference === "workspace:.") { @@ -132,6 +133,21 @@ function npmLocator(locator: Locator): Locator | undefined { } } +function extractPatchPaths(locator: Locator): string[] { + if (locator.reference.startsWith("patch:")) { + return patchUtils + .parseLocator(locator) + .patchPaths.filter((patchPath) => !patchPath.startsWith("~builtin")) + .map((patchPath) => { + // Replace final slash with a colon to make it a bazel label + const parts = patchPath.split("/"); + return `//${parts.slice(0, -1).join("/")}:${parts.slice(-1)}`; + }); + } else { + return []; + } +} + function bzlId(locator: Locator): string | undefined { if ( locator.reference.startsWith("http:") || diff --git a/util/starlark/src/index.ts b/util/starlark/src/index.ts index 4e562b3d..3a1da72a 100644 --- a/util/starlark/src/index.ts +++ b/util/starlark/src/index.ts @@ -98,7 +98,7 @@ function printValue(value: StarlarkValue, indent?: string | undefined): string { if (value instanceof StarlarkString) { return printString(value); } - throw new Error("Unreognized value"); + throw new Error("Unrecognized value"); } function printVariable(value: StarlarkVariable): string { From 238b84e27fdf7db438b28f3e4617870eee86ebbc Mon Sep 17 00:00:00 2001 From: Nevada Perry Date: Tue, 31 Dec 2024 01:39:15 +0000 Subject: [PATCH 2/6] Re-tar the .tgz if any patches were applied --- npm/workspace.bzl | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/npm/workspace.bzl b/npm/workspace.bzl index 75f1de64..090628c3 100644 --- a/npm/workspace.bzl +++ b/npm/workspace.bzl @@ -33,10 +33,24 @@ def _npm_import_external_impl(ctx, plugins): fail("Could not list files") files = [file[len("npm/"):] for file in files_result.stdout.split("\n")] + final_package_path = ctx.attr.package + if ctx.attr.patches: + tar_result = ctx.execute([ + "tar", "czf", "patched-package.tgz", "--strip-components=1", "npm/" + ]) + if tar_result.return_code: + fail("Could not tar up patched-package.tgz") + final_package_path = "patched-package.tgz" + + # Don't leave the package contents sitting around now that we're done. Bazel + # builds will always extract from the .tgz file, so anyone wanting to tinker + # should go poke at the .tgz. + ctx.execute(["rm", "-r", "npm/"]) + build = "" package = struct( - archive = ctx.attr.package, + archive = final_package_path, deps = deps, extra_deps = extra_deps, name = package_name, From e4ff915965d0be439a87665a476d8737ea212774 Mon Sep 17 00:00:00 2001 From: Nevada Perry Date: Tue, 31 Dec 2024 19:54:15 +0000 Subject: [PATCH 3/6] Update docs/docker.md to use rules_oci syntax --- docs/docker.md | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/docs/docker.md b/docs/docker.md index 2efb4741..19c504ff 100644 --- a/docs/docker.md +++ b/docs/docker.md @@ -7,26 +7,32 @@ -Use `nodejs_binary_archive` to create a tar and add that to the docker image. +Use `nodejs_binary_package` to create a tar and add that to the docker image. # Example ```bzl -load("@better_rules_javascript//nodejs:rules.bzl", "nodejs_binary_archive") -load("@io_bazel_rules_docker//container:container.bzl", "container_image") +load("@better_rules_javascript//nodejs:rules.bzl", "nodejs_binary_package") +load("@rules_oci//oci:defs.bzl", "oci_image") -container_image( +oci_image( name = "image", - base = "@base//image", - directory = "/opt/example", + base = "@base", entrypoint = ["/usr/local/bin/example"], + tars = ["image_layer"], +) + +pkg_tar( + name = "image_layer", + package_dir = "/opt/example", symlinks = { "/usr/local/bin/example": "/opt/example/bin", }, - tars = [":tar"], + # Merges these tars together into one layer + deps = [":tar"], ) -nodejs_binary_archive( +nodejs_binary_package( name = "tar", dep = ":lib", main = "src/main.js", From 4a6fdc1301861db6f7291121931b478eb52e13c6 Mon Sep 17 00:00:00 2001 From: nevadaperry <107876546+nevadaperry@users.noreply.github.com> Date: Tue, 7 Jan 2025 13:30:14 -0600 Subject: [PATCH 4/6] Fix lint --- npm/workspace.bzl | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/npm/workspace.bzl b/npm/workspace.bzl index 090628c3..f0a2de52 100644 --- a/npm/workspace.bzl +++ b/npm/workspace.bzl @@ -22,7 +22,9 @@ def _npm_import_external_impl(ctx, plugins): patch_args = "--directory=npm --strip=1 --forward --reject-file=-" for patch in ctx.attr.patches: patch_result = ctx.execute([ - "sh", "-c", "patch %s < %s" % (patch_args, ctx.path(patch)), + "sh", + "-c", + "patch %s < %s" % (patch_args, ctx.path(patch)), ]) # Ignore return code 2, which signals the patch has already been applied if patch_result.return_code != 0 and patch_result.return_code != 2: @@ -36,7 +38,11 @@ def _npm_import_external_impl(ctx, plugins): final_package_path = ctx.attr.package if ctx.attr.patches: tar_result = ctx.execute([ - "tar", "czf", "patched-package.tgz", "--strip-components=1", "npm/" + "tar", + "czf", + "patched-package.tgz", + "--strip-components=1", + "npm/", ]) if tar_result.return_code: fail("Could not tar up patched-package.tgz") From 0355c22bc61d93326cc690989f49c8be50b35f20 Mon Sep 17 00:00:00 2001 From: nevadaperry <107876546+nevadaperry@users.noreply.github.com> Date: Tue, 7 Jan 2025 14:58:44 -0600 Subject: [PATCH 5/6] Fix lint --- npm/workspace.bzl | 1 + 1 file changed, 1 insertion(+) diff --git a/npm/workspace.bzl b/npm/workspace.bzl index f0a2de52..6fbfc80d 100644 --- a/npm/workspace.bzl +++ b/npm/workspace.bzl @@ -26,6 +26,7 @@ def _npm_import_external_impl(ctx, plugins): "-c", "patch %s < %s" % (patch_args, ctx.path(patch)), ]) + # Ignore return code 2, which signals the patch has already been applied if patch_result.return_code != 0 and patch_result.return_code != 2: fail("Could not apply patch %s: %s" % (patch, patch_result.stderr)) From b8919bb325a8330f63a1b87d24b1fe85a5f5cb40 Mon Sep 17 00:00:00 2001 From: Nevada Perry Date: Wed, 8 Jan 2025 04:04:14 +0000 Subject: [PATCH 6/6] Fix lint --- npm/yarn-resolve/src/resolve.ts | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/npm/yarn-resolve/src/resolve.ts b/npm/yarn-resolve/src/resolve.ts index eac28107..8fdeae5c 100644 --- a/npm/yarn-resolve/src/resolve.ts +++ b/npm/yarn-resolve/src/resolve.ts @@ -134,18 +134,16 @@ function npmLocator(locator: Locator): Locator | undefined { } function extractPatchPaths(locator: Locator): string[] { - if (locator.reference.startsWith("patch:")) { - return patchUtils - .parseLocator(locator) - .patchPaths.filter((patchPath) => !patchPath.startsWith("~builtin")) - .map((patchPath) => { - // Replace final slash with a colon to make it a bazel label - const parts = patchPath.split("/"); - return `//${parts.slice(0, -1).join("/")}:${parts.slice(-1)}`; - }); - } else { - return []; - } + return locator.reference.startsWith("patch:") + ? patchUtils + .parseLocator(locator) + .patchPaths.filter((patchPath) => !patchPath.startsWith("~builtin")) + .map((patchPath) => { + // Replace final slash with a colon to make it a bazel label + const parts = patchPath.split("/"); + return `//${parts.slice(0, -1).join("/")}:${parts.slice(-1)}`; + }) + : []; } function bzlId(locator: Locator): string | undefined {