Add --incompatible_strict_objc_module_maps.

This flag changes the behavior of objc_library module map propagation so that module maps are only propagated to direct dependents, not transitive dependents. swift_library targets that import Objective-C code must then list those dependencies directly in its deps instead of depending on them being transitively present.

PiperOrigin-RevId: 187184692

4 files changed, 67 insertions, 23 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java
index b940580870..cc23c7cf83 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java
@@ -1460,7 +1460,7 @@ public class AppleBinaryTest extends ObjcRuleTestCase {
 
   @Test
   public void testCustomModuleMap() throws Exception {
-    checkCustomModuleMap(getRuleType());
+    checkCustomModuleMapNotPropagatedByTargetUnderTest(getRuleType());
   }
 
   @Test
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 ec3dea8d41..0a83e02975 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
@@ -1928,7 +1928,7 @@ public class ObjcLibraryTest extends ObjcRuleTestCase {
 
   @Test
   public void testCustomModuleMap() throws Exception {
-    checkCustomModuleMap(RULE_TYPE);
+    checkCustomModuleMapPropagatedByTargetUnderTest(RULE_TYPE);
   }
 
   private boolean containsObjcFeature(String srcName) throws Exception {
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibraryTest.java
index 955ce658a7..2dbd7db711 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibraryTest.java
@@ -399,27 +399,32 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase {
 
   @Test
   public void testModulemapCreatedForNonLinkingTargets() throws Exception {
-    checkOnlyLibModuleMapsArePresentForTarget("//package:opl_protobuf");
+    // TODO(b/73943026): Remove this flag once everyone has migrated to the new strict behavior and
+    // it is made the default.
+    useConfiguration("--incompatible_strict_objc_module_maps");
+
+    // The library target should propagate its module map.
+    ObjcProvider provider = providerForTarget("//package:opl_protobuf");
+    assertThat(Artifact.toRootRelativePaths(provider.get(ObjcProvider.MODULE_MAP).toSet()))
+        .containsExactly("package/opl_protobuf.modulemaps/module.modulemap");
   }
 
   @Test
   public void testModulemapNotCreatedForLinkingTargets() throws Exception {
-    checkOnlyLibModuleMapsArePresentForTarget("//package:opl_binary");
+    // TODO(b/73943026): Remove this flag once everyone has migrated to the new strict behavior and
+    // it is made the default.
+    useConfiguration("--incompatible_strict_objc_module_maps");
+
+    // The binary target should not propagate the module map from the library it depends on.
+    ObjcProvider provider = providerForTarget("//package:opl_binary");
+    assertThat(Artifact.toRootRelativePaths(provider.get(ObjcProvider.MODULE_MAP).toSet()))
+        .isEmpty();
   }
 
   private static String sortedJoin(Iterable<String> elements) {
     return Joiner.on('\n').join(Ordering.natural().immutableSortedCopy(elements));
   }
 
-  private void checkOnlyLibModuleMapsArePresentForTarget(String target) throws Exception {
-    ObjcProvider provider = providerForTarget(target);
-    assertThat(Artifact.toRootRelativePaths(provider.get(ObjcProvider.MODULE_MAP).toSet()))
-        .containsExactly(
-            "package/opl_protobuf.modulemaps/module.modulemap",
-            TestConstants.TOOLS_REPOSITORY_PATH_PREFIX
-                + "objcproto/protobuf_lib.modulemaps/module.modulemap");
-  }
-
   @Test
   public void testObjcProvider() throws Exception {
     ConfiguredTarget target = getConfiguredTarget("//package:opl_protobuf");
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
index 6da95762ab..9a8dfff2db 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
@@ -3351,19 +3351,40 @@ public abstract class ObjcRuleTestCase extends BuildViewTestCase {
         .isEqualTo("a.o");
   }
 
-  protected void checkCustomModuleMap(RuleType ruleType) throws Exception {
-    useConfiguration("--experimental_objc_enable_module_maps");
-    ruleType.scratchTarget(scratch, "deps", "['//z:testModuleMap']");
+  protected void checkCustomModuleMapNotPropagatedByTargetUnderTest(
+      RuleType ruleType) throws Exception {
+    checkCustomModuleMap(ruleType, false);
+  }
+
+  protected void checkCustomModuleMapPropagatedByTargetUnderTest(
+      RuleType ruleType) throws Exception {
+    checkCustomModuleMap(ruleType, true);
+  }
+
+  private void checkCustomModuleMap(RuleType ruleType, boolean targetUnderTestShouldPropagate)
+      throws Exception {
+    useConfiguration(
+        "--experimental_objc_enable_module_maps", "--incompatible_strict_objc_module_maps");
+    ruleType.scratchTarget(scratch, "deps", "['//z:a']");
+    scratch.file("z/a.m");
+    scratch.file("z/a.h");
     scratch.file("z/b.m");
     scratch.file("z/b.h");
     scratch.file("y/module.modulemap", "module my_module_b { export *\n header b.h }");
-    scratch.file("z/BUILD",
+    scratch.file(
+        "z/BUILD",
+        "objc_library(",
+        "name = 'testModuleMap',",
+        "hdrs = ['b.h'],",
+        "srcs = ['b.m'],",
+        "module_map = '//y:mm'",
+        ")",
         "objc_library(",
-            "name = 'testModuleMap',",
-            "hdrs = ['b.h'],",
-            "srcs = ['b.m'],",
-            "module_map = '//y:mm'",
-         ")");
+        "name = 'a',",
+        "hdrs = ['a.h'],",
+        "srcs = ['a.m'],",
+        "deps = [':testModuleMap']",
+        ")");
     scratch.file("y/BUILD",
         "filegroup(",
             "name = 'mm',",
@@ -3374,12 +3395,30 @@ public abstract class ObjcRuleTestCase extends BuildViewTestCase {
     assertThat(compileActionA.getArguments()).doesNotContain("-fmodule-maps");
     assertThat(compileActionA.getArguments()).doesNotContain("-fmodule-name");
 
+    String x8664Genfiles =
+        configurationGenfiles("x86_64", ConfigurationDistinguisher.UNKNOWN, null);
+
+    // The target with the module map should propagate it to its direct dependers...
     ObjcProvider provider = providerForTarget("//z:testModuleMap");
     assertThat(Artifact.toExecPaths(provider.get(MODULE_MAP)))
         .containsExactly("y/module.modulemap");
 
+    // ...and the target depending on //z:testModuleMap will see it (as well as its own)...
+    provider = providerForTarget("//z:a");
+    assertThat(Artifact.toExecPaths(provider.get(MODULE_MAP)))
+        .containsExactly(x8664Genfiles + "/z/a.modulemaps/module.modulemap", "y/module.modulemap");
+
     provider = providerForTarget("//x:x");
-    assertThat(Artifact.toExecPaths(provider.get(MODULE_MAP))).contains("y/module.modulemap");
+    if (targetUnderTestShouldPropagate) {
+      // ...and //x:x should propagate //z:a but not //z:testModuleMap.
+      assertThat(Artifact.toExecPaths(provider.get(MODULE_MAP)))
+          .containsExactly(
+              x8664Genfiles + "/x/x.modulemaps/module.modulemap",
+              x8664Genfiles + "/z/a.modulemaps/module.modulemap");
+    } else {
+      // ...but //x:x should not see them.
+      assertThat(Artifact.toExecPaths(provider.get(MODULE_MAP))).isEmpty();
+    }
   }
 
   /**