diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkymeldModule.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkymeldModule.java index ef2facbd2d2824..ca8f1899f24f89 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkymeldModule.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkymeldModule.java @@ -31,13 +31,13 @@ boolean determineIfMergingAnalysisExecution(CommandEnvironment env) { PathPackageLocator packageLocator = env.getPackageLocator(); BuildRequestOptions buildRequestOptions = env.getOptions().getOptions(BuildRequestOptions.class); + boolean plainValueFromFlag = getPlainValueFromFlag(buildRequestOptions); // --nobuild means no execution will be carried out, hence it doesn't make sense to interleave // analysis and execution in that case and --experimental_merged_skyframe_analysis_execution // should be ignored. // Aquery and Cquery implicitly set --nobuild, so there's no need to have a warning here: it // makes no different from the users' perspective. - if (buildRequestOptions != null - && buildRequestOptions.mergedSkyframeAnalysisExecutionDoNotUseDirectly + if (plainValueFromFlag && !buildRequestOptions.performExecutionPhase && !(commandName.equals("aquery") || commandName.equals("cquery"))) { env.getReporter() @@ -46,7 +46,15 @@ boolean determineIfMergingAnalysisExecution(CommandEnvironment env) { "--experimental_merged_skyframe_analysis_execution is incompatible with --nobuild" + " and will be ignored.")); } - boolean plainValueFromFlag = getPlainValueFromFlag(buildRequestOptions); + // TODO(b/245922903): Make --explain compatible with Skymeld. + if (plainValueFromFlag && buildRequestOptions.explanationPath != null) { + env.getReporter() + .handle( + Event.warn( + "--experimental_merged_skyframe_analysis_execution is incompatible with --explain" + + " and will be ignored.")); + } + boolean havingMultiPackagePath = packageLocator != null && packageLocator.getPathEntries().size() > 1; // TODO(b/246324830): Skymeld and multi-package_path are incompatible. @@ -61,6 +69,7 @@ boolean determineIfMergingAnalysisExecution(CommandEnvironment env) { } return plainValueFromFlag && buildRequestOptions.performExecutionPhase + && buildRequestOptions.explanationPath == null && !havingMultiPackagePath; } diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/SkymeldBuildIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/SkymeldBuildIntegrationTest.java index 7a0ec6e18abed0..c26cfd75ca394a 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/SkymeldBuildIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildtool/SkymeldBuildIntegrationTest.java @@ -475,10 +475,22 @@ public void targetWithNoConfiguration_success() throws Exception { assertThat(result.getSuccess()).isTrue(); } + @Test + public void explain_ignoreSkymeldWithWarning() throws Exception { + addOptions("--explain=/dev/null"); + write("foo/BUILD", "genrule(name = 'foo', outs = ['foo.out'], cmd = 'touch $@')"); + BuildResult buildResult = buildTarget("//foo"); + + assertThat(buildResult.getSuccess()).isTrue(); + + events.assertContainsWarning( + "--experimental_merged_skyframe_analysis_execution is incompatible with --explain"); + events.assertContainsWarning("and will be ignored."); + } + @Test public void multiplePackagePath_ignoreSkymeldWithWarning() throws Exception { write("foo/BUILD", "genrule(name = 'foo', outs = ['foo.out'], cmd = 'touch $@')"); - // write("foo/root.sh"); write("otherroot/bar/BUILD", "genrule(name = 'bar', outs = ['bar.out'], cmd = 'touch $@')"); addOptions("--package_path=%workspace%:otherroot");