[bazel] master build failing

[dbeitel-opentitan]

Description

$ bash bazelisk.sh --output_user_root=/home/dbeitel/.opentitan build --jobs 8 //...
Starting local Bazel server and connecting to it...
WARNING:root:Closest bitstream to HEAD is 26b0ee2.
WARNING:root:No manifest found. Attempting to generate manifest from legacy file paths.
ERROR: file 'sw/device/silicon_creator/rom/_objs/bootstrap_unittest/bootstrap_unittest.pic.o' is generated by these conflicting actions:
Label: //sw/device/silicon_creator/rom:bootstrap_unittest
RuleClass: cc_test rule
Configuration: 53b8c927cfc8add4f78c4b39cd8e606485545cb950239ee335b895fb868a0026, 80fabee21eb8c48b020ddd938fdc819f71d11cfeb7b4da2aea369a10416885d4
Mnemonic: CppCompile
Action key: 353a44e7d8dde53ffc5dd47dec423a2e82aecaccc8830555f88fb974c7ac486c
Progress message: Compiling sw/device/silicon_creator/rom/bootstrap_unittest.cc
PrimaryInput: File:[/home/dbeitel/rivos/org.lowrisc.opentitan[source]]sw/device/silicon_creator/rom/bootstrap_unittest.cc
PrimaryOutput: File:[[<execution_root>]bazel-out/k8-fastbuild/bin]sw/device/silicon_creator/rom/_objs/bootstrap_unittest/bootstrap_unittest.pic.o
Owner information: ConfiguredTargetKey{label=//sw/device/silicon_creator/rom:bootstrap_unittest, config=BuildConfigurationValue.Key[53b8c927cfc8add4f78c4b39cd8e606485545cb950239ee335b895fb868a0026]}, ConfiguredTargetKey{label=//sw/device/silicon_creator/rom:bootstrap_unittest, config=BuildConfigurationValue.Key[80fabee21eb8c48b020ddd938fdc819f71d11cfeb7b4da2aea369a10416885d4]}
MandatoryInputs: are equal
Outputs: are equal
ERROR: file 'sw/device/silicon_creator/rom/_objs/boot_policy_unittest/boot_policy_unittest.pic.o' is generated by these conflicting actions:
Label: //sw/device/silicon_creator/rom:boot_policy_unittest
RuleClass: cc_test rule
Configuration: 53b8c927cfc8add4f78c4b39cd8e606485545cb950239ee335b895fb868a0026, 80fabee21eb8c48b020ddd938fdc819f71d11cfeb7b4da2aea369a10416885d4
Mnemonic: CppCompile
Action key: 98110f0566b867521539254560728ed5ce02f36c70d58adb1df27eb3762786c9
Progress message: Compiling sw/device/silicon_creator/rom/boot_policy_unittest.cc
PrimaryInput: File:[/home/dbeitel/rivos/org.lowrisc.opentitan[source]]sw/device/silicon_creator/rom/boot_policy_unittest.cc
PrimaryOutput: File:[[<execution_root>]bazel-out/k8-fastbuild/bin]sw/device/silicon_creator/rom/_objs/boot_policy_unittest/boot_policy_unittest.pic.o
Owner information: ConfiguredTargetKey{label=//sw/device/silicon_creator/rom:boot_policy_unittest, config=BuildConfigurationValue.Key[53b8c927cfc8add4f78c4b39cd8e606485545cb950239ee335b895fb868a0026]}, ConfiguredTargetKey{label=//sw/device/silicon_creator/rom:boot_policy_unittest, config=BuildConfigurationValue.Key[80fabee21eb8c48b020ddd938fdc819f71d11cfeb7b4da2aea369a10416885d4]}
MandatoryInputs: are equal
Outputs: are equal
ERROR: com.google.devtools.build.lib.skyframe.ArtifactConflictFinder$ConflictException: com.google.devtools.build.lib.actions.MutableActionGraph$ActionConflictException: for sw/device/silicon_creator/rom/_objs/boot_policy_unittest/boot_policy_unittest.pic.o, previous action: action 'Compiling sw/device/silicon_creator/rom/boot_policy_unittest.cc', attempted action: action 'Compiling sw/device/silicon_creator/rom/boot_policy_unittest.cc'
INFO: Elapsed time: 261.624s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (832 packages loaded, 94723 targets configured)

[pamaury]

I can reproduce this, I will look into it. We never usually build the entire repository like this, but only parts of it at a timewhich is why this issue has gone unnoticed.

[arunthomas]

Thanks, @pamaury! It'd be great if we could add a CI build check for the //... target to catch these issues in the future.

[alphan]

Thanks, @pamaury! It'd be great if we could add a CI build check for the //... target to catch these issues in the future.

Not sure if we want this since this would just add extra build time, right?

[a-will]

We could probably add a check for just the loading and analysis phases, though:

bazel build --nobuild //...

This test of loading and analysis phases does trigger the error mentioned here.

For actually performing the build, I'm not sure what that wildcard picks up, however. It might not be a reasonable thing to do if it creates a graph that includes targets that are only meant to be built on the other side of a configuration transition. Though I suppose one could argue that we should make use of all the proper attributes so such targets don't get built under the host configuration...

[alphan]

I can reproduce this, I will look into it. We never usually build the entire repository like this, but only parts of it at a timewhich is why this issue has gone unnoticed.

Right, I typically build individual targets or wildcards on subdirs,e.g. //sw/device/silicon_creator/... or //sw/device/....

[a-will]

In case it helps with debugging, the difference between the two configurations is apparently a test configuration key:

$ diff -u <(bazel-5.1.1 config 53b8c927) <(bazel-5.1.1 config 80fabee21e)
--- /dev/fd/63	2023-05-24 08:07:42.970968076 -0700
+++ /dev/fd/62	2023-05-24 08:07:42.970968076 -0700
@@ -1,5 +1,5 @@
-BuildConfiguration 53b8c927cfc8add4f78c4b39cd8e606485545cb950239ee335b895fb868a0026:
-Skyframe Key: BuildConfigurationValue.Key[53b8c927cfc8add4f78c4b39cd8e606485545cb950239ee335b895fb868a0026]
+BuildConfiguration 80fabee21eb8c48b020ddd938fdc819f71d11cfeb7b4da2aea369a10416885d4:
+Skyframe Key: BuildConfigurationValue.Key[80fabee21eb8c48b020ddd938fdc819f71d11cfeb7b4da2aea369a10416885d4]
 Fragments: com.google.devtools.build.lib.analysis.PlatformConfiguration: [com.google.devtools.build.lib.analysis.PlatformOptions], com.google.devtools.build.lib.analysis.ShellConfiguration: [com.google.devtools.build.lib.analysis.ShellConfiguration$Options], com.google.devtools.build.lib.analysis.test.CoverageConfiguration: [com.google.devtools.build.lib.analysis.config.CoreOptions,com.google.devtools.build.lib.analysis.test.CoverageConfiguration$CoverageOptions], com.google.devtools.build.lib.analysis.test.TestConfiguration: [com.google.devtools.build.lib.analysis.test.TestConfiguration$TestOptions], com.google.devtools.build.lib.bazel.rules.BazelRuleClassProvider$StrictActionEnvConfiguration: [com.google.devtools.build.lib.bazel.rules.BazelRuleClassProvider$StrictActionEnvOptions], com.google.devtools.build.lib.bazel.rules.python.BazelPythonConfiguration: [com.google.devtools.build.lib.bazel.rules.python.BazelPythonConfiguration$Options,com.google.devtools.build.lib.rules.python.PythonOptions], com.google.devtools.build.lib.rules.android.AndroidConfiguration: [com.google.devtools.build.lib.rules.android.AndroidConfiguration$Options], com.google.devtools.build.lib.rules.android.AndroidLocalTestConfiguration: [com.google.devtools.build.lib.rules.android.AndroidLocalTestConfiguration$Options], com.google.devtools.build.lib.rules.apple.AppleConfiguration: [com.google.devtools.build.lib.rules.apple.AppleCommandLineOptions], com.google.devtools.build.lib.rules.apple.swift.SwiftConfiguration: [com.google.devtools.build.lib.rules.apple.swift.SwiftCommandLineOptions], com.google.devtools.build.lib.rules.config.ConfigFeatureFlagConfiguration: [com.google.devtools.build.lib.rules.config.ConfigFeatureFlagOptions], com.google.devtools.build.lib.rules.cpp.CppConfiguration: [com.google.devtools.build.lib.rules.apple.AppleCommandLineOptions,com.google.devtools.build.lib.rules.cpp.CppOptions], com.google.devtools.build.lib.rules.genquery.GenQueryConfiguration: [com.google.devtools.build.lib.rules.genquery.GenQueryConfiguration$GenQueryOptions], com.google.devtools.build.lib.rules.java.JavaConfiguration: [com.google.devtools.build.lib.analysis.PlatformOptions,com.google.devtools.build.lib.rules.java.JavaOptions], com.google.devtools.build.lib.rules.objc.J2ObjcConfiguration: [com.google.devtools.build.lib.rules.objc.J2ObjcCommandLineOptions], com.google.devtools.build.lib.rules.objc.ObjcConfiguration: [com.google.devtools.build.lib.rules.cpp.CppOptions,com.google.devtools.build.lib.rules.objc.ObjcCommandLineOptions], com.google.devtools.build.lib.rules.proto.ProtoConfiguration: [com.google.devtools.build.lib.rules.proto.ProtoConfiguration$Options], com.google.devtools.build.lib.rules.python.PythonConfiguration: [com.google.devtools.build.lib.rules.python.PythonOptions], 
 FragmentOptions com.google.devtools.build.lib.analysis.PlatformOptions {
   experimental_add_exec_constraints_to_targets: []
@@ -79,6 +79,26 @@
 FragmentOptions com.google.devtools.build.lib.analysis.test.CoverageConfiguration$CoverageOptions {
   coverage_output_generator: @bazel_tools//tools/test:lcov_merger
 }
+FragmentOptions com.google.devtools.build.lib.analysis.test.TestConfiguration$TestOptions {
+  cache_test_results: AUTO
+  coverage_report_generator: @bazel_tools//tools/test:coverage_report_generator
+  coverage_support: @bazel_tools//tools/test:coverage_support
+  experimental_cancel_concurrent_tests: false
+  experimental_fetch_all_coverage_outputs: false
+  experimental_persistent_test_runner: false
+  experimental_retain_test_configuration_across_testonly: false
+  experimental_split_coverage_postprocessing: false
+  incompatible_exclusive_test_sandboxed: false
+  runs_per_test: [(?:(?>.*)) Options: [1]]
+  runs_per_test_detects_flakes: false
+  test_arg: []
+  test_filter: null
+  test_result_expiration: -1
+  test_runner_fail_fast: false
+  test_sharding_strategy: EXPLICIT
+  test_timeout: {short=PT1M, moderate=PT5M, long=PT15M, eternal=PT1H}
+  trim_test_configuration: true
+}
 FragmentOptions com.google.devtools.build.lib.bazel.rules.BazelRuleClassProvider$StrictActionEnvOptions {
   incompatible_strict_action_env: false
 }

Not sure why there is a configuration transition to this target.

[a-will]

A quick bisection reveals this started with 74f6805, from #18537

[pamaury]

I am not sure building //... in CI is a good idea because that could be extremely computer intensive and would duplicate a lot of building work. This could go into nightly perhaps? But it looks the problem is quite bazel specific so at least we could add a test to ask bazel to check that there are no such build conflicts?

[pamaury]

The problem seems to be specifically with this rule

clang_tidy_check(
    name = "clang_tidy_check_host",
    testonly = True,
    deps = [
        "//sw/device/silicon_creator/rom:boot_policy_unittest",
        "//sw/device/silicon_creator/rom:bootstrap_unittest",
    ],
)

I am still not sure why, maybe it's a side-effect how how the clang_tidy_check rule is implemented.

[pamaury]

After some investigation, I believe the problem may be due to a bazel bug-but-not-bug. In short, a non-test rule in bazel should not depend on a test rule, ortherwise (and unless instructed by some experimental flag), bazel could produce conflicting actions (I do not understand why). This is exactly what happens here:

clang_tidy_check(
    name = "clang_tidy_check_host",
    testonly = True,
    deps = [
        "//sw/device/silicon_creator/rom:boot_policy_unittest",
        "//sw/device/silicon_creator/rom:bootstrap_unittest",
    ],
)

since clang_tidy_check is a non-test rule but it depends on unit tests. Note that using testonly does not seem to prevent the problem (see bazel bug report).

Tagging @dmcardle who created clang_tidy_check to see if he has some thoughts on how to rework this.

[dmcardle]

I'll take a look today. I think this may be as simple as making clang_tidy_check into a test rule by adding test = True to its definition.

Thanks for the detailed investigation, @pamaury!

[pamaury]

I think this could work, but it might require a bit of rework because (unless I am mistaken), a test rule has some constraints like ending with _test also to be executable (so instead of the rule producing a file, it should run the executable producing the file). Thanks for looking into it.

[dmcardle]

I'll declare this issue fixed now that #18763 is in, but I haven't actually run a full build of //... because that takes a very long time. I have added the quick loading/analysis check to CI, though, so future regressions like the clang-tidy issue should be caught early.

opentitan/azure-pipelines.yml

Lines 220 to 222 in 511b94e

	# Check the entire build graph for conflicts in loading or analysis
	# phases. For context, see issue #18726.
	ci/bazelisk.sh build --nobuild //...