From 2884b863109bf6a3e706e781ea8b08fbad674cc1 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Fri, 6 Mar 2026 13:46:32 -0800 Subject: [PATCH 1/2] fix: give an error message t accept the patch which works in any working directory --- MODULE.bazel.lock | 11 ++++++----- diff/private/diff.bzl | 4 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index fbb9d6e..766c34f 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -35,8 +35,9 @@ "https://bcr.bazel.build/modules/bazel_features/1.3.0/MODULE.bazel": "cdcafe83ec318cda34e02948e81d790aab8df7a929cec6f6969f13a489ccecd9", "https://bcr.bazel.build/modules/bazel_features/1.30.0/MODULE.bazel": "a14b62d05969a293b80257e72e597c2da7f717e1e69fa8b339703ed6731bec87", "https://bcr.bazel.build/modules/bazel_features/1.33.0/MODULE.bazel": "8b8dc9d2a4c88609409c3191165bccec0e4cb044cd7a72ccbe826583303459f6", - "https://bcr.bazel.build/modules/bazel_features/1.33.0/source.json": "13617db3930328c2cd2807a0f13d52ca870ac05f96db9668655113265147b2a6", "https://bcr.bazel.build/modules/bazel_features/1.4.1/MODULE.bazel": "e45b6bb2350aff3e442ae1111c555e27eac1d915e77775f6fdc4b351b758b5d7", + "https://bcr.bazel.build/modules/bazel_features/1.42.1/MODULE.bazel": "275a59b5406ff18c01739860aa70ad7ccb3cfb474579411decca11c93b951080", + "https://bcr.bazel.build/modules/bazel_features/1.42.1/source.json": "fcd4396b2df85f64f2b3bb436ad870793ecf39180f1d796f913cc9276d355309", "https://bcr.bazel.build/modules/bazel_features/1.9.0/MODULE.bazel": "885151d58d90d8d9c811eb75e3288c11f850e1d6b481a8c9f766adee4712358b", "https://bcr.bazel.build/modules/bazel_features/1.9.1/MODULE.bazel": "8f679097876a9b609ad1f60249c49d68bfab783dd9be012faf9d82547b14815a", "https://bcr.bazel.build/modules/bazel_lib/3.0.0/MODULE.bazel": "22b70b80ac89ad3f3772526cd9feee2fa412c2b01933fea7ed13238a448d370d", @@ -59,8 +60,8 @@ "https://bcr.bazel.build/modules/bazelrc-preset.bzl/1.6.0/source.json": "ad6cee6b42b5def78f6b3082c0bd7041a330003cbace330aaf1d3f94a122c48b", "https://bcr.bazel.build/modules/buildifier_prebuilt/8.2.1/MODULE.bazel": "57f2736616367163651d9d3e918cddfd6c67ec7a541d924f2c4544d3fae8630e", "https://bcr.bazel.build/modules/buildifier_prebuilt/8.2.1/source.json": "a3e79a347f9d8256d83eba2b621b5b6affa17c8b3236127f030db03b72a44a91", - "https://bcr.bazel.build/modules/buildozer/8.2.1/MODULE.bazel": "61e9433c574c2bd9519cad7fa66b9c1d2b8e8d5f3ae5d6528a2c2d26e68d874d", - "https://bcr.bazel.build/modules/buildozer/8.2.1/source.json": "7c33f6a26ee0216f85544b4bca5e9044579e0219b6898dd653f5fb449cf2e484", + "https://bcr.bazel.build/modules/buildozer/8.5.1/MODULE.bazel": "a35d9561b3fc5b18797c330793e99e3b834a473d5fbd3d7d7634aafc9bdb6f8f", + "https://bcr.bazel.build/modules/buildozer/8.5.1/source.json": "e3386e6ff4529f2442800dee47ad28d3e6487f36a1f75ae39ae56c70f0cd2fbd", "https://bcr.bazel.build/modules/google_benchmark/1.8.2/MODULE.bazel": "a70cf1bba851000ba93b58ae2f6d76490a9feb74192e57ab8e8ff13c34ec50cb", "https://bcr.bazel.build/modules/googletest/1.11.0/MODULE.bazel": "3a83f095183f66345ca86aa13c58b59f9f94a2f81999c093d4eeaa2d262d12f4", "https://bcr.bazel.build/modules/googletest/1.14.0.bcr.1/MODULE.bazel": "22c31a561553727960057361aa33bf20fb2e98584bc4fec007906e27053f80c6", @@ -122,8 +123,8 @@ "https://bcr.bazel.build/modules/rules_cc/0.1.5/MODULE.bazel": "88dfc9361e8b5ae1008ac38f7cdfd45ad738e4fa676a3ad67d19204f045a1fd8", "https://bcr.bazel.build/modules/rules_cc/0.2.0/MODULE.bazel": "b5c17f90458caae90d2ccd114c81970062946f49f355610ed89bebf954f5783c", "https://bcr.bazel.build/modules/rules_cc/0.2.13/MODULE.bazel": "eecdd666eda6be16a8d9dc15e44b5c75133405e820f620a234acc4b1fdc5aa37", - "https://bcr.bazel.build/modules/rules_cc/0.2.14/MODULE.bazel": "353c99ed148887ee89c54a17d4100ae7e7e436593d104b668476019023b58df8", - "https://bcr.bazel.build/modules/rules_cc/0.2.14/source.json": "55d0a4587c5592fad350f6e698530f4faf0e7dd15e69d43f8d87e220c78bea54", + "https://bcr.bazel.build/modules/rules_cc/0.2.17/MODULE.bazel": "1849602c86cb60da8613d2de887f9566a6d354a6df6d7009f9d04a14402f9a84", + "https://bcr.bazel.build/modules/rules_cc/0.2.17/source.json": "3832f45d145354049137c0090df04629d9c2b5493dc5c2bf46f1834040133a07", "https://bcr.bazel.build/modules/rules_cc/0.2.8/MODULE.bazel": "f1df20f0bf22c28192a794f29b501ee2018fa37a3862a1a2132ae2940a23a642", "https://bcr.bazel.build/modules/rules_foreign_cc/0.9.0/MODULE.bazel": "c9e8c682bf75b0e7c704166d79b599f93b72cfca5ad7477df596947891feeef6", "https://bcr.bazel.build/modules/rules_fuzzing/0.5.2/MODULE.bazel": "40c97d1144356f52905566c55811f13b299453a14ac7769dfba2ac38192337a8", diff --git a/diff/private/diff.bzl b/diff/private/diff.bzl index 671fcca..31307ee 100644 --- a/diff/private/diff.bzl +++ b/diff/private/diff.bzl @@ -64,13 +64,13 @@ echo $EXIT_CODE > {} validate = ctx.attr._options[DiffOptionsInfo].validate_diffs if validate: + # NB: the error message we print here allows the user to be in any working directory. validation_outputs.append(_validate_no_diff(ctx, ctx.outputs.exit_code, """\ ERROR: diff command exited with non-zero status. To accept the diff, run: - patch -d \\$(bazel info workspace) -p0 < {patch} + ( cd \\$(bazel info workspace); patch -p0 < {patch} ) """.format(patch = ctx.outputs.patch.path))) - return [ DefaultInfo(files = depset(outputs)), OutputGroupInfo( From 9f2bd8bfc3ab7993f5a47860d6b996289284ff50 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Wed, 11 Mar 2026 17:43:43 -0700 Subject: [PATCH 2/2] WIP: allow multiple to/from files with a provider saying how to diff them Supports more complex scenarios in downstream rulesets like copying multiple proto outputs --- diff/defs.bzl | 7 +- diff/private/diff.bzl | 177 ++++++++++++++++++------- diff/tests/case.3.dir_a/file.txt.orig | 1 + diff/tests/case.3.dir_a/file.txt.rej | 5 + examples/multiple-files/BUILD | 39 ++++++ examples/multiple-files/diff_files.bzl | 21 +++ examples/multiple-files/file1.txt | 0 examples/multiple-files/file2.txt | 0 8 files changed, 200 insertions(+), 50 deletions(-) create mode 100644 diff/tests/case.3.dir_a/file.txt.orig create mode 100644 diff/tests/case.3.dir_a/file.txt.rej create mode 100644 examples/multiple-files/BUILD create mode 100644 examples/multiple-files/diff_files.bzl create mode 100644 examples/multiple-files/file1.txt create mode 100644 examples/multiple-files/file2.txt diff --git a/diff/defs.bzl b/diff/defs.bzl index 3e97146..52c8cfa 100644 --- a/diff/defs.bzl +++ b/diff/defs.bzl @@ -1,7 +1,12 @@ "Public API re-exports" load("@bazel_skylib//lib:partial.bzl", "partial") -load("//diff/private:diff.bzl", "diff_rule") +load("//diff/private:diff.bzl", "diff_rule", _FilesToDiffInfo = "FilesToDiffInfo", _diff_multiple = "diff_multiple") + +# Re-export the FilesToDiffInfo provider +FilesToDiffInfo = _FilesToDiffInfo + +diff_multiple = _diff_multiple def diff(name, file1, file2, patch = None, exit_code = None, **kwargs): """Runs a diff between two files and returns the exit code. diff --git a/diff/private/diff.bzl b/diff/private/diff.bzl index 31307ee..5a0d999 100644 --- a/diff/private/diff.bzl +++ b/diff/private/diff.bzl @@ -2,6 +2,14 @@ load("//diff/private:options.bzl", "DiffOptionsInfo") +FilesToDiffInfo = provider( + doc = "Which files among the sources are meant to be diffed.", + fields = { + "src_subpaths": "The files to diff among the srcs. Names are expected to match relative to the workspace root.", + "dep_subpaths": "The files to diff among the deps. Names are expected to match relative to the workspace root.", + }, +) + # We run diff in actions, so we want to use the execution platform toolchain. DIFFUTILS_TOOLCHAIN_TYPE = "@diff.bzl//diff/toolchain:execution_type" @@ -17,13 +25,31 @@ def _validate_no_diff(ctx, exit_code_file, error_message): >&2 echo "{}" exit 1 fi - """.format(exit_code_valid.path, ctx.outputs.exit_code.path, error_message), + """.format(exit_code_valid.path, exit_code_file.path, error_message), ) return exit_code_valid -def _diff_rule_impl(ctx): - DIFF_BIN = ctx.toolchains[DIFFUTILS_TOOLCHAIN_TYPE].diffutilsinfo.diff_bin +def _maybe_validate_diff(ctx, exit_code_file, patch_file): + if ctx.attr.validate == 1: + validate = True + elif ctx.attr.validate == 0: + validate = False + else: + validate = ctx.attr._options[DiffOptionsInfo].validate_diffs + if not validate: + return [] + + # NB: the error message we print here allows the user to be in any working directory. + return [_validate_no_diff(ctx, exit_code_file, """\ + ERROR: diff command exited with non-zero status. + + To accept the diff, run: + ( cd \\$(bazel info workspace); patch -p0 < {patch} ) + """.format(patch = patch_file.path))] + +def _diff_action(ctx, file1, file2, patch, exit_code): + diff_bin = ctx.toolchains[DIFFUTILS_TOOLCHAIN_TYPE].diffutilsinfo.diff_bin command = """\ {} {} {} {} > {} EXIT_CODE=$? @@ -32,63 +58,130 @@ if [[ $EXIT_CODE == '2' ]]; then fi echo $EXIT_CODE > {} """.format( - DIFF_BIN.path, + diff_bin.path, " ".join(ctx.attr.args), - ctx.file.file1.path, - ctx.file.file2.path, - ctx.outputs.patch.path, - ctx.outputs.exit_code.path, + file1.path, + file2.path, + patch.path, + exit_code.path, ) - # If both inputs are generated, there's no writable file to patch. - is_copy_to_source = ctx.file.file1.is_source or ctx.file.file2.is_source - outputs = [ctx.outputs.patch, ctx.outputs.exit_code] + outputs = [patch, exit_code] ctx.actions.run_shell( - inputs = [ctx.file.file1, ctx.file.file2], + inputs = [file1, file2], outputs = outputs, command = command, mnemonic = "DiffutilsDiff", progress_message = "Diffing %{input} to %{output}", - tools = [DIFF_BIN], + tools = [diff_bin], toolchain = "@diff.bzl//diffutils/toolchain:execution_type", ) + return outputs + +def _diff_rule_impl(ctx): + # If both inputs are generated, there's no writable file to patch. + is_copy_to_source = ctx.file.file1.is_source or ctx.file.file2.is_source + outputs = _diff_action( + ctx, + ctx.file.file1, + ctx.file.file2, + ctx.outputs.patch, + ctx.outputs.exit_code, + ) - validation_outputs = [] copy_to_source_outputs = [ctx.outputs.patch] if is_copy_to_source else [] + validation_outputs = _maybe_validate_diff(ctx, ctx.outputs.exit_code, ctx.outputs.patch) + return [ + DefaultInfo(files = depset(outputs)), + OutputGroupInfo( + _validation = depset(validation_outputs), + # By reading the Build Events, a Bazel wrapper can identify this diff output group and apply the patch. + diff_bzl__patch = depset(copy_to_source_outputs), + ), + ] - if ctx.attr.validate == 1: - validate = True - elif ctx.attr.validate == 0: - validate = False - else: - validate = ctx.attr._options[DiffOptionsInfo].validate_diffs +# Borrowed from https://github.com/bazelbuild/bazel-skylib/blob/f7718b7b8e2003b9359248e9632c875cb48a6e48/rules/select_file.bzl +def _select_file(deps, subpath): + out = None + canonical = subpath.replace("\\", "/") + candidates = [f for d in deps for f in d[DefaultInfo].files.to_list()] + for file_ in candidates: + if file_.path.replace("\\", "/").endswith(canonical): + out = file_ + break + if not out: + files_str = ",\n".join([ + str(f.path) + for f in candidates + ]) + fail("Can not find specified file {} in {}".format(canonical, files_str)) + return out - if validate: - # NB: the error message we print here allows the user to be in any working directory. - validation_outputs.append(_validate_no_diff(ctx, ctx.outputs.exit_code, """\ - ERROR: diff command exited with non-zero status. +def _diff_multiple_impl(ctx): + outputs = [] + infos = [dep[FilesToDiffInfo] for dep in ctx.attr.deps] - To accept the diff, run: - ( cd \\$(bazel info workspace); patch -p0 < {patch} ) - """.format(patch = ctx.outputs.patch.path))) + patches = [] + exit_codes = [] + validation_outputs = [] + for info in infos: + for src_subpath, dep_subpath in zip(info.src_subpaths, info.dep_subpaths): + patch = ctx.actions.declare_file(src_subpath + ".patch") + exit_code = ctx.actions.declare_file(src_subpath + ".exit_code") + outputs.extend(_diff_action( + ctx, + _select_file(ctx.attr.srcs, src_subpath), + _select_file(ctx.attr.deps, dep_subpath), + patch, + exit_code, + )) + patches.append(patch) + exit_codes.append(exit_code) + validation_outputs.extend(_maybe_validate_diff(ctx, exit_code, patch)) return [ DefaultInfo(files = depset(outputs)), OutputGroupInfo( _validation = depset(validation_outputs), # By reading the Build Events, a Bazel wrapper can identify this diff output group and apply the patch. - diff_bzl__patch = depset(copy_to_source_outputs), + #diff_bzl__patch = depset(copy_to_source_outputs), ), ] +common_attrs = { + "args": attr.string_list( + doc = """\ + Additional arguments to pass to the diff command. + """, + default = [], + ), + "validate": attr.int( + doc = """\ + Whether to treat a non-empty diff (exit 1) as a build validation failure. + + -1: default to the flag value --@diff.bzl//diff:validate_diffs + 0: never validate + 1: always validate + + An individual Bazel invocation can run with --norun_validations to skip this behavior. + """, + default = -1, + values = [-1, 0, 1], + ), + "_options": attr.label(default = "//diff:diff_options"), +} + +diff_multiple = rule( + implementation = _diff_multiple_impl, + attrs = common_attrs | { + "srcs": attr.label_list(allow_files = True), + "deps": attr.label_list(providers = [FilesToDiffInfo]), + }, + toolchains = [DIFFUTILS_TOOLCHAIN_TYPE], +) + diff_rule = rule( implementation = _diff_rule_impl, - attrs = { - "args": attr.string_list( - doc = """\ - Additional arguments to pass to the diff command. - """, - default = [], - ), + attrs = common_attrs | { "file1": attr.label(allow_single_file = True), "file2": attr.label(allow_single_file = True), "exit_code": attr.output( @@ -107,20 +200,6 @@ diff_rule = rule( """, mandatory = True, ), - "validate": attr.int( - doc = """\ - Whether to treat a non-empty diff (exit 1) as a build validation failure. - - -1: default to the flag value --@diff.bzl//diff:validate_diffs - 0: never validate - 1: always validate - - An individual Bazel invocation can run with --norun_validations to skip this behavior. - """, - default = -1, - values = [-1, 0, 1], - ), - "_options": attr.label(default = "//diff:diff_options"), }, toolchains = [DIFFUTILS_TOOLCHAIN_TYPE], ) diff --git a/diff/tests/case.3.dir_a/file.txt.orig b/diff/tests/case.3.dir_a/file.txt.orig new file mode 100644 index 0000000..ce01362 --- /dev/null +++ b/diff/tests/case.3.dir_a/file.txt.orig @@ -0,0 +1 @@ +hello diff --git a/diff/tests/case.3.dir_a/file.txt.rej b/diff/tests/case.3.dir_a/file.txt.rej new file mode 100644 index 0000000..3b71697 --- /dev/null +++ b/diff/tests/case.3.dir_a/file.txt.rej @@ -0,0 +1,5 @@ +*************** +*** 3 +- > moo, moooooo! +--- 3 ----- ++ > moo, moooeooo! diff --git a/examples/multiple-files/BUILD b/examples/multiple-files/BUILD new file mode 100644 index 0000000..bcdcde7 --- /dev/null +++ b/examples/multiple-files/BUILD @@ -0,0 +1,39 @@ +load("@bazel_skylib//rules:write_file.bzl", "write_file") +load("@diff.bzl//diff:defs.bzl", "diff_multiple") +load(":diff_files.bzl", "diff_files") + +write_file( + name = "file1", + out = "file1.gen", + content = [ + "file1_content", + "", + ], +) + +write_file( + name = "file2", + out = "file2.gen", + content = [ + "file2_content", + "", + ], +) + +diff_files( + name = "diff_files", + srcs = [ + ":file1", + ":file2", + ], +) + +diff_multiple( + name = "diff", + srcs = [ + "file1.txt", + "file2.txt", + ], + validate = 1, + deps = [":diff_files"], +) diff --git a/examples/multiple-files/diff_files.bzl b/examples/multiple-files/diff_files.bzl new file mode 100644 index 0000000..889c518 --- /dev/null +++ b/examples/multiple-files/diff_files.bzl @@ -0,0 +1,21 @@ +"A specialization of filegroup that also provides FilesToDiffInfo instructions to diff" + +load("@diff.bzl//diff:defs.bzl", "FilesToDiffInfo") + +def _diff_files_impl(ctx): + # we expect file paths to appear in the same path under bazel-bin and the source tree + subpaths = [f.short_path for f in ctx.files.srcs] + return [ + FilesToDiffInfo( + src_subpaths = [f.replace("gen", "txt") for f in subpaths], + dep_subpaths = subpaths, + ), + DefaultInfo(files = depset(ctx.files.srcs)), + ] + +diff_files = rule( + implementation = _diff_files_impl, + attrs = { + "srcs": attr.label_list(allow_files = True), + }, +) diff --git a/examples/multiple-files/file1.txt b/examples/multiple-files/file1.txt new file mode 100644 index 0000000..e69de29 diff --git a/examples/multiple-files/file2.txt b/examples/multiple-files/file2.txt new file mode 100644 index 0000000..e69de29