Enable analysis caching codepaths only for commands that execute actions.
For example, this includes `test` and `build`, but does not include `cquery`.
This prevents the use case where the someone adds `build
--experimental_remote_analysis_cache_mode=download` by default into their
bazelrc, and runs a cquery command (which, WAI, picks up the flag, because
`CqueryCommand` inherits options from `TestCommand`, which inherits from
`BuildCommand`), and fails to this cryptic error:
```
$ bazel cquery 'somepath(//foo, //bar)'
ERROR: Failed to find PROJECT.scl file for: [//foo:foo, //bar:bar]
INFO: Elapsed time: 0.078s
ERROR: Build did NOT complete successfully
```
It also doesn't make sense to support analysis caching with cquery anyway, because correct results require a complete, unpruned analysis graph.
Same goes for `aquery`.
PiperOrigin-RevId: 689397164
Change-Id: Ib63ab41450b84e782d69e148196b2bfa6413e65f
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
index b0ce8b4..c15e337 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
@@ -208,7 +208,8 @@
throws LoadingFailedException, InvalidConfigurationException {
EnumSet<ProjectFileFeature> featureFlags = EnumSet.noneOf(ProjectFileFeature.class);
- if (env.getOptions().getOptions(RemoteAnalysisCachingOptions.class).mode != OFF) {
+ if (env.getCommand().buildPhase().executes()
+ && env.getOptions().getOptions(RemoteAnalysisCachingOptions.class).mode != OFF) {
// RemoteAnalysisCachingOptions is never null because it's a build command flag, and this
// method only runs for build commands.
featureFlags.add(ANALYSIS_CACHING);
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
index 53333e1..1718530 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -850,7 +850,7 @@
}
RemoteAnalysisCachingOptions options = request.getOptions(RemoteAnalysisCachingOptions.class);
- if (options == null) {
+ if (options == null || !env.getCommand().buildPhase().executes()) {
return;
}
@@ -1108,6 +1108,7 @@
CommandEnvironment env, Optional<PathFragmentPrefixTrie> activeDirectoriesMatcher) {
var options = env.getOptions().getOptions(RemoteAnalysisCachingOptions.class);
if (options == null
+ || !env.getCommand().buildPhase().executes()
|| !options.mode.downloadForAnalysis()
|| activeDirectoriesMatcher.isEmpty()) {
return DisabledDependenciesProvider.INSTANCE;
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD
index e062e57..ee48774 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD
@@ -15,6 +15,7 @@
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target_value",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/pkgcache",
+ "//src/main/java/com/google/devtools/build/lib/runtime/commands",
"//src/main/java/com/google/devtools/build/lib/skyframe:aspect_key_creator",
"//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key",
"//src/main/java/com/google/devtools/build/lib/skyframe:rule_configured_target_value",
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FrontierSerializerTestBase.java b/src/test/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FrontierSerializerTestBase.java
index edee016..d74be69 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FrontierSerializerTestBase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FrontierSerializerTestBase.java
@@ -31,6 +31,7 @@
import com.google.devtools.build.lib.buildtool.util.BuildIntegrationTestCase;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.pkgcache.LoadingFailedException;
+import com.google.devtools.build.lib.runtime.commands.CqueryCommand;
import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectBaseKey;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.skyframe.RemoteConfiguredTargetValue;
@@ -534,6 +535,37 @@
.hasCount(SkyFunctions.CONFIGURED_TARGET, serializedConfiguredTargetCount - 1);
}
+ @Test
+ public void cquery_succeedsAndDoesNotTriggerUpload() throws Exception {
+ setupScenarioWithConfiguredTargets();
+ addOptions("--experimental_remote_analysis_cache_mode=upload");
+ runtimeWrapper.newCommand(CqueryCommand.class);
+ buildTarget("//foo:A"); // passes, even though there's no PROJECT.scl
+ assertThat(
+ getCommandEnvironment()
+ .getRemoteAnalysisCachingEventListener()
+ .getSerializedKeysCount())
+ .isEqualTo(0);
+ }
+
+ @Test
+ public void cquery_succeedsAndDoesNotTriggerUploadWithProjectScl() throws Exception {
+ setupScenarioWithConfiguredTargets();
+ write(
+ "foo/PROJECT.scl",
+ """
+active_directories = { "default": ["foo"] }
+""");
+ addOptions("--experimental_remote_analysis_cache_mode=upload");
+ runtimeWrapper.newCommand(CqueryCommand.class);
+ buildTarget("//foo:A");
+ assertThat(
+ getCommandEnvironment()
+ .getRemoteAnalysisCachingEventListener()
+ .getSerializedKeysCount())
+ .isEqualTo(0);
+ }
+
protected void setupScenarioWithConfiguredTargets() throws Exception {
// ┌───────┐ ┌───────┐
// │ bar:C │ ◀── │ foo:A │
