Repository: bazelbuild/bazel
Issue: Feature Request: Objc Library Module Name
Labels: under investigation
category: rules > ObjC / iOS / J2ObjC

I'd like the ability to control the module name created by the objc_library rule. For more details refer to https://github.com/bazelbuild/rules_apple/issues/51.


Issue comments:
any updates on this?
No updates yet -- we'll be chatting about this over the next few business days to prioritize.
Is there a hint so that we could fix it and perhaps submit a PR ?
We're not likely to get to this ourselves; we deemed this low priority internally.
If you'd like to go ahead and create a PR, you'll need to trace through ObjcLibrary.java and understand where the module name gets set; you can then hook through an attribute value to optionally change the default. 

(I don't know off the top of my head what decides the default module name in terms of the end command line.. You might start there? You can build an example with `-s` to see the command lines invoked in your build)
Makes total sense that's it's low priority.

That's exactly what I needed, we'll see how bad we want it and if so we'll submit a PR. Thanks!

Pull request review comments:

Patch that resolved the issue:
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 602089b691fb3f..37cc3ee659013d 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
@@ -49,10 +49,10 @@ public final class CppModuleMapAction extends AbstractFileWriteAction {
 
   // C++ module map of the current target
   private final CppModuleMap cppModuleMap;
-  
+
   /**
    * If set, the paths in the module map are relative to the current working directory instead
-   * of relative to the module map file's location. 
+   * of relative to the module map file's location.
    */
   private final boolean moduleMapHomeIsCwd;
 
@@ -217,7 +217,7 @@ private void appendHeader(Appendable content, String visibilitySpecifier,
     }
     content.append("\n");
   }
-  
+
   private boolean shouldCompileHeader(PathFragment path) {
     return compiledModule && !CppFileTypes.CPP_TEXTUAL_INCLUDE.matches(path);
   }
@@ -258,6 +258,9 @@ protected void computeKey(ActionKeyContext actionKeyContext, Fingerprint fp) {
     fp.addBoolean(externDependencies);
   }
 
+  @VisibleForTesting
+  public CppModuleMap getCppModuleMap() { return cppModuleMap; }
+
   @VisibleForTesting
   public Collection<Artifact> getPublicHeaders() {
     return publicHeaders;
@@ -267,7 +270,7 @@ public Collection<Artifact> getPublicHeaders() {
   public Collection<Artifact> getPrivateHeaders() {
     return privateHeaders;
   }
-  
+
   @VisibleForTesting
   public ImmutableList<PathFragment> getAdditionalExportedHeaders() {
     return additionalExportedHeaders;
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 5ef84aba9b088c..a1d423a946f4bb 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
@@ -604,6 +604,10 @@ static final class ExtraCompileArgs extends IterableWrapper<String> {
   static final String FILE_IN_SRCS_AND_NON_ARC_SRCS_ERROR_FORMAT =
       "File '%s' is present in both srcs and non_arc_srcs which is forbidden.";
 
+  @VisibleForTesting
+  static final String BOTH_MODULE_NAME_AND_MODULE_MAP_SPECIFIED =
+      "Specifying both module_name and module_map is invalid, please remove one of them.";
+
   static final ImmutableList<String> DEFAULT_COMPILER_FLAGS = ImmutableList.of("-DOS_IOS");
 
   static final ImmutableList<String> DEFAULT_LINKER_FLAGS = ImmutableList.of("-ObjC");
@@ -936,6 +940,12 @@ CompilationSupport validateAttributes() throws RuleErrorException {
       }
     }
 
+    if (ruleContext.attributes().isAttributeValueExplicitlySpecified("module_name")
+        && ruleContext.attributes().isAttributeValueExplicitlySpecified("module_map")) {
+      ruleContext.attributeError(
+          "module_name", BOTH_MODULE_NAME_AND_MODULE_MAP_SPECIFIED);
+    }
+
     ruleContext.assertNoErrors();
     return this;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java
index 90b46950371306..7c79a11c35bd1c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java
@@ -23,6 +23,7 @@
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.rules.cpp.CppModuleMap;
 import com.google.devtools.build.lib.rules.cpp.CppModuleMap.UmbrellaHeaderStrategy;
+import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
@@ -370,15 +371,20 @@ public Artifact linkmap(String arch) {
    * {@link CppModuleMap} that provides the clang module map for this target.
    */
   public CppModuleMap moduleMap() {
-    String moduleName =
-        ruleContext
-            .getLabel()
-            .toString()
-            .replace("//", "")
-            .replace("@", "")
-            .replace("-", "_")
-            .replace("/", "_")
-            .replace(":", "_");
+    String moduleName;
+    if (ruleContext.attributes().isAttributeValueExplicitlySpecified("module_name")) {
+      moduleName = ruleContext.attributes().get("module_name", Type.STRING);
+    } else {
+      moduleName =
+          ruleContext
+              .getLabel()
+              .toString()
+              .replace("//", "")
+              .replace("@", "")
+              .replace("-", "_")
+              .replace("/", "_")
+              .replace(":", "_");
+    }
     Optional<Artifact> customModuleMap = CompilationSupport.getCustomModuleMap(ruleContext);
     if (customModuleMap.isPresent()) {
       return new CppModuleMap(customModuleMap.get(), moduleName);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
index c17fa0692650b3..2270b281768d31 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
@@ -684,6 +684,11 @@ Enables clang module support (via -fmodules).
           provided module map to the compiler.
           <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/
           .add(attr("module_map", LABEL).allowedFileTypes(FileType.of(".modulemap")))
+          /* <!-- #BLAZE_RULE($objc_compiling_rule).ATTRIBUTE(module_name) -->
+          Sets the module name for this target. By default the module name is the target path with
+          all special symbols replaced by _, e.g. //foo/baz:bar can be imported as foo_baz_bar.
+          <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/
+          .add(attr("module_name", STRING))
           /* Provides the label for header_scanner tool that is used to scan inclusions for ObjC
           sources and provide a list of required headers via a .header_list file.
 


Test patch:
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java
index 16aee317cbf29a..a4a45892fca126 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java
@@ -18,6 +18,7 @@
 import static com.google.common.truth.Truth.assertWithMessage;
 import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.baseArtifactNames;
 import static com.google.devtools.build.lib.rules.objc.CompilationSupport.ABSOLUTE_INCLUDES_PATH_FORMAT;
+import static com.google.devtools.build.lib.rules.objc.CompilationSupport.BOTH_MODULE_NAME_AND_MODULE_MAP_SPECIFIED;
 import static com.google.devtools.build.lib.rules.objc.CompilationSupport.FILE_IN_SRCS_AND_HDRS_WARNING_FORMAT;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.ASSET_CATALOG;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.BUNDLE_FILE;
@@ -52,6 +53,7 @@
 import com.google.devtools.build.lib.rules.apple.ApplePlatform;
 import com.google.devtools.build.lib.rules.apple.AppleToolchain;
 import com.google.devtools.build.lib.rules.cpp.CppCompileAction;
+import com.google.devtools.build.lib.rules.cpp.CppModuleMap;
 import com.google.devtools.build.lib.rules.cpp.CppModuleMapAction;
 import com.google.devtools.build.lib.rules.cpp.LinkerInput;
 import com.google.devtools.build.lib.rules.objc.ObjcProvider.Key;
@@ -587,6 +589,15 @@ public void testObjcCopts_argumentOrdering() throws Exception {
     assertThat(args).containsAllOf("-fobjc-arc", "-foo", "-bar").inOrder();
   }
 
+  @Test
+  public void testBothModuleNameAndModuleMapGivesError() throws Exception {
+    checkError(
+        "x",
+        "x",
+        BOTH_MODULE_NAME_AND_MODULE_MAP_SPECIFIED,
+        "objc_library( name = 'x', module_name = 'x', module_map = 'x.modulemap' )");
+  }
+
   @Test
   public void testCompilationActionsWithModuleMapsEnabled() throws Exception {
     useConfiguration(
@@ -676,6 +687,22 @@ public void testCompilationActionsWithBitcode_simulator() throws Exception {
     assertThat(compileActionA.getArguments()).doesNotContain("-fembed-bitcode-marker");
   }
 
+  @Test
+  public void testModuleNameAttributeChangesName() throws Exception {
+    RULE_TYPE.scratchTarget(
+        scratch,
+        "module_name",
+        "'foo'");
+
+    ConfiguredTarget configuredTarget = getConfiguredTarget("//x:x");
+    Artifact moduleMap = getGenfilesArtifact("x.modulemaps/module.modulemap", configuredTarget);
+
+    CppModuleMapAction genMap = (CppModuleMapAction) getGeneratingAction(moduleMap);
+
+    CppModuleMap cppModuleMap = genMap.getCppModuleMap();
+    assertThat(cppModuleMap.getName()).isEqualTo("foo");
+  }
+
   @Test
   public void testModuleMapActionFiltersHeaders() throws Exception {
     RULE_TYPE.scratchTarget(
@@ -692,6 +719,10 @@ public void testModuleMapActionFiltersHeaders() throws Exception {
 
     assertThat(Artifact.toRootRelativePaths(genMap.getPrivateHeaders())).isEmpty();
     assertThat(Artifact.toRootRelativePaths(genMap.getPublicHeaders())).containsExactly("x/a.h");
+
+    // now check the generated name
+    CppModuleMap cppModuleMap = genMap.getCppModuleMap();
+    assertThat(cppModuleMap.getName()).isEqualTo("x_x");
   }
 
   @Test