bazel: validate foreign_cc outputs before the action exits (#1517)

novas0x2a · web-flow · commit 60fee81ef42e · 2026-04-04T10:18:35.000Z
When a foreign build's outputs are set up wrong, Bazel fails with an
opaque "output ... was not created" error. That skips the existing
foreign_cc failure path, so users lose both the build log and the usual
--sandbox_debug workflow.

Teach the generated build script to validate every expected installed
output before the action completes. Missing outputs now fail the
original action, print the build log, and keep the temp tree around for
postmortem debug.

The failure output looks something like:
```
_____ BEGIN OUTPUT VALIDATION _____
rules_foreign_cc: install root: /[...]/execroot/_main/bazel-out/k8-fastbuild/bin/test/output_validation/repro
rules_foreign_cc: validating expected installed outputs
rules_foreign_cc: missing expected installed output: lib/librepro.a
rules_foreign_cc: other files in the same directory (max 5):
rules_foreign_cc:   librepro-renamed.a
rules_foreign_cc: other files in the install root with the same name (max 5):
rules_foreign_cc:   wrong/librepro.a
rules_foreign_cc: missing expected installed output: lib/librepro_same_dir.a
rules_foreign_cc: other files in the same directory (max 5):
rules_foreign_cc:   librepro-renamed.a
rules_foreign_cc: expected output validation failed
_____ END OUTPUT VALIDATION _____

```

Because it's awful to sit through an entire build just to have it fail,
then realize you didn't set --sandbox_debug and have to do the entire
build again to learn anything, it also performs two checks:
- what other files are in the same dir? (max 5)
- what other files are in the install tree with the same name? (max 5)

This makes it more likely you learn something from the failed build.

If this malfunctions for a specific target, you can disable it by
setting the attribute: `experimental_validate_outputs_in_action=False`.
It defaults to True.
diff --git a/examples/cmake_defines/BUILD.bazel b/examples/cmake_defines/BUILD.bazel
@@ -3,6 +3,7 @@ load("@rules_foreign_cc//foreign_cc:defs.bzl", "cmake")
 cmake(
     name = "lib_a",
     lib_source = ":lib_a_sources",
+    out_include_dir = "",
     out_static_libs = select({
         "@platforms//os:windows": ["lib_a.lib"],
         "//conditions:default": ["liblib_a.a"],
@@ -14,6 +15,7 @@ cmake(
     name = "lib_b",
     defines = ["FOO"],
     lib_source = ":lib_b_sources",
+    out_include_dir = "",
     out_static_libs = select({
         "@platforms//os:windows": ["lib_b.lib"],
         "//conditions:default": ["liblib_b.a"],
diff --git a/examples/cmake_defines/nocrosstool/BUILD.bazel b/examples/cmake_defines/nocrosstool/BUILD.bazel
@@ -4,6 +4,7 @@ cmake(
     name = "lib_a",
     generate_crosstool_file = False,
     lib_source = "//cmake_defines:lib_a_sources",
+    out_include_dir = "",
     out_static_libs = select({
         "@platforms//os:windows": ["lib_a.lib"],
         "//conditions:default": ["liblib_a.a"],
@@ -16,6 +17,7 @@ cmake(
     defines = ["FOO"],
     generate_crosstool_file = False,
     lib_source = "//cmake_defines:lib_b_sources",
+    out_include_dir = "",
     out_static_libs = select({
         "@platforms//os:windows": ["lib_b.lib"],
         "//conditions:default": ["liblib_b.a"],
diff --git a/foreign_cc/private/framework.bzl b/foreign_cc/private/framework.bzl
@@ -122,6 +122,11 @@ CC_EXTERNAL_RULE_ATTRIBUTES = {
             "Variables containing `PATH` (e.g. `PATH`, `LD_LIBRARY_PATH`, `CPATH`) entries will be prepended to the existing variable."
         ),
     ),
+    "experimental_validate_outputs_in_action": attr.bool(
+        doc = "Validate expected installed outputs inside the main foreign build action before it exits.",
+        mandatory = False,
+        default = True,
+    ),
     "includes": attr.string_list(
         doc = (
             "Optional list of include dirs to be passed to the dependencies of this library. " +
@@ -476,6 +481,13 @@ def cc_external_rule_impl(ctx, attrs):
     else:
         postfix_script = [expand_locations_and_make_variables(ctx, attrs.postfix_script, "postfix_script", data_dependencies)]
 
+    validation_script = []
+    if attrs.experimental_validate_outputs_in_action:
+        validation_script = _validate_expected_outputs_script(
+            ctx,
+            outputs.expected_output_paths,
+        )
+
     script_lines = [
         "##echo## \"\"",
         "##echo## \"{}\"".format(lib_header),
@@ -490,7 +502,7 @@ def cc_external_rule_impl(ctx, attrs):
         "##mkdirs## $$EXT_BUILD_DEPS$$",
     ] + _print_env() + _copy_deps_and_tools(inputs) + [
         "cd $$BUILD_TMPDIR$$",
-    ] + attrs.create_configure_script(ConfigureParameters(ctx = ctx, attrs = attrs, inputs = inputs)) + postfix_script + [
+    ] + attrs.create_configure_script(ConfigureParameters(ctx = ctx, attrs = attrs, inputs = inputs)) + postfix_script + validation_script + [
         # replace references to the root directory when building ($BUILD_TMPDIR)
         # and the root where the dependencies were installed ($EXT_BUILD_DEPS)
         # for the results which are in $INSTALLDIR (with placeholder)
@@ -852,6 +864,7 @@ _Outputs = provider(
         data_dirs = "Directory containing additional files generated by the build",
         data_files = "Data files, which will be created by the action",
         declared_outputs = "All output files and directories of the action",
+        expected_output_paths = "Install-root-relative paths expected to exist after the foreign build finishes",
     ),
 )
 
@@ -884,19 +897,27 @@ def _define_outputs(ctx, attrs, lib_name):
     else:
         out_include_dir = ""
 
+    expected_output_paths = [attrs.out_include_dir] if out_include_dir else []
+
     out_data_dirs = []
     for dir in attr_out_data_dirs:
         out_data_dirs.append(ctx.actions.declare_directory(lib_name + "/" + dir.lstrip("/")))
+        expected_output_paths.append(dir.lstrip("/"))
 
     out_data_files = _declare_out(ctx, lib_name, "/", attr_out_data_files)
+    expected_output_paths += [_expected_output_path("/", file) for file in attr_out_data_files]
 
     out_binary_files = _declare_out(ctx, lib_name, attrs.out_bin_dir, attr_binaries_libs)
+    expected_output_paths += [_expected_output_path(attrs.out_bin_dir, file) for file in attr_binaries_libs]
 
     libraries = LibrariesToLinkInfo(
         static_libraries = _declare_out(ctx, lib_name, attrs.out_lib_dir, static_libraries),
         shared_libraries = _declare_out(ctx, lib_name, attrs.out_dll_dir if targets_windows(ctx, None) else attrs.out_lib_dir, attr_shared_libs),
         interface_libraries = _declare_out(ctx, lib_name, attrs.out_lib_dir, attr_interface_libs),
     )
+    expected_output_paths += [_expected_output_path(attrs.out_lib_dir, file) for file in static_libraries]
+    expected_output_paths += [_expected_output_path(attrs.out_dll_dir if targets_windows(ctx, None) else attrs.out_lib_dir, file) for file in attr_shared_libs]
+    expected_output_paths += [_expected_output_path(attrs.out_lib_dir, file) for file in attr_interface_libs]
 
     declared_outputs = [out_include_dir] if out_include_dir else []
     declared_outputs += out_data_dirs + out_binary_files + out_data_files
@@ -910,13 +931,91 @@ def _define_outputs(ctx, attrs, lib_name):
         data_dirs = out_data_dirs,
         data_files = out_data_files,
         declared_outputs = declared_outputs,
+        expected_output_paths = expected_output_paths,
     )
 
 def _declare_out(ctx, lib_name, dir_, files):
     if files and len(files) > 0:
         return [ctx.actions.declare_file("/".join([lib_name, dir_, file])) for file in files]
     return []
 
+def _validate_expected_outputs_script(ctx, expected_output_paths):
+    if not expected_output_paths:
+        return []
+
+    validate_output_function = create_function(
+        ctx,
+        "validate_expected_output",
+        "\n".join([
+            "local expected_output_path=\"$1\"",
+            "local expected_basename=\"$2\"",
+            "local rel_expected_output_path=\"${expected_output_path#\"$INSTALLDIR\"/}\"",
+            "local expected_dir",
+            "local find_bin=\"${REAL_FIND:-find}\"",
+            "expected_dir=$(dirname \"$expected_output_path\")",
+            "if [[ -e \"$expected_output_path\" ]]; then",
+            "  return 0",
+            "fi",
+            "echo \"rules_foreign_cc: missing expected installed output: $rel_expected_output_path\" >&2",
+            "if [[ -d \"$expected_dir\" ]]; then",
+            "  local rel_expected_dir=\"${expected_dir#\"$INSTALLDIR\"/}\"",
+            "  local sibling_candidates",
+            "  sibling_candidates=$($find_bin \"$expected_dir\" -maxdepth 1 -mindepth 1 -print 2>/dev/null | head -n 5 || true)",
+            "  if [[ -n \"$sibling_candidates\" ]]; then",
+            "    echo \"rules_foreign_cc: other files in the same directory (max 5):\" >&2",
+            "    while IFS= read -r sibling_candidate; do",
+            "      [[ -n \"$sibling_candidate\" ]] || continue",
+            "      echo \"rules_foreign_cc:   ${sibling_candidate##*/}\" >&2",
+            "    done <<< \"$sibling_candidates\"",
+            "  fi",
+            "fi",
+            "local nearby_candidates",
+            "nearby_candidates=$($find_bin \"$INSTALLDIR\" -mindepth 1 -name \"$expected_basename\" -print 2>/dev/null | head -n 5 || true)",
+            "if [[ -n \"$nearby_candidates\" ]]; then",
+            "  echo \"rules_foreign_cc: other files in the install root with the same name (max 5):\" >&2",
+            "  while IFS= read -r nearby_candidate; do",
+            "    [[ -n \"$nearby_candidate\" ]] || continue",
+            "    echo \"rules_foreign_cc:   ${nearby_candidate#\"$INSTALLDIR\"/}\" >&2",
+            "  done <<< \"$nearby_candidates\"",
+            "fi",
+            "return 1",
+        ]),
+    )
+
+    lines = [
+        validate_output_function,
+        "echo \"_____ BEGIN OUTPUT VALIDATION _____\"",
+        "missing_foreign_cc_outputs=0",
+        "echo \"rules_foreign_cc: install root: $INSTALLDIR\"",
+        "echo \"rules_foreign_cc: validating expected installed outputs\"",
+    ]
+
+    for rel in expected_output_paths:
+        basename = rel.rsplit("/", 1)[-1]
+        lines.extend([
+            "if ! validate_expected_output \"$$INSTALLDIR$$/{rel}\" \"{basename}\"; then".format(
+                rel = rel,
+                basename = basename,
+            ),
+            "  missing_foreign_cc_outputs=1",
+            "fi",
+        ])
+
+    lines.extend([
+        "if [[ \"$missing_foreign_cc_outputs\" -ne 0 ]]; then",
+        "  echo \"rules_foreign_cc: expected output validation failed\" >&2",
+        "  echo \"_____ END OUTPUT VALIDATION _____\" >&2",
+        "  exit 1",
+        "fi",
+        "echo \"_____ END OUTPUT VALIDATION _____\"",
+    ])
+    return lines
+
+def _expected_output_path(dir_, file):
+    if dir_ == "/" or not dir_:
+        return file.lstrip("/")
+    return dir_.rstrip("/") + "/" + file.lstrip("/")
+
 # buildifier: disable=name-conventions
 InputFiles = provider(
     doc = (
diff --git a/test/BUILD.bazel b/test/BUILD.bazel
@@ -1,5 +1,6 @@
 load("@bazel_lib//lib:diff_test.bzl", "diff_test")
 load("@rules_shell//shell:sh_library.bzl", "sh_library")
+load("@rules_shell//shell:sh_test.bzl", "sh_test")
 load("//tools/lint:linters.bzl", "shellcheck_test")
 load(":cmake_text_tests.bzl", "cmake_script_test_suite")
 load(":convert_shell_script_test.bzl", "shell_script_conversion_suite")
@@ -103,3 +104,11 @@ diff_test(
     file1 = "expected/out_symlinked_dirs.txt",
     file2 = ":symlink_dirs",
 )
+
+sh_test(
+    name = "output_validation_script_test",
+    size = "small",
+    srcs = ["assert_output_validation_script.sh"],
+    args = ["$(locations //test/output_validation:wrong_output_in_action_check_script)"],
+    data = ["//test/output_validation:wrong_output_in_action_check_script"],
+)
diff --git a/test/assert_output_validation_script.sh b/test/assert_output_validation_script.sh
@@ -0,0 +1,21 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+script=""
+for candidate in "$@"; do
+  if [[ "$candidate" == *"/build_script.sh" ]]; then
+    script="$candidate"
+    break
+  fi
+done
+
+if [[ -z "$script" ]]; then
+  echo "expected one generated build_script.sh path in args" >&2
+  exit 1
+fi
+
+grep -F 'function validate_expected_output() {' "$script"
+grep -F 'rules_foreign_cc: validating expected installed outputs' "$script"
+grep -F 'rules_foreign_cc: other files in the same directory (max 5):' "$script"
+grep -F 'rules_foreign_cc: other files in the install root with the same name (max 5):' "$script"
+grep -F "find_bin=\"\${REAL_FIND:-find}\"" "$script"
diff --git a/test/output_group_file_rule.bzl b/test/output_group_file_rule.bzl
@@ -0,0 +1,18 @@
+"""Expose files from a target output group through DefaultInfo for tests."""
+
+def _output_group_file_impl(ctx):
+    files = getattr(ctx.attr.target[OutputGroupInfo], ctx.attr.output_group).to_list()
+    if not files:
+        fail("Expected at least one file in output group {} for {}".format(
+            ctx.attr.output_group,
+            ctx.attr.target.label,
+        ))
+    return [DefaultInfo(files = depset(files))]
+
+output_group_file = rule(
+    implementation = _output_group_file_impl,
+    attrs = {
+        "output_group": attr.string(mandatory = True),
+        "target": attr.label(mandatory = True),
+    },
+)
diff --git a/test/output_validation/BUILD.bazel b/test/output_validation/BUILD.bazel
@@ -0,0 +1,64 @@
+load("@rules_foreign_cc//foreign_cc:defs.bzl", "cmake")
+load("//test:output_group_file_rule.bzl", "output_group_file")
+
+package(default_visibility = ["//visibility:public"])
+
+filegroup(
+    name = "srcs",
+    srcs = [
+        "CMakeLists.txt",
+        "repro.c",
+        "repro.h",
+    ],
+)
+
+cmake(
+    name = "output_validation_script_fixture",
+    generate_args = ["-GNinja"],
+    lib_name = "repro_fixture",
+    lib_source = ":srcs",
+    out_static_libs = select({
+        "@platforms//os:windows": ["repro.lib"],
+        "//conditions:default": ["librepro.a"],
+    }),
+    tags = ["manual"],
+)
+
+output_group_file(
+    name = "wrong_output_in_action_check_script",
+    output_group = "CMake_logs",
+    target = ":output_validation_script_fixture",
+)
+
+# TODO: We don't have a good way of validating that actions fail with the
+# expected error message, so this target can be bazel built to perform a manual
+# test.
+cmake(
+    name = "wrong_output_in_action_check",
+    generate_args = ["-GNinja"],
+    lib_name = "repro",
+    lib_source = ":srcs",
+    out_static_libs = select({
+        "@platforms//os:windows": [
+            "repro.lib",
+            "repro_same_dir.lib",
+        ],
+        "//conditions:default": [
+            "librepro.a",
+            "librepro_same_dir.a",
+        ],
+    }),
+    postfix_script = select({
+        "@platforms//os:windows": "\n".join([
+            "mkdir -p \"$$INSTALLDIR/wrong\"",
+            "mv \"$$INSTALLDIR/lib/repro.lib\" \"$$INSTALLDIR/wrong/repro.lib\"",
+            "cp \"$$INSTALLDIR/wrong/repro.lib\" \"$$INSTALLDIR/lib/repro-renamed.lib\"",
+        ]),
+        "//conditions:default": "\n".join([
+            "mkdir -p \"$$INSTALLDIR/wrong\"",
+            "mv \"$$INSTALLDIR/lib/librepro.a\" \"$$INSTALLDIR/wrong/librepro.a\"",
+            "cp \"$$INSTALLDIR/wrong/librepro.a\" \"$$INSTALLDIR/lib/librepro-renamed.a\"",
+        ]),
+    }),
+    tags = ["manual"],
+)
diff --git a/test/output_validation/CMakeLists.txt b/test/output_validation/CMakeLists.txt
@@ -0,0 +1,8 @@
+cmake_minimum_required(VERSION 3.10)
+project(output_validation_repro C)
+
+add_library(repro STATIC repro.c)
+target_include_directories(repro PUBLIC ${CMAKE_CURRENT_SOURCE_DIR})
+
+install(TARGETS repro ARCHIVE DESTINATION lib)
+install(FILES repro.h DESTINATION include)
diff --git a/test/output_validation/repro.c b/test/output_validation/repro.c
@@ -0,0 +1,3 @@
+#include "repro.h"
+
+int repro_answer(void) { return 42; }
diff --git a/test/output_validation/repro.h b/test/output_validation/repro.h
@@ -0,0 +1,6 @@
+#ifndef TEST_OUTPUT_VALIDATION_REPRO_H_
+#define TEST_OUTPUT_VALIDATION_REPRO_H_
+
+int repro_answer(void);
+
+#endif

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+#include "repro.h"`
	`2`	`+`
	`3`	`+int repro_answer(void) { return 42; }`