feat: add repository_ctx.patch(directory) (https://github.com/bazelbuild/bazel/pull/30987) ### Description Add a `directory` parameter to `repository_ctx.patch` so a patch can be applied inside a subdirectory of the repository instead of only at its root. Useful for rulesets applying patches to artifacts where the patch-author may not know the directory structure where the patch is being applied. ### Motivation https://github.com/bazelbuild/bazel/issues/19772 outlines it well - essentially allowing rulesets to avoid things like dependencies on native `patch` being on the `PATH` etc. ### Build API Changes Adds `repository_ctx.patch(directory)` ### Checklist - [x] I have added tests for the new use cases (if any). - [x] I have updated the documentation (if applicable). ### Release Notes RELNOTES: Add `repository_ctx.patch(directory)` Closes #30987. PiperOrigin-RevId: 976091181 Change-Id: Ibdea6c57827ac18e387037bd06dd3485a7be0180
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java index 61f111f..bd91d8e 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java
@@ -478,8 +478,8 @@ name = "patch", doc = """ - Apply a patch file to the root directory of external repository. \ - The patch file should be a standard \ + Apply a patch file to the root directory of external repository, or to \ + <code>directory</code> within it. The patch file should be a standard \ <a href="https://en.wikipedia.org/wiki/Diff#Unified_format"> \ unified diff format</a> file. \ The Bazel-native patch implementation doesn't support binary patch \ @@ -505,6 +505,22 @@ defaultValue = "0", doc = "Strip the specified number of leading components from file names."), @Param( + name = "directory", + allowedTypes = { + @ParamType(type = String.class), + @ParamType(type = Label.class), + @ParamType(type = StarlarkPath.class) + }, + defaultValue = "''", + positional = false, + named = true, + doc = + """ + Directory to apply the patch in, relative to the repository directory. \ + File names in the patch are resolved relative to this directory. \ + Defaults to the repository directory. + """), + @Param( name = "watch_patch", defaultValue = "'auto'", positional = false, @@ -519,10 +535,17 @@ information. """), }) - public void patch(Object patchFile, StarlarkInt stripI, String watchPatch, StarlarkThread thread) + public void patch( + Object patchFile, + StarlarkInt stripI, + Object directory, + String watchPatch, + StarlarkThread thread) throws EvalException, RepositoryFunctionException, InterruptedException { int strip = Starlark.toInt(stripI, "strip"); StarlarkPath starlarkPath = getPath(patchFile); + StarlarkPath directoryPath = getPath(directory); + checkInOutputDirectory("write", directoryPath); WorkspaceRuleEvent w = WorkspaceRuleEvent.newPatchEvent( starlarkPath.toString(), @@ -535,7 +558,7 @@ } maybeWatch(starlarkPath, ShouldWatch.fromString(watchPatch)); try { - PatchUtil.apply(starlarkPath.getPath(), strip, workingDirectory); + PatchUtil.apply(starlarkPath.getPath(), strip, directoryPath.getPath()); } catch (PatchFailedException e) { throw new RepositoryFunctionException( Starlark.errorf("Error applying patch %s: %s", starlarkPath, e.getMessage()),
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java index f33c9fd..7043caf 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java
@@ -366,18 +366,45 @@ StarlarkPath patchFile = context.getPath("my.patch"); context.createFile( context.getPath("my.patch"), "--- foo\n+++ foo\n" + ONE_LINE_PATCH, false, true, thread); - context.patch(patchFile, StarlarkInt.of(0), "auto", thread); + context.patch(patchFile, StarlarkInt.of(0), "", "auto", thread); testOutputFile(foo.getPath(), "line one\nline two\n"); } @Test + public void testPatchInDirectory() throws Exception { + setUpRepo("test"); + StarlarkPath foo = context.getPath("sub/foo"); + context.createFile(foo, "line one\n", false, true, thread); + StarlarkPath patchFile = context.getPath("my.patch"); + context.createFile(patchFile, "--- a/foo\n+++ b/foo\n" + ONE_LINE_PATCH, false, true, thread); + context.patch(patchFile, StarlarkInt.of(1), "sub", "auto", thread); + testOutputFile(foo.getPath(), "line one\nline two\n"); + } + + @Test + public void testPatchInDirectoryOutsideOfExternalRepository() throws Exception { + setUpRepo("test"); + StarlarkPath patchFile = context.getPath("my.patch"); + context.createFile(patchFile, "--- foo\n+++ foo\n" + ONE_LINE_PATCH, false, true, thread); + try { + context.patch(patchFile, StarlarkInt.of(0), "/other_root", "auto", thread); + fail("Expected RepositoryFunctionException"); + } catch (RepositoryFunctionException ex) { + assertThat(ex) + .hasCauseThat() + .hasMessageThat() + .isEqualTo("Cannot write outside of the repository directory for path /other_root"); + } + } + + @Test public void testCannotFindFileToPatch() throws Exception { setUpRepo("test"); StarlarkPath patchFile = context.getPath("my.patch"); context.createFile( context.getPath("my.patch"), "--- foo\n+++ foo\n" + ONE_LINE_PATCH, false, true, thread); try { - context.patch(patchFile, StarlarkInt.of(0), "auto", thread); + context.patch(patchFile, StarlarkInt.of(0), "", "auto", thread); fail("Expected RepositoryFunctionException"); } catch (RepositoryFunctionException ex) { assertThat(ex) @@ -400,7 +427,7 @@ true, thread); try { - context.patch(patchFile, StarlarkInt.of(0), "auto", thread); + context.patch(patchFile, StarlarkInt.of(0), "", "auto", thread); fail("Expected RepositoryFunctionException"); } catch (RepositoryFunctionException ex) { assertThat(ex) @@ -433,7 +460,7 @@ """; context.createFile(context.getPath("my.patch"), patch, false, true, thread); try { - context.patch(patchFile, StarlarkInt.of(0), "auto", thread); + context.patch(patchFile, StarlarkInt.of(0), "", "auto", thread); fail("Expected RepositoryFunctionException"); } catch (RepositoryFunctionException ex) { assertThat(ex)
diff --git a/src/test/shell/bazel/external_patching_test.sh b/src/test/shell/bazel/external_patching_test.sh index eec7f00..ebb4c24 100755 --- a/src/test/shell/bazel/external_patching_test.sh +++ b/src/test/shell/bazel/external_patching_test.sh
@@ -848,4 +848,139 @@ grep -q 'New version' $foopath || fail "expected patch to be applied" } +test_patch_directory() { + EXTREPODIR=`pwd` + EXTREPOURL="$(get_extrepourl ${EXTREPODIR})" + + # Verify that repository_ctx.patch can apply a patch inside a subdirectory + # of the repository. + mkdir main + cd main + cat > patch_foo.sh <<'EOF' +--- a/foo.sh ++++ b/foo.sh +@@ -1,3 +1,3 @@ + #!/usr/bin/env sh + +-echo Here be dragons... ++echo There are dragons... +EOF + cat > ext.bzl <<'EOF' +def _impl(ctx): + ctx.download_and_extract( + url = ctx.attr.url, + output = "sub", + strip_prefix = "ext-0.1.2", + ) + ctx.patch(ctx.attr.patch, strip = 1, directory = "sub") + ctx.file("BUILD", "exports_files([\"sub/foo.sh\"])") + +ext = repository_rule( + implementation = _impl, + attrs = { + "url": attr.string(), + "patch": attr.label(), + }, +) +EOF + cat > $(setup_module_dot_bazel) <<EOF +ext = use_repo_rule("//:ext.bzl", "ext") +ext( + name="ext", + url="${EXTREPOURL}/ext.zip", + patch="//:patch_foo.sh", +) +EOF + cat > BUILD <<'EOF' +genrule( + name = "foo", + outs = ["foo.sh"], + srcs = ["@ext//:sub/foo.sh"], + cmd = "cp $< $@; chmod u+x $@", + executable = True, +) +EOF + bazel build :foo.sh + foopath=`bazel info bazel-genfiles`/foo.sh + grep -q 'There are' $foopath || fail "expected patch to be applied" +} + +do_test_utils_patch_directory() { + EXTREPODIR=`pwd` + EXTREPOURL="$(get_extrepourl ${EXTREPODIR})" + + # Verify that the patch() helper in @bazel_tools honors patch_directory. + mkdir main + cd main + cat > patch_foo.sh <<'EOF' +--- a/foo.sh ++++ b/foo.sh +@@ -1,3 +1,3 @@ + #!/usr/bin/env sh + +-echo Here be dragons... ++echo There are dragons... +EOF + cat > ext.bzl <<EOF +load("@bazel_tools//tools/build_defs/repo:utils.bzl", "patch") + +def _impl(ctx): + ctx.download_and_extract( + url = ctx.attr.url, + output = "sub", + strip_prefix = "ext-0.1.2", + ) + patch( + ctx, + patch_args = ["-p1"], + patch_cmds = ["echo patched >> foo.sh"], + $1 + ) + ctx.file("BUILD", "exports_files([\"sub/foo.sh\"])") + +ext = repository_rule( + implementation = _impl, + attrs = { + "url": attr.string(), + "patches": attr.label_list(), + "patch_directory": attr.string(), + }, +) +EOF + cat > $(setup_module_dot_bazel) <<EOF +ext = use_repo_rule("//:ext.bzl", "ext") +ext( + name="ext", + url="${EXTREPOURL}/ext.zip", + patches=["//:patch_foo.sh"], + $2 +) +EOF + cat > BUILD <<'EOF' +genrule( + name = "foo", + outs = ["foo.sh"], + srcs = ["@ext//:sub/foo.sh"], + cmd = "cp $< $@; chmod u+x $@", + executable = True, +) +EOF + bazel build :foo.sh + foopath=`bazel info bazel-genfiles`/foo.sh + grep -q 'There are' $foopath || fail "expected patch to be applied" + grep -q '^patched$' $foopath || fail "expected patch commands to run in patch_directory" +} + +test_utils_patch_directory() { + do_test_utils_patch_directory 'patch_directory = "sub",' "" +} + +test_utils_patch_directory_with_patch_tool() { + do_test_utils_patch_directory 'patch_directory = "sub", patch_tool = "patch",' "" +} + +test_utils_patch_directory_from_attr() { + do_test_utils_patch_directory "" 'patch_directory="sub",' +} + run_suite "external patching tests"
diff --git a/src/test/tools/bzlmod/MODULE.bazel.lock b/src/test/tools/bzlmod/MODULE.bazel.lock index 4edc3ab..b6d8d83 100644 --- a/src/test/tools/bzlmod/MODULE.bazel.lock +++ b/src/test/tools/bzlmod/MODULE.bazel.lock
@@ -199,7 +199,7 @@ "moduleExtensions": { "@@rules_kotlin+//src/main/starlark/core/repositories:bzlmod_setup.bzl%rules_kotlin_extensions": { "general": { - "bzlTransitiveDigest": "i/i90DMdIifmbl+U6MMrM7QayQUvoNf2GsYd738TxxY=", + "bzlTransitiveDigest": "rmf8Oxala2gciqij2vzqzi+3SiXQs72hr72k3e/mIBo=", "usagesDigest": "ZQiGVXrj8yl+WvAOGWZCipKxak7/2zZWT+1/qI4Ac4U=", "recordedInputs": [ "REPO_MAPPING:rules_kotlin+,bazel_tools bazel_tools" @@ -256,7 +256,7 @@ }, "@@rules_python+//python/extensions:config.bzl%config": { "general": { - "bzlTransitiveDigest": "K2CtbBVV3aCFwN1jN9HinfYTSjqp8rV4DuXb3kXzyDM=", + "bzlTransitiveDigest": "STd/6iQ6aDNwdI3ktrtw7n6zefNS/ZSjr/4MOg+SouI=", "usagesDigest": "cwDr/pxIBBufM3ypvgH7wsGwkQH2zIyIbFPGTAtKXhw=", "recordedInputs": [ "REPO_MAPPING:rules_python+,bazel_tools bazel_tools", @@ -424,7 +424,7 @@ }, "@@rules_python+//python/uv:uv.bzl%uv": { "general": { - "bzlTransitiveDigest": "ijW9KS7qsIY+yBVvJ+Nr1mzwQox09j13DnE3iIwaeTM=", + "bzlTransitiveDigest": "IgkgxNrtlAec7OnaFrWoCC5NQhHcY/aIz4jsqcNMm8Y=", "usagesDigest": "T2gL2j7jWAaLy4VVKveM57Xr5plhW5gICghTUpDs6dM=", "recordedInputs": [ "REPO_MAPPING:rules_python+,bazel_tools bazel_tools",
diff --git a/tools/build_defs/repo/utils.bzl b/tools/build_defs/repo/utils.bzl index 15ad8be..9691732 100644 --- a/tools/build_defs/repo/utils.bzl +++ b/tools/build_defs/repo/utils.bzl
@@ -140,13 +140,13 @@ ctx.delete(path) ctx.symlink(src_path, path) -def patch(ctx, patches = None, patch_cmds = None, patch_cmds_win = None, patch_tool = None, patch_args = None, auth = None): +def patch(ctx, patches = None, patch_cmds = None, patch_cmds_win = None, patch_tool = None, patch_args = None, auth = None, patch_directory = None): """Implementation of patching an already extracted repository. This rule is intended to be used in the implementation function of a repository rule. If the parameters `patches`, `patch_tool`, - `patch_args`, `patch_cmds` and `patch_cmds_win` are not specified - then they are taken from `ctx.attr`. + `patch_args`, `patch_cmds`, `patch_cmds_win` and `patch_directory` + are not specified then they are taken from `ctx.attr`. Args: ctx: The repository context of the repository rule calling this utility @@ -162,6 +162,9 @@ patches. String. patch_args: Arguments to pass to the patch tool. List of strings. auth: An optional dict specifying authentication information for some of the URLs. + patch_directory: Directory relative to the repository root in which to + apply `patches` and run `patch_cmds`. Remote patches are always applied + at the repository root. Defaults to the repository root. Returns: dict mapping remote patch URLs to a download info. @@ -209,6 +212,9 @@ new_patch_args.extend(patch_args) patch_args = new_patch_args + if patch_directory == None and hasattr(ctx.attr, "patch_directory"): + patch_directory = ctx.attr.patch_directory + if len(remote_patches) > 0 or len(patches) > 0 or len(patch_cmds) > 0: ctx.report_progress("Patching repository") @@ -248,7 +254,7 @@ else: strip = 0 for patchfile in patches: - ctx.patch(patchfile, strip) + ctx.patch(patchfile, strip, directory = patch_directory or "") else: for patchfile in patches: command = "{patchtool} {patch_args} < {patchfile}".format( @@ -259,20 +265,20 @@ for arg in patch_args ]), ) - st = ctx.execute([bash_exe, "-c", command]) + st = ctx.execute([bash_exe, "-c", command], working_directory = patch_directory or "") if st.return_code: fail("Error applying patch %s:\n%s%s" % (str(patchfile), st.stderr, st.stdout)) if _is_windows(ctx) and patch_cmds_win: for cmd in patch_cmds_win: - st = ctx.execute([powershell_exe, "/c", cmd]) + st = ctx.execute([powershell_exe, "/c", cmd], working_directory = patch_directory or "") if st.return_code: fail("Error applying patch command %s:\n%s%s" % (cmd, st.stdout, st.stderr)) else: for cmd in patch_cmds: - st = ctx.execute([bash_exe, "-c", cmd]) + st = ctx.execute([bash_exe, "-c", cmd], working_directory = patch_directory or "") if st.return_code: fail("Error applying patch command %s:\n%s%s" % (cmd, st.stdout, st.stderr))