From 5f98726fbfada03f35bb142593259e6dd8010228 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Sun, 20 Jun 2021 10:45:33 -0700 Subject: [PATCH] Improve message when docs are out-of-date. Checking in generated build artifacts makes things harder for contributors. The get a red CI status but have to know to look at the definition of the diff_test target to discover there's a command to auto-update. Instead we just print that command on failure. The failure now looks like this: ``` $ bazel test //doc:all INFO: Analyzed 9 targets (0 packages loaded, 0 targets configured). INFO: Found 9 test targets... FAIL: //doc:check_ios.doc (see /home/alexeagle/.cache/bazel/_bazel_alexeagle/32c0f21b6a1d2ed917381b593d4bcb34/execroot/build_bazel_rules_apple/bazel-out/k8-fastbuild/testlogs/doc/check_ios.doc/test.log) INFO: From Testing //doc:check_ios.doc: ==================== Test output for //doc:check_ios.doc: 393c393 < | entitlements | The entitlements file required for device builds of this target. If absent, the default entitlements from the provisioning profile will be used.

The following variables are substituted in the entitlements file: $(CFBundleIdentifier) with the bundle ID of the application and $(AppIdentifierPrefix) with the value of the ApplicationIdentifierPrefix key from the target's provisioning profile. | Label | optional | None | --- > | entitlements | The entitlements file required for device builds of this target. If absent, the default entitlements from the provisioning profile will be used.

The following variables are substituted in the entitlements file: $(CFBundleIdentifier) with the bundle ID of the application and $(AppIdentifierPrefix) with the value of the ApplicationIdentifierPrefixe key from the target's provisioning profile. | Label | optional | None | FAIL: files "doc/ios.doc.md" and "doc/rules-ios.md" differ. Please update the docs by running bazel run //doc:update ================================================================================ INFO: Elapsed time: 0.067s, Critical Path: 0.02s ``` --- .bazelci/update_workspace_to_deps_heads.sh | 2 + apple/repositories.bzl | 2 + doc/BUILD.bazel | 7 ++- doc/bazel-skylib.pr307.patch | 50 ++++++++++++++++++++++ 4 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 doc/bazel-skylib.pr307.patch diff --git a/.bazelci/update_workspace_to_deps_heads.sh b/.bazelci/update_workspace_to_deps_heads.sh index 59d97e48f8..123e9eef12 100755 --- a/.bazelci/update_workspace_to_deps_heads.sh +++ b/.bazelci/update_workspace_to_deps_heads.sh @@ -28,6 +28,8 @@ load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")\ \ git_repository(\ \ name = "bazel_skylib",\ +\ patches = ["@build_bazel_rules_apple//doc:bazel-skylib.pr307.patch"],\ +\ patch_args = ["-p1"],\ \ remote = "https://github.com/bazelbuild/bazel-skylib.git",\ \ branch = "main",\ )\ diff --git a/apple/repositories.bzl b/apple/repositories.bzl index 971ae022a7..d5c084ae7c 100644 --- a/apple/repositories.bzl +++ b/apple/repositories.bzl @@ -99,6 +99,8 @@ def apple_rules_dependencies(ignore_version_differences = False): _maybe( http_archive, name = "bazel_skylib", + patches = ["@build_bazel_rules_apple//doc:bazel-skylib.pr307.patch"], + patch_args = ["-p1"], urls = [ "https://github.com/bazelbuild/bazel-skylib/releases/download/1.0.3/bazel-skylib-1.0.3.tar.gz", ], diff --git a/doc/BUILD.bazel b/doc/BUILD.bazel index a7b6de03a6..66196a8ec2 100644 --- a/doc/BUILD.bazel +++ b/doc/BUILD.bazel @@ -35,11 +35,13 @@ _DOC_SRCS = [ for file in _DOC_SRCS ] -# If this test fails, run -# bazel run //doc:update +# Help developers who get a red CI result by telling them how to fix it +_failure_message = "\nPlease update the docs by running\n bazel run //doc:update" + [ diff_test( name = "check_" + file, + failure_message = _failure_message, file1 = file + ".md", file2 = "rules-%s.md" % file.replace(".doc", ""), ) @@ -57,6 +59,7 @@ stardoc( diff_test( name = "check_providers", + failure_message = _failure_message, file1 = ":providers_doc", file2 = "providers.md", ) diff --git a/doc/bazel-skylib.pr307.patch b/doc/bazel-skylib.pr307.patch new file mode 100644 index 0000000000..f333042c72 --- /dev/null +++ b/doc/bazel-skylib.pr307.patch @@ -0,0 +1,50 @@ +commit 58ad64fb010ce54e4530f2d0a5426e17c6199dce +Author: Alex Eagle +Date: Sun Jun 20 10:39:19 2021 -0700 + + diff_test: add ability for caller to specify a message printed on failure. + + Useful for the use case of a pair of .test and .update targets for checked-in golden files + +diff --git a/rules/diff_test.bzl b/rules/diff_test.bzl +index 71faf40..acde2ea 100644 +--- a/rules/diff_test.bzl ++++ b/rules/diff_test.bzl +@@ -58,7 +58,7 @@ if "!RF2!" equ "" ( + fc.exe 2>NUL 1>NUL /B "!RF1!" "!RF2!" + if %ERRORLEVEL% neq 0 ( + if %ERRORLEVEL% equ 1 ( +- echo>&2 FAIL: files "{file1}" and "{file2}" differ ++ echo>&2 FAIL: files "{file1}" and "{file2}" differ. {fail_msg} + exit /b 1 + ) else ( + fc.exe /B "!RF1!" "!RF2!" +@@ -66,6 +66,7 @@ if %ERRORLEVEL% neq 0 ( + ) + ) + """.format( ++ fail_msg = ctx.attr.failure_message, + file1 = _runfiles_path(ctx.file.file1), + file2 = _runfiles_path(ctx.file.file2), + ), +@@ -95,10 +96,11 @@ else + exit 1 + fi + if ! diff "$RF1" "$RF2"; then +- echo >&2 "FAIL: files \"{file1}\" and \"{file2}\" differ" ++ echo >&2 "FAIL: files \"{file1}\" and \"{file2}\" differ. {fail_msg}" + exit 1 + fi + """.format( ++ fail_msg = ctx.attr.failure_message, + file1 = _runfiles_path(ctx.file.file1), + file2 = _runfiles_path(ctx.file.file2), + ), +@@ -112,6 +114,7 @@ fi + + _diff_test = rule( + attrs = { ++ "failure_message": attr.string(), + "file1": attr.label( + allow_single_file = True, + mandatory = True,