Automated rollback of commit 09bf7cc5865f7edd9a3eeabb42792e8de3ad862f.

*** Reason for rollback ***

Causes crashes in fake_cc_binary in combination with --experimental_nodotd_scanning_with_modules.

*** Original change description ***

Stop generating .d files when they won't be looked at later.

While at it remove CppCompilationActionBuilder.setAllowUsingHeaderModules,
which isn't used anymore and would make the logic here (even) more complicated.

RELNOTES: None.
PiperOrigin-RevId: 207932548

2 files changed, 26 insertions, 7 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
index ea5ee63f04..27409c97e3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
@@ -1458,7 +1458,8 @@ public final class CcCompilationHelper {
       userCompileFlags.addAll(collectPerFileCopts(sourceFile, sourceLabel));
     }
     String dotdFileExecPath = null;
-    if (builder.getDotdFile() != null) {
+    if (isGenerateDotdFile(builder.getSourceFile())) {
+      Preconditions.checkNotNull(builder.getDotdFile());
       dotdFileExecPath = builder.getDotdFile().getSafeExecPath().getPathString();
     }
     ImmutableMap.Builder<String, String> allAdditionalBuildVariables = ImmutableMap.builder();
@@ -1509,12 +1510,22 @@ public final class CcCompilationHelper {
    * initialized.
    */
   private CppCompileActionBuilder initializeCompileAction(Artifact sourceArtifact) {
+    CppCompileActionBuilder builder = createCompileActionBuilder(sourceArtifact);
+    builder.setFeatureConfiguration(featureConfiguration);
+
+    return builder;
+  }
+
+  /**
+   * Creates a basic cpp compile action builder for source file. Configures options, crosstool
+   * inputs, output and dotd file names, {@code CcCompilationContext} and copts.
+   */
+  private CppCompileActionBuilder createCompileActionBuilder(Artifact source) {
     CppCompileActionBuilder builder =
         new CppCompileActionBuilder(ruleContext, ccToolchain, configuration);
-    builder.setSourceFile(sourceArtifact);
+    builder.setSourceFile(source);
     builder.setCcCompilationContext(ccCompilationContext);
     builder.setCoptsFilter(coptsFilter);
-    builder.setFeatureConfiguration(featureConfiguration);
     return builder;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
index 36be89ef59..f119f5de22 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
@@ -66,6 +66,7 @@ public class CppCompileActionBuilder {
   private CoptsFilter coptsFilter = CoptsFilter.alwaysPasses();
   private ImmutableList<PathFragment> extraSystemIncludePrefixes = ImmutableList.of();
   private boolean usePic;
+  private boolean allowUsingHeaderModules;
   private UUID actionClassId = GUID;
   private CppConfiguration cppConfiguration;
   private final ArrayList<Artifact> additionalIncludeScanningRoots;
@@ -116,6 +117,7 @@ public class CppCompileActionBuilder {
     this.cppConfiguration = configuration.getFragment(CppConfiguration.class);
     this.mandatoryInputsBuilder = NestedSetBuilder.stableOrder();
     this.additionalIncludeScanningRoots = new ArrayList<>();
+    this.allowUsingHeaderModules = true;
     this.env = configuration.getActionEnvironment();
     this.codeCoverageEnabled = configuration.isCodeCoverageEnabled();
     this.ccToolchain = ccToolchain;
@@ -149,6 +151,7 @@ public class CppCompileActionBuilder {
     this.cppConfiguration = other.cppConfiguration;
     this.configuration = other.configuration;
     this.usePic = other.usePic;
+    this.allowUsingHeaderModules = other.allowUsingHeaderModules;
     this.shouldScanIncludes = other.shouldScanIncludes;
     this.executionInfo = new LinkedHashMap<>(other.executionInfo);
     this.env = other.env;
@@ -395,9 +398,8 @@ public class CppCompileActionBuilder {
   }
 
   private boolean useHeaderModules() {
-    Preconditions.checkNotNull(featureConfiguration);
-    Preconditions.checkNotNull(sourceFile);
-    return featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES)
+    return allowUsingHeaderModules
+        && featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES)
         && (sourceFile.isFileType(CppFileTypes.CPP_SOURCE)
             || sourceFile.isFileType(CppFileTypes.CPP_HEADER)
             || sourceFile.isFileType(CppFileTypes.CPP_MODULE_MAP));
@@ -515,7 +517,7 @@ public class CppCompileActionBuilder {
         ruleContext,
         CppHelper.getArtifactNameForCategory(ruleContext, ccToolchain, outputCategory, outputName),
         configuration);
-    if (generateDotd && (!cppConfiguration.getNoDotdScanningWithModules() || !useHeaderModules())) {
+    if (generateDotd) {
       String dotdFileName =
           CppHelper.getDotdFileName(ruleContext, ccToolchain, outputCategory, outputName);
       if (cppConfiguration.getInmemoryDotdFiles()) {
@@ -588,6 +590,12 @@ public class CppCompileActionBuilder {
     return this;
   }
 
+  /** Sets whether the CompileAction should use header modules. */
+  public CppCompileActionBuilder setAllowUsingHeaderModules(boolean allowUsingHeaderModules) {
+    this.allowUsingHeaderModules = allowUsingHeaderModules;
+    return this;
+  }
+
   /** Sets the CppSemantics for this compile. */
   public CppCompileActionBuilder setSemantics(CppSemantics semantics) {
     this.cppSemantics = semantics;