diff options
6 files changed, 56 insertions, 3 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java index 53db2a2aeb..0bab371346 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java @@ -703,7 +703,22 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { * Creates a BuildOptions class for the given options taken from an optionsProvider. */ public BuildOptions createBuildOptions(OptionsClassProvider optionsProvider) { - return BuildOptions.of(configurationOptions, optionsProvider); + BuildOptions buildOptions = BuildOptions.of(configurationOptions, optionsProvider); + // Possibly disable dynamic configurations if they won't work with this build. It's + // best to do this as early in the build as possible, because as the build goes on the number + // of BuildOptions references grows and the more dangerous it becomes to modify them. We do + // this here instead of in BlazeRuntime because tests and production logic don't use + // BlazeRuntime the same way. + if (buildOptions.useStaticConfigurationsOverride() + && buildOptions.get(BuildConfiguration.Options.class).useDynamicConfigurations + == BuildConfiguration.Options.DynamicConfigsMode.NOTRIM_PARTIAL) { + // It's not, generally speaking, safe to mutate BuildOptions instances when the original + // reference might persist. + buildOptions = buildOptions.clone(); + buildOptions.get(BuildConfiguration.Options.class).useDynamicConfigurations = + BuildConfiguration.Options.DynamicConfigsMode.OFF; + } + return buildOptions; } private Environment.Frame createGlobals( diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java index 97f113f8e1..725062541d 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java @@ -846,13 +846,20 @@ public final class BuildConfiguration { /** * Values for --experimental_dynamic_configs. */ - public static enum DynamicConfigsMode { + public enum DynamicConfigsMode { /** Don't use dynamic configurations. */ OFF, /** Use dynamic configurations, including only the fragments each rule needs. */ ON, /** Use dynamic configurations, always including all fragments known to Blaze. */ NOTRIM, + /** + * Use untrimmed dynamic configurations unless an {@link Options} fragment needs static + * configurations. This is used to exempt features that don't yet work with dynamic configs. + */ + // TODO(gregce): make this mode unnecesary by making everything compatible with dynamic + // configs. b/23280991 tracks the effort (LIPO is the main culprit). + NOTRIM_PARTIAL } /** diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java index 46842ae6bd..4fdae568b1 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java @@ -210,6 +210,19 @@ public final class BuildOptions implements Cloneable, Serializable { } /** + * Returns {@code true} if static configurations should be used with + * {@link BuildConfiguration.Options.DynamicConfigsMode.NOTRIM_PARTIAL}. + */ + public boolean useStaticConfigurationsOverride() { + for (FragmentOptions fragment : fragmentOptionsMap.values()) { + if (fragment.useStaticConfigurationsOverride()) { + return true; + } + } + return false; + } + + /** * The cache key for the options collection. Recomputes cache key every time it's called. */ public String computeCacheKey() { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/FragmentOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/FragmentOptions.java index 02df85e2f9..11998bb5f1 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/FragmentOptions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/FragmentOptions.java @@ -125,4 +125,12 @@ public abstract class FragmentOptions extends OptionsBase implements Cloneable, return null; } } + + /** + * Returns {@code true} if static configurations should be used with + * {@link BuildConfiguration.Options.DynamicConfigsMode.NOTRIM_PARTIAL}. + */ + public boolean useStaticConfigurationsOverride() { + return false; + } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java index 8f369680b2..9067be9ef0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java @@ -76,7 +76,7 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { CppConfiguration cppConfig = new CppConfiguration(params); if (options.get(BuildConfiguration.Options.class).useDynamicConfigurations != BuildConfiguration.Options.DynamicConfigsMode.OFF - && cppConfig.getLipoMode() != CrosstoolConfig.LipoMode.OFF) { + && (cppConfig.isFdo() || cppConfig.getLipoMode() != CrosstoolConfig.LipoMode.OFF)) { throw new InvalidConfigurationException( "LIPO does not currently work with dynamic configurations"); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java index 0af42a5aee..fbc8fb80fe 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java @@ -689,4 +689,14 @@ public class CppOptions extends FragmentOptions { public LipoMode getLipoMode() { return lipoMode; } + + /** + * FDO/LIPO is not yet compatible with dynamic configurations. + **/ + @Override + public boolean useStaticConfigurationsOverride() { + // --lipo=binary is technically possible without FDO, even though it doesn't do anything. + return isFdo() || getLipoMode() == LipoMode.BINARY; + } + } |