BEP: report visibility errors
On finding a visibility error, report it directly for that target, instead of
relying on the implict "abort" message for targets that have not been built.
Change-Id: I5e45722a1117afca3bc8eeebd05179425b995172
PiperOrigin-RevId: 157592518
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 2afd928..08f2703 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -666,6 +666,7 @@
":bazel-repository",
":build-base",
":build-info",
+ ":buildeventstream",
":collect",
":concurrent",
":events",
@@ -678,6 +679,7 @@
":util",
":vfs",
"//src/main/java/com/google/devtools/build/lib/actions",
+ "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
"//src/main/java/com/google/devtools/build/lib/query2:query-output",
"//src/main/java/com/google/devtools/build/lib/rules/apple",
"//src/main/java/com/google/devtools/build/lib/rules/apple/cpp",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 2077a887..02fb3e5 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -52,6 +52,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.AspectDescriptor;
import com.google.devtools.build.lib.packages.Attribute;
@@ -1780,6 +1781,10 @@
return mapBuilder.build();
}
+ public void post(Postable event) {
+ reporter.post(event);
+ }
+
public void reportError(Location location, String message) {
reporter.reportError(location, message);
}
@@ -2127,6 +2132,10 @@
env.getEventHandler().handle(Event.error(location, message));
}
+ public void post(Postable event) {
+ env.getEventHandler().post(event);
+ }
+
@Override
public void ruleError(String message) {
reportError(rule.getLocation(), prefixRuleMessage(message));
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelPrerequisiteValidator.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelPrerequisiteValidator.java
index 49d8f8f..81c097f 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelPrerequisiteValidator.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelPrerequisiteValidator.java
@@ -48,22 +48,28 @@
.getPackageIdentifier()
.equals(AliasProvider.getDependencyLabel(prerequisite).getPackageIdentifier())
&& !context.isVisible(prerequisite)) {
+ String errorMessage;
if (!context.getConfiguration().checkVisibility()) {
- context.ruleWarning(
+ errorMessage =
String.format(
"Target '%s' violates visibility of target "
+ "%s. Continuing because --nocheck_visibility is active",
- rule.getLabel(), AliasProvider.printLabelWithAliasChain(prerequisite)));
+ rule.getLabel(), AliasProvider.printLabelWithAliasChain(prerequisite));
+ context.ruleWarning(errorMessage);
} else {
// Oddly enough, we use reportError rather than ruleError here.
- context.reportError(
- rule.getLocation(),
+ errorMessage =
String.format(
"Target %s is not visible from target '%s'. Check "
+ "the visibility declaration of the former target if you think "
+ "the dependency is legitimate",
- AliasProvider.printLabelWithAliasChain(prerequisite), rule.getLabel()));
+ AliasProvider.printLabelWithAliasChain(prerequisite), rule.getLabel());
+ context.reportError(rule.getLocation(), errorMessage);
}
+ // We can always post the visibility error as, regardless of the value of keep going,
+ // that target will not be built.
+ context.post(
+ new VisibilityErrorEvent(context.getConfiguration(), rule.getLabel(), errorMessage));
}
if (prerequisiteTarget instanceof PackageGroup && !attrName.equals("visibility")) {
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/VisibilityErrorEvent.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/VisibilityErrorEvent.java
new file mode 100644
index 0000000..a2c51a8
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/VisibilityErrorEvent.java
@@ -0,0 +1,61 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.bazel.rules;
+
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.analysis.config.BuildEventWithConfiguration;
+import com.google.devtools.build.lib.buildeventstream.BuildEventConverters;
+import com.google.devtools.build.lib.buildeventstream.BuildEventId;
+import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
+import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent;
+import com.google.devtools.build.lib.cmdline.Label;
+import java.util.Collection;
+
+/** Class reporting that a configured target will not be built due an error in the analysis phase */
+public class VisibilityErrorEvent implements BuildEventWithConfiguration {
+ BuildConfiguration configuration;
+ Label label;
+ String errorMessage;
+
+ VisibilityErrorEvent(BuildConfiguration configuration, Label label, String errorMessage) {
+ this.configuration = configuration;
+ this.label = label;
+ this.errorMessage = errorMessage;
+ }
+
+ @Override
+ public BuildEventId getEventId() {
+ return BuildEventId.targetCompleted(label, configuration.getEventId());
+ }
+
+ @Override
+ public Collection<BuildEventId> getChildrenEvents() {
+ return ImmutableList.<BuildEventId>of();
+ }
+
+ @Override
+ public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventConverters converters) {
+ return GenericBuildEvent.protoChaining(this)
+ .setAnalysisFailed(
+ BuildEventStreamProtos.AnalysisFailure.newBuilder().setDetails(errorMessage).build())
+ .build();
+ }
+
+ @Override
+ public Collection<BuildConfiguration> getConfigurations() {
+ return ImmutableList.of(configuration);
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
index 6d150d5..0dc3d12 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
@@ -204,6 +204,13 @@
string details = 1;
}
+// Payload of an event indicating that an expected event will not come, as
+// something went wrong during analysis. If the analysis of a target failed,
+// the id of the event will be the corresponding TargetCompletedId.
+message AnalysisFailure {
+ string details = 1;
+}
+
// Payload of an event indicating the beginning of a new build. Usually, events
// of those type start a new build-event stream. The target pattern requested
// to be build is contained in one of the announced child events; it is an
@@ -464,6 +471,7 @@
Progress progress = 3;
Aborted aborted = 4;
LoadingFailure loading_failed = 11;
+ AnalysisFailure analysis_failed = 19;
BuildStarted started = 5;
CommandLine command_line = 12;
OptionsParsed options_parsed = 13;
