Automated rollback of commit 4607ad439fe8869c8e8951d953e2d3adb613e6d6.
*** Reason for rollback ***
[]
*** Original change description ***
Automated rollback of commit 76b42af100b0a1079a25a7ba24bffcd6bc53f6e0.
*** Reason for rollback ***
The internal tests were fixed.
*** Original change description ***
Automated rollback of commit 750f0a1c05f56403e3b4a3d08b6d7de6ce3ed7f7.
*** Reason for rollback ***
This CL breaks internal tests.
*** Original change description ***
Fix `_validation` output group merging
While https://github.com/bazelbuild/bazel/commit/cd725834eba3edb53b7a7dd9867363ef1fe00e57 made it so that a rul...
***
PiperOrigin-RevId: 678831804
Change-Id: I7725ff31af4c184c79162249283018d808b6b41e
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 9b62db0..017fc9a 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -334,7 +334,6 @@
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_options",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_phase_complete_event",
- "//src/main/java/com/google/devtools/build/lib/analysis:aspect_collection",
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info",
"//src/main/java/com/google/devtools/build/lib/analysis:build_info_event",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java
index 2c122c3..1c2f21f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java
@@ -148,9 +148,6 @@
*/
@Immutable
public final class AspectCollection {
- /** The name of the native aspect that collects validation outputs. */
- public static final String VALIDATION_ASPECT_NAME = "ValidateTarget";
-
/** aspects that should be visible to a dependency */
private final ImmutableSet<AspectDeps> usedAspects;
@@ -286,14 +283,11 @@
ImmutableList.copyOf(aspectMap.entrySet()).reverse()) {
for (AspectDescriptor depAspectDescriptor : deps.keySet()) {
Aspect depAspect = aspectMap.get(depAspectDescriptor);
- // As any aspect can add validation outputs, the special validation aspect that collects
- // their outputs has to depend on all aspects.
if (depAspect
.getDefinition()
.getRequiredProvidersForAspects()
.isSatisfiedBy(aspect.getValue().getDefinition().getAdvertisedProviders())
- || depAspect.getDefinition().requires(aspect.getValue())
- || depAspect.getAspectClass().getName().equals(VALIDATION_ASPECT_NAME)) {
+ || depAspect.getDefinition().requires(aspect.getValue())) {
deps.get(depAspectDescriptor).add(aspect.getKey());
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
index 9bae6a5..85de020 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
@@ -209,9 +209,6 @@
"Output group " + VALIDATION + " provided twice with incompatible depset orders");
}
}
- if (!validationOutputs.isEmpty()) {
- outputGroups.put(VALIDATION, validationOutputs.build());
- }
return createInternal(ImmutableMap.copyOf(outputGroups));
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
index 3496572..2a8b293 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
@@ -22,7 +22,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.analysis.AnalysisOptions;
-import com.google.devtools.build.lib.analysis.AspectCollection;
import com.google.devtools.build.lib.analysis.OutputGroupInfo;
import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.analysis.ViewCreationFailedException;
@@ -58,6 +57,7 @@
* as --keep_going, --jobs, etc.
*/
public class BuildRequest implements OptionsProvider {
+ public static final String VALIDATION_ASPECT_NAME = "ValidateTarget";
private static final ImmutableList<Class<? extends OptionsBase>> MANDATORY_OPTIONS =
ImmutableList.of(
@@ -429,8 +429,8 @@
public ImmutableList<String> getAspects() {
List<String> aspects = getBuildOptions().aspects;
ImmutableList.Builder<String> result = ImmutableList.<String>builder().addAll(aspects);
- if (!aspects.contains(AspectCollection.VALIDATION_ASPECT_NAME) && useValidationAspect()) {
- result.add(AspectCollection.VALIDATION_ASPECT_NAME);
+ if (!aspects.contains(VALIDATION_ASPECT_NAME) && useValidationAspect()) {
+ result.add(VALIDATION_ASPECT_NAME);
}
return result.build();
}
@@ -453,7 +453,7 @@
}
}
- /** Whether {@value AspectCollection#VALIDATION_ASPECT_NAME} is in use. */
+ /** Whether {@value #VALIDATION_ASPECT_NAME} is in use. */
public boolean useValidationAspect() {
return validationMode() == OutputGroupInfo.ValidationMode.ASPECT;
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
index 596b423..9ec081f 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
@@ -19,7 +19,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.analysis.AspectCollection;
import com.google.devtools.build.lib.analysis.ConfiguredAspect;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FileProvider;
@@ -390,7 +389,7 @@
var aspectsToPrintBuilder = ImmutableSet.<AspectKey>builder();
var validationAspectsBuilder = ImmutableList.<AspectKey>builder();
for (AspectKey key : keys) {
- if (Objects.equals(key.getAspectClass().getName(), AspectCollection.VALIDATION_ASPECT_NAME)) {
+ if (Objects.equals(key.getAspectClass().getName(), BuildRequest.VALIDATION_ASPECT_NAME)) {
validationAspectsBuilder.add(key);
} else {
aspectsToPrintBuilder.add(key);
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD b/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD
index 8ed4ede..4c7bd40 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD
@@ -36,7 +36,6 @@
"//src/main/java/com/google/devtools/build/lib/actions:commandline_item",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_options",
- "//src/main/java/com/google/devtools/build/lib/analysis:aspect_collection",
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
index f02d132..17816ad 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
@@ -19,7 +19,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.analysis.AspectCollection;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
import com.google.devtools.build.lib.buildtool.BuildRequest;
@@ -244,7 +243,7 @@
if (buildRequest.useValidationAspect()) {
validatedTargets =
buildResult.getSuccessfulAspects().stream()
- .filter(key -> AspectCollection.VALIDATION_ASPECT_NAME.equals(key.getAspectName()))
+ .filter(key -> BuildRequest.VALIDATION_ASPECT_NAME.equals(key.getAspectName()))
.map(AspectKey::getBaseConfiguredTargetKey)
.collect(ImmutableSet.toImmutableSet());
} else {
diff --git a/src/test/shell/integration/validation_actions_test.sh b/src/test/shell/integration/validation_actions_test.sh
index a7cd089..1f68bba 100755
--- a/src/test/shell/integration/validation_actions_test.sh
+++ b/src/test/shell/integration/validation_actions_test.sh
@@ -514,7 +514,9 @@
expect_log "Target //validation_actions:foo0 up-to-date:"
}
-function setup_validation_tool_aspect() {
+function test_validation_actions_in_rule_and_aspect() {
+ setup_test_project
+
mkdir -p aspect
cat > aspect/BUILD <<'EOF'
exports_files(["aspect_validation_tool"])
@@ -546,20 +548,10 @@
echo "aspect validation output" > $1
EOF
chmod +x aspect/aspect_validation_tool
-}
-
-function test_validation_actions_in_rule_and_aspect_no_use_validation_aspect() {
- setup_test_project
- setup_validation_tool_aspect
setup_passing_validation_action
bazel build --run_validations --aspects=//aspect:def.bzl%validation_aspect \
- --noexperimental_use_validation_aspect \
//validation_actions:foo0 >& "$TEST_log" || fail "Expected build to succeed"
- expect_log "Target //validation_actions:foo0 up-to-date:"
- expect_log "validation_actions/foo0.main"
- assert_exists bazel-bin/validation_actions/foo0.validation
- assert_exists bazel-bin/validation_actions/foo0.aspect_validation
cat > aspect/aspect_validation_tool <<'EOF'
#!/bin/bash
@@ -568,32 +560,6 @@
EOF
bazel build --run_validations --aspects=//aspect:def.bzl%validation_aspect \
- --noexperimental_use_validation_aspect \
- //validation_actions:foo0 >& "$TEST_log" && fail "Expected build to fail"
- expect_log "aspect validation failed!"
-}
-
-function test_validation_actions_in_rule_and_aspect_use_validation_aspect() {
- setup_test_project
- setup_validation_tool_aspect
- setup_passing_validation_action
-
- bazel build --run_validations --aspects=//aspect:def.bzl%validation_aspect \
- --experimental_use_validation_aspect \
- //validation_actions:foo0 >& "$TEST_log" || fail "Expected build to succeed"
- expect_log "Target //validation_actions:foo0 up-to-date:"
- expect_log "validation_actions/foo0.main"
- assert_exists bazel-bin/validation_actions/foo0.validation
- assert_exists bazel-bin/validation_actions/foo0.aspect_validation
-
- cat > aspect/aspect_validation_tool <<'EOF'
-#!/bin/bash
-echo "aspect validation failed!"
-exit 1
-EOF
-
- bazel build --run_validations --aspects=//aspect:def.bzl%validation_aspect \
- --experimental_use_validation_aspect \
//validation_actions:foo0 >& "$TEST_log" && fail "Expected build to fail"
expect_log "aspect validation failed!"
}
