Add a flag to make Apple rules not share C++ compile actions.

This is accomplished by:

- Setting the APPLE_CROSSTOOL configuration distinguisher in AppleCrosstoolTransition
- Doing nothing in AppleCrosstoolTransition in case we are already in an Apple configuration so that funky use cases like a binary having a dependency on a swift_library both directly and through an objc_library work
- Adding the "apl-" prefix to the output directory name in APPLE_CROSSTOOL configurations

Plus a few minor cleanups:

- Removed some unused methods
- Nopped out --enable_apple_crosstool_transition if the new flag is set
- Nopped out --target_uses_apple_crosstool if the new flag is set

These latter reduce the possible space of Apple configurations, thus making the code a bit more comprehensible.

RELNOTES: None.
PiperOrigin-RevId: 206157413

6 files changed, 56 insertions, 63 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java
index 5a2ff9d259..af55552e95 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java
@@ -15,7 +15,6 @@
 package com.google.devtools.build.lib.rules.apple;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.DefaultLabelConverter;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.LabelConverter;
@@ -333,6 +332,16 @@ public class AppleCommandLineOptions extends FragmentOptions {
   public boolean enableAppleCrosstoolTransition;
 
   @Option(
+      name = "apple_crosstool_in_output_directory_name",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+      effectTags =  {OptionEffectTag.LOADING_AND_ANALYSIS},
+      metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+      help = "If true, all Apple configurations have a different output directory than non-Apple "
+          + "ones")
+  public boolean appleCrosstoolInOutputDirectoryName;
+
+  @Option(
     name = "target_uses_apple_crosstool",
     defaultValue = "false",
     documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
@@ -358,34 +367,6 @@ public class AppleCommandLineOptions extends FragmentOptions {
   }
 
   /**
-   * Returns the architecture implied by these options.
-   *
-   * <p>In contexts in which a configuration instance is present, prefer {@link
-   * AppleConfiguration#getSingleArchitecture}.
-   */
-  public String getSingleArchitecture() {
-    if (!Strings.isNullOrEmpty(appleSplitCpu)) {
-      return appleSplitCpu;
-    }
-    switch (applePlatformType) {
-      case IOS:
-        if (!iosMultiCpus.isEmpty()) {
-          return iosMultiCpus.get(0);
-        } else {
-          return iosCpu;
-        }
-      case WATCHOS:
-        return watchosCpus.get(0);
-      case TVOS:
-        return tvosCpus.get(0);
-      case MACOS:
-        return macosCpus.get(0);
-      default:
-        throw new IllegalArgumentException("Unhandled platform type " + applePlatformType);
-    }
-  }
-
-  /**
    * Represents the Apple Bitcode mode for compilation steps.
    *
    * <p>Bitcode is an intermediate representation of a compiled program. For many platforms, Apple
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java
index 22333bb41a..530e62ad5c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java
@@ -383,24 +383,18 @@ public class AppleConfiguration extends BuildConfiguration.Fragment
     return xcodeConfigLabel;
   }
 
-  /**
-   * Returns the unique identifier distinguishing configurations that are otherwise the same.
-   *
-   * <p>Use this value for situations in which two configurations create two outputs that are the
-   * same but are not collapsed due to their different configuration owners.
-   */
-  public ConfigurationDistinguisher getConfigurationDistinguisher() {
-    return configurationDistinguisher;
-  }
-
   private boolean shouldDistinguishOutputDirectory() {
-    if (configurationDistinguisher == ConfigurationDistinguisher.UNKNOWN) {
-      return false;
-    } else if (configurationDistinguisher == ConfigurationDistinguisher.APPLE_CROSSTOOL
-        && isAppleCrosstoolEnabled()) {
-      return false;
+    if (options.appleCrosstoolInOutputDirectoryName) {
+      return configurationDistinguisher != ConfigurationDistinguisher.UNKNOWN;
     } else {
-      return true;
+      if (configurationDistinguisher == ConfigurationDistinguisher.UNKNOWN) {
+        return false;
+      } else if (configurationDistinguisher == ConfigurationDistinguisher.APPLE_CROSSTOOL
+          && enableAppleCrosstool) {
+        return false;
+      } else {
+        return true;
+      }
     }
   }
 
@@ -439,11 +433,6 @@ public class AppleConfiguration extends BuildConfiguration.Fragment
     return objcProviderFromLinked;
   }
 
-  /** Returns true if {@link AppleCrosstoolTransition} should be applied to every apple rule. */
-  public boolean isAppleCrosstoolEnabled() {
-    return enableAppleCrosstool;
-  }
-
   @Override
   public boolean equals(Object obj) {
     if (this == obj) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleCrosstoolTransition.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleCrosstoolTransition.java
index 3a2e636a13..7cd9cc986c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleCrosstoolTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleCrosstoolTransition.java
@@ -21,6 +21,7 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
 import com.google.devtools.build.lib.rules.apple.AppleCommandLineOptions;
 import com.google.devtools.build.lib.rules.apple.AppleConfiguration;
+import com.google.devtools.build.lib.rules.apple.AppleConfiguration.ConfigurationDistinguisher;
 import com.google.devtools.build.lib.rules.apple.ApplePlatform;
 import com.google.devtools.build.lib.rules.cpp.CppOptions;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
@@ -39,13 +40,22 @@ public class AppleCrosstoolTransition implements PatchTransition {
   public BuildOptions patch(BuildOptions buildOptions) {
     BuildOptions result = buildOptions.clone();
 
-    if (!appleCrosstoolTransitionIsAppliedForAllObjc(buildOptions)) {
-      return buildOptions;
-    }
-
     AppleCommandLineOptions appleOptions = buildOptions.get(AppleCommandLineOptions.class);
     BuildConfiguration.Options configOptions = buildOptions.get(BuildConfiguration.Options.class);
 
+    if (appleOptions.appleCrosstoolInOutputDirectoryName) {
+      if (appleOptions.configurationDistinguisher != ConfigurationDistinguisher.UNKNOWN) {
+        // The configuration distinguisher is only set by AppleCrosstoolTransition and
+        // AppleBinaryTransition, both of which also set the Crosstool and the CPU to Apple ones.
+        // So we are fine not doing anything.
+        return buildOptions;
+      }
+    } else {
+      if (!appleCrosstoolTransitionIsAppliedForAllObjc(buildOptions)) {
+        return buildOptions;
+      }
+    }
+
     String cpu =
         ApplePlatform.cpuStringForTarget(
             appleOptions.applePlatformType,
@@ -69,6 +79,10 @@ public class AppleCrosstoolTransition implements PatchTransition {
     to.get(CppOptions.class).crosstoolTop =
         from.get(AppleCommandLineOptions.class).appleCrosstoolTop;
     to.get(AppleCommandLineOptions.class).targetUsesAppleCrosstool = true;
+    if (from.get(AppleCommandLineOptions.class).appleCrosstoolInOutputDirectoryName) {
+      to.get(AppleCommandLineOptions.class).configurationDistinguisher =
+          ConfigurationDistinguisher.APPLE_CROSSTOOL;
+    }
 
     // --compiler = "compiler" for all OSX toolchains.  We do not support asan/tsan, cfi, etc. on
     // darwin.
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 da2d3124b9..0501930cbb 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
@@ -461,7 +461,7 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase {
 
   @Test
   public void testCompilationAction() throws Exception {
-    useConfiguration("--cpu=ios_i386");
+    useConfiguration("--cpu=ios_i386", "--apple_crosstool_in_output_directory_name");
     ApplePlatform platform = ApplePlatform.IOS_SIMULATOR;
 
     // Because protos are linked/compiled within the apple_binary context, we need to traverse the
@@ -510,7 +510,7 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase {
             .addAll(
                 ObjcLibraryTest.iquoteArgs(
                     providerForTarget("//package:opl_binary"),
-                    getTargetConfiguration()))
+                    getAppleCrosstoolConfiguration()))
             .add("-I")
             .add(sourceFile.getExecPath().getParentDirectory().getParentDirectory().toString())
             .add("-fno-objc-arc")
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 4227116991..2090c7642a 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
@@ -192,6 +192,12 @@ public abstract class ObjcRuleTestCase extends BuildViewTestCase {
     switch (configurationDistinguisher) {
       case UNKNOWN:
         return String.format("%s-out/ios_%s-%s/", TestConstants.PRODUCT_NAME, arch, modeSegment);
+      case APPLE_CROSSTOOL:
+        return String.format("%1$s-out/apl-ios_%2$s%3$s-%4$s/",
+            TestConstants.PRODUCT_NAME,
+            arch,
+            minOsSegment,
+            modeSegment);
       case APPLEBIN_IOS:
         return String.format(
             "%1$s-out/ios-%2$s%4$s-%3$s-ios_%2$s-%5$s/",
@@ -2107,7 +2113,9 @@ public abstract class ObjcRuleTestCase extends BuildViewTestCase {
   private void checkCustomModuleMap(RuleType ruleType, boolean targetUnderTestShouldPropagate)
       throws Exception {
     useConfiguration(
-        "--experimental_objc_enable_module_maps", "--incompatible_strict_objc_module_maps");
+        "--apple_crosstool_in_output_directory_name",
+        "--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");
@@ -2139,7 +2147,7 @@ public abstract class ObjcRuleTestCase extends BuildViewTestCase {
     assertThat(compileActionA.getArguments()).doesNotContain("-fmodule-name");
 
     String x8664Genfiles =
-        configurationGenfiles("x86_64", ConfigurationDistinguisher.UNKNOWN, null);
+        configurationGenfiles("x86_64", ConfigurationDistinguisher.APPLE_CROSSTOOL, null);
 
     // The target with the module map should propagate it to its direct dependers...
     ObjcProvider provider = providerForTarget("//z:testModuleMap");
diff --git a/src/test/shell/bazel/apple/bazel_objc_test.sh b/src/test/shell/bazel/apple/bazel_objc_test.sh
index e60c98b957..09dd0536ff 100755
--- a/src/test/shell/bazel/apple/bazel_objc_test.sh
+++ b/src/test/shell/bazel/apple/bazel_objc_test.sh
@@ -49,9 +49,10 @@ function test_build_app() {
   setup_objc_test_support
   make_lib
 
-  bazel build --verbose_failures --ios_sdk_version=$IOS_SDK_VERSION \
+  bazel build --verbose_failures --apple_crosstool_in_output_directory_name \
+      --ios_sdk_version=$IOS_SDK_VERSION \
       //ios:lib >$TEST_log 2>&1 || fail "should pass"
-  ls bazel-out/ios_x86_64-fastbuild/bin/ios/liblib.a \
+  ls bazel-out/apl-ios_x86_64-fastbuild/bin/ios/liblib.a \
       || fail "should generate lib.a"
 }
 
@@ -101,7 +102,7 @@ int aFunction() {
 }
 EOF
 
-  bazel build --verbose_failures --apple_crosstool_transition \
+  bazel build --verbose_failures --apple_crosstool_in_output_directory_name \
       --ios_sdk_version=$IOS_SDK_VERSION //objclib:objclib >"$TEST_log" 2>&1 \
       || fail "Should build objc_library"
 
@@ -109,9 +110,9 @@ EOF
   # Dec 31 1969 or Jan 1 1970 -- either is fine.
   # We would use 'date' here, but the format is slightly different (Jan 1 vs.
   # Jan 01).
-  ar -tv bazel-out/ios_x86_64-fastbuild/bin/objclib/libobjclib.a \
+  ar -tv bazel-out/apl-ios_x86_64-fastbuild/bin/objclib/libobjclib.a \
       | grep "mysrc" | grep "Dec 31" | grep "1969" \
-      || ar -tv bazel-out/ios_x86_64-fastbuild/bin/objclib/libobjclib.a \
+      || ar -tv bazel-out/apl-ios_x86_64-fastbuild/bin/objclib/libobjclib.a \
       | grep "mysrc" | grep "Jan  1" | grep "1970" || \
       fail "Timestamp of contents of archive file should be zero"
 }