Skip to content

Commit

Permalink
Flip --experimental_output_directory_naming_scheme and `--experimen…
Browse files Browse the repository at this point in the history
…tal_exec_configuration_distinguisher`

Flipping as per
bazelbuild#14023 (comment).

Also changes the platform suffix in the exec cfg to a fixed `exec` for
compatibility with logic that looks for the substring `-exec-` in paths
to determine whether the current configuration is the exec
configuration.

Work towards bazelbuild#14023
  • Loading branch information
fmeum committed Jan 20, 2023
1 parent 33fed24 commit 8f1810b
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ public ExecConfigurationDistinguisherSchemeConverter() {

@Option(
name = "experimental_exec_configuration_distinguisher",
defaultValue = "legacy",
defaultValue = "off",
converter = ExecConfigurationDistinguisherSchemeConverter.class,
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
Expand Down Expand Up @@ -579,7 +579,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},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ private static BuildOptions transitionImpl(BuildOptionsView options, Label execu
break;
default:
// else if OFF do nothing
coreOptions.platformSuffix = "exec";
}

return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 2 additions & 0 deletions src/test/java/com/google/devtools/build/lib/rules/objc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,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",
Expand All @@ -93,6 +94,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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -176,6 +180,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);
Expand All @@ -185,20 +212,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();
}
Expand Down

0 comments on commit 8f1810b

Please sign in to comment.