Enable revised output directory hash suffix computation

Flips `--experimental_output_directory_naming_scheme` and `--experimental_exec_configuration_distinguisher` to `diff_against_baseline` and `off`, respectively, which makes it so that Starlark transitions can return to previous configurations, thus improving cache hit rates.

Also changes the fixed platform suffix in the exec configuration from an empty string to `"exec"` for compatibility with logic that looks for the substring `-exec-` in paths to determine whether the current configuration is the exec configuration.

As a result of this commit, output directory paths can change in the following ways:
* `-ST-<hash>` suffixes can change or disappear and, in rare cases, appear where they previously didn't
* for exec configuration directories, `k8-opt-exec-2B5CBBC6` becomes `k8-opt-exec-ST-011b9bdc32ed`

This may result in tool paths that are seven characters longer, which can cause builds on Windows to fail that were already very close to the `MAX_PATH` limit. A follow-up commit will reduce the length of the hash by three characters, bringing that increase down to four.

Work towards #14023

Closes #16910.

PiperOrigin-RevId: 523999034
Change-Id: Id5548a00b62ebfe7889cd40177995210ecc224c1

Author: fmeum

src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java

@@ -348,7 +348,7 @@ public ExecConfigurationDistinguisherSchemeConverter() {
 
   @Option(
       name = "experimental_exec_configuration_distinguisher",
-      defaultValue = "legacy",
+      defaultValue = "off",
       converter = ExecConfigurationDistinguisherSchemeConverter.class,
       documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
       effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
@@ -593,7 +593,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},

src/main/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactory.java

@@ -184,7 +184,8 @@ private static BuildOptions transitionImpl(BuildOptionsView options, Label execu
           result = result.clone();
           break;
         default:
-          // else if OFF do nothing
+          // else if OFF just mark that we are now in an exec transition
+          coreOptions.platformSuffix = "exec";
       }
 
       return result;

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)) {

src/test/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactoryTest.java

@@ -211,7 +211,6 @@ public void executionTransition_confDist_off()
             new StoredEventHandler());
 
     assertThat(result.get(CoreOptions.class).affectedByStarlarkTransition).isEmpty();
-    assertThat(result.get(CoreOptions.class).platformSuffix)
-        .isEqualTo(options.get(CoreOptions.class).platformSuffix);
+    assertThat(result.get(CoreOptions.class).platformSuffix).isEqualTo("exec");
   }
 }

src/test/java/com/google/devtools/build/lib/analysis/util/BUILD

@@ -50,6 +50,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:config/config_conditions",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/config_matching_provider",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/core_option_converters",
+        "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/execution_transition_factory",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_factory",

src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java

@@ -24,6 +24,7 @@
 import com.google.devtools.build.lib.analysis.ServerDirectories;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.analysis.config.CoreOptions;
 import com.google.devtools.build.lib.analysis.config.FragmentFactory;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
@@ -130,6 +131,10 @@ public final void initializeSkyframeExecutor() throws Exception {
     SkyframeExecutorTestHelper.process(skyframeExecutor);
     skyframeExecutor.injectExtraPrecomputedValues(
         ImmutableList.of(
+            PrecomputedValue.injected(
+                PrecomputedValue.BASELINE_CONFIGURATION,
+                BuildOptions.getDefaultBuildOptionsForFragments(
+                    ImmutableList.of(CoreOptions.class))),
             PrecomputedValue.injected(
                 RepositoryDelegatorFunction.RESOLVED_FILE_INSTEAD_OF_WORKSPACE, Optional.empty()),
             PrecomputedValue.injected(

src/test/java/com/google/devtools/build/lib/rules/objc/BUILD

@@ -80,6 +80,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",
@@ -94,6 +95,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",

src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java

@@ -39,6 +39,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;
@@ -48,12 +50,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;
@@ -169,13 +173,39 @@ private static String toolExecutable(String toolSrcPath) {
         TestConstants.PRODUCT_NAME, TestConstants.TOOLS_REPOSITORY_PATH_PREFIX + toolSrcPath);
   }
 
+  @SuppressWarnings("MissingCasesInEnumSwitch")
   private String configurationDir(
       String arch,
       ConfigurationDistinguisher configurationDistinguisher,
       DottedVersion minOsVersion,
       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);
@@ -185,20 +215,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();
     }
@@ -669,7 +701,7 @@ protected void checkProvidesHdrsAndIncludes(RuleType ruleType, Optional<String>
         getConfiguredTarget("//x:x", getAppleCrosstoolConfiguration())
             .get(CcInfo.PROVIDER)
             .getCcCompilationContext();
-    List<String> declaredIncludeSrcs =
+    ImmutableList<String> declaredIncludeSrcs =
         ccCompilationContext.getDeclaredIncludeSrcs().toList().stream()
             .map(x -> removeConfigFragment(x.getExecPathString()))
             .collect(toImmutableList());