From a40b6a055d84ec5274302aca2bc010527265aabc Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 2 Dec 2022 22:24:11 +0100 Subject: [PATCH] Flip `--experimental_output_directory_naming_scheme` to `diff_against_baseline` Flipping as per https://github.com/bazelbuild/bazel/issues/14023#issuecomment-1335726213. Work towards #14023 --- .../lib/analysis/config/CoreOptions.java | 2 +- .../skyframe/BuildConfigurationFunction.java | 3 +- .../devtools/build/lib/rules/objc/BUILD | 2 + .../lib/rules/objc/ObjcRuleTestCase.java | 37 +++++++++++++++++-- 4 files changed, 38 insertions(+), 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 index 2fc7ee409cc986..ce5aaa70229e6b 100644 --- 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 index 6a95da81ecdc5d..0906ec65ff0999 100644 --- 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 index 07b5b2631d10ac..9458b00b947cb7 100644 --- 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 index 6459c6522c0ef6..c4b2eb416d5bd8 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 @@ -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(); }