From bdee8f91f3afcc8a6b2afb600a9c48cb4bd7651c Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Thu, 7 Sep 2023 14:28:51 -0700 Subject: [PATCH] Don't allow tree artifacts in mixed builds Tree artifacts aren't fully correct in incremental builds with ninja, so add a check that we don't write rules using them. Bug: 293609369 Test: m nothing Change-Id: I2e49cb6ec24124baf00adb0860f3c1f1f80178bb --- bazel/aquery.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/bazel/aquery.go b/bazel/aquery.go index 76cd97254..c35571239 100644 --- a/bazel/aquery.go +++ b/bazel/aquery.go @@ -177,6 +177,21 @@ func newAqueryHandler(aqueryResult *analysis_v2_proto.ActionGraphContainer) (*aq if err != nil { return nil, err } + if artifact.IsTreeArtifact && + !strings.HasPrefix(artifactPath, "bazel-out/io_bazel_rules_go/") && + !strings.HasPrefix(artifactPath, "bazel-out/rules_java_builtin/") { + // Since we're using ninja as an executor, we can't use tree artifacts. Ninja only + // considers a file/directory "dirty" when it's mtime changes. Directories' mtimes will + // only change when a file in the directory is added/removed, but not when files in + // the directory are changed, or when files in subdirectories are changed/added/removed. + // Bazel handles this by walking the directory and generating a hash for it after the + // action runs, which we would have to do as well if we wanted to support these + // artifacts in mixed builds. + // + // However, there are some bazel built-in rules that use tree artifacts. Allow those, + // but keep in mind that they'll have incrementality issues. + return nil, fmt.Errorf("tree artifacts are currently not supported in mixed builds: " + artifactPath) + } artifactIdToPath[artifactId(artifact.Id)] = artifactPath }