Add module map support to ExperimentalObjcLibrary, which contains experimental support

for building objc code using the c++ crosstool.  This will eventually replace the current module support in blaze.

Note: This required injecting a CppModuleMap into the cc logic.  The reason that objc cannot rely on standard CppModuleMap creation logic is that there is different naming semantics for module maps between cpp and objc.  In particular:

 - In cc, module maps can be inputs to a compilation action. Thus, the module maps are given labels.
 - In objc, if interoping with swift, module maps are explicitly referenced in swift code.  Thus, their names cannot contain illegal characters for swift source.  Those, some name mangling occurs to get rid of "//" and ":".  To enforce that this does not cause problems with compilation actions, the CPP_MODULE_COMPILE action has been disabled for the objc CcLibraryHelper.SourceCategory

--
MOS_MIGRATED_REVID=124177067

2 files changed, 37 insertions, 28 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 aaf5e95410..58205e7e6e 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
@@ -118,7 +118,6 @@ public final class CcLibraryHelper {
             CppCompileAction.OBJCPP_COMPILE,
             CppCompileAction.CPP_HEADER_PARSING,
             CppCompileAction.CPP_HEADER_PREPROCESSING,
-            CppCompileAction.CPP_MODULE_COMPILE,
             CppCompileAction.ASSEMBLE,
             CppCompileAction.PREPROCESS_ASSEMBLE,
             Link.LinkTargetType.STATIC_LIBRARY.getActionName(),
@@ -274,8 +273,9 @@ public final class CcLibraryHelper {
   private boolean emitDynamicLibrary = true;
   private boolean checkDepsGenerateCpp = true;
   private boolean emitCompileProviders;
-  private SourceCategory sourceCatagory;
+  private final SourceCategory sourceCategory;
   private List<VariablesExtension> variablesExtensions = new ArrayList<>();
+  @Nullable private CppModuleMap injectedCppModuleMap;
   
   private final FeatureConfiguration featureConfiguration;
 
@@ -296,7 +296,7 @@ public final class CcLibraryHelper {
     this.configuration = ruleContext.getConfiguration();
     this.semantics = Preconditions.checkNotNull(semantics);
     this.featureConfiguration = Preconditions.checkNotNull(featureConfiguration);
-    this.sourceCatagory = Preconditions.checkNotNull(sourceCatagory);
+    this.sourceCategory = Preconditions.checkNotNull(sourceCatagory);
   }
 
   public CcLibraryHelper(
@@ -443,7 +443,7 @@ public final class CcLibraryHelper {
   private void addSource(Artifact source, Label label) {
     boolean isHeader = CppFileTypes.CPP_HEADER.matches(source.getExecPath());
     boolean isTextualInclude = CppFileTypes.CPP_TEXTUAL_INCLUDE.matches(source.getExecPath());
-    boolean isCompiledSource = sourceCatagory.getSourceTypes().matches(source.getExecPathString());
+    boolean isCompiledSource = sourceCategory.getSourceTypes().matches(source.getExecPathString());
     if (isHeader || isTextualInclude) {
       privateHeaders.add(source);
     }
@@ -685,6 +685,15 @@ public final class CcLibraryHelper {
   }
   
   /**
+   * Sets the module map artifact for this build.
+   */
+  public CcLibraryHelper setCppModuleMap(CppModuleMap cppModuleMap) {
+    Preconditions.checkNotNull(cppModuleMap);
+    this.injectedCppModuleMap = cppModuleMap;
+    return this;
+  }
+  
+  /**
    * Overrides the path for the generated dynamic library - this should only be called if the
    * dynamic library is an implicit or explicit output of the rule, i.e., if it is accessible by
    * name from other rules in the same package. Set to {@code null} to use the default computation.
@@ -1031,24 +1040,24 @@ public final class CcLibraryHelper {
     }
 
     if (featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAPS)) {
-      CppModuleMap cppModuleMap = CppHelper.addCppModuleMapToContext(ruleContext, contextBuilder);
-      // 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),
-                !featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAP_WITHOUT_EXTERN_MODULE));
-        ruleContext.registerAction(action);
-      }
+      CppModuleMap cppModuleMap =
+          injectedCppModuleMap == null
+              ? CppHelper.createDefaultCppModuleMap(ruleContext)
+              : injectedCppModuleMap;
+      contextBuilder.setCppModuleMap(cppModuleMap);
+      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()) {
         contextBuilder.setPicHeaderModule(model.getPicHeaderModule(cppModuleMap.getArtifact()));
       }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
index 7069c8fbcd..f554eb9c34 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -418,17 +418,17 @@ public class CppHelper {
     return (dep != null) ? dep.getProvider(LipoContextProvider.class) : null;
   }
 
-  // Creates CppModuleMap object, and adds it to C++ compilation context.
-  public static CppModuleMap addCppModuleMapToContext(RuleContext ruleContext,
-      CppCompilationContext.Builder contextBuilder) {
+  /**
+   * Creates a CppModuleMap object for pure c++ builds.  The module map artifact becomes a
+   * candidate input to a CppCompileAction.
+   */
+  public static CppModuleMap createDefaultCppModuleMap(RuleContext ruleContext) {
     // Create the module map artifact as a genfile.
     Artifact mapFile = ruleContext.getPackageRelativeArtifact(
         ruleContext.getLabel().getName()
             + Iterables.getOnlyElement(CppFileTypes.CPP_MODULE_MAP.getExtensions()),
         ruleContext.getConfiguration().getGenfilesDirectory());
-        CppModuleMap moduleMap = new CppModuleMap(mapFile, ruleContext.getLabel().toString());
-    contextBuilder.setCppModuleMap(moduleMap);
-    return moduleMap;
+    return new CppModuleMap(mapFile, ruleContext.getLabel().toString());
   }
 
   /**