Add a feature to require explicitly passing module maps.

Currently, module maps contain both "use <module>" entries that specify which
modules the current module map depends on, and "extern module" entries that
provide paths where to load the dependent module maps from.

This change adds a feature "module_map_without_extern_module", which instructs
blaze to not write the "extern module" entries into the module map. Instead,
the crosstool needs to add -fmodule-file flags for each dependent module file
where needed for the compile via the new build variable
"dependent_module_map_files".

Note that the feature is phrased negatively ("_without_") in order to simplify
the roll-out of this feature: as long as crosstools do not specify any
features, they still want the old behavior.
We cannot make the feature positive and add it to the legacy configuration, as
we currently cannot remove features that have already been set in the crosstool
file.

--
MOS_MIGRATED_REVID=104757413

6 files changed, 66 insertions, 26 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
index 5e97bce53c..ac06521f8c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
@@ -789,15 +789,18 @@ public final class CcLibraryHelper {
       // TODO(bazel-team): addCppModuleMapToContext second-guesses whether module maps should
       // actually be enabled, so we need to double-check here. Who would write code like this?
       if (cppModuleMap != null) {
-        CppModuleMapAction action = new CppModuleMapAction(ruleContext.getActionOwner(),
-            cppModuleMap,
-            privateHeaders,
-            publicHeaders,
-            collectModuleMaps(),
-            additionalExportedHeaders,
-            featureConfiguration.isEnabled(CppRuleClasses.HEADER_MODULES),
-            featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAP_HOME_CWD),
-            featureConfiguration.isEnabled(CppRuleClasses.GENERATE_SUBMODULES));
+        CppModuleMapAction action =
+            new CppModuleMapAction(
+                ruleContext.getActionOwner(),
+                cppModuleMap,
+                privateHeaders,
+                publicHeaders,
+                collectModuleMaps(),
+                additionalExportedHeaders,
+                featureConfiguration.isEnabled(CppRuleClasses.HEADER_MODULES),
+                featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAP_HOME_CWD),
+                featureConfiguration.isEnabled(CppRuleClasses.GENERATE_SUBMODULES),
+                !featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAP_WITHOUT_EXTERN_MODULE));
         ruleContext.registerAction(action);
       }
       if (model.getGeneratesPicHeaderModule()) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompilationContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompilationContext.java
index 7658794e6c..d8527fe71b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompilationContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompilationContext.java
@@ -273,7 +273,7 @@ public final class CppCompilationContext implements TransitiveInfoProvider {
   /**
    * @return modules maps from direct dependencies.
    */
-  private NestedSet<Artifact> getDirectModuleMaps() {
+  public NestedSet<Artifact> getDirectModuleMaps() {
     NestedSetBuilder<Artifact> builder = NestedSetBuilder.stableOrder();
     for (DepsContext depsContext : depsContexts) {
       builder.addTransitive(depsContext.directModuleMaps);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
index bd7ea07a74..262dd3794e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
@@ -346,6 +346,12 @@ public final class CppModel {
       buildVariables.addVariable("module_name", cppModuleMap.getName());
       buildVariables.addVariable("module_map_file",
           cppModuleMap.getArtifact().getExecPathString());
+      CcToolchainFeatures.Variables.NestedSequence.Builder sequence =
+          new CcToolchainFeatures.Variables.NestedSequence.Builder();
+      for (Artifact artifact : context.getDirectModuleMaps()) {
+        sequence.addValue(artifact.getExecPathString());
+      }
+      buildVariables.addSequence("dependent_module_map_files", sequence.build());
     }
     if (featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES)) {
       buildVariables.addSequenceVariable("module_files", getHeaderModulePaths(builder, usePic));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java
index 05da7857a1..758a5bbd80 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java
@@ -58,11 +58,19 @@ public class CppModuleMapAction extends AbstractFileWriteAction {
   private final ImmutableList<PathFragment> additionalExportedHeaders;
   private final boolean compiledModule;
   private final boolean generateSubmodules;
-
-  public CppModuleMapAction(ActionOwner owner, CppModuleMap cppModuleMap,
-      Iterable<Artifact> privateHeaders, Iterable<Artifact> publicHeaders,
-      Iterable<CppModuleMap> dependencies, Iterable<PathFragment> additionalExportedHeaders,
-      boolean compiledModule, boolean moduleMapHomeIsCwd, boolean generateSubmodules) {
+  private final boolean externDependencies;
+
+  public CppModuleMapAction(
+      ActionOwner owner,
+      CppModuleMap cppModuleMap,
+      Iterable<Artifact> privateHeaders,
+      Iterable<Artifact> publicHeaders,
+      Iterable<CppModuleMap> dependencies,
+      Iterable<PathFragment> additionalExportedHeaders,
+      boolean compiledModule,
+      boolean moduleMapHomeIsCwd,
+      boolean generateSubmodules,
+      boolean externDependencies) {
     super(owner, ImmutableList.<Artifact>of(), cppModuleMap.getArtifact(),
         /*makeExecutable=*/false);
     this.cppModuleMap = cppModuleMap;
@@ -73,6 +81,7 @@ public class CppModuleMapAction extends AbstractFileWriteAction {
     this.additionalExportedHeaders = ImmutableList.copyOf(additionalExportedHeaders);
     this.compiledModule = compiledModule;
     this.generateSubmodules = generateSubmodules;
+    this.externDependencies = externDependencies;
   }
 
   @Override
@@ -93,11 +102,16 @@ public class CppModuleMapAction extends AbstractFileWriteAction {
         HashSet<PathFragment> deduper = new HashSet<>();
         for (Artifact artifact : publicHeaders) {
           appendHeader(
-              content, "", artifact.getExecPath(), leadingPeriods, /*canCompile=*/true, deduper);
+              content, "", artifact.getExecPath(), leadingPeriods, /*canCompile=*/ true, deduper);
         }
         for (Artifact artifact : privateHeaders) {
-          appendHeader(content, "private", artifact.getExecPath(), leadingPeriods,
-              /*canCompile=*/true, deduper);
+          appendHeader(
+              content,
+              "private",
+              artifact.getExecPath(),
+              leadingPeriods,
+              /*canCompile=*/ true,
+              deduper);
         }
         for (PathFragment additionalExportedHeader : additionalExportedHeaders) {
           appendHeader(
@@ -107,13 +121,16 @@ public class CppModuleMapAction extends AbstractFileWriteAction {
           content.append("  use \"").append(dep.getName()).append("\"\n");
         }
         content.append("}");
-        for (CppModuleMap dep : dependencies) {
-          content.append("\nextern module \"")
-              .append(dep.getName())
-              .append("\" \"")
-              .append(leadingPeriods)
-              .append(dep.getArtifact().getExecPath())
-              .append("\"");
+        if (externDependencies) {
+          for (CppModuleMap dep : dependencies) {
+            content
+                .append("\nextern module \"")
+                .append(dep.getName())
+                .append("\" \"")
+                .append(leadingPeriods)
+                .append(dep.getArtifact().getExecPath())
+                .append("\"");
+          }
         }
         out.write(content.toString().getBytes(StandardCharsets.ISO_8859_1));
       }
@@ -178,6 +195,7 @@ public class CppModuleMapAction extends AbstractFileWriteAction {
     f.addBoolean(moduleMapHomeIsCwd);
     f.addBoolean(compiledModule);
     f.addBoolean(generateSubmodules);
+    f.addBoolean(externDependencies);
     return f.hexDigestAndReset();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
index b7bfaf85ef..77a9719923 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
@@ -101,6 +101,18 @@ public class CppRuleClasses {
   public static final String MODULE_MAP_HOME_CWD = "module_map_home_cwd";
 
   /**
+   * A string constant for the module_map_without_extern_module feature.
+   *
+   * <p>This features is a transitional feature; enabling it means that generated module maps
+   * will not have "extern module" declarations inside them; instead, the module maps need
+   * to be passed via the dependent_module_map_files build variable.
+   *
+   * <p>This variable is phrased negatively to aid the roll-out: currently, the default is that
+   * "extern module" declarations are generated.
+   */
+  public static final String MODULE_MAP_WITHOUT_EXTERN_MODULE = "module_map_without_extern_module";
+
+  /**
    * A string constant for the layering_check feature.
    */
   public static final String LAYERING_CHECK = "layering_check";
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
index 6ba0442d15..1c92409383 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
@@ -675,7 +675,8 @@ public final class CompilationSupport {
             ImmutableList.<PathFragment>of(),
             /*compiledModule=*/ true,
             /*moduleMapHomeIsCwd=*/ false,
-            /*generateSubModules=*/ false));
+            /*generateSubModules=*/ false,
+            /*externDependencies=*/ true));
   }
 
   private void registerLinkAction(ObjcProvider objcProvider, ExtraLinkArgs extraLinkArgs,