Flip --experimental_output_directory_naming_scheme to `diff_against…

…_baseline` Flipping as per bazelbuild#14023 (comment). Work towards bazelbuild#14023
iancha1992 · Dec 5, 2022 · 7871c5a · 7871c5a
1 parent 1a8f999
commit 7871c5a
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 6 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java
@@ -556,7 +556,7 @@ public OutputDirectoryNamingSchemeConverter() {
 
   @Option(
       name = "experimental_output_directory_naming_scheme",
-      defaultValue = "legacy",
+      defaultValue = "diff_against_baseline",
       converter = OutputDirectoryNamingSchemeConverter.class,
       documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
       effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationFunction.java
@@ -140,7 +140,8 @@ private static final class BuildConfigurationFunctionException extends SkyFuncti
    * Compute the hash for the new BuildOptions based on the names and values of all options (both
    * native and Starlark) that are different from some supplied baseline configuration.
    */
-  private static String computeNameFragmentWithDiff(
+  @VisibleForTesting
+  public static String computeNameFragmentWithDiff(
       BuildOptions toOptions, BuildOptions baselineOptions) {
     // Quick short-circuit for trivial case.
     if (toOptions.equals(baselineOptions)) {

diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/BUILD b/src/test/java/com/google/devtools/build/lib/rules/objc/BUILD
@@ -77,6 +77,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/compilation_mode",
+        "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/invalid_configuration_exception",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/split_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
@@ -91,6 +92,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/rules/apple/cpp",
         "//src/main/java/com/google/devtools/build/lib/rules/cpp",
         "//src/main/java/com/google/devtools/build/lib/rules/objc",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",
         "//src/main/java/com/google/devtools/build/lib/util",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",

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
@@ -40,6 +40,8 @@
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
 import com.google.devtools.build.lib.analysis.config.CompilationMode;
+import com.google.devtools.build.lib.analysis.config.CoreOptions;
+import com.google.devtools.build.lib.analysis.config.CoreOptions.OutputDirectoryNamingScheme;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
@@ -49,12 +51,14 @@
 import com.google.devtools.build.lib.packages.util.MockObjcSupport;
 import com.google.devtools.build.lib.rules.apple.AppleCommandLineOptions;
 import com.google.devtools.build.lib.rules.apple.AppleConfiguration.ConfigurationDistinguisher;
+import com.google.devtools.build.lib.rules.apple.ApplePlatform.PlatformType;
 import com.google.devtools.build.lib.rules.apple.AppleToolchain;
 import com.google.devtools.build.lib.rules.apple.DottedVersion;
 import com.google.devtools.build.lib.rules.cpp.CcCompilationContext;
 import com.google.devtools.build.lib.rules.cpp.CcInfo;
 import com.google.devtools.build.lib.rules.cpp.CppLinkAction;
 import com.google.devtools.build.lib.rules.objc.CompilationSupport.ExtraLinkArgs;
+import com.google.devtools.build.lib.skyframe.BuildConfigurationFunction;
 import com.google.devtools.build.lib.testutil.Scratch;
 import com.google.devtools.build.lib.testutil.TestConstants;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -177,6 +181,29 @@ private String configurationDir(
       CompilationMode compilationMode) {
     String minOsSegment = minOsVersion == null ? "" : "-min" + minOsVersion;
     String modeSegment = compilationModeFlag(compilationMode);
+
+    String hash = "";
+    if (targetConfig.getOptions().get(CoreOptions.class).outputDirectoryNamingScheme
+        == OutputDirectoryNamingScheme.DIFF_AGAINST_BASELINE) {
+      PlatformType platformType = null;
+      switch (configurationDistinguisher) {
+        case APPLEBIN_IOS:
+          platformType = PlatformType.IOS;
+          break;
+        case APPLEBIN_WATCHOS:
+          platformType = PlatformType.WATCHOS;
+          break;
+      }
+      BuildOptions transitionedConfig = targetConfig.cloneOptions();
+      transitionedConfig.get(CoreOptions.class).cpu = platformType + "_" + arch;
+      transitionedConfig.get(
+          AppleCommandLineOptions.class).configurationDistinguisher = configurationDistinguisher;
+      transitionedConfig.get(AppleCommandLineOptions.class).applePlatformType = platformType;
+      transitionedConfig.get(AppleCommandLineOptions.class).appleSplitCpu = arch;
+      hash = "-" + BuildConfigurationFunction.computeNameFragmentWithDiff(
+          transitionedConfig, targetConfig.getOptions());
+    }
+
     switch (configurationDistinguisher) {
       case UNKNOWN:
         return String.format("%s-out/ios_%s-%s/", TestConstants.PRODUCT_NAME, arch, modeSegment);
@@ -186,20 +213,22 @@ private String configurationDir(
             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/",
+            "%1$s-out/ios-%2$s%4$s-%3$s-ios_%2$s-%5$s%6$s/",
             TestConstants.PRODUCT_NAME,
             arch,
             configurationDistinguisher.toString().toLowerCase(Locale.US),
             minOsSegment,
-            modeSegment);
+            modeSegment,
+            hash);
       case APPLEBIN_WATCHOS:
         return String.format(
-            "%1$s-out/watchos-%2$s%4$s-%3$s-watchos_%2$s-%5$s/",
+            "%1$s-out/watchos-%2$s%4$s-%3$s-watchos_%2$s-%5$s%6$s/",
             TestConstants.PRODUCT_NAME,
             arch,
             configurationDistinguisher.toString().toLowerCase(Locale.US),
             minOsSegment,
-            modeSegment);
+            modeSegment,
+            hash);
       default:
         throw new AssertionError();
     }