diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build')
51 files changed, 181 insertions, 1974 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java index fac653b402..d1f1ecc848 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java @@ -169,14 +169,16 @@ public class BaseRuleClasses { attr("$test_runtime", LABEL_LIST) .cfg(HostTransition.INSTANCE) .value(ImmutableList.of(env.getToolsLabel("//tools/test:runtime")))) - .add(attr("$test_setup_script", LABEL) - .cfg(HostTransition.INSTANCE) - .singleArtifact() - .value(env.getToolsLabel("//tools/test:test_setup"))) - .add(attr("$collect_coverage_script", LABEL) - .cfg(HostTransition.INSTANCE) - .singleArtifact() - .value(env.getToolsLabel("//tools/test:collect_coverage"))) + .add( + attr("$test_setup_script", LABEL) + .cfg(HostTransition.INSTANCE) + .singleArtifact() + .value(env.getToolsLabel("//tools/test:test_setup"))) + .add( + attr("$collect_coverage_script", LABEL) + .cfg(HostTransition.INSTANCE) + .singleArtifact() + .value(env.getToolsLabel("//tools/test:collect_coverage"))) // Input files for test actions collecting code coverage .add( attr(":coverage_support", LABEL) @@ -190,14 +192,8 @@ public class BaseRuleClasses { coverageReportGeneratorAttribute( env.getToolsLabel(DEFAULT_COVERAGE_REPORT_GENERATOR_VALUE))) .singleArtifact()) - - // The target itself and run_under both run on the same machine. We use the DATA config - // here because the run_under acts like a data dependency (e.g. no LIPO optimization). - .add( - attr(":run_under", LABEL) - .cfg(env.getLipoDataTransition()) - .value(RUN_UNDER) - .skipPrereqValidatorCheck()) + // The target itself and run_under both run on the same machine. + .add(attr(":run_under", LABEL).value(RUN_UNDER).skipPrereqValidatorCheck()) .executionPlatformConstraintsAllowed(ExecutionPlatformConstraintsAllowed.PER_TARGET) .build(); } @@ -382,9 +378,10 @@ public class BaseRuleClasses { public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) { return builder .add(attr("deps", LABEL_LIST).legacyAllowAnyFileType()) - .add(attr("data", LABEL_LIST).cfg(env.getLipoDataTransition()) - .allowedFileTypes(FileTypeSet.ANY_FILE) - .dontCheckConstraints()) + .add( + attr("data", LABEL_LIST) + .allowedFileTypes(FileTypeSet.ANY_FILE) + .dontCheckConstraints()) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java index 8a3faea0a3..c5b411c881 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java @@ -69,16 +69,6 @@ public class CachingAnalysisEnvironment implements AnalysisEnvironment { private final boolean isSystemEnv; private final boolean extendedSanityChecks; - /** - * If false, no actions will be registered, they'll all be just dropped. - * - * <p>Usually, an analysis environment should register all actions. However, in some scenarios we - * analyze some targets twice, but the first one only serves the purpose of collecting information - * for the second analysis. In this case we don't register actions created by the first pass in - * order to avoid action conflicts. - */ - private final boolean allowRegisteringActions; - private final ActionKeyContext actionKeyContext; private boolean enabled = true; @@ -100,8 +90,7 @@ public class CachingAnalysisEnvironment implements AnalysisEnvironment { boolean isSystemEnv, boolean extendedSanityChecks, ExtendedEventHandler errorEventListener, - SkyFunction.Environment env, - boolean allowRegisteringActions) { + SkyFunction.Environment env) { this.artifactFactory = artifactFactory; this.actionKeyContext = actionKeyContext; this.owner = Preconditions.checkNotNull(owner); @@ -109,13 +98,12 @@ public class CachingAnalysisEnvironment implements AnalysisEnvironment { this.extendedSanityChecks = extendedSanityChecks; this.errorEventListener = errorEventListener; this.skyframeEnv = env; - this.allowRegisteringActions = allowRegisteringActions; middlemanFactory = new MiddlemanFactory(artifactFactory, this); artifacts = new HashMap<>(); } public void disable(Target target) { - if (!hasErrors() && allowRegisteringActions) { + if (!hasErrors()) { verifyGeneratedArtifactHaveActions(target); } artifacts = null; @@ -163,18 +151,11 @@ public class CachingAnalysisEnvironment implements AnalysisEnvironment { @Override public ImmutableSet<Artifact> getOrphanArtifacts() { - if (!allowRegisteringActions) { - return ImmutableSet.of(); - } return ImmutableSet.copyOf(getOrphanArtifactMap().keySet()); } @Override public ImmutableSet<Artifact> getTreeArtifactsConflictingWithFiles() { - if (!allowRegisteringActions) { - return ImmutableSet.of(); - } - boolean hasTreeArtifacts = false; for (Artifact artifact : artifacts.keySet()) { if (artifact.isTreeArtifact()) { @@ -300,14 +281,11 @@ public class CachingAnalysisEnvironment implements AnalysisEnvironment { @Override public void registerAction(ActionAnalysisMetadata... actions) { Preconditions.checkState(enabled); - if (allowRegisteringActions) { - Collections.addAll(this.actions, actions); - } + Collections.addAll(this.actions, actions); } @Override public ActionAnalysisMetadata getLocalGeneratingAction(Artifact artifact) { - Preconditions.checkState(allowRegisteringActions); for (ActionAnalysisMetadata action : actions) { if (action.getOutputs().contains(artifact)) { return action; 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 8b6f7c6060..0831578ee4 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 @@ -36,7 +36,6 @@ import com.google.devtools.build.lib.analysis.config.ComposingRuleTransitionFact import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory; import com.google.devtools.build.lib.analysis.config.DefaultsPackage; import com.google.devtools.build.lib.analysis.config.FragmentOptions; -import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; import com.google.devtools.build.lib.analysis.constraints.ConstraintSemantics; import com.google.devtools.build.lib.analysis.skylark.SkylarkModules; import com.google.devtools.build.lib.cmdline.Label; @@ -234,7 +233,6 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { private final Map<Class<? extends RuleDefinition>, RuleClass> ruleMap = new HashMap<>(); private final Digraph<Class<? extends RuleDefinition>> dependencyGraph = new Digraph<>(); - private PatchTransition lipoDataTransition; private List<Class<? extends BuildConfiguration.Fragment>> universalFragments = new ArrayList<>(); @Nullable private RuleTransitionFactory trimmingTransitionFactory; @@ -391,21 +389,6 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { } /** - * Sets the C++ LIPO data transition, as defined in {@link - * com.google.devtools.build.lib.rules.cpp.transitions.DisableLipoTransition}. - * - * <p>This is language-specific, so doesn't really belong here. But since non-C++ rules declare - * this transition, we need universal access to it. The need for this interface should go away - * on the deprecation of LIPO for - * <a href="https://clang.llvm.org/docs/ThinLTO.html">ThinLTO</a>. - */ - public Builder setLipoDataTransition(PatchTransition transition) { - Preconditions.checkState(lipoDataTransition == null, "LIPO data transition already set"); - lipoDataTransition = Preconditions.checkNotNull(transition); - return this; - } - - /** * Adds a transition factory that produces a trimming transition to be run over all targets * after other transitions. * @@ -428,20 +411,14 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { /** * Overrides the transition factory run over all targets. * - * @see #setTrimmingTransitionFactory(RuleTransitionFactory) + * @see {@link #addTrimmingTransitionFactory(RuleTransitionFactory)} */ - @VisibleForTesting(/* for testing trimming transition factories without relying on prod use */) + @VisibleForTesting(/* for testing trimming transition factories without relying on prod use */ ) public Builder overrideTrimmingTransitionFactoryForTesting(RuleTransitionFactory factory) { trimmingTransitionFactory = null; return this.addTrimmingTransitionFactory(factory); } - @Override - public PatchTransition getLipoDataTransition() { - Preconditions.checkState(lipoDataTransition != null); - return lipoDataTransition; - } - private RuleConfiguredTargetFactory createFactory( Class<? extends RuleConfiguredTargetFactory> factoryClass) { try { @@ -515,7 +492,6 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { ImmutableList.copyOf(buildInfoFactories), ImmutableList.copyOf(configurationOptions), ImmutableList.copyOf(configurationFragmentFactories), - lipoDataTransition, ImmutableList.copyOf(universalFragments), trimmingTransitionFactory, prerequisiteValidator, @@ -615,8 +591,6 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { /** The set of configuration fragment factories. */ private final ImmutableList<ConfigurationFragmentFactory> configurationFragmentFactories; - private final PatchTransition lipoDataTransition; - /** The transition factory used to produce the transition that will trim targets. */ @Nullable private final RuleTransitionFactory trimmingTransitionFactory; @@ -652,7 +626,6 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { ImmutableList<BuildInfoFactory> buildInfoFactories, ImmutableList<Class<? extends FragmentOptions>> configurationOptions, ImmutableList<ConfigurationFragmentFactory> configurationFragments, - PatchTransition lipoDataTransition, ImmutableList<Class<? extends BuildConfiguration.Fragment>> universalFragments, @Nullable RuleTransitionFactory trimmingTransitionFactory, PrerequisiteValidator prerequisiteValidator, @@ -672,7 +645,6 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { this.buildInfoFactories = buildInfoFactories; this.configurationOptions = configurationOptions; this.configurationFragmentFactories = configurationFragments; - this.lipoDataTransition = lipoDataTransition; this.universalFragments = universalFragments; this.trimmingTransitionFactory = trimmingTransitionFactory; this.prerequisiteValidator = prerequisiteValidator; @@ -732,18 +704,6 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { } /** - * Returns the C++ LIPO data transition, as defined in {@link - * com.google.devtools.build.lib.rules.cpp.transitions.DisableLipoTransition}. - * - * <p>This is language-specific, so doesn't really belong here. But since non-C++ rules declare - * this transition, we need universal access to it. The need for this interface should go away on - * the deprecation of LIPO for <a href="https://clang.llvm.org/docs/ThinLTO.html">ThinLTO</a>. - */ - public PatchTransition getLipoDataTransition() { - return lipoDataTransition; - } - - /** * Returns the transition factory used to produce the transition to trim targets. * * <p>This is a temporary measure for supporting manual trimming of feature flags, and support @@ -844,7 +804,6 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { .build(); SkylarkUtils.setToolsRepository(env, toolsRepository); SkylarkUtils.setFragmentMap(env, configurationFragmentMap); - SkylarkUtils.setLipoDataTransition(env, getLipoDataTransition()); return env; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java index 40ed396d07..bd55ea99a9 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java @@ -332,7 +332,6 @@ public final class ConfiguredTargetFactory { ImmutableList.of(), configuration, hostConfiguration, - ruleClassProvider.getLipoDataTransition(), ruleClassProvider.getPrerequisiteValidator(), rule.getRuleClassObject().getConfigurationFragmentPolicy()) .setVisibility(convertVisibility(prerequisiteMap, env.getEventHandler(), rule, null)) @@ -449,7 +448,6 @@ public final class ConfiguredTargetFactory { aspectPath, aspectConfiguration, hostConfiguration, - ruleClassProvider.getLipoDataTransition(), ruleClassProvider.getPrerequisiteValidator(), aspect.getDefinition().getConfigurationFragmentPolicy()); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index b8173dbaf0..ecac68f078 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java @@ -181,7 +181,6 @@ public final class RuleContext extends TargetContext private final ImmutableSet<String> disabledFeatures; private final String ruleClassNameForLogging; private final BuildConfiguration hostConfiguration; - private final PatchTransition disableLipoTransition; private final ConfigurationFragmentPolicy configurationFragmentPolicy; private final ImmutableList<Class<? extends BuildConfiguration.Fragment>> universalFragments; private final ErrorReporter reporter; @@ -227,7 +226,6 @@ public final class RuleContext extends TargetContext this.disabledFeatures = ImmutableSortedSet.copyOf(allDisabledFeatures); this.ruleClassNameForLogging = ruleClassNameForLogging; this.hostConfiguration = builder.hostConfiguration; - this.disableLipoTransition = builder.disableLipoTransition; reporter = builder.reporter; this.toolchainContext = toolchainContext; this.constraintSemantics = constraintSemantics; @@ -1431,7 +1429,6 @@ public final class RuleContext extends TargetContext private ImmutableList<Class<? extends BuildConfiguration.Fragment>> universalFragments; private final BuildConfiguration configuration; private final BuildConfiguration hostConfiguration; - private PatchTransition disableLipoTransition; private final PrerequisiteValidator prerequisiteValidator; private final ErrorReporter reporter; private OrderedSetMultimap<Attribute, ConfiguredTargetAndData> prerequisiteMap; @@ -1449,7 +1446,6 @@ public final class RuleContext extends TargetContext ImmutableList<Aspect> aspects, BuildConfiguration configuration, BuildConfiguration hostConfiguration, - PatchTransition disableLipoTransition, PrerequisiteValidator prerequisiteValidator, ConfigurationFragmentPolicy configurationFragmentPolicy) { this.env = Preconditions.checkNotNull(env); @@ -1458,7 +1454,6 @@ public final class RuleContext extends TargetContext this.configurationFragmentPolicy = Preconditions.checkNotNull(configurationFragmentPolicy); this.configuration = Preconditions.checkNotNull(configuration); this.hostConfiguration = Preconditions.checkNotNull(hostConfiguration); - this.disableLipoTransition = disableLipoTransition; this.prerequisiteValidator = prerequisiteValidator; reporter = new ErrorReporter(env, rule, getRuleClassNameForLogging()); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleDefinitionEnvironment.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleDefinitionEnvironment.java index 315abc800f..bd76287fd1 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleDefinitionEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleDefinitionEnvironment.java @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.analysis; -import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; import com.google.devtools.build.lib.cmdline.Label; import javax.annotation.Nullable; @@ -49,14 +48,4 @@ public interface RuleDefinitionEnvironment { */ @Nullable Label getLauncherLabel(); - - /** - * Returns the C++ LIPO data transition, as defined in {@link - * com.google.devtools.build.lib.rules.cpp.transitions.DisableLipoTransition}. - * - * <p>This is language-specific, so doesn't really belong here. But since non-C++ rules declare - * this transition, we need universal access to it. The need for this interface should go away on - * the deprecation of LIPO for <a href="https://clang.llvm.org/docs/ThinLTO.html">ThinLTO</a>. - */ - PatchTransition getLipoDataTransition(); } 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 046ba3c651..9a0a54d270 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 @@ -1075,10 +1075,6 @@ public class BuildConfiguration implements BuildConfigurationApi { private final boolean separateGenfilesDirectory; - // Cache this value for quicker access. We don't cache it inside BuildOptions because BuildOptions - // is mutable, so a cached value there could fall out of date when it's updated. - private final boolean actionsEnabled; - /** * The global "make variables" such as "$(TARGET_CPU)"; these get applied to all rules analyzed in * this configuration. @@ -1121,7 +1117,6 @@ public class BuildConfiguration implements BuildConfigurationApi { // "bazel-out/arm-linux-fastbuild/bin" while the parent bindir is // "bazel-out/android-arm-linux-fastbuild/bin". That's pretty awkward to check here. // && outputRoots.equals(other.outputRoots) - && actionsEnabled == other.actionsEnabled && fragments.values().containsAll(other.fragments.values()) && buildOptions.getOptions().containsAll(other.buildOptions.getOptions())); } @@ -1139,17 +1134,15 @@ public class BuildConfiguration implements BuildConfigurationApi { return false; } BuildConfiguration otherConfig = (BuildConfiguration) other; - return actionsEnabled == otherConfig.actionsEnabled - && fragments.values().equals(otherConfig.fragments.values()) + return fragments.values().equals(otherConfig.fragments.values()) && buildOptions.equals(otherConfig.buildOptions); } private int computeHashCode() { - return Objects.hash(isActionsEnabled(), fragments, buildOptions.getOptions()); + return Objects.hash(fragments, buildOptions.getOptions()); } public void describe(StringBuilder sb) { - sb.append(isActionsEnabled()).append('\n'); for (Fragment fragment : fragments.values()) { sb.append(fragment.getClass().getName()).append('\n'); } @@ -1243,7 +1236,6 @@ public class BuildConfiguration implements BuildConfigurationApi { this.skylarkVisibleFragments = buildIndexOfSkylarkVisibleFragments(); this.buildOptions = buildOptions.clone(); this.buildOptionsDiff = buildOptionsDiff; - this.actionsEnabled = buildOptions.enableActions(); this.options = buildOptions.get(Options.class); this.separateGenfilesDirectory = options.separateGenfilesDirectory; this.mainRepositoryName = mainRepositoryName; @@ -1744,11 +1736,6 @@ public class BuildConfiguration implements BuildConfigurationApi { return options.experimentalJavaCoverage; } - /** If false, AnalysisEnvironment doesn't register any actions created by the ConfiguredTarget. */ - public boolean isActionsEnabled() { - return actionsEnabled; - } - public RunUnder getRunUnder() { return options.runUnder; } @@ -1773,7 +1760,7 @@ public class BuildConfiguration implements BuildConfigurationApi { } public List<Label> getActionListeners() { - return isActionsEnabled() ? options.actionListeners : ImmutableList.<Label>of(); + return options.actionListeners; } /** 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 33cd98afc3..cc31da8a31 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 @@ -176,20 +176,6 @@ public final class BuildOptions implements Cloneable, Serializable { } /** - * Returns true if actions should be enabled for this configuration. - */ - public boolean enableActions() { - // It's not necessarily safe to cache this value. This is because BuildOptions is not immutable. - // So caching the value correctly would require keeping it updated after relevant changes. - for (FragmentOptions fragment : fragmentOptionsMap.values()) { - if (!fragment.enableActions()) { - return false; - } - } - return true; - } - - /** * 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 9933b6fad3..4487a2101a 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 @@ -35,18 +35,6 @@ public abstract class FragmentOptions extends OptionsBase implements Cloneable, return ImmutableMap.of(); } - /** - * Returns true if actions should be enabled for this configuration. If <b>any</b> fragment - * sets this to false, <i>all</i> actions are disabled for the configuration. - * - * <p>Disabling actions is unusual behavior that should only be triggered under exceptionally - * special circumstances. In practice this only exists to support LIPO in C++. Don't override - * this method for any other purpose. - */ - public boolean enableActions() { - return true; - } - @Override public FragmentOptions clone() { try { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java index b1eef7dc85..2de684f82c 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java @@ -39,7 +39,6 @@ import com.google.devtools.build.lib.analysis.DefaultInfo; import com.google.devtools.build.lib.analysis.TemplateVariableInfo; import com.google.devtools.build.lib.analysis.config.ConfigAwareRuleClassBuilder; import com.google.devtools.build.lib.analysis.config.HostTransition; -import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; import com.google.devtools.build.lib.analysis.skylark.SkylarkAttr.Descriptor; import com.google.devtools.build.lib.analysis.test.TestConfiguration; import com.google.devtools.build.lib.cmdline.Label; @@ -138,8 +137,7 @@ public class SkylarkRuleClassFunctions implements SkylarkRuleFunctionsApi<Artifa .build(); /** Parent rule class for test Skylark rules. */ - public static final RuleClass getTestBaseRule(String toolsRepository, - PatchTransition lipoDataTransition) { + public static final RuleClass getTestBaseRule(String toolsRepository) { return new RuleClass.Builder("$test_base_rule", RuleClassType.ABSTRACT, true, baseRule) .requiresConfigurationFragments(TestConfiguration.class) .add( @@ -200,7 +198,7 @@ public class SkylarkRuleClassFunctions implements SkylarkRuleFunctionsApi<Artifa toolsRepository + BaseRuleClasses.DEFAULT_COVERAGE_REPORT_GENERATOR_VALUE))) .singleArtifact()) - .add(attr(":run_under", LABEL).cfg(lipoDataTransition).value(RUN_UNDER)) + .add(attr(":run_under", LABEL).value(RUN_UNDER)) .executionPlatformConstraintsAllowed(ExecutionPlatformConstraintsAllowed.PER_TARGET) .build(); } @@ -327,9 +325,7 @@ public class SkylarkRuleClassFunctions implements SkylarkRuleFunctionsApi<Artifa RuleClassType type = test ? RuleClassType.TEST : RuleClassType.NORMAL; RuleClass parent = test - ? getTestBaseRule( - SkylarkUtils.getToolsRepository(funcallEnv), - (PatchTransition) SkylarkUtils.getLipoDataTransition(funcallEnv)) + ? getTestBaseRule(SkylarkUtils.getToolsRepository(funcallEnv)) : (executable ? binaryBaseRule : baseRule); // We'll set the name later, pass the empty string for now. diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java index da9a0f32b8..8be4821791 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java @@ -80,7 +80,6 @@ import com.google.devtools.build.lib.rules.config.ConfigRules; import com.google.devtools.build.lib.rules.core.CoreRules; import com.google.devtools.build.lib.rules.cpp.proto.CcProtoAspect; import com.google.devtools.build.lib.rules.cpp.proto.CcProtoLibraryRule; -import com.google.devtools.build.lib.rules.cpp.transitions.LipoDataTransitionRuleSet; import com.google.devtools.build.lib.rules.java.JavaSemantics; import com.google.devtools.build.lib.rules.platform.PlatformRules; import com.google.devtools.build.lib.rules.proto.BazelProtoLibraryRule; @@ -381,10 +380,6 @@ public class BazelRuleClassProvider { private static final ImmutableSet<RuleSet> RULE_SETS = ImmutableSet.of( - // Rules defined before LipoDataTransitionRuleSet will fail when trying to declare a data - // transition. - // TODO(b/73071922): remove this when LIPO support is phased out - LipoDataTransitionRuleSet.INSTANCE, BAZEL_SETUP, CoreRules.INSTANCE, CoreWorkspaceRules.INSTANCE, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/common/BazelFilegroupRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/common/BazelFilegroupRule.java index f8c275b155..d13ec9fb7a 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/common/BazelFilegroupRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/common/BazelFilegroupRule.java @@ -67,7 +67,6 @@ public final class BazelFilegroupRule implements RuleDefinition { <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/ .add( attr("data", LABEL_LIST) - .cfg(env.getLipoDataTransition()) .allowedFileTypes(FileTypeSet.ANY_FILE) .dontCheckConstraints()) .add(attr("output_licenses", LICENSE)) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcBinaryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcBinaryRule.java index 23b090a43a..71e312e85f 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcBinaryRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcBinaryRule.java @@ -23,7 +23,6 @@ import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.bazel.rules.cpp.BazelCppRuleClasses.CcBinaryBaseRule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.rules.cpp.CppConfiguration; -import com.google.devtools.build.lib.rules.cpp.CppRuleClasses; /** Rule definition for cc_binary rules. */ public final class BazelCcBinaryRule implements RuleDefinition { @@ -72,7 +71,6 @@ public final class BazelCcBinaryRule implements RuleDefinition { attr("linkshared", BOOLEAN) .value(false) .nonconfigurable("used to *determine* the rule's configuration")) - .cfg(CppRuleClasses.LIPO_ON_DEMAND) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcTestRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcTestRule.java index d893a657cc..1b277cbbd0 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcTestRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcTestRule.java @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.bazel.rules.cpp; import static com.google.devtools.build.lib.packages.Attribute.attr; -import static com.google.devtools.build.lib.packages.BuildType.LABEL; import static com.google.devtools.build.lib.packages.BuildType.TRISTATE; import static com.google.devtools.build.lib.syntax.Type.BOOLEAN; @@ -43,10 +42,6 @@ public final class BazelCcTestRule implements RuleDefinition { // to decorate data symbols imported from DLL. .override(attr("linkstatic", BOOLEAN).value(OS.getCurrent() == OS.WINDOWS)) .override(attr("stamp", TRISTATE).value(TriState.NO)) - // Oh weary adventurer, endeavour not to remove this attribute, enticing as that may be, - // given that no code referenceth it. Should ye be on the verge of yielding to the - // temptation, lay your eyes on the Javadoc of {@link FdoSupport} to find out why. - .add(attr(":lipo_context", LABEL).value(BazelCppRuleClasses.LIPO_CONTEXT)) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java index 4cbfa065d8..2f025439bb 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java @@ -58,8 +58,6 @@ import com.google.devtools.build.lib.rules.cpp.CppConfiguration; import com.google.devtools.build.lib.rules.cpp.CppFileTypes; import com.google.devtools.build.lib.rules.cpp.CppRuleClasses; import com.google.devtools.build.lib.rules.cpp.CppRuleClasses.CcIncludeScanningRule; -import com.google.devtools.build.lib.rules.cpp.TransitiveLipoInfoProvider; -import com.google.devtools.build.lib.rules.cpp.transitions.LipoContextCollectorTransition; import com.google.devtools.build.lib.util.FileTypeSet; /** @@ -224,11 +222,6 @@ public class BazelCppRuleClasses { </p> <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/ .add(attr("includes", STRING_LIST)) - .add( - attr(TransitiveLipoInfoProvider.LIPO_CONTEXT_COLLECTOR, LABEL) - .cfg(LipoContextCollectorTransition.INSTANCE) - .value(CppRuleClasses.LIPO_CONTEXT_COLLECTOR) - .skipPrereqValidatorCheck()) .build(); } @@ -464,16 +457,6 @@ public class BazelCppRuleClasses { } } - /** Implementation for the :lipo_context attribute. */ - static final LabelLateBoundDefault<?> LIPO_CONTEXT = - LabelLateBoundDefault.fromTargetConfiguration( - CppConfiguration.class, - null, - (rule, attributes, cppConfig) -> { - Label result = cppConfig.getLipoContextLabel(); - return (rule == null || rule.getLabel().equals(result)) ? null : result; - }); - /** * Helper rule class. */ diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java index 5e429f0f38..6737e5dde1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java @@ -63,9 +63,7 @@ import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; -import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; /** * A ConfiguredTarget for <code>cc_binary</code> rules. @@ -216,8 +214,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { } List<String> linkopts = common.getLinkopts(); - LinkingMode linkingMode = - getLinkStaticness(ruleContext, linkopts, cppConfiguration, ccToolchain); + LinkingMode linkingMode = getLinkStaticness(ruleContext, linkopts, cppConfiguration); FdoSupportProvider fdoSupport = common.getFdoSupport(); FeatureConfiguration featureConfiguration = CcCommon.configureFeaturesOrReportRuleError( @@ -491,13 +488,6 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { RunfilesSupport runfilesSupport = RunfilesSupport.withExecutable( ruleContext, runfiles, executable); - TransitiveLipoInfoProvider transitiveLipoInfo; - if (cppConfiguration.isLipoContextCollector()) { - transitiveLipoInfo = common.collectTransitiveLipoLabels(ccCompilationOutputs); - } else { - transitiveLipoInfo = TransitiveLipoInfoProvider.EMPTY; - } - RuleConfiguredTargetBuilder ruleBuilder = new RuleConfiguredTargetBuilder(ruleContext); addTransitiveInfoProviders( ruleContext, @@ -510,22 +500,8 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { ccCompilationContext, linkingOutputs, dwoArtifacts, - transitiveLipoInfo, fake); - Map<Artifact, IncludeScannable> scannableMap = new LinkedHashMap<>(); - Map<PathFragment, Artifact> sourceFileMap = new LinkedHashMap<>(); - if (cppConfiguration.isLipoContextCollector()) { - for (IncludeScannable scannable : transitiveLipoInfo.getTransitiveIncludeScannables()) { - // These should all be CppCompileActions, which should have only one source file. - // This is also checked when they are put into the nested set. - Artifact source = - Iterables.getOnlyElement(scannable.getIncludeScannerSources()); - scannableMap.put(source, scannable); - sourceFileMap.put(source.getExecPath(), source); - } - } - // Support test execution on darwin. if (ApplePlatform.isApplePlatform(ccToolchain.getTargetCpu()) && TargetUtils.isTestRule(ruleContext.getRule())) { @@ -553,12 +529,6 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { new DebugPackageProvider( ruleContext.getLabel(), strippedFile, executable, explicitDwpFile)) .setRunfilesSupport(runfilesSupport, executable) - .addProvider( - LipoContextProvider.class, - new LipoContextProvider( - ccCompilationContext, - ImmutableMap.copyOf(scannableMap), - ImmutableMap.copyOf(sourceFileMap))) .addProvider(CppLinkAction.Context.class, linkContext) .addSkylarkTransitiveInfo(CcSkylarkApiProvider.NAME, new CcSkylarkApiProvider()) .build(); @@ -653,15 +623,12 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { } private static final LinkingMode getLinkStaticness( - RuleContext context, - List<String> linkopts, - CppConfiguration cppConfiguration, - CcToolchainProvider toolchain) { - if (CppHelper.getDynamicMode(cppConfiguration, toolchain) == DynamicMode.FULLY) { + RuleContext context, List<String> linkopts, CppConfiguration cppConfiguration) { + if (cppConfiguration.getDynamicModeFlag() == DynamicMode.FULLY) { return LinkingMode.DYNAMIC; } else if (dashStaticInLinkopts(linkopts, cppConfiguration)) { return Link.LinkingMode.LEGACY_FULLY_STATIC; - } else if (CppHelper.getDynamicMode(cppConfiguration, toolchain) == DynamicMode.OFF + } else if (cppConfiguration.getDynamicModeFlag() == DynamicMode.OFF || context.attributes().get("linkstatic", Type.BOOLEAN)) { return LinkingMode.STATIC; } else { @@ -889,7 +856,6 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { CcCompilationContext ccCompilationContext, CcLinkingOutputs linkingOutputs, DwoArtifactsCollector dwoArtifacts, - TransitiveLipoInfoProvider transitiveLipoInfo, boolean fake) { List<Artifact> instrumentedObjectFiles = new ArrayList<>(); instrumentedObjectFiles.addAll(ccCompilationOutputs.getObjectFiles(false)); @@ -901,7 +867,6 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { CcCompilationHelper.collectHeaderTokens(ruleContext, ccCompilationOutputs); NestedSet<Artifact> filesToCompile = ccCompilationOutputs.getFilesToCompile( - cppConfiguration.isLipoContextCollector(), cppConfiguration.processHeadersInDependencies(), CppHelper.usePicForDynamicLibraries(ruleContext, toolchain)); @@ -917,7 +882,6 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { builder .setFilesToBuild(filesToBuild) .addNativeDeclaredProvider(ccCompilationInfoBuilder.build()) - .addProvider(TransitiveLipoInfoProvider.class, transitiveLipoInfo) .addNativeDeclaredProvider(ccLinkingInfoBuilder.build()) .addProvider( CcNativeLibraryProvider.class, @@ -929,8 +893,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { CppDebugFileProvider.class, new CppDebugFileProvider( dwoArtifacts.getDwoArtifacts(), dwoArtifacts.getPicDwoArtifacts())) - .addOutputGroup( - OutputGroupInfo.TEMP_FILES, getTemps(cppConfiguration, ccCompilationOutputs)) + .addOutputGroup(OutputGroupInfo.TEMP_FILES, ccCompilationOutputs.getTemps()) .addOutputGroup(OutputGroupInfo.FILES_TO_COMPILE, filesToCompile) // For CcBinary targets, we only want to ensure that we process headers in dependencies and // thus only add header tokens to HIDDEN_TOP_LEVEL. If we add all HIDDEN_TOP_LEVEL artifacts @@ -976,13 +939,6 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { return builder.build(); } - private static NestedSet<Artifact> getTemps(CppConfiguration cppConfiguration, - CcCompilationOutputs compilationOutputs) { - return cppConfiguration.isLipoContextCollector() - ? NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER) - : compilationOutputs.getTemps(); - } - private static boolean usePic(RuleContext ruleContext, CcToolchainProvider ccToolchainProvider) { if (isLinkShared(ruleContext)) { return CppHelper.usePicForDynamicLibraries(ruleContext, ccToolchainProvider); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java index 04dab1586b..097b809f7e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java @@ -37,7 +37,6 @@ import com.google.devtools.build.lib.analysis.skylark.SkylarkRuleContext; import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector; import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector.LocalMetadataCollector; import com.google.devtools.build.lib.analysis.test.InstrumentedFilesProvider; -import com.google.devtools.build.lib.analysis.test.InstrumentedFilesProviderImpl; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.collect.nestedset.NestedSet; @@ -246,7 +245,7 @@ public final class CcCommon { if (ourLinkopts != null) { boolean allowDashStatic = !cppConfiguration.forceIgnoreDashStatic() - && (CppHelper.getDynamicMode(cppConfiguration, ccToolchain) != DynamicMode.FULLY); + && (cppConfiguration.getDynamicModeFlag() != DynamicMode.FULLY); if (!allowDashStatic) { ourLinkopts = Iterables.filter(ourLinkopts, (v) -> !"-static".equals(v)); } @@ -301,25 +300,12 @@ public final class CcCommon { deps.add(CppHelper.mallocForTarget(ruleContext)); } - return compilationOutputs == null // Possible in LIPO collection mode (see initializationHook). - ? DwoArtifactsCollector.emptyCollector() - : DwoArtifactsCollector.transitiveCollector( - compilationOutputs, - deps.build(), - generateDwo, - ltoBackendArtifactsUsePic, - ltoBackendArtifacts); - } - - public TransitiveLipoInfoProvider collectTransitiveLipoLabels(CcCompilationOutputs outputs) { - if (fdoSupport.getFdoSupport().getFdoRoot() == null - || !cppConfiguration.isLipoContextCollector()) { - return TransitiveLipoInfoProvider.EMPTY; - } - - NestedSetBuilder<IncludeScannable> scannableBuilder = NestedSetBuilder.stableOrder(); - CppHelper.addTransitiveLipoInfoForCommonAttributes(ruleContext, outputs, scannableBuilder); - return new TransitiveLipoInfoProvider(scannableBuilder.build()); + return DwoArtifactsCollector.transitiveCollector( + compilationOutputs, + deps.build(), + generateDwo, + ltoBackendArtifactsUsePic, + ltoBackendArtifacts); } /** @@ -729,13 +715,14 @@ public final class CcCommon { */ public InstrumentedFilesProvider getInstrumentedFilesProvider(Iterable<Artifact> files, boolean withBaselineCoverage) { - return cppConfiguration.isLipoContextCollector() - ? InstrumentedFilesProviderImpl.EMPTY - : InstrumentedFilesCollector.collect( - ruleContext, CppRuleClasses.INSTRUMENTATION_SPEC, CC_METADATA_COLLECTOR, files, - CppHelper.getGcovFilesIfNeeded(ruleContext, ccToolchain), - CppHelper.getCoverageEnvironmentIfNeeded(ruleContext, ccToolchain), - withBaselineCoverage); + return InstrumentedFilesCollector.collect( + ruleContext, + CppRuleClasses.INSTRUMENTATION_SPEC, + CC_METADATA_COLLECTOR, + files, + CppHelper.getGcovFilesIfNeeded(ruleContext, ccToolchain), + CppHelper.getCoverageEnvironmentIfNeeded(ruleContext, ccToolchain), + withBaselineCoverage); } public static ImmutableList<String> getCoverageFeatures(CcToolchainProvider toolchain) { @@ -920,16 +907,6 @@ public final class CcCommon { if (cppConfiguration.getFdoPrefetchHintsLabel() != null) { allRequestedFeaturesBuilder.add(CppRuleClasses.FDO_PREFETCH_HINTS); } - if (cppConfiguration.isLipoOptimizationOrInstrumentation()) { - // Map LIPO to ThinLTO for LLVM builds. - if (toolchain.isLLVMCompiler() && fdoMode != FdoMode.OFF) { - if (!allUnsupportedFeatures.contains(CppRuleClasses.THIN_LTO)) { - allFeatures.add(CppRuleClasses.THIN_LTO); - } - } else { - allFeatures.add(CppRuleClasses.LIPO); - } - } for (String feature : allFeatures.build()) { if (!allUnsupportedFeatures.contains(feature)) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java index 0dd346c424..7153cf62a8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java @@ -292,73 +292,6 @@ public final class CcCompilationContext implements CcCompilationContextApi { ccCompilationContext.propagateModuleMapAsActionInput); } - /** - * Returns the context for a LIPO compile action. This uses the include dirs and defines of the - * library, but the declared inclusion dirs/srcs from both the library and the owner binary. - * - * <p>TODO(bazel-team): this might make every LIPO target have an unnecessary large set of - * inclusion dirs/srcs. The correct behavior would be to merge only the contexts of actual - * referred targets (as listed in .imports file). - * - * <p>Undeclared inclusion checking ({@link #getDeclaredIncludeDirs()}, {@link - * #getDeclaredIncludeWarnDirs()}, and {@link #getDeclaredIncludeSrcs()}) needs to use the union - * of the contexts of the involved source files. - * - * <p>For include and define command line flags ({@link #getIncludeDirs()} {@link - * #getQuoteIncludeDirs()}, {@link #getSystemIncludeDirs()}, and {@link #getDefines()}) LIPO - * compilations use the same values as non-LIPO compilation. - * - * <p>Include scanning is not handled by this method. See {@code - * IncludeScannable#getAuxiliaryScannables()} instead. - * - * @param ownerCcCompilationContext the {@code CcCompilationContext} of the owner binary - * @param libCcCompilationContext the {@code CcCompilationContext} of the library - */ - public static CcCompilationContext mergeForLipo( - CcCompilationContext ownerCcCompilationContext, - CcCompilationContext libCcCompilationContext) { - ImmutableSet.Builder<Artifact> prerequisites = ImmutableSet.builder(); - prerequisites.addAll(ownerCcCompilationContext.compilationPrerequisites); - prerequisites.addAll(libCcCompilationContext.compilationPrerequisites); - ModuleInfo.Builder moduleInfo = new ModuleInfo.Builder(); - moduleInfo.merge(ownerCcCompilationContext.moduleInfo); - moduleInfo.merge(libCcCompilationContext.moduleInfo); - ModuleInfo.Builder picModuleInfo = new ModuleInfo.Builder(); - picModuleInfo.merge(ownerCcCompilationContext.picModuleInfo); - picModuleInfo.merge(libCcCompilationContext.picModuleInfo); - return new CcCompilationContext( - libCcCompilationContext.commandLineCcCompilationContext, - prerequisites.build(), - mergeSets( - ownerCcCompilationContext.declaredIncludeDirs, - libCcCompilationContext.declaredIncludeDirs), - mergeSets( - ownerCcCompilationContext.declaredIncludeWarnDirs, - libCcCompilationContext.declaredIncludeWarnDirs), - mergeSets( - ownerCcCompilationContext.declaredIncludeSrcs, - libCcCompilationContext.declaredIncludeSrcs), - mergeSets(ownerCcCompilationContext.pregreppedHdrs, libCcCompilationContext.pregreppedHdrs), - mergeSets(ownerCcCompilationContext.nonCodeInputs, libCcCompilationContext.nonCodeInputs), - moduleInfo.build(), - picModuleInfo.build(), - mergeSets( - ownerCcCompilationContext.directModuleMaps, libCcCompilationContext.directModuleMaps), - libCcCompilationContext.cppModuleMap, - libCcCompilationContext.verificationModuleMap, - libCcCompilationContext.propagateModuleMapAsActionInput); - } - - /** - * Return a nested set containing all elements from {@code s1} and {@code s2}. - */ - private static <T> NestedSet<T> mergeSets(NestedSet<T> s1, NestedSet<T> s2) { - NestedSetBuilder<T> builder = NestedSetBuilder.stableOrder(); - builder.addTransitive(s1); - builder.addTransitive(s2); - return builder.build(); - } - /** @return the C++ module map of the owner. */ public CppModuleMap getCppModuleMap() { return cppModuleMap; @@ -695,12 +628,7 @@ public final class CcCompilationContext implements CcCompilationContextApi { Preconditions.checkState( Objects.equals(moduleInfo.textualHeaders, picModuleInfo.textualHeaders), "Module and PIC module's textual headers are expected to be identical"); - // We don't create middlemen in LIPO collector subtree, because some target CT - // will do that instead. - Artifact prerequisiteStampFile = (ruleContext != null - && ruleContext.getFragment(CppConfiguration.class).isLipoContextCollector()) - ? getMiddlemanArtifact(middlemanFactory) - : createMiddleman(owner, middlemanFactory); + Artifact prerequisiteStampFile = createMiddleman(owner, middlemanFactory); return new CcCompilationContext( new CommandLineCcCompilationContext( @@ -709,7 +637,7 @@ public final class CcCompilationContext implements CcCompilationContextApi { ImmutableList.copyOf(systemIncludeDirs), ImmutableList.copyOf(defines)), prerequisiteStampFile == null - ? ImmutableSet.<Artifact>of() + ? ImmutableSet.of() : ImmutableSet.of(prerequisiteStampFile), declaredIncludeDirs.build(), declaredIncludeWarnDirs.build(), @@ -760,22 +688,6 @@ public final class CcCompilationContext implements CcCompilationContextApi { ruleContext.getConfiguration().getMiddlemanDirectory( ruleContext.getRule().getRepository())); } - - /** - * Returns the same set of artifacts as createMiddleman() would, but without - * actually creating middlemen. - */ - private Artifact getMiddlemanArtifact(MiddlemanFactory middlemanFactory) { - if (compilationPrerequisites.isEmpty()) { - return null; - } - - return middlemanFactory.getErrorPropagatingMiddlemanArtifact( - ruleContext.getLabel().toString(), - purpose, - ruleContext.getConfiguration().getMiddlemanDirectory( - ruleContext.getRule().getRepository())); - } } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java index 931db5361a..e5f0191a3a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java @@ -42,7 +42,6 @@ import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; -import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; @@ -76,8 +75,8 @@ import javax.annotation.Nullable; * lower-level APIs in CppHelper and CppCompileActionBuilder. * * <p>Rules that want to use this class are required to have implicit dependencies on the toolchain, - * the STL, the lipo context, and so on. Optionally, they can also have copts, and malloc - * attributes, but note that these require explicit calls to the corresponding setter methods. + * the STL, and so on. Optionally, they can also have copts, and malloc attributes, but note that + * these require explicit calls to the corresponding setter methods. */ public final class CcCompilationHelper { /** Similar to {@code OutputGroupInfo.HIDDEN_TOP_LEVEL}, but specific to header token files. */ @@ -112,7 +111,7 @@ public final class CcCompilationHelper { private final FileTypeSet sourceTypeSet; - private SourceCategory(FileTypeSet sourceTypeSet) { + SourceCategory(FileTypeSet sourceTypeSet) { this.sourceTypeSet = sourceTypeSet; } @@ -762,21 +761,19 @@ public final class CcCompilationHelper { new TransitiveInfoProviderMapBuilder() .add( new CppDebugFileProvider( - dwoArtifacts.getDwoArtifacts(), dwoArtifacts.getPicDwoArtifacts()), - collectTransitiveLipoInfo(ccOutputs)); + dwoArtifacts.getDwoArtifacts(), dwoArtifacts.getPicDwoArtifacts())); CcCompilationInfo.Builder ccCompilationInfoBuilder = CcCompilationInfo.Builder.create(); ccCompilationInfoBuilder.setCcCompilationContext(ccCompilationContext); providers.put(ccCompilationInfoBuilder.build()); Map<String, NestedSet<Artifact>> outputGroups = new TreeMap<>(); - outputGroups.put(OutputGroupInfo.TEMP_FILES, getTemps(ccOutputs)); + outputGroups.put(OutputGroupInfo.TEMP_FILES, ccOutputs.getTemps()); if (emitCompileProviders) { - boolean isLipoCollector = cppConfiguration.isLipoContextCollector(); boolean processHeadersInDependencies = cppConfiguration.processHeadersInDependencies(); boolean usePic = CppHelper.usePicForDynamicLibraries(ruleContext, ccToolchain); outputGroups.put( OutputGroupInfo.FILES_TO_COMPILE, - ccOutputs.getFilesToCompile(isLipoCollector, processHeadersInDependencies, usePic)); + ccOutputs.getFilesToCompile(processHeadersInDependencies, usePic)); outputGroups.put( OutputGroupInfo.COMPILATION_PREREQUISITES, CcCommon.collectCompilationPrerequisites(ruleContext, ccCompilationContext)); @@ -868,7 +865,7 @@ public final class CcCompilationHelper { } if (ruleContext.hasErrors()) { - return new PublicHeaders(ImmutableList.<Artifact>of(), ImmutableList.<Artifact>of(), null); + return new PublicHeaders(ImmutableList.of(), ImmutableList.of(), null); } ImmutableList.Builder<Artifact> moduleHeadersBuilder = ImmutableList.builder(); @@ -1115,7 +1112,7 @@ public final class CcCompilationHelper { ruleContext.getActionOwner(), moduleMap, featureConfiguration.isEnabled(CppRuleClasses.EXCLUDE_PRIVATE_HEADERS_IN_MODULE_MAPS) - ? ImmutableList.<Artifact>of() + ? ImmutableList.of() : privateHeaders, featureConfiguration.isEnabled(CppRuleClasses.ONLY_DOTH_HEADERS_IN_MODULE_MAPS) ? Iterables.filter(publicHeaders.getModuleMapHeaders(), CppFileTypes.MODULE_MAP_HEADER) @@ -1147,39 +1144,7 @@ public final class CcCompilationHelper { result.add(additionalCppModuleMap); } - return Iterables.filter(result, Predicates.<CppModuleMap>notNull()); - } - - private TransitiveLipoInfoProvider collectTransitiveLipoInfo(CcCompilationOutputs outputs) { - if (fdoSupport.getFdoSupport().getFdoRoot() == null) { - return TransitiveLipoInfoProvider.EMPTY; - } - NestedSetBuilder<IncludeScannable> scannableBuilder = NestedSetBuilder.stableOrder(); - // TODO(bazel-team): Only fetch the STL prerequisite in one place. - TransitiveInfoCollection stl = ruleContext.getPrerequisite(":stl", Mode.TARGET); - if (stl != null) { - TransitiveLipoInfoProvider provider = stl.getProvider(TransitiveLipoInfoProvider.class); - if (provider != null) { - scannableBuilder.addTransitive(provider.getTransitiveIncludeScannables()); - } - } - - for (TransitiveLipoInfoProvider dep : - AnalysisUtils.getProviders(deps, TransitiveLipoInfoProvider.class)) { - scannableBuilder.addTransitive(dep.getTransitiveIncludeScannables()); - } - - for (IncludeScannable scannable : outputs.getLipoScannables()) { - Preconditions.checkState(scannable.getIncludeScannerSources().size() == 1); - scannableBuilder.add(scannable); - } - return new TransitiveLipoInfoProvider(scannableBuilder.build()); - } - - private NestedSet<Artifact> getTemps(CcCompilationOutputs compilationOutputs) { - return cppConfiguration.isLipoContextCollector() - ? NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER) - : compilationOutputs.getTemps(); + return Iterables.filter(result, Predicates.notNull()); } /** @return whether this target needs to generate a pic header module. */ @@ -1194,8 +1159,7 @@ public final class CcCompilationHelper { /** @return whether we want to provide header modules for the current target. */ private boolean shouldProvideHeaderModules() { - return featureConfiguration.isEnabled(CppRuleClasses.HEADER_MODULES) - && !cppConfiguration.isLipoContextCollector(); + return featureConfiguration.isEnabled(CppRuleClasses.HEADER_MODULES); } /** @return the no-PIC header module artifact for the current target. */ @@ -1299,8 +1263,8 @@ public final class CcCompilationHelper { /** * Constructs the C++ compiler actions. It generally creates one action for every specified source - * file. It takes into account LIPO, fake-ness, coverage, and PIC, in addition to using the - * settings specified on the current object. This method should only be called once. + * file. It takes into account fake-ness, coverage, and PIC, in addition to using the settings + * specified on the current object. This method should only be called once. */ private CcCompilationOutputs createCcCompileActions() throws RuleErrorException { CcCompilationOutputs.Builder result = new CcCompilationOutputs.Builder(); @@ -1452,7 +1416,6 @@ public final class CcCompilationHelper { /* outputName= */ null, usePic, /* ccRelativeName= */ null, - /* autoFdoImportPath= */ null, ccCompilationContext.getCppModuleMap(), /* gcnoFile= */ null, /* dwoFile= */ null, @@ -1503,7 +1466,6 @@ public final class CcCompilationHelper { String outputName, boolean usePic, PathFragment ccRelativeName, - PathFragment autoFdoImportPath, CppModuleMap cppModuleMap, Artifact gcnoFile, Artifact dwoFile, @@ -1530,8 +1492,6 @@ public final class CcCompilationHelper { .configureCompilation( builder, ruleContext, - ccRelativeName, - autoFdoImportPath, PathFragment.create(outputName), usePic, featureConfiguration, @@ -1618,7 +1578,7 @@ public final class CcCompilationHelper { ruleContext, ccToolchain, ArtifactCategory.COVERAGE_DATA_FILE, outputName); // TODO(djasper): This is now duplicated. Refactor the various create..Action functions. Artifact gcnoFile = - isCodeCoverageEnabled() && !CppHelper.isLipoOptimization(cppConfiguration, ccToolchain) + isCodeCoverageEnabled() ? CppHelper.getCompileOutputArtifact(ruleContext, gcnoFileName, configuration) : null; @@ -1637,7 +1597,6 @@ public final class CcCompilationHelper { outputName, /* usePic= */ pic, ccRelativeName, - module.getExecPath(), ccCompilationContext.getCppModuleMap(), gcnoFile, dwoFile, @@ -1688,7 +1647,6 @@ public final class CcCompilationHelper { /* outputName= */ null, generatePicAction, /* ccRelativeName= */ null, - /* autoFdoImportPath= */ null, ccCompilationContext.getCppModuleMap(), /* gcnoFile= */ null, /* dwoFile= */ null, @@ -1749,16 +1707,6 @@ public final class CcCompilationHelper { throws RuleErrorException { ImmutableList.Builder<Artifact> directOutputs = new ImmutableList.Builder<>(); PathFragment ccRelativeName = sourceArtifact.getRootRelativePath(); - if (CppHelper.isLipoOptimization(cppConfiguration, ccToolchain)) { - // TODO(bazel-team): we shouldn't be needing this, merging ccCompilationContext with the - // binary - // is a superset of necessary information. - LipoContextProvider lipoProvider = - Preconditions.checkNotNull(CppHelper.getLipoContextProvider(ruleContext), outputName); - builder.setCcCompilationContext( - CcCompilationContext.mergeForLipo( - lipoProvider.getLipoCcCompilationContext(), ccCompilationContext)); - } if (fake) { boolean usePic = !generateNoPicAction; createFakeSourceAction( @@ -1770,7 +1718,6 @@ public final class CcCompilationHelper { outputCategory, addObject, ccRelativeName, - sourceArtifact.getExecPath(), usePic, generateDotd); } else { @@ -1804,7 +1751,6 @@ public final class CcCompilationHelper { outputName, /* usePic= */ true, ccRelativeName, - sourceArtifact.getExecPath(), ccCompilationContext.getCppModuleMap(), gcnoFile, dwoFile, @@ -1840,9 +1786,6 @@ public final class CcCompilationHelper { // Host targets don't produce .dwo files. result.addPicDwoFile(dwoFile); } - if (cppConfiguration.isLipoContextCollector() && !generateNoPicAction) { - result.addLipoScannable(picAction); - } } if (generateNoPicAction) { @@ -1859,7 +1802,7 @@ public final class CcCompilationHelper { // Create no-PIC compile actions Artifact gcnoFile = - !CppHelper.isLipoOptimization(cppConfiguration, ccToolchain) && enableCoverage + enableCoverage ? CppHelper.getCompileOutputArtifact(ruleContext, gcnoFileName, configuration) : null; @@ -1874,7 +1817,6 @@ public final class CcCompilationHelper { outputName, /* usePic= */ false, ccRelativeName, - sourceArtifact.getExecPath(), cppModuleMap, gcnoFile, noPicDwoFile, @@ -1910,9 +1852,6 @@ public final class CcCompilationHelper { // Host targets don't produce .dwo files. result.addDwoFile(noPicDwoFile); } - if (cppConfiguration.isLipoContextCollector()) { - result.addLipoScannable(compileAction); - } } } return directOutputs.build(); @@ -1950,7 +1889,6 @@ public final class CcCompilationHelper { ArtifactCategory outputCategory, boolean addObject, PathFragment ccRelativeName, - PathFragment execPath, boolean usePic, boolean generateDotd) throws RuleErrorException { @@ -1979,7 +1917,6 @@ public final class CcCompilationHelper { outputName, usePic, ccRelativeName, - execPath, ccCompilationContext.getCppModuleMap(), /* gcnoFile= */ null, /* dwoFile= */ null, @@ -2089,7 +2026,6 @@ public final class CcCompilationHelper { outputName, usePic, ccRelativeName, - source.getExecPath(), ccCompilationContext.getCppModuleMap(), /* gcnoFile= */ null, /* dwoFile= */ null, @@ -2111,7 +2047,6 @@ public final class CcCompilationHelper { outputName, usePic, ccRelativeName, - source.getExecPath(), ccCompilationContext.getCppModuleMap(), /* gcnoFile= */ null, /* dwoFile= */ null, diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java index f1ec721ded..14d68fee2b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java @@ -21,12 +21,9 @@ import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; -import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.skylarkbuildapi.cpp.CcCompilationOutputsApi; import com.google.devtools.build.lib.syntax.SkylarkList; -import java.util.ArrayList; import java.util.LinkedHashSet; -import java.util.List; import java.util.Set; /** A structured representation of the compilation outputs of a C++ rule. */ @@ -67,8 +64,6 @@ public class CcCompilationOutputs implements CcCompilationOutputsApi { */ private final ImmutableList<Artifact> headerTokenFiles; - private final List<IncludeScannable> lipoScannables; - private CcCompilationOutputs( ImmutableList<Artifact> objectFiles, ImmutableList<Artifact> picObjectFiles, @@ -76,8 +71,7 @@ public class CcCompilationOutputs implements CcCompilationOutputsApi { ImmutableList<Artifact> dwoFiles, ImmutableList<Artifact> picDwoFiles, NestedSet<Artifact> temps, - ImmutableList<Artifact> headerTokenFiles, - ImmutableList<IncludeScannable> lipoScannables) { + ImmutableList<Artifact> headerTokenFiles) { this.objectFiles = objectFiles; this.picObjectFiles = picObjectFiles; this.ltoBitcodeFiles = ltoBitcodeFiles; @@ -85,7 +79,6 @@ public class CcCompilationOutputs implements CcCompilationOutputsApi { this.picDwoFiles = picDwoFiles; this.temps = temps; this.headerTokenFiles = headerTokenFiles; - this.lipoScannables = lipoScannables; } /** @@ -142,22 +135,8 @@ public class CcCompilationOutputs implements CcCompilationOutputsApi { return headerTokenFiles; } - /** - * Returns the {@link IncludeScannable} objects this C++ compile action contributes to a - * LIPO context collector. - */ - public List<IncludeScannable> getLipoScannables() { - return lipoScannables; - } - - /** - * Returns the output files that are considered "compiled" by this C++ compile action. - */ - NestedSet<Artifact> getFilesToCompile( - boolean isLipoContextCollector, boolean parseHeaders, boolean usePic) { - if (isLipoContextCollector) { - return NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER); - } + /** Returns the output files that are considered "compiled" by this C++ compile action. */ + NestedSet<Artifact> getFilesToCompile(boolean parseHeaders, boolean usePic) { NestedSetBuilder<Artifact> files = NestedSetBuilder.stableOrder(); files.addAll(getObjectFiles(usePic)); if (parseHeaders) { @@ -175,7 +154,6 @@ public class CcCompilationOutputs implements CcCompilationOutputsApi { private final Set<Artifact> picDwoFiles = new LinkedHashSet<>(); private final NestedSetBuilder<Artifact> temps = NestedSetBuilder.stableOrder(); private final Set<Artifact> headerTokenFiles = new LinkedHashSet<>(); - private final List<IncludeScannable> lipoScannables = new ArrayList<>(); public CcCompilationOutputs build() { return new CcCompilationOutputs( @@ -185,8 +163,7 @@ public class CcCompilationOutputs implements CcCompilationOutputsApi { ImmutableList.copyOf(dwoFiles), ImmutableList.copyOf(picDwoFiles), temps.build(), - ImmutableList.copyOf(headerTokenFiles), - ImmutableList.copyOf(lipoScannables)); + ImmutableList.copyOf(headerTokenFiles)); } public Builder merge(CcCompilationOutputs outputs) { @@ -196,7 +173,6 @@ public class CcCompilationOutputs implements CcCompilationOutputsApi { this.picDwoFiles.addAll(outputs.picDwoFiles); this.temps.addTransitive(outputs.temps); this.headerTokenFiles.addAll(outputs.headerTokenFiles); - this.lipoScannables.addAll(outputs.lipoScannables); this.ltoBitcodeFiles.putAll(outputs.ltoBitcodeFiles); return this; } @@ -261,14 +237,5 @@ public class CcCompilationOutputs implements CcCompilationOutputsApi { headerTokenFiles.add(artifact); return this; } - - /** - * Adds an {@link IncludeScannable} that this compilation output object contributes to a - * LIPO context collector. - */ - public Builder addLipoScannable(IncludeScannable scannable) { - lipoScannables.add(scannable); - return this; - } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImportRule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImportRule.java index 5f31e98462..7d40924fa8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImportRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImportRule.java @@ -96,7 +96,6 @@ public final class CcImportRule implements RuleDefinition { .add(attr("alwayslink", BOOLEAN)) .add( attr("data", LABEL_LIST) - .cfg(env.getLipoDataTransition()) .allowedFileTypes(FileTypeSet.ANY_FILE) .dontCheckConstraints()) .addRequiredToolchains(CppRuleClasses.ccToolchainTypeAttribute(env)) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java index 0a46d7f9df..7c8791b697 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java @@ -405,12 +405,10 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { // Ensure that we build all the dependencies, otherwise users may get confused. NestedSetBuilder<Artifact> artifactsToForceBuilder = NestedSetBuilder.stableOrder(); CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class); - boolean isLipoCollector = cppConfiguration.isLipoContextCollector(); boolean processHeadersInDependencies = cppConfiguration.processHeadersInDependencies(); boolean usePic = CppHelper.usePicForDynamicLibraries(ruleContext, toolchain); artifactsToForceBuilder.addTransitive( - ccCompilationOutputs.getFilesToCompile( - isLipoCollector, processHeadersInDependencies, usePic)); + ccCompilationOutputs.getFilesToCompile(processHeadersInDependencies, usePic)); for (OutputGroupInfo dep : ruleContext.getPrerequisites( "deps", Mode.TARGET, OutputGroupInfo.SKYLARK_CONSTRUCTOR)) { @@ -423,12 +421,6 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { private static void warnAboutEmptyLibraries(RuleContext ruleContext, CcCompilationOutputs ccCompilationOutputs, boolean linkstaticAttribute) { - if (ruleContext.getFragment(CppConfiguration.class).isLipoContextCollector()) { - // Do not signal warnings in the lipo context collector configuration. These will be duly - // signaled in the target configuration, and there can be spurious warnings since targets in - // the LIPO context collector configuration do not compile anything. - return; - } if (ccCompilationOutputs.getObjectFiles(false).isEmpty() && ccCompilationOutputs.getObjectFiles(true).isEmpty()) { if (!linkstaticAttribute && appearsToHaveObjectFiles(ruleContext.attributes())) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java index aba22e42f3..8ffc398ae6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java @@ -65,8 +65,8 @@ import javax.annotation.Nullable; * lower-level APIs in CppHelper and CppLinkActionBuilder. * * <p>Rules that want to use this class are required to have implicit dependencies on the toolchain, - * the STL, the lipo context, and so on. Optionally, they can also have copts, and malloc - * attributes, but note that these require explicit calls to the corresponding setter methods. + * the STL, and so on. Optionally, they can also have copts, and malloc attributes, but note that + * these require explicit calls to the corresponding setter methods. */ public final class CcLinkingHelper { @@ -710,11 +710,6 @@ public final class CcLinkingHelper { linkType.linkerOrArchiver() == LinkerOrArchiver.ARCHIVER, "can only handle static links"); CcLinkingOutputs.Builder result = new CcLinkingOutputs.Builder(); - if (cppConfiguration.isLipoContextCollector()) { - // Don't try to create LIPO link actions in collector mode, - // because it needs some data that's not available at this point. - return result.build(); - } AnalysisEnvironment env = ruleContext.getAnalysisEnvironment(); boolean usePicForBinaries = CppHelper.usePicForBinaries(ruleContext, ccToolchain); boolean usePicForDynamicLibs = CppHelper.usePicForDynamicLibraries(ruleContext, ccToolchain); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java index 811ec110d1..ffad5c5084 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java @@ -325,15 +325,6 @@ public class CcToolchain implements RuleConfiguredTargetFactory { @Override public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedException, RuleErrorException, ActionConflictException { - TransitiveInfoCollection lipoContextCollector = - ruleContext.getPrerequisite( - TransitiveLipoInfoProvider.LIPO_CONTEXT_COLLECTOR, Mode.DONT_CHECK); - if (lipoContextCollector != null - && lipoContextCollector.getProvider(LipoContextProvider.class) == null) { - ruleContext.ruleError("--lipo_context must point to a cc_binary or a cc_test rule"); - return null; - } - BuildConfiguration configuration = Preconditions.checkNotNull(ruleContext.getConfiguration()); CppConfiguration cppConfiguration = Preconditions.checkNotNull(configuration.getFragment(CppConfiguration.class)); @@ -407,7 +398,6 @@ public class CcToolchain implements RuleConfiguredTargetFactory { SkyKey fdoKey = FdoSupportValue.key( - cppConfiguration.getLipoMode(), fdoZip, prefetchHints, cppConfiguration.getFdoInstrument(), diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java index 02dcdbeca8..ad65e53ef0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java @@ -38,7 +38,6 @@ import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain; -import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode; import java.util.Map; import javax.annotation.Nullable; @@ -772,49 +771,32 @@ public final class CcToolchainProvider extends ToolchainInfo implements CcToolch return toolchainInfo.getCxxFlagsByCompilationMode(); } - /** Returns compiler flags arising from the {@link CToolchain} for C compilation by lipo mode. */ - ImmutableListMultimap<LipoMode, String> getLipoCFlags() { - return toolchainInfo.getLipoCFlags(); - } - - /** - * Returns compiler flags arising from the {@link CToolchain} for C++ compilation by lipo mode. - */ - ImmutableListMultimap<LipoMode, String> getLipoCxxFlags() { - return toolchainInfo.getLipoCxxFlags(); - } - /** Returns linker flags for fully statically linked outputs. */ - ImmutableList<String> getLegacyFullyStaticLinkFlags( - CompilationMode compilationMode, LipoMode lipoMode) { - return configureAllLegacyLinkOptions( - compilationMode, lipoMode, LinkingMode.LEGACY_FULLY_STATIC); + ImmutableList<String> getLegacyFullyStaticLinkFlags(CompilationMode compilationMode) { + return configureAllLegacyLinkOptions(compilationMode, LinkingMode.LEGACY_FULLY_STATIC); } /** Returns linker flags for mostly static linked outputs. */ - ImmutableList<String> getLegacyMostlyStaticLinkFlags( - CompilationMode compilationMode, LipoMode lipoMode) { - return configureAllLegacyLinkOptions(compilationMode, lipoMode, LinkingMode.STATIC); + ImmutableList<String> getLegacyMostlyStaticLinkFlags(CompilationMode compilationMode) { + return configureAllLegacyLinkOptions(compilationMode, LinkingMode.STATIC); } /** Returns linker flags for mostly static shared linked outputs. */ - ImmutableList<String> getLegacyMostlyStaticSharedLinkFlags( - CompilationMode compilationMode, LipoMode lipoMode) { + ImmutableList<String> getLegacyMostlyStaticSharedLinkFlags(CompilationMode compilationMode) { return configureAllLegacyLinkOptions( - compilationMode, lipoMode, LinkingMode.LEGACY_MOSTLY_STATIC_LIBRARIES); + compilationMode, LinkingMode.LEGACY_MOSTLY_STATIC_LIBRARIES); } /** Returns linker flags for artifacts that are not fully or mostly statically linked. */ - ImmutableList<String> getLegacyDynamicLinkFlags( - CompilationMode compilationMode, LipoMode lipoMode) { - return configureAllLegacyLinkOptions(compilationMode, lipoMode, LinkingMode.DYNAMIC); + ImmutableList<String> getLegacyDynamicLinkFlags(CompilationMode compilationMode) { + return configureAllLegacyLinkOptions(compilationMode, LinkingMode.DYNAMIC); } /** * Return all flags coming from naked {@code linker_flag} fields in the crosstool. {@code * linker_flag}s coming from linking_mode_flags and compilation_mode_flags are not included. If * you need all possible linker flags, use {@link #configureAllLegacyLinkOptions(CompilationMode, - * LipoMode, LinkingMode)}. + * LinkingMode)}. */ public ImmutableList<String> getLegacyLinkOptions() { return toolchainInfo.getLegacyLinkOptions(); @@ -828,8 +810,7 @@ public final class CcToolchainProvider extends ToolchainInfo implements CcToolch ImmutableList.Builder<String> coptsBuilder = ImmutableList.<String>builder() .addAll(getToolchainCompilerFlags()) - .addAll(getCFlagsByCompilationMode().get(cppConfiguration.getCompilationMode())) - .addAll(getLipoCFlags().get(cppConfiguration.getLipoMode())); + .addAll(getCFlagsByCompilationMode().get(cppConfiguration.getCompilationMode())); if (cppConfiguration.isOmitfp()) { coptsBuilder.add("-fomit-frame-pointer"); @@ -849,8 +830,8 @@ public final class CcToolchainProvider extends ToolchainInfo implements CcToolch /** Return all possible {@code linker_flag} flags from the crosstool. */ ImmutableList<String> configureAllLegacyLinkOptions( - CompilationMode compilationMode, LipoMode lipoMode, LinkingMode linkingMode) { - return toolchainInfo.configureAllLegacyLinkOptions(compilationMode, lipoMode, linkingMode); + CompilationMode compilationMode, LinkingMode linkingMode) { + return toolchainInfo.configureAllLegacyLinkOptions(compilationMode, linkingMode); } /** Returns the GNU System Name */ @@ -914,7 +895,6 @@ public final class CcToolchainProvider extends ToolchainInfo implements CcToolch return ImmutableList.<String>builder() .addAll(getToolchainCxxFlags()) .addAll(getCxxFlagsByCompilationMode().get(cppConfiguration.getCompilationMode())) - .addAll(getLipoCxxFlags().get(cppConfiguration.getLipoMode())) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java index f5ca99384c..aa13189481 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java @@ -34,7 +34,6 @@ import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.Target; -import com.google.devtools.build.lib.rules.cpp.transitions.LipoContextCollectorTransition; import com.google.devtools.build.lib.syntax.Type; /** @@ -187,11 +186,6 @@ public final class CcToolchainRule implements RuleDefinition { .allowedRuleClasses("fdo_prefetch_hints") .mandatoryProviders(ImmutableList.of(FdoPrefetchHintsProvider.PROVIDER.id())) .value(FDO_PREFETCH_HINTS)) - .add( - attr(TransitiveLipoInfoProvider.LIPO_CONTEXT_COLLECTOR, LABEL) - .cfg(LipoContextCollectorTransition.INSTANCE) - .value(CppRuleClasses.LIPO_CONTEXT_COLLECTOR) - .skipPrereqValidatorCheck()) .add(attr("proto", Type.STRING)) .add( attr("toolchain_identifier", Type.STRING) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java index 55be4165fd..e933ae382c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java @@ -448,21 +448,6 @@ public class CppActionConfigs { " }", " }", "}"), - ifTrue( - !existingFeatureNames.contains(CppRuleClasses.LIPO), - "feature {", - " name: 'lipo'", - " requires { feature: 'autofdo' }", - " requires { feature: 'fdo_optimize' }", - " requires { feature: 'fdo_instrument' }", - " flag_set {", - " action: 'c-compile'", - " action: 'c++-compile'", - " flag_group {", - " flag: '-fripa'", - " }", - " }", - "}"), "action_config {", " config_name: 'c++-link-executable'", " action_name: 'c++-link-executable'", diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java index 3c541e8eec..f2c55bca6b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java @@ -122,9 +122,8 @@ public class CppCompileAction extends AbstractAction private final IncludeProcessing includeProcessing; private final CcCompilationContext ccCompilationContext; - private final Iterable<IncludeScannable> lipoScannables; private final ImmutableList<Artifact> builtinIncludeFiles; - // A list of files to include scan that are not source files, pcm files, lipo scannables, or + // A list of files to include scan that are not source files, pcm files, or // included via a command-line "-include file.h". Actions that use non C++ files as source // files--such as Clif--may use this mechanism. private final ImmutableList<Artifact> additionalIncludeScanningRoots; @@ -186,7 +185,6 @@ public class CppCompileAction extends AbstractAction * if Fission mode is disabled) * @param ccCompilationContext the {@code CcCompilationContext} * @param coptsFilter regular expression to remove options from {@code copts} - * @param lipoScannables List of artifacts to include-scan when this action is a lipo action * @param additionalIncludeScanningRoots list of additional artifacts to include-scan * @param actionClassId TODO(bazel-team): Add parameter description * @param actionName a string giving the name of this action for the purpose of toolchain @@ -218,7 +216,6 @@ public class CppCompileAction extends AbstractAction ActionEnvironment env, CcCompilationContext ccCompilationContext, CoptsFilter coptsFilter, - Iterable<IncludeScannable> lipoScannables, ImmutableList<Artifact> additionalIncludeScanningRoots, UUID actionClassId, ImmutableMap<String, String> executionInfo, @@ -254,7 +251,6 @@ public class CppCompileAction extends AbstractAction useHeaderModules, isStrictSystemIncludes, ccCompilationContext, - lipoScannables, builtinIncludeFiles, ImmutableList.copyOf(additionalIncludeScanningRoots), CompileCommandLine.builder(sourceFile, coptsFilter, actionName, dotdFile) @@ -296,7 +292,6 @@ public class CppCompileAction extends AbstractAction boolean useHeaderModules, boolean isStrictSystemIncludes, CcCompilationContext ccCompilationContext, - Iterable<IncludeScannable> lipoScannables, ImmutableList<Artifact> builtinIncludeFiles, ImmutableList<Artifact> additionalIncludeScanningRoots, CompileCommandLine compileCommandLine, @@ -327,7 +322,6 @@ public class CppCompileAction extends AbstractAction this.useHeaderModules = useHeaderModules; this.isStrictSystemIncludes = isStrictSystemIncludes; this.ccCompilationContext = ccCompilationContext; - this.lipoScannables = lipoScannables; this.builtinIncludeFiles = builtinIncludeFiles; this.additionalIncludeScanningRoots = additionalIncludeScanningRoots; this.compileCommandLine = compileCommandLine; @@ -661,11 +655,6 @@ public class CppCompileAction extends AbstractAction return builder.build().toCollection(); } - @Override - public Iterable<IncludeScannable> getAuxiliaryScannables() { - return lipoScannables; - } - /** * Returns the list of "-D" arguments that should be used by this gcc * invocation. Only used for testing. @@ -1009,14 +998,6 @@ public class CppCompileAction extends AbstractAction /** Return explicitly listed header files. */ @Override public NestedSet<Artifact> getDeclaredIncludeSrcs() { - if (lipoScannables != null && lipoScannables.iterator().hasNext()) { - NestedSetBuilder<Artifact> srcs = NestedSetBuilder.stableOrder(); - srcs.addTransitive(ccCompilationContext.getDeclaredIncludeSrcs()); - for (IncludeScannable lipoScannable : lipoScannables) { - srcs.addTransitive(lipoScannable.getDeclaredIncludeSrcs()); - } - return srcs.build(); - } return ccCompilationContext.getDeclaredIncludeSrcs(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java index f188e09c52..1bedc437fd 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java @@ -15,9 +15,7 @@ package com.google.devtools.build.lib.rules.cpp; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Functions; import com.google.common.base.Preconditions; -import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -35,7 +33,6 @@ import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory. import com.google.devtools.build.lib.rules.cpp.CcCommon.CoptsFilter; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CppCompileAction.DotdFile; -import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; @@ -72,7 +69,6 @@ public class CppCompileActionBuilder { private boolean allowUsingHeaderModules; private UUID actionClassId = GUID; private CppConfiguration cppConfiguration; - private ImmutableMap<Artifact, IncludeScannable> lipoScannableMap; private final ArrayList<Artifact> additionalIncludeScanningRoots; private Boolean shouldScanIncludes; private Map<String, String> executionInfo = new LinkedHashMap<>(); @@ -102,7 +98,6 @@ public class CppCompileActionBuilder { this( ruleContext.getActionOwner(), configuration, - getLipoScannableMap(ruleContext, ccToolchain), ccToolchain, ruleContext.attributes().has("$grep_includes") ? ruleContext.getPrerequisiteArtifact("$grep_includes", Mode.HOST) @@ -113,13 +108,11 @@ public class CppCompileActionBuilder { private CppCompileActionBuilder( ActionOwner actionOwner, BuildConfiguration configuration, - Map<Artifact, IncludeScannable> lipoScannableMap, CcToolchainProvider ccToolchain, @Nullable Artifact grepIncludes) { this.owner = actionOwner; this.configuration = configuration; this.cppConfiguration = configuration.getFragment(CppConfiguration.class); - this.lipoScannableMap = ImmutableMap.copyOf(lipoScannableMap); this.mandatoryInputsBuilder = NestedSetBuilder.stableOrder(); this.additionalIncludeScanningRoots = new ArrayList<>(); this.allowUsingHeaderModules = true; @@ -156,7 +149,6 @@ public class CppCompileActionBuilder { this.configuration = other.configuration; this.usePic = other.usePic; this.allowUsingHeaderModules = other.allowUsingHeaderModules; - this.lipoScannableMap = other.lipoScannableMap; this.shouldScanIncludes = other.shouldScanIncludes; this.executionInfo = new LinkedHashMap<>(other.executionInfo); this.env = other.env; @@ -167,26 +159,6 @@ public class CppCompileActionBuilder { this.grepIncludes = other.grepIncludes; } - private static ImmutableMap<Artifact, IncludeScannable> getLipoScannableMap( - RuleContext ruleContext, CcToolchainProvider toolchain) { - if (!CppHelper.isLipoOptimization(ruleContext.getFragment(CppConfiguration.class), toolchain) - // Rules that do not contain sources that are compiled into object files, but may - // contain headers, will still create CppCompileActions without providing a - // lipo_context_collector. - || ruleContext - .attributes() - .getAttributeDefinition(TransitiveLipoInfoProvider.LIPO_CONTEXT_COLLECTOR) - == null) { - return ImmutableMap.<Artifact, IncludeScannable>of(); - } - LipoContextProvider provider = - ruleContext.getPrerequisite( - TransitiveLipoInfoProvider.LIPO_CONTEXT_COLLECTOR, - Mode.DONT_CHECK, - LipoContextProvider.class); - return provider.getIncludeScannables(); - } - public PathFragment getTempOutputFile() { return tempOutputFile; } @@ -208,24 +180,6 @@ public class CppCompileActionBuilder { return mandatoryInputsBuilder.build(); } - private Iterable<IncludeScannable> getLipoScannables(NestedSet<Artifact> realMandatoryInputs) { - boolean fake = tempOutputFile != null; - - return lipoScannableMap.isEmpty() || fake - ? ImmutableList.<IncludeScannable>of() - : Iterables.filter( - Iterables.transform( - Iterables.filter( - FileType.filter( - realMandatoryInputs, - CppFileTypes.C_SOURCE, - CppFileTypes.CPP_SOURCE, - CppFileTypes.ASSEMBLER_WITH_C_PREPROCESSOR), - Predicates.not(Predicates.equalTo(getSourceFile()))), - Functions.forMap(lipoScannableMap, null)), - Predicates.notNull()); - } - private String getActionName() { if (actionName != null) { return actionName; @@ -337,20 +291,6 @@ public class CppCompileActionBuilder { prunableHeadersBuilder.addTransitive(ccCompilationContext.getDeclaredIncludeSrcs()); prunableHeadersBuilder.addTransitive(cppSemantics.getAdditionalPrunableIncludes()); - Iterable<IncludeScannable> lipoScannables = getLipoScannables(realMandatoryInputs); - // We need to add "legal generated scanner files" coming through LIPO scannables here. These - // usually contain pre-grepped source files, i.e. files just containing the #include lines - // extracted from generated files. With LIPO, some of these files can be accessed, even though - // there is no direct dependency on them. Adding the artifacts as inputs to this compile - // action ensures that the action generating them is actually executed. - for (IncludeScannable lipoScannable : lipoScannables) { - for (Artifact value : lipoScannable.getLegalGeneratedScannerFileMap().values()) { - if (value != null) { - prunableHeadersBuilder.add(value); - } - } - } - NestedSet<Artifact> prunableHeaders = prunableHeadersBuilder.build(); // Copying the collections is needed to make the builder reusable. @@ -380,7 +320,6 @@ public class CppCompileActionBuilder { env, ccCompilationContext, coptsFilter, - getLipoScannables(realMandatoryInputs), cppSemantics, ccToolchain, ImmutableMap.copyOf(executionInfo), @@ -411,7 +350,6 @@ public class CppCompileActionBuilder { env, ccCompilationContext, coptsFilter, - getLipoScannables(realMandatoryInputs), ImmutableList.copyOf(additionalIncludeScanningRoots), actionClassId, ImmutableMap.copyOf(executionInfo), diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java index 73d6c538c1..d8337f6160 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java @@ -26,7 +26,6 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.CompilationMode; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.PerLabelOptions; -import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; import com.google.devtools.build.lib.analysis.skylark.annotations.SkylarkConfigurationField; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; @@ -34,20 +33,15 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.Location; -import com.google.devtools.build.lib.packages.OutputFile; -import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.rules.cpp.CppConfigurationLoader.CppConfigurationParameters; import com.google.devtools.build.lib.rules.cpp.CrosstoolConfigurationLoader.CrosstoolFile; import com.google.devtools.build.lib.rules.cpp.Link.LinkingMode; -import com.google.devtools.build.lib.rules.cpp.transitions.ContextCollectorOwnerTransition; -import com.google.devtools.build.lib.rules.cpp.transitions.DisableLipoTransition; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skylarkbuildapi.cpp.CppConfigurationApi; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain; -import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode; import java.util.Map; import javax.annotation.Nullable; @@ -169,8 +163,8 @@ public final class CppConfiguration extends BuildConfiguration.Fragment } /** - * This macro will be passed as a command-line parameter (eg. -DBUILD_FDO_TYPE="LIPO"). - * For possible values see {@code CppModel.getFdoBuildStamp()}. + * This macro will be passed as a command-line parameter (eg. -DBUILD_FDO_TYPE="AUTOFDO"). For + * possible values see {@code CppModel.getFdoBuildStamp()}. */ public static final String FDO_STAMP_MACRO = "BUILD_FDO_TYPE"; @@ -181,7 +175,6 @@ public final class CppConfiguration extends BuildConfiguration.Fragment // it here so that the output directory doesn't depend on the CToolchain. When we will eventually // verify that the two are the same, we can remove one of desiredCpu and targetCpu. private final String desiredCpu; - private final boolean convertLipoToThinLto; private final PathFragment crosstoolTopPathFragment; private final PathFragment fdoPath; @@ -221,12 +214,6 @@ public final class CppConfiguration extends BuildConfiguration.Fragment private final boolean shouldProvideMakeVariables; private final boolean dropFullyStaticLinkingMode; - /** - * If true, the ConfiguredTarget is only used to get the necessary cross-referenced {@code - * CcCompilationContext}s, but registering build actions is disabled. - */ - private final boolean lipoContextCollector; - private final CppToolchainInfo cppToolchainInfo; static CppConfiguration create(CppConfigurationParameters params) @@ -243,8 +230,7 @@ public final class CppConfiguration extends BuildConfiguration.Fragment ImmutableList.Builder<String> coptsBuilder = ImmutableList.<String>builder() .addAll(cppToolchainInfo.getCompilerFlags()) - .addAll(cppToolchainInfo.getCFlagsByCompilationMode().get(compilationMode)) - .addAll(cppToolchainInfo.getLipoCFlags().get(cppOptions.getLipoMode())); + .addAll(cppToolchainInfo.getCFlagsByCompilationMode().get(compilationMode)); if (cppOptions.experimentalOmitfp) { coptsBuilder.add("-fomit-frame-pointer"); coptsBuilder.add("-fasynchronous-unwind-tables"); @@ -256,7 +242,6 @@ public final class CppConfiguration extends BuildConfiguration.Fragment ImmutableList.<String>builder() .addAll(cppToolchainInfo.getCxxFlags()) .addAll(cppToolchainInfo.getCxxFlagsByCompilationMode().get(compilationMode)) - .addAll(cppToolchainInfo.getLipoCxxFlags().get(cppOptions.getLipoMode())) .addAll(cppOptions.cxxoptList) .build(); @@ -266,17 +251,10 @@ public final class CppConfiguration extends BuildConfiguration.Fragment linkoptsBuilder.add("-Wl,--eh-frame-hdr"); } - if (cppOptions.getLipoMode() != LipoMode.OFF - && !cppOptions.convertLipoToThinLto - && !cppOptions.allowLipo) { - throw new InvalidConfigurationException("LIPO is disallowed"); - } - return new CppConfiguration( params.crosstoolTop, params.crosstoolFile, Preconditions.checkNotNull(params.commonOptions.cpu), - cppOptions.convertLipoToThinLto, crosstoolTopPathFragment, params.fdoPath, params.fdoOptimizeLabel, @@ -290,12 +268,10 @@ public final class CppConfiguration extends BuildConfiguration.Fragment cxxOpts, ImmutableList.copyOf(toolchain.getUnfilteredCxxFlagList()), ImmutableList.copyOf(cppOptions.conlyoptList), + cppToolchainInfo.configureAllLegacyLinkOptions(compilationMode, LinkingMode.STATIC), cppToolchainInfo.configureAllLegacyLinkOptions( - compilationMode, cppOptions.getLipoMode(), LinkingMode.STATIC), - cppToolchainInfo.configureAllLegacyLinkOptions( - compilationMode, cppOptions.getLipoMode(), LinkingMode.LEGACY_MOSTLY_STATIC_LIBRARIES), - cppToolchainInfo.configureAllLegacyLinkOptions( - compilationMode, cppOptions.getLipoMode(), LinkingMode.DYNAMIC), + compilationMode, LinkingMode.LEGACY_MOSTLY_STATIC_LIBRARIES), + cppToolchainInfo.configureAllLegacyLinkOptions(compilationMode, LinkingMode.DYNAMIC), ImmutableList.copyOf(cppOptions.coptList), ImmutableList.copyOf(cppOptions.cxxoptList), linkoptsBuilder.build(), @@ -309,7 +285,6 @@ public final class CppConfiguration extends BuildConfiguration.Fragment compilationMode, params.commonOptions.makeVariableSource == MakeVariableSource.CONFIGURATION, cppOptions.dropFullyStaticLinkingMode, - cppOptions.isLipoContextCollector(), cppToolchainInfo); } @@ -318,7 +293,6 @@ public final class CppConfiguration extends BuildConfiguration.Fragment Label crosstoolTop, CrosstoolFile crosstoolFile, String desiredCpu, - boolean convertLipoToThinLto, PathFragment crosstoolTopPathFragment, PathFragment fdoPath, Label fdoOptimizeLabel, @@ -344,12 +318,10 @@ public final class CppConfiguration extends BuildConfiguration.Fragment CompilationMode compilationMode, boolean shouldProvideMakeVariables, boolean dropFullyStaticLinkingMode, - boolean lipoContextCollector, CppToolchainInfo cppToolchainInfo) { this.crosstoolTop = crosstoolTop; this.crosstoolFile = crosstoolFile; this.desiredCpu = desiredCpu; - this.convertLipoToThinLto = convertLipoToThinLto; this.crosstoolTopPathFragment = crosstoolTopPathFragment; this.fdoPath = fdoPath; this.fdoOptimizeLabel = fdoOptimizeLabel; @@ -375,7 +347,6 @@ public final class CppConfiguration extends BuildConfiguration.Fragment this.compilationMode = compilationMode; this.shouldProvideMakeVariables = shouldProvideMakeVariables; this.dropFullyStaticLinkingMode = dropFullyStaticLinkingMode; - this.lipoContextCollector = lipoContextCollector; this.cppToolchainInfo = cppToolchainInfo; } @@ -402,8 +373,8 @@ public final class CppConfiguration extends BuildConfiguration.Fragment } /** - * Returns the toolchain identifier, which uniquely identifies the compiler - * version, target libc version, target cpu, and LIPO linkage. + * Returns the toolchain identifier, which uniquely identifies the compiler version, target libc + * version, and target cpu. */ public String getToolchainIdentifier() { return cppToolchainInfo.getToolchainIdentifier(); @@ -694,7 +665,7 @@ public final class CppConfiguration extends BuildConfiguration.Fragment * @param featuresNotUsedAnymore * @param sharedLib true if the output is a shared lib, false if it's an executable * <p>Deprecated: Use {@link CppHelper#getFullyStaticLinkOptions(CppConfiguration, - * CcToolchainProvider, Boolean)} + * CcToolchainProvider, boolean)} */ // TODO(b/64384912): Migrate skylark users to cc_common and remove. @Override @@ -795,14 +766,6 @@ public final class CppConfiguration extends BuildConfiguration.Fragment return cppOptions.linkCompileOutputSeparately; } - /* - * If true then the directory name for non-LIPO targets will have a '-lipodata' suffix in - * AutoFDO mode. - */ - public boolean getAutoFdoLipoData() { - return cppOptions.getAutoFdoLipoData(); - } - /** * Returns the STL label if given on the command line. {@code null} * otherwise. @@ -828,18 +791,6 @@ public final class CppConfiguration extends BuildConfiguration.Fragment return stlLabel; } - /** - * Returns the currently active LIPO compilation mode. - */ - public LipoMode getLipoMode() { - return cppOptions.getLipoMode(); - } - - /** Returns true if lipo should be converted to thinlto. */ - public boolean shouldConvertLipoToThinLto() { - return convertLipoToThinLto; - } - public boolean dropFullyStaticLinkingMode() { return dropFullyStaticLinkingMode; } @@ -848,44 +799,6 @@ public final class CppConfiguration extends BuildConfiguration.Fragment return cppOptions.isFdo(); } - /** Deprecated: Use {@link CcToolchainProvider#isLLVMCompiler()} */ - // TODO(b/64384912): Remove in favor of CcToolchainProvider#isLLVMCompiler - @Deprecated - private final boolean isLLVMCompiler() { - return cppToolchainInfo.isLLVMCompiler(); - } - - /** Returns true if LIPO optimization is implied by the flags of this build. */ - public boolean lipoOptimizationIsActivated() { - return cppOptions.isLipoOptimization(); - } - - /** - * Returns true if LIPO optimization should be applied for this configuration. - * - * <p>Deprecated: Use {@link CppHelper#isLipoOptimization(CppConfiguration, CcToolchainProvider)} - */ - // TODO(b/64384912): Remove usage in topLevelConfigurationHook and CppRuleClasses and delete. - @Deprecated - public boolean isLipoOptimization() { - // The LIPO optimization bits are set in the LIPO context collector configuration, too. - // If compiler is LLVM, then LIPO gets auto-converted to ThinLTO. - return cppOptions.isLipoOptimization() && !isLLVMCompiler(); - } - - public boolean isLipoOptimizationOrInstrumentation() { - return cppOptions.isLipoOptimizationOrInstrumentation(); - } - - /** - * Returns true if it is AutoFDO LIPO build. - */ - public boolean isAutoFdoLipo() { - return cppOptions.getFdoOptimize() != null - && CppFileTypes.GCC_AUTO_PROFILE.matches(cppOptions.getFdoOptimize()) - && getLipoMode() != LipoMode.OFF; - } - /** * Returns whether or not to strip the binaries. */ @@ -925,31 +838,6 @@ public final class CppConfiguration extends BuildConfiguration.Fragment } /** - * Returns the LIPO context for this configuration. - * - * <p>This only exists for configurations that apply LIPO in LIPO-optimized builds. It does - * <b>not</b> exist for data configurations, which contain LIPO state but don't actually apply - * LIPO. Nor does it exist for host configurations, which contain no LIPO state. - */ - public Label getLipoContextLabel() { - return cppOptions.getLipoContext(); - } - - /** - * Returns the LIPO context for this build, even if LIPO isn't enabled in the current - * configuration. - * - * <p>Unlike {@link #getLipoContextLabel}, this returns the LIPO context for the data - * configuration. - * - * <p>Unless you have a clear reason to use this version (which basically involves - * inspecting oher configurations' state), always use {@link #getLipoContextLabel}. - */ - public Label getLipoContextForBuild() { - return cppOptions.getLipoContextForBuild(); - } - - /** * Returns the custom malloc library label. */ public Label customMalloc() { @@ -1093,13 +981,6 @@ public final class CppConfiguration extends BuildConfiguration.Fragment return cppToolchainInfo.getTargetGnuSystemName(); } - /** - * Returns whether the configuration's purpose is only to collect LIPO-related data. - */ - public boolean isLipoContextCollector() { - return lipoContextCollector; - } - /** Returns whether this configuration will use libunwind for stack unwinding. */ public boolean isOmitfp() { return cppOptions.experimentalOmitfp; @@ -1134,12 +1015,12 @@ public final class CppConfiguration extends BuildConfiguration.Fragment } // FDO - if (cppOptions.getFdoOptimize() != null && cppOptions.getFdoProfileLabel() != null) { + if (cppOptions.getFdoOptimize() != null && cppOptions.fdoProfileLabel != null) { reporter.handle(Event.error("Both --fdo_optimize and --fdo_profile specified")); } - if (cppOptions.getFdoInstrument() != null) { - if (cppOptions.getFdoOptimize() != null || cppOptions.getFdoProfileLabel() != null) { + if (cppOptions.fdoInstrumentForBuild != null) { + if (cppOptions.getFdoOptimize() != null || cppOptions.fdoProfileLabel != null) { reporter.handle( Event.error( "Cannot instrument and optimize for FDO at the same time. Remove one of the " @@ -1152,59 +1033,10 @@ public final class CppConfiguration extends BuildConfiguration.Fragment } } - if (cppOptions.getLipoMode() != LipoMode.OFF && cppOptions.getFdoProfileLabel() != null) { - reporter.handle( - Event.error( - "LIPO options can not be used with --fdo_profile. Use --fdo_optimize instead")); - } - - if (cppOptions.getLipoMode() != LipoMode.OFF - && isLLVMCompiler() - && !cppOptions.convertLipoToThinLto) { - reporter.handle( - Event.error( - "The LLVM compiler does not support LIPO. Use --convert_lipo_to_thinlto to " - + "automatically fall back to thinlto.")); - } - if (cppOptions.lipoContextForBuild != null) { - if (!cppOptions.linkoptList.contains("-Wl,--warn-unresolved-symbols")) { - // This is effectively impossible. --lipo_context adds these values, and only invocation - // policy could remove them. - reporter.handle( - Event.error( - "The --lipo_context option cannot be used without -Wl,--warn-unresolved-symbols " - + "included as a linkoption")); - } - if (isLLVMCompiler()) { - reporter.handle( - Event.warn("LIPO options are not applicable with a LLVM compiler and will be " - + "converted to ThinLTO")); - } else if (cppOptions.getLipoMode() != LipoMode.BINARY - || cppOptions.getFdoOptimize() == null) { - reporter.handle( - Event.warn( - "The --lipo_context option can only be used together with --fdo_optimize=" - + "<profile zip> and --lipo=binary. LIPO context will be ignored.")); - } - } else { - if (!isLLVMCompiler() - && cppOptions.getLipoMode() == LipoMode.BINARY - && cppOptions.getFdoOptimize() != null) { - reporter.handle( - Event.error( - "The --lipo_context option must be specified when using " - + "--fdo_optimize=<profile zip> and --lipo=binary")); - } - } - if (cppOptions.getLipoMode() == LipoMode.BINARY && compilationMode != CompilationMode.OPT) { - reporter.handle(Event.error( - "'--lipo=binary' can only be used with '--compilation_mode=opt' (or '-c opt')")); - } - // This is an assertion check vs. user error because users can't trigger this state. Verify.verify( !(buildOptions.get(BuildConfiguration.Options.class).isHost && cppOptions.isFdo()), - "FDO/LIPO state should not propagate to the host configuration"); + "FDO state should not propagate to the host configuration"); } @Override @@ -1230,20 +1062,12 @@ public final class CppConfiguration extends BuildConfiguration.Fragment @Override public String getOutputDirectoryName() { - String lipoSuffix; - if (getLipoMode() != LipoMode.OFF && !isAutoFdoLipo()) { - lipoSuffix = "-lipo"; - } else if (getAutoFdoLipoData()) { - lipoSuffix = "-lipodata"; - } else { - lipoSuffix = ""; - } String toolchainPrefix = desiredCpu; if (!cppOptions.outputDirectoryTag.isEmpty()) { toolchainPrefix += "-" + cppOptions.outputDirectoryTag; } - return toolchainPrefix + lipoSuffix; + return toolchainPrefix; } /** @@ -1266,7 +1090,7 @@ public final class CppConfiguration extends BuildConfiguration.Fragment } public String getFdoInstrument() { - return cppOptions.getFdoInstrument(); + return cppOptions.fdoInstrumentForBuild; } public PathFragment getFdoPath() { @@ -1282,7 +1106,7 @@ public final class CppConfiguration extends BuildConfiguration.Fragment } public Label getFdoProfileLabel() { - return cppOptions.getFdoProfileLabel(); + return cppOptions.fdoProfileLabel; } public boolean isFdoAbsolutePathEnabled() { @@ -1304,24 +1128,4 @@ public final class CppConfiguration extends BuildConfiguration.Fragment } return PathFragment.create(builtInSysroot); } - - @Override - public PatchTransition getArtifactOwnerTransition() { - return isLipoContextCollector() ? ContextCollectorOwnerTransition.INSTANCE : null; - } - - @Nullable - @Override - public PatchTransition topLevelConfigurationHook(Target toTarget) { - // Top-level output files that aren't outputs of the LIPO context should be built in - // the data config. This is so their output path prefix doesn't have "-lipo" in it, which - // is a confusing and unnecessary deviation from how they would normally look. - if (toTarget instanceof OutputFile - && isLipoOptimization() - && !toTarget.getAssociatedRule().getLabel().equals(getLipoContextLabel())) { - return DisableLipoTransition.INSTANCE; - } else { - return null; - } - } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java index 69f212bff7..8ff93ddec6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java @@ -60,7 +60,6 @@ import com.google.devtools.build.lib.packages.RuleErrorConsumer; import com.google.devtools.build.lib.rules.cpp.CcLinkParams.Linkstamp; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Tool; -import com.google.devtools.build.lib.rules.cpp.CppConfiguration.DynamicMode; import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; import com.google.devtools.build.lib.shell.ShellUtils; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; @@ -68,7 +67,6 @@ import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode; import java.util.ArrayList; import java.util.HashSet; import java.util.LinkedHashMap; @@ -228,11 +226,9 @@ public class CppHelper { CppConfiguration config, CcToolchainProvider toolchain, boolean sharedLib) { if (sharedLib) { return toolchain.getSharedLibraryLinkOptions( - toolchain.getLegacyMostlyStaticLinkFlags( - config.getCompilationMode(), config.getLipoMode())); + toolchain.getLegacyMostlyStaticLinkFlags(config.getCompilationMode())); } else { - return toolchain.getLegacyFullyStaticLinkFlags( - config.getCompilationMode(), config.getLipoMode()); + return toolchain.getLegacyFullyStaticLinkFlags(config.getCompilationMode()); } } @@ -252,13 +248,10 @@ public class CppHelper { if (sharedLib) { return toolchain.getSharedLibraryLinkOptions( shouldStaticallyLinkCppRuntimes - ? toolchain.getLegacyMostlyStaticSharedLinkFlags( - config.getCompilationMode(), config.getLipoMode()) - : toolchain.getLegacyDynamicLinkFlags( - config.getCompilationMode(), config.getLipoMode())); + ? toolchain.getLegacyMostlyStaticSharedLinkFlags(config.getCompilationMode()) + : toolchain.getLegacyDynamicLinkFlags(config.getCompilationMode())); } else { - return toolchain.getLegacyMostlyStaticLinkFlags( - config.getCompilationMode(), config.getLipoMode()); + return toolchain.getLegacyMostlyStaticLinkFlags(config.getCompilationMode()); } } @@ -276,9 +269,9 @@ public class CppHelper { Boolean sharedLib) { if (sharedLib) { return toolchain.getSharedLibraryLinkOptions( - toolchain.getLegacyDynamicLinkFlags(config.getCompilationMode(), config.getLipoMode())); + toolchain.getLegacyDynamicLinkFlags(config.getCompilationMode())); } else { - return toolchain.getLegacyDynamicLinkFlags(config.getCompilationMode(), config.getLipoMode()); + return toolchain.getLegacyDynamicLinkFlags(config.getCompilationMode()); } } @@ -326,28 +319,6 @@ public class CppHelper { } /** - * Returns the dynamic linking mode (full, off, or default) implied by the given configuration and - * toolchain. - */ - public static DynamicMode getDynamicMode(CppConfiguration config, CcToolchainProvider toolchain) { - // With LLVM, ThinLTO is automatically used in place of LIPO. ThinLTO works fine with dynamic - // linking (and in fact creates a lot more work when dynamic linking is off). - if (config.getLipoMode() == LipoMode.BINARY && !toolchain.isLLVMCompiler()) { - // TODO(bazel-team): implement dynamic linking with LIPO - return DynamicMode.OFF; - } else { - return config.getDynamicModeFlag(); - } - } - - /** Returns true if LIPO optimization should be applied for this configuration and toolchain. */ - public static boolean isLipoOptimization(CppConfiguration config, CcToolchainProvider toolchain) { - // The LIPO optimization bits are set in the LIPO context collector configuration, too. - // If compiler is LLVM, then LIPO gets auto-converted to ThinLTO. - return config.lipoOptimizationIsActivated() && !toolchain.isLLVMCompiler(); - } - - /** * Return {@link FdoSupportProvider} using default cc_toolchain attribute name. * * <p>Be careful to provide explicit attribute name if the rule doesn't store cc_toolchain under @@ -675,41 +646,6 @@ public class CppHelper { } } - public static void addTransitiveLipoInfoForCommonAttributes( - RuleContext ruleContext, - CcCompilationOutputs outputs, - NestedSetBuilder<IncludeScannable> scannableBuilder) { - - TransitiveLipoInfoProvider stl = null; - if (ruleContext.getRule().getAttributeDefinition(":stl") != null - && ruleContext.getPrerequisite(":stl", Mode.TARGET) != null) { - // If the attribute is defined, it is never null. - stl = ruleContext.getPrerequisite(":stl", Mode.TARGET) - .getProvider(TransitiveLipoInfoProvider.class); - } - if (stl != null) { - scannableBuilder.addTransitive(stl.getTransitiveIncludeScannables()); - } - - for (TransitiveLipoInfoProvider dep : - ruleContext.getPrerequisites("deps", Mode.TARGET, TransitiveLipoInfoProvider.class)) { - scannableBuilder.addTransitive(dep.getTransitiveIncludeScannables()); - } - - if (ruleContext.attributes().has("malloc", LABEL)) { - TransitiveInfoCollection malloc = mallocForTarget(ruleContext); - TransitiveLipoInfoProvider provider = malloc.getProvider(TransitiveLipoInfoProvider.class); - if (provider != null) { - scannableBuilder.addTransitive(provider.getTransitiveIncludeScannables()); - } - } - - for (IncludeScannable scannable : outputs.getLipoScannables()) { - Preconditions.checkState(scannable.getIncludeScannerSources().size() == 1); - scannableBuilder.add(scannable); - } - } - // TODO(bazel-team): figure out a way to merge these 2 methods. See the Todo in // CcCommonConfiguredTarget.noCoptsMatches(). /** @@ -739,29 +675,6 @@ public class CppHelper { } /** - * Returns true if shared libraries must be compiled with position independent code for the build - * implied by the given config and toolchain. - */ - - /** - * Returns the LIPO context provider for configured target, - * or null if such a provider doesn't exist. - */ - public static LipoContextProvider getLipoContextProvider(RuleContext ruleContext) { - if (ruleContext - .getRule() - .getAttributeDefinition(TransitiveLipoInfoProvider.LIPO_CONTEXT_COLLECTOR) - == null) { - return null; - } - - TransitiveInfoCollection dep = - ruleContext.getPrerequisite( - TransitiveLipoInfoProvider.LIPO_CONTEXT_COLLECTOR, Mode.DONT_CHECK); - return (dep != null) ? dep.getProvider(LipoContextProvider.class) : null; - } - - /** * Creates a CppModuleMap object for pure c++ builds. The module map artifact becomes a candidate * input to a CppCompileAction. */ @@ -864,10 +777,10 @@ public class CppHelper { public static String getFdoBuildStamp(RuleContext ruleContext, FdoSupport fdoSupport) { CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class); if (fdoSupport.isAutoFdoEnabled()) { - return (cppConfiguration.getLipoMode() == LipoMode.BINARY) ? "ALIPO" : "AFDO"; + return "AFDO"; } if (cppConfiguration.isFdo()) { - return (cppConfiguration.getLipoMode() == LipoMode.BINARY) ? "LIPO" : "FDO"; + return "FDO"; } if (fdoSupport.isXBinaryFdoEnabled()) { return "XFDO"; @@ -875,24 +788,6 @@ public class CppHelper { return null; } - /** - * Returns a relative path to the bin directory for data in AutoFDO LIPO mode. - */ - public static PathFragment getLipoDataBinFragment(BuildConfiguration configuration) { - PathFragment parent = configuration.getBinFragment().getParentDirectory(); - return parent.replaceName(parent.getBaseName() + "-lipodata") - .getChild(configuration.getBinFragment().getBaseName()); - } - - /** - * Returns a relative path to the genfiles directory for data in AutoFDO LIPO mode. - */ - public static PathFragment getLipoDataGenfilesFragment(BuildConfiguration configuration) { - PathFragment parent = configuration.getGenfilesFragment().getParentDirectory(); - return parent.replaceName(parent.getBaseName() + "-lipodata") - .getChild(configuration.getGenfilesFragment().getBaseName()); - } - /** Creates an action to strip an executable. */ public static void createStripAction( RuleContext context, 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 822ab07494..d06e6a78a5 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 @@ -27,7 +27,6 @@ import com.google.devtools.build.lib.rules.cpp.CppConfiguration.DynamicMode; import com.google.devtools.build.lib.rules.cpp.CppConfiguration.StripMode; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.util.OS; -import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode; import com.google.devtools.common.options.Converter; import com.google.devtools.common.options.EnumConverter; import com.google.devtools.common.options.Option; @@ -40,7 +39,6 @@ import java.util.List; import java.util.ListIterator; import java.util.Map; import java.util.Set; -import javax.annotation.Nullable; /** Command-line options for C++. */ @AutoCodec(strategy = AutoCodec.Strategy.PUBLIC_FIELDS) @@ -118,24 +116,6 @@ public class CppOptions extends FragmentOptions { } } - /** - * Converter for the --lipo option. - */ - public static class LipoModeConverter extends EnumConverter<LipoMode> { - public LipoModeConverter() { - super(LipoMode.class, "LIPO mode"); - } - } - - @Option( - name = "experimental_allow_lipo", - defaultValue = "true", - documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, - effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, - metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, - help = "Flag to roll out the removal of LIPO.") - public boolean allowLipo; - @Option( name = "crosstool_top", defaultValue = "@bazel_tools//tools/cpp:toolchain", @@ -434,21 +414,8 @@ public class CppOptions extends FragmentOptions { + "With Clang/LLVM compiler, it also accepts the directory name under" + "which the raw profile file(s) will be dumped at runtime." ) - /** - * Never read FDO/LIPO options directly. This is because {@link #lipoConfigurationState} - * determines whether these options are actually "active" for this configuration. Instead, use the - * equivalent getter method, which takes that into account. - */ public String fdoInstrumentForBuild; - /** - * Returns the --fdo_instrument value if FDO is specified and active for this configuration, - * the default value otherwise. - */ - public String getFdoInstrument() { - return enableLipoSettings() ? fdoInstrumentForBuild : null; - } - @Option( name = "fdo_optimize", allowMultiple = true, @@ -468,17 +435,13 @@ public class CppOptions extends FragmentOptions { + "the file visible to Bazel. It also accepts a raw or an indexed LLVM profile file. " + "This flag will be superseded by fdo_profile rule." ) - /** - * Never read FDO/LIPO options directly. This is because {@link #lipoConfigurationState} - * determines whether these options are actually "active" for this configuration. Instead, use the - * equivalent getter method, which takes that into account. - */ public List<String> fdoProfiles; /** - * Select profile from the list of profiles passed through multiple -fdo_optimize options. + * Returns the --fdo_optimize value if FDO is specified and active for this configuration, the + * default value otherwise. */ - private String selectProfile() { + public String getFdoOptimize() { if (fdoProfiles == null) { return null; } @@ -499,14 +462,6 @@ public class CppOptions extends FragmentOptions { return lastXBinaryProfile; } - /** - * Returns the --fdo_optimize value if FDO is specified and active for this - * configuration, the default value otherwise. - */ - public String getFdoOptimize() { - return enableLipoSettings() ? selectProfile() : null; - } - @Option( name = "fdo_prefetch_hints", defaultValue = "null", @@ -525,147 +480,6 @@ public class CppOptions extends FragmentOptions { return fdoPrefetchHintsLabel; } - /** - * Returns the --autofdo_lipo_data value for this configuration. This is false except for data - * configurations under LIPO builds. - */ - public boolean getAutoFdoLipoData() { - if (enableLipoSettings()) { - return false; - } - - String fdoProfile = selectProfile(); - return lipoModeForBuild != LipoMode.OFF - && fdoProfile != null - && FdoSupport.isAutoFdo(fdoProfile); - } - - @Option( - name = "convert_lipo_to_thinlto", - defaultValue = "false", - documentationCategory = OptionDocumentationCategory.TOOLCHAIN, - effectTags = {OptionEffectTag.EXECUTION, OptionEffectTag.AFFECTS_OUTPUTS}, - help = - "If set, builds using LIPO will automatically be converted to ThinLTO for the LLVM " - + "compiler." - ) - public boolean convertLipoToThinLto; - - @Option( - name = "lipo", - defaultValue = "off", - converter = LipoModeConverter.class, - documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS, - effectTags = {OptionEffectTag.AFFECTS_OUTPUTS}, - help = - "Enable LIPO optimization (lightweight inter-procedural optimization, The allowed " - + "values for this option are 'off' and 'binary', which enables LIPO. This option " - + "only has an effect when FDO is also enabled. Currently LIPO is only supported " - + "when building a single cc_binary rule." - ) - /** - * Never read FDO/LIPO options directly. This is because {@link #lipoConfigurationState} - * determines whether these options are actually "active" for this configuration. Instead, use the - * equivalent getter method, which takes that into account. - */ - public LipoMode lipoModeForBuild; - - /** - * Returns the --lipo value if LIPO is specified and active for this configuration, - * the default value otherwise. - */ - public LipoMode getLipoMode() { - return enableLipoSettings() ? lipoModeForBuild : LipoMode.OFF; - } - - @Option( - name = "lipo_context", - defaultValue = "null", - converter = LabelConverter.class, - implicitRequirements = {"--linkopt=-Wl,--warn-unresolved-symbols"}, - documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS, - effectTags = {OptionEffectTag.AFFECTS_OUTPUTS}, - help = "Specifies the binary from which the LIPO profile information comes." - ) - /** - * Never read FDO/LIPO options directly. This is because {@link #lipoConfigurationState} - * determines whether these options are actually "active" for this configuration. Instead, use the - * equivalent getter method, which takes that into account. - */ - public Label lipoContextForBuild; - - /** - * Returns the --lipo_context value if LIPO is specified and active for this configuration, - * null otherwise. - */ - @Nullable - public Label getLipoContext() { - return isLipoOptimization() ? lipoContextForBuild : null; - } - - /** - * Returns the LIPO context for this build, even if LIPO isn't enabled in the current - * configuration. - */ - public Label getLipoContextForBuild() { - return lipoContextForBuild; - } - - /** - * Internal state determining how to apply LIPO settings under this configuration. - */ - public enum LipoConfigurationState { - /** Don't LIPO-optimize targets under this configuration. */ - IGNORE_LIPO, - /** LIPO-optimize targets under this configuration if this is a LIPO build. */ - APPLY_LIPO, - /** - * Evaluate targets in this configuration in "LIPO context collector" mode. See - * {@link FdoSupport} for details. - */ - LIPO_CONTEXT_COLLECTOR, - } - - /** - * Converter for {@link LipoConfigurationState}. - */ - public static class LipoConfigurationStateConverter - extends EnumConverter<LipoConfigurationState> { - public LipoConfigurationStateConverter() { - super(LipoConfigurationState.class, "LIPO configuration state"); - } - } - - @Option( - name = "lipo configuration state", - defaultValue = "apply_lipo", - documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, - effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION}, - metadataTags = {OptionMetadataTag.INTERNAL}, - converter = LipoConfigurationStateConverter.class - ) - public LipoConfigurationState lipoConfigurationState; - - /** - * Returns true if targets under this configuration should use the build's LIPO settings. - * - * <p>Even when we switch off LIPO (e.g. by switching to a data configuration), we still need to - * remember the LIPO settings in case we need to switch back (e.g. if we build a genrule with a - * data dependency on the LIPO context). - * - * <p>We achieve this by maintaining a "configuration state" flag that flips on / off when we - * want to enable / disable LIPO respectively. This means we need to be careful to distinguish - * between the existence of LIPO settings and LIPO actually applying to the configuration. So when - * buiding a target, it's not enough to see if {@link #lipoContextForBuild} or - * {@link #lipoModeForBuild} are set. We also need to check this flag. - * - * <p>This class exposes appropriate convenience methods to make these checks convenient and easy. - * Use them and read the documentation carefully. - */ - private boolean enableLipoSettings() { - return lipoConfigurationState != LipoConfigurationState.IGNORE_LIPO; - } - @Option( name = "fdo_profile", defaultValue = "null", @@ -677,14 +491,6 @@ public class CppOptions extends FragmentOptions { ) public Label fdoProfileLabel; - /** - * Returns the --fdo_optimize value if FDO is specified and active for this configuration, the - * default value otherwise. - */ - public Label getFdoProfileLabel() { - return enableLipoSettings() ? fdoProfileLabel : null; - } - @Option( name = "enable_fdo_profile_absolute_path", defaultValue = "true", @@ -1013,7 +819,6 @@ public class CppOptions extends FragmentOptions { host.stripBinaries = StripMode.ALWAYS; host.fdoProfiles = null; host.fdoProfileLabel = null; - host.lipoModeForBuild = LipoMode.OFF; host.inmemoryDotdFiles = inmemoryDotdFiles; host.pruneCppInputDiscovery = pruneCppInputDiscovery; @@ -1042,53 +847,6 @@ public class CppOptions extends FragmentOptions { * Returns true if targets under this configuration should apply FDO. */ public boolean isFdo() { - return getFdoOptimize() != null || getFdoInstrument() != null || getFdoProfileLabel() != null; - } - - /** - * Returns true if this configuration has LIPO optimization settings (even if they're - * not necessarily active). - */ - private boolean hasLipoOptimizationState() { - return lipoModeForBuild == LipoMode.BINARY && selectProfile() != null - && lipoContextForBuild != null; - } - - /** - * Returns true if targets under this configuration should LIPO-optimize. - */ - public boolean isLipoOptimization() { - return hasLipoOptimizationState() && enableLipoSettings() && !isLipoContextCollector(); - } - - /** - * Returns true if this is a data configuration for a LIPO-optimizing build. - * - * <p>This means LIPO is not applied for this configuration, but LIPO might be reenabled further - * down the dependency tree. - */ - public boolean isDataConfigurationForLipoOptimization() { - return hasLipoOptimizationState() && !enableLipoSettings(); - } - - /** - * Returns true if targets under this configuration should LIPO-optimize or LIPO-instrument. - */ - public boolean isLipoOptimizationOrInstrumentation() { - return getLipoMode() == LipoMode.BINARY - && ((getFdoOptimize() != null && getLipoContext() != null) || getFdoInstrument() != null) - && !isLipoContextCollector(); - } - - /** - * Returns true if this is the LIPO context collector configuration. - */ - public boolean isLipoContextCollector() { - return lipoConfigurationState == LipoConfigurationState.LIPO_CONTEXT_COLLECTOR; - } - - @Override - public boolean enableActions() { - return !isLipoContextCollector(); + return getFdoOptimize() != null || fdoInstrumentForBuild != null || fdoProfileLabel != null; } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java index 6aa0c220fd..fc62f040b7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java @@ -42,8 +42,6 @@ import com.google.devtools.build.lib.packages.Attribute.LabelLateBoundDefault; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; -import com.google.devtools.build.lib.packages.RuleTransitionFactory; -import com.google.devtools.build.lib.rules.cpp.transitions.EnableLipoTransition; import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.util.OsUtils; @@ -51,26 +49,6 @@ import com.google.devtools.build.lib.util.OsUtils; * Rule class definitions for C++ rules. */ public class CppRuleClasses { - /** - * Implementation for the :lipo_context_collector attribute. - * - * <p>This attribute connects a target to the LIPO context target configured with the lipo input - * collector configuration. - */ - public static final LabelLateBoundDefault<?> LIPO_CONTEXT_COLLECTOR = - LabelLateBoundDefault.fromTargetConfiguration( - CppConfiguration.class, - null, - // TODO(b/69548520): Remove call to isLipoOptimization - (rule, attributes, cppConfig) -> - cppConfig.isLipoOptimization() ? cppConfig.getLipoContextLabel() : null); - - /** - * Rule transition factory that enables LIPO on the LIPO context binary (i.e. applies a DATA -> - * TARGET transition). - */ - public static final RuleTransitionFactory LIPO_ON_DEMAND = - (rule) -> new EnableLipoTransition(rule.getLabel()); /** * Label of a pseudo-filegroup that contains all crosstool and libcfiles for all configurations, @@ -390,11 +368,6 @@ public class CppRuleClasses { public static final String XBINARYFDO = "xbinaryfdo"; /** - * A string constant for the lipo feature. - */ - public static final String LIPO = "lipo"; - - /** * A string constant for the coverage feature. */ public static final String COVERAGE = "coverage"; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppToolchainInfo.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppToolchainInfo.java index 88c06ad67e..b71c213e11 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppToolchainInfo.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppToolchainInfo.java @@ -35,7 +35,6 @@ import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain.ArtifactNamePattern; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LinkingModeFlags; -import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.ToolPath; import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.TextFormat; @@ -80,7 +79,6 @@ public final class CppToolchainInfo { private final ImmutableList<String> dynamicLibraryLinkFlags; private final ImmutableList<String> legacyLinkOptions; private final ImmutableListMultimap<LinkingMode, String> legacyLinkOptionsFromLinkingMode; - private final ImmutableListMultimap<LipoMode, String> legacyLinkOptionsFromLipoMode; private final ImmutableListMultimap<CompilationMode, String> legacyLinkOptionsFromCompilationMode; private final ImmutableList<String> testOnlyLinkFlags; private final ImmutableList<String> ldOptionsForEmbedding; @@ -100,8 +98,6 @@ public final class CppToolchainInfo { private final ImmutableListMultimap<CompilationMode, String> cFlagsByCompilationMode; private final ImmutableListMultimap<CompilationMode, String> cxxFlagsByCompilationMode; - private final ImmutableListMultimap<LipoMode, String> lipoCFlags; - private final ImmutableListMultimap<LipoMode, String> lipoCxxFlags; private final ImmutableList<String> unfilteredCompilerFlags; @@ -154,15 +150,6 @@ public final class CppToolchainInfo { cxxFlagsBuilder.putAll(realmode, flags.getCxxFlagList()); } - ImmutableListMultimap.Builder<LipoMode, String> lipoCFlagsBuilder = - ImmutableListMultimap.builder(); - ImmutableListMultimap.Builder<LipoMode, String> lipoCxxFlagsBuilder = - ImmutableListMultimap.builder(); - for (CrosstoolConfig.LipoModeFlags flags : toolchain.getLipoModeFlagsList()) { - LipoMode realmode = flags.getMode(); - lipoCFlagsBuilder.putAll(realmode, flags.getCompilerFlagList()); - lipoCxxFlagsBuilder.putAll(realmode, flags.getCxxFlagList()); - } try { return new CppToolchainInfo( toolchain, @@ -185,7 +172,6 @@ public final class CppToolchainInfo { ImmutableList.copyOf(toolchain.getDynamicLibraryLinkerFlagList()), ImmutableList.copyOf(toolchain.getLinkerFlagList()), linkOptionsFromLinkingModeBuilder.build(), - computeLinkOptionsFromLipoMode(toolchain), computeLinkOptionsFromCompilationMode(toolchain), ImmutableList.copyOf(toolchain.getTestOnlyLinkerFlagList()), ImmutableList.copyOf(toolchain.getLdEmbedFlagList()), @@ -207,8 +193,6 @@ public final class CppToolchainInfo { ImmutableList.copyOf(toolchain.getCxxFlagList()), cFlagsBuilder.build(), cxxFlagsBuilder.build(), - lipoCFlagsBuilder.build(), - lipoCxxFlagsBuilder.build(), ImmutableList.copyOf(toolchain.getUnfilteredCxxFlagList()), toolchain.getSupportsFission(), toolchain.getSupportsStartEndLib(), @@ -242,7 +226,6 @@ public final class CppToolchainInfo { ImmutableList<String> dynamicLibraryLinkFlags, ImmutableList<String> legacyLinkOptions, ImmutableListMultimap<LinkingMode, String> legacyLinkOptionsFromLinkingMode, - ImmutableListMultimap<LipoMode, String> legacyLinkOptionsFromLipoMode, ImmutableListMultimap<CompilationMode, String> legacyLinkOptionsFromCompilationMode, ImmutableList<String> testOnlyLinkFlags, ImmutableList<String> ldOptionsForEmbedding, @@ -258,8 +241,6 @@ public final class CppToolchainInfo { ImmutableList<String> crosstoolCxxFlags, ImmutableListMultimap<CompilationMode, String> cFlagsByCompilationMode, ImmutableListMultimap<CompilationMode, String> cxxFlagsByCompilationMode, - ImmutableListMultimap<LipoMode, String> lipoCFlags, - ImmutableListMultimap<LipoMode, String> lipoCxxFlags, ImmutableList<String> unfilteredCompilerFlags, boolean supportsFission, boolean supportsStartEndLib, @@ -287,7 +268,6 @@ public final class CppToolchainInfo { this.dynamicLibraryLinkFlags = dynamicLibraryLinkFlags; this.legacyLinkOptions = legacyLinkOptions; this.legacyLinkOptionsFromLinkingMode = legacyLinkOptionsFromLinkingMode; - this.legacyLinkOptionsFromLipoMode = legacyLinkOptionsFromLipoMode; this.legacyLinkOptionsFromCompilationMode = legacyLinkOptionsFromCompilationMode; this.testOnlyLinkFlags = testOnlyLinkFlags; this.ldOptionsForEmbedding = ldOptionsForEmbedding; @@ -303,8 +283,6 @@ public final class CppToolchainInfo { this.crosstoolCxxFlags = crosstoolCxxFlags; this.cFlagsByCompilationMode = cFlagsByCompilationMode; this.cxxFlagsByCompilationMode = cxxFlagsByCompilationMode; - this.lipoCFlags = lipoCFlags; - this.lipoCxxFlags = lipoCxxFlags; this.unfilteredCompilerFlags = unfilteredCompilerFlags; this.supportsFission = supportsFission; this.supportsStartEndLib = supportsStartEndLib; @@ -452,16 +430,13 @@ public final class CppToolchainInfo { return legacyLinkOptions; } - /** - * @see CcToolchainProvider#configureAllLegacyLinkOptions(CompilationMode, LipoMode, LinkingMode). - */ + /** @see CcToolchainProvider#configureAllLegacyLinkOptions(CompilationMode, LinkingMode). */ ImmutableList<String> configureAllLegacyLinkOptions( - CompilationMode compilationMode, LipoMode lipoMode, LinkingMode linkingMode) { + CompilationMode compilationMode, LinkingMode linkingMode) { List<String> result = new ArrayList<>(); result.addAll(legacyLinkOptions); result.addAll(legacyLinkOptionsFromCompilationMode.get(compilationMode)); - result.addAll(legacyLinkOptionsFromLipoMode.get(lipoMode)); result.addAll(legacyLinkOptionsFromLinkingMode.get(linkingMode)); return ImmutableList.copyOf(result); } @@ -475,7 +450,7 @@ public final class CppToolchainInfo { /** * Returns the toolchain identifier, which uniquely identifies the compiler version, target libc - * version, target cpu, and LIPO linkage. + * version, and target cpu. */ public String getToolchainIdentifier() { return toolchainIdentifier; @@ -736,16 +711,6 @@ public final class CppToolchainInfo { return cxxFlagsByCompilationMode; } - /** Returns compiler flags for C compilation by lipo mode. */ - public ImmutableListMultimap<LipoMode, String> getLipoCFlags() { - return lipoCFlags; - } - - /** Returns compiler flags for C compilation by lipo mode. */ - public ImmutableListMultimap<LipoMode, String> getLipoCxxFlags() { - return lipoCxxFlags; - } - /** Returns unfiltered compiler options for C++ from this toolchain. */ public ImmutableList<String> getUnfilteredCompilerOptions(@Nullable PathFragment sysroot) { if (sysroot == null) { @@ -784,17 +749,6 @@ public final class CppToolchainInfo { return linkOptionsFromCompilationModeBuilder.build(); } - private static ImmutableListMultimap<LipoMode, String> computeLinkOptionsFromLipoMode( - CToolchain toolchain) { - ImmutableListMultimap.Builder<LipoMode, String> linkOptionsFromLipoModeBuilder = - ImmutableListMultimap.builder(); - for (CrosstoolConfig.LipoModeFlags flags : toolchain.getLipoModeFlagsList()) { - LipoMode realmode = flags.getMode(); - linkOptionsFromLipoModeBuilder.putAll(realmode, flags.getLinkerFlagList()); - } - return linkOptionsFromLipoModeBuilder.build(); - } - private static ImmutableMap<String, PathFragment> computeToolPaths( CToolchain toolchain, PathFragment crosstoolTopPathFragment) { Map<String, PathFragment> toolPathsCollector = Maps.newHashMap(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java index fece268cd2..63093dbd08 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java @@ -78,7 +78,6 @@ public class FakeCppCompileAction extends CppCompileAction { ActionEnvironment env, CcCompilationContext ccCompilationContext, CoptsFilter nocopts, - Iterable<IncludeScannable> lipoScannables, CppSemantics cppSemantics, CcToolchainProvider cppProvider, ImmutableMap<String, String> executionInfo, @@ -114,7 +113,6 @@ public class FakeCppCompileAction extends CppCompileAction { // time, so they can't depend on the contents of the ".d" file.) CcCompilationContext.disallowUndeclaredHeaders(ccCompilationContext), nocopts, - lipoScannables, /* additionalIncludeScanningRoots=*/ ImmutableList.of(), GUID, executionInfo, diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java index 512661dfc4..e4990dee28 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java @@ -16,9 +16,7 @@ package com.google.devtools.build.lib.rules.cpp; import static java.nio.charset.StandardCharsets.ISO_8859_1; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultimap; @@ -30,11 +28,9 @@ import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.analysis.AnalysisEnvironment; import com.google.devtools.build.lib.analysis.RuleContext; -import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; -import com.google.devtools.build.lib.skyframe.PrecomputedValue; import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization; @@ -43,7 +39,6 @@ import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; -import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode; import com.google.devtools.build.skyframe.SkyFunction; import java.io.File; import java.io.FileNotFoundException; @@ -51,18 +46,16 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.Collection; import java.util.Enumeration; import java.util.zip.ZipEntry; import java.util.zip.ZipException; import java.util.zip.ZipFile; /** - * Support class for FDO (feedback directed optimization) and LIPO (lightweight inter-procedural - * optimization). + * Support class for FDO (feedback directed optimization). * - * <p>Here follows a quick run-down of how FDO/LIPO builds work (for non-FDO/LIPO builds, none of - * this applies): + * <p>Here follows a quick run-down of how FDO builds work (for non-FDO builds, none of this + * applies): * * <p>{@link FdoSupport#create} is called from {@link FdoSupportFunction} (a {@link SkyFunction}), * which is requested from Skyframe by the {@code cc_toolchain} rule. It extracts the FDO .zip (in @@ -80,66 +73,21 @@ import java.util.zip.ZipFile; * original source file or the .gcda file for such a referenced file. They both are added to the * imports map. * - * <p>If we do LIPO, we create an extra configuration that is called the "LIPO context collector", - * whose job it is to collect information that every configured target compiled with LIPO needs. The - * top-level target of this configuration is the LIPO context (always a cc_binary) and is an - * implicit dependency of every cc_* rule through their :lipo_context_collector attribute. The - * collected information is encapsulated in {@link LipoContextProvider}. - * - * <p>Note that the LIPO context can be different from the actual binary we are compiling because - * it's beneficial to compile sources in a test in the exact same way as they would be compiled for - * a particular {@code cc_binary} so that the code tested is the same as the one being run in - * production. Thus, the {@code --lipo_context} command line flag, which takes the label of a {@code - * cc_binary} rule as an argument which will be used as the LIPO context. - * - * <p>In this case, it can happen that files are needed for the compilation (because code in them is - * inlined) that are not in the transitive closure of the tests being run. To cover this case, we - * have the otherwise unused {@code :lipo_context} attribute, which depends on the LIPO context - * without any configuration transition. Its purpose is to give a chance for the configured targets - * containing the inlined code to run and thus create generating actions for the artifacts {@link - * LipoContextProvider} contains. That is, configured targets in the LIPO context collector - * configuration collect these artifacts but do not generate actions for them, and configured - * targets under {@code :lipo_context} generate actions, but the artifacts they create are - * discarded. This works because {@link Artifact} is a value object and the artifacts in {@link - * LipoContextProvider} are {@code #equals()} to the ones created under {@code :lipo_context}. - * * <p>For each C++ compile action in the target configuration, {@link #configureCompilation} is - * called, which adds command line options and input files required for the build. There are three + * called, which adds command line options and input files required for the build. There are 2 * cases: * * <ul> * <li>If we do AutoFDO, the .afdo file and the source files containing the functions imported by * the original source file (as determined from the inputs map) are added. * <li>If we do FDO, the .gcda file corresponding to the source file is added. - * <li>If we do LIPO, in addition to the .gcda file corresponding to the source file (like for - * FDO) the source files that contain the functions referenced by the source file and their - * .gcda files are added, too. * </ul> - * - * <p>If we do LIPO, the actual {@code CcCompilationContext} for LIPO compilation actions is pieced - * together from the {@code CcCompilationContext} in LipoContextProvider and that of the rule being - * compiled. (see {@link CcCompilationContext#mergeForLipo}) This is so that the include files for - * the extra LIPO sources are found and is, strictly speaking, incorrect, since it also changes the - * declared include directories of the main source file, which in theory can result in the - * compilation passing even though it should fail with undeclared inclusion errors. - * - * <p>During the actual execution of the C++ compile action, the extra sources also need to be - * include scanned, which is the reason why they are {@link IncludeScannable} objects and not simple - * artifacts. We currently create these {@link IncludeScannable} objects by creating actual C++ - * compile actions in the LIPO context collector configuration which are then never executed. In - * fact, these C++ compile actions are never even registered with Skyframe. For this we propagate a - * bit from {@code BuildConfiguration.isActionsEnabled} to {@code - * CachingAnalysisEnvironment.allowRegisteringActions}, which causes actions to be silently - * discarded after configured targets are created. */ @Immutable @AutoCodec public class FdoSupport { /** * The FDO mode we are operating in. - * - * LIPO can only be active if this is not <code>OFF</code>, but all of the modes below can work - * with LIPO either off or on. */ @VisibleForSerialization enum FdoMode { @@ -160,13 +108,6 @@ public class FdoSupport { } /** - * Returns true if the given fdoFile represents an AutoFdo profile. - */ - public static final boolean isAutoFdo(String fdoFile) { - return CppFileTypes.GCC_AUTO_PROFILE.matches(fdoFile); - } - - /** * Coverage information output directory passed to {@code --fdo_instrument}, * or {@code null} if FDO instrumentation is disabled. */ @@ -200,12 +141,6 @@ public class FdoSupport { private final PathFragment fdoPath; /** - * LIPO mode passed to {@code --lipo}. This is only used if - * {@code fdoProfile != null}. - */ - private final LipoMode lipoMode; - - /** * FDO mode. */ private final FdoMode fdoMode; @@ -233,13 +168,11 @@ public class FdoSupport { * * @param fdoInstrument value of the --fdo_instrument option * @param fdoProfile path to the profile file passed to --fdo_optimize option - * @param lipoMode value of the --lipo_mode option */ @VisibleForSerialization @AutoCodec.Instantiator FdoSupport( FdoMode fdoMode, - LipoMode lipoMode, ArtifactRoot fdoRoot, PathFragment fdoRootExecPath, String fdoInstrument, @@ -253,7 +186,6 @@ public class FdoSupport { ? null : FileSystemUtils.removeExtension(PathFragment.create("_fdo").getChild( fdoProfile.getBaseName())); - this.lipoMode = lipoMode; this.fdoMode = fdoMode; if (fdoZipContents != null) { this.gcdaFiles = fdoZipContents.gcdaFiles; @@ -264,10 +196,6 @@ public class FdoSupport { } } - public ArtifactRoot getFdoRoot() { - return fdoRoot; - } - public Path getFdoProfile() { return fdoProfile; } @@ -283,16 +211,11 @@ public class FdoSupport { SkyFunction.Environment env, String fdoInstrument, Path fdoProfile, - LipoMode lipoMode, Path execRoot, String productName, FdoMode fdoMode) throws IOException, FdoException, InterruptedException { - if (fdoProfile == null) { - lipoMode = LipoMode.OFF; - } - ArtifactRoot fdoRoot = (fdoProfile == null) ? null @@ -304,18 +227,12 @@ public class FdoSupport { PathFragment.create("_fdo").getChild(fdoProfile.getBaseName()))); if (fdoProfile != null) { - if (lipoMode != LipoMode.OFF) { - // Incrementality is not supported for LIPO builds, see FdoSupport#scannables. - // Ensure that the Skyframe value containing the configuration will not be reused to avoid - // incrementality issues. - PrecomputedValue.dependOnBuildId(env); - } else { Path path = fdoMode == FdoMode.AUTO_FDO ? getAutoFdoImportsPath(fdoProfile) : fdoProfile; env.getValue( FileValue.key( RootedPath.toRootedPathMaybeUnderRoot( path, ImmutableList.of(Root.fromPath(execRoot))))); - } + } if (env.valuesMissing()) { @@ -323,14 +240,13 @@ public class FdoSupport { } if (fdoMode == FdoMode.LLVM_FDO) { - return new FdoSupport( - fdoMode, LipoMode.OFF, fdoRoot, fdoRootExecPath, fdoInstrument, fdoProfile, null); + return new FdoSupport(fdoMode, fdoRoot, fdoRootExecPath, fdoInstrument, fdoProfile, null); } FdoZipContents fdoZipContents = - extractFdoZip(fdoMode, lipoMode, execRoot, fdoProfile, fdoRootExecPath, productName); + extractFdoZip(fdoMode, execRoot, fdoProfile, fdoRootExecPath, productName); return new FdoSupport( - fdoMode, lipoMode, fdoRoot, fdoRootExecPath, fdoInstrument, fdoProfile, fdoZipContents); + fdoMode, fdoRoot, fdoRootExecPath, fdoInstrument, fdoProfile, fdoZipContents); } @Immutable @@ -354,13 +270,17 @@ public class FdoSupport { /** * Extracts the FDO zip file and collects data from it that's needed during analysis. * - * <p>When an {@code --fdo_optimize} compile is requested, unpacks the given - * FDO gcda zip file into a clean working directory under execRoot. + * <p>When an {@code --fdo_optimize} compile is requested, unpacks the given FDO zip file + * into a clean working directory under execRoot. * * @throws FdoException if the FDO ZIP contains a file of unknown type */ - private static FdoZipContents extractFdoZip(FdoMode fdoMode, LipoMode lipoMode, Path execRoot, - Path fdoProfile, PathFragment fdoRootExecPath, String productName) + private static FdoZipContents extractFdoZip( + FdoMode fdoMode, + Path execRoot, + Path fdoProfile, + PathFragment fdoRootExecPath, + String productName) throws IOException, FdoException { // The execRoot != null case is only there for testing. We cannot provide a real ZIP file in // tests because ZipFileSystem does not work with a ZIP on an in-memory file system. @@ -375,9 +295,6 @@ public class FdoSupport { FileSystemUtils.createDirectoryAndParents(fdoDirPath); if (fdoMode == FdoMode.AUTO_FDO || fdoMode == FdoMode.XBINARY_FDO) { - if (lipoMode != LipoMode.OFF) { - imports = readAutoFdoImports(getAutoFdoImportsPath(fdoProfile)); - } FileSystemUtils.ensureSymbolicLink( execRoot.getRelative(getAutoProfilePath(fdoProfile, fdoRootExecPath)), fdoProfile); } else { @@ -490,7 +407,7 @@ public class FdoSupport { /** * Reads a .gcda.imports file and stores the imports information. * - * @throws FdoException if an auxiliary LIPO input was not found + * @throws FdoException */ private static void readCoverageImports( ZipFile zipFile, @@ -516,84 +433,11 @@ public class FdoSupport { } } - /** - * Reads a .afdo.imports file and stores the imports information. - */ - private static ImmutableMultimap<PathFragment, PathFragment> readAutoFdoImports(Path importsFile) - throws IOException, FdoException { - ImmutableMultimap.Builder<PathFragment, PathFragment> importBuilder = - ImmutableMultimap.builder(); - for (String line : FileSystemUtils.iterateLinesAsLatin1(importsFile)) { - line = line.trim(); - if (line.isEmpty()) { - continue; - } - - int colonIndex = line.indexOf(':'); - if (colonIndex < 0) { - continue; - } - PathFragment key = PathFragment.create(line.substring(0, colonIndex)); - if (key.isAbsolute()) { - throw new FdoException("Absolute paths not allowed in afdo imports file " + importsFile - + ": " + key); - } - for (String auxFile : line.substring(colonIndex + 1).split(" ")) { - if (auxFile.length() == 0) { - continue; - } - - importBuilder.put(key, PathFragment.create(auxFile)); - } - } - return importBuilder.build(); - } - private static Path getAutoFdoImportsPath(Path fdoProfile) { return fdoProfile.getParentDirectory().getRelative(fdoProfile.getBaseName() + ".imports"); } /** - * Returns the imports from the .afdo.imports file of a source file. - * - * @param sourceExecPath the source file - */ - private Collection<Artifact> getAutoFdoImports(RuleContext ruleContext, - PathFragment sourceExecPath, LipoContextProvider lipoContextProvider) { - Preconditions.checkState(lipoMode != LipoMode.OFF); - ImmutableCollection<PathFragment> afdoImports = imports.get(sourceExecPath); - Preconditions.checkState(afdoImports != null, - "AutoFDO import data missing for %s", sourceExecPath); - ImmutableList.Builder<Artifact> result = ImmutableList.builder(); - for (PathFragment afdoImport : afdoImports) { - Artifact afdoArtifact = lipoContextProvider.getSourceArtifactMap().get(afdoImport); - if (afdoArtifact != null) { - result.add(afdoArtifact); - } else { - ruleContext.ruleError(String.format( - "cannot find source file '%s' referenced from '%s'", afdoImport, sourceExecPath)); - } - } - return result.build(); - } - - /** - * Returns the imports from the .gcda.imports file of an object file. - * - * @param objDirectory the object directory of the object file's target - * @param objectName the object file - */ - private Iterable<PathFragment> getImports(PathFragment objDirectory, PathFragment objectName) { - Preconditions.checkState(lipoMode != LipoMode.OFF); - Preconditions.checkState(imports != null, - "Tried to look up imports of uninitialized FDOSupport"); - PathFragment key = objDirectory.getRelative(FileSystemUtils.removeExtension(objectName)); - ImmutableCollection<PathFragment> importsForObject = imports.get(key); - Preconditions.checkState(importsForObject != null, "Import data missing for %s", key); - return importsForObject; - } - - /** * Configures a compile action builder by setting up command line options and auxiliary inputs * according to the FDO configuration. This method does nothing If FDO is disabled. */ @@ -601,8 +445,6 @@ public class FdoSupport { public ImmutableMap<String, String> configureCompilation( CppCompileActionBuilder builder, RuleContext ruleContext, - PathFragment sourceName, - PathFragment sourceExecPath, PathFragment outputName, boolean usePic, FeatureConfiguration featureConfiguration, @@ -635,8 +477,7 @@ public class FdoSupport { return ImmutableMap.of(); } Iterable<Artifact> auxiliaryInputs = - getAuxiliaryInputs( - ruleContext, sourceName, sourceExecPath, outputName, usePic, fdoSupportProvider); + getAuxiliaryInputs(ruleContext, outputName, usePic, fdoSupportProvider); builder.addMandatoryInputs(auxiliaryInputs); if (!Iterables.isEmpty(auxiliaryInputs)) { if (featureConfiguration.isEnabled(CppRuleClasses.AUTOFDO) @@ -664,16 +505,9 @@ public class FdoSupport { /** Returns the auxiliary files that need to be added to the {@link CppCompileAction}. */ private Iterable<Artifact> getAuxiliaryInputs( RuleContext ruleContext, - PathFragment sourceName, - PathFragment sourceExecPath, PathFragment outputName, boolean usePic, FdoSupportProvider fdoSupportProvider) { - CcToolchainProvider toolchain = - CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext); - LipoContextProvider lipoContextProvider = - toolchain.isLLVMCompiler() ? null : CppHelper.getLipoContextProvider(ruleContext); - ImmutableSet.Builder<Artifact> auxiliaryInputs = ImmutableSet.builder(); if (fdoSupportProvider.getPrefetchHintsArtifact() != null) { @@ -686,85 +520,37 @@ public class FdoSupport { || fdoMode == FdoMode.AUTO_FDO || fdoMode == FdoMode.XBINARY_FDO) { auxiliaryInputs.add(fdoSupportProvider.getProfileArtifact()); - if (lipoContextProvider != null) { - auxiliaryInputs.addAll(getAutoFdoImports(ruleContext, sourceExecPath, lipoContextProvider)); - } return auxiliaryInputs.build(); } else { PathFragment objectName = FileSystemUtils.appendExtension(outputName, usePic ? ".pic.o" : ".o"); - Label lipoLabel = ruleContext.getLabel(); auxiliaryInputs.addAll( - getGcdaArtifactsForObjectFileName( - ruleContext, fdoSupportProvider, objectName, lipoLabel)); - - if (lipoContextProvider != null) { - for (PathFragment importedFile : getImports( - getNonLipoObjDir(ruleContext, lipoLabel), objectName)) { - if (CppFileTypes.COVERAGE_DATA.matches(importedFile.getBaseName())) { - Artifact gcdaArtifact = - getGcdaArtifactsForGcdaPath(fdoSupportProvider, importedFile); - if (gcdaArtifact == null) { - ruleContext.ruleError(String.format( - ".gcda file %s is not in the FDO zip (referenced by source file %s). Check if " - + "your profile is generated from the same sources you are building the " - + "optimized binary from", - importedFile, sourceName)); - } else { - auxiliaryInputs.add(gcdaArtifact); - } - } else { - Artifact importedArtifact = lipoContextProvider.getSourceArtifactMap() - .get(importedFile); - if (importedArtifact != null) { - auxiliaryInputs.add(importedArtifact); - } else { - ruleContext.ruleError(String.format( - "cannot find source file '%s' referenced from '%s' by LIPO inclusion. Check if " - + "your profile is generated from the same sources you are building the " - + "optimized binary from", - importedFile, objectName)); - } - } - } - } + getGcdaArtifactsForObjectFileName(ruleContext, fdoSupportProvider, objectName)); return auxiliaryInputs.build(); } } - /** - * Returns the .gcda file artifacts for a .gcda path from the .gcda.imports file or null if the - * referenced .gcda file is not in the FDO zip. - */ - private Artifact getGcdaArtifactsForGcdaPath(FdoSupportProvider fdoSupportProvider, - PathFragment gcdaPath) { - if (!gcdaFiles.contains(gcdaPath)) { - return null; - } - - return fdoSupportProvider.getGcdaArtifacts().get(gcdaPath); - } - - private PathFragment getNonLipoObjDir(RuleContext ruleContext, Label label) { - return ruleContext.getConfiguration().getBinFragment() - .getRelative(CppHelper.getObjDirectory(label)); + private PathFragment getObjDir(RuleContext ruleContext) { + return ruleContext + .getConfiguration() + .getBinFragment() + .getRelative(CppHelper.getObjDirectory(ruleContext.getLabel())); } /** * Returns a list of .gcda file artifacts for an object file path. * - * <p>The resulting set is either empty (because no .gcda file exists for the - * given object file) or contains one or two artifacts (the file itself and a - * symlink to it). + * <p>The resulting set is either empty (because no .gcda file exists for the given object file) + * or contains one or two artifacts (the file itself and a symlink to it). */ - private ImmutableList<Artifact> getGcdaArtifactsForObjectFileName(RuleContext ruleContext, - FdoSupportProvider fdoSupportProvider, PathFragment objectFileName, Label lipoLabel) { + private ImmutableList<Artifact> getGcdaArtifactsForObjectFileName( + RuleContext ruleContext, FdoSupportProvider fdoSupportProvider, PathFragment objectFileName) { // We put the .gcda files relative to the location of the .o file in the instrumentation run. String gcdaExt = Iterables.getOnlyElement(CppFileTypes.COVERAGE_DATA.getExtensions()); PathFragment baseName = FileSystemUtils.replaceExtension(objectFileName, gcdaExt); - PathFragment gcdaFile = getNonLipoObjDir(ruleContext, lipoLabel).getRelative(baseName); + PathFragment gcdaFile = getObjDir(ruleContext).getRelative(baseName); if (!gcdaFiles.contains(gcdaFile)) { // If the object is a .pic.o file and .pic.gcda is not found, we should try finding .gcda too @@ -774,7 +560,7 @@ public class FdoSupport { // Object file is not .pic.o return ImmutableList.of(); } - gcdaFile = getNonLipoObjDir(ruleContext, lipoLabel).getRelative(baseName); + gcdaFile = getObjDir(ruleContext).getRelative(baseName); if (!gcdaFiles.contains(gcdaFile)) { // .gcda file not found return ImmutableList.of(); @@ -827,8 +613,7 @@ public class FdoSupport { public ProfileArtifacts buildProfileForLtoBackend( FdoSupportProvider fdoSupportProvider, FeatureConfiguration featureConfiguration, - CcToolchainVariables.Builder buildVariables, - RuleContext ruleContext) { + CcToolchainVariables.Builder buildVariables) { Artifact prefetch = fdoSupportProvider.getPrefetchHintsArtifact(); if (prefetch != null) { buildVariables.addStringVariable("fdo_prefetch_hints_path", prefetch.getExecPathString()); @@ -843,15 +628,6 @@ public class FdoSupport { return new ProfileArtifacts(profile, prefetch); } - /** - * Returns the path of the FDO output tree (relative to the execution root) - * containing the .gcda profile files, or null if FDO is not enabled. - */ - @VisibleForTesting - public PathFragment getFdoOptimizeDir() { - return fdoRootExecPath; - } - public FdoSupportProvider createFdoSupportProvider( RuleContext ruleContext, ProfileArtifacts profiles) { if (fdoRoot == null) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java index 742835a9b1..c8a3569607 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java @@ -66,7 +66,6 @@ public class FdoSupportFunction implements SkyFunction { env, key.getFdoInstrument(), fdoZip, - key.getLipoMode(), execRoot, directories.getProductName(), key.getFdoMode()); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportValue.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportValue.java index f6bad1f0b2..5d2192b889 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportValue.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportValue.java @@ -19,7 +19,6 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoMode; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; @@ -45,19 +44,16 @@ public class FdoSupportValue implements SkyValue { public static class Key implements SkyKey { private static final Interner<Key> interner = BlazeInterners.newWeakInterner(); - private final LipoMode lipoMode; private final PathFragment fdoZip; private final FdoInputFile fdoPrefetchHintsFile; private final String fdoInstrument; private final FdoMode fdoMode; private Key( - LipoMode lipoMode, PathFragment fdoZip, FdoInputFile fdoPrefetchHintsFile, String fdoInstrument, FdoMode fdoMode) { - this.lipoMode = lipoMode; this.fdoZip = fdoZip; this.fdoPrefetchHintsFile = fdoPrefetchHintsFile; this.fdoInstrument = fdoInstrument; @@ -67,17 +63,11 @@ public class FdoSupportValue implements SkyValue { @AutoCodec.Instantiator @AutoCodec.VisibleForSerialization static Key of( - LipoMode lipoMode, PathFragment fdoZip, FdoInputFile fdoPrefetchHintsFile, String fdoInstrument, FdoMode fdoMode) { - return interner.intern( - new Key(lipoMode, fdoZip, fdoPrefetchHintsFile, fdoInstrument, fdoMode)); - } - - public LipoMode getLipoMode() { - return lipoMode; + return interner.intern(new Key(fdoZip, fdoPrefetchHintsFile, fdoInstrument, fdoMode)); } public PathFragment getFdoZip() { @@ -107,8 +97,7 @@ public class FdoSupportValue implements SkyValue { } Key that = (Key) o; - return Objects.equals(this.lipoMode, that.lipoMode) - && Objects.equals(this.fdoZip, that.fdoZip) + return Objects.equals(this.fdoZip, that.fdoZip) && Objects.equals(this.fdoPrefetchHintsFile, that.fdoPrefetchHintsFile) && Objects.equals(this.fdoMode, that.fdoMode) && Objects.equals(this.fdoInstrument, that.fdoInstrument); @@ -116,7 +105,7 @@ public class FdoSupportValue implements SkyValue { @Override public int hashCode() { - return Objects.hash(lipoMode, fdoZip, fdoPrefetchHintsFile, fdoInstrument); + return Objects.hash(fdoZip, fdoPrefetchHintsFile, fdoInstrument); } @Override @@ -136,11 +125,10 @@ public class FdoSupportValue implements SkyValue { } public static SkyKey key( - LipoMode lipoMode, PathFragment fdoZip, FdoInputFile fdoPrefetchHintsFile, String fdoInstrument, FdoMode fdoMode) { - return Key.of(lipoMode, fdoZip, fdoPrefetchHintsFile, fdoInstrument, fdoMode); + return Key.of(fdoZip, fdoPrefetchHintsFile, fdoInstrument, fdoMode); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScannable.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScannable.java index 0b46c5293a..c9dbcd120e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScannable.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScannable.java @@ -93,12 +93,6 @@ public interface IncludeScannable { NestedSet<Artifact> getDeclaredIncludeSrcs(); /** - * Returns additional scannables that need also be scanned when scanning this - * scannable. May be empty but not null. This is not evaluated recursively. - */ - Iterable<IncludeScannable> getAuxiliaryScannables(); - - /** * Returns a map of (generated header:.includes file listing the header's includes) which may be * reached during include scanning. Generated files which are reached, but not in the key set, * must be ignored. diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanner.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanner.java index d6d46dfec4..0512adcc4f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanner.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanner.java @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.rules.cpp; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Iterables; import com.google.common.collect.Sets; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.Artifact; @@ -111,21 +110,16 @@ public interface IncludeScanner { Profiler profiler = Profiler.instance(); try (SilentCloseable c = profiler.profile(ProfilerTask.SCANNER, profilerTaskName)) { - // We need to scan the action itself, but also the auxiliary scannables - // (for LIPO). There is no need to call getAuxiliaryScannables - // recursively. - for (IncludeScannable scannable : - Iterables.concat(ImmutableList.of(action), action.getAuxiliaryScannables())) { - - Map<Artifact, Artifact> legalOutputPaths = scannable.getLegalGeneratedScannerFileMap(); - // Deduplicate include directories. This can occur especially with "built-in" and "system" - // include directories because of the way we retrieve them. Duplicate include directories - // really mess up #include_next directives. - Set<PathFragment> includeDirs = new LinkedHashSet<>(scannable.getIncludeDirs()); - List<PathFragment> quoteIncludeDirs = scannable.getQuoteIncludeDirs(); - List<String> cmdlineIncludes = scannable.getCmdlineIncludes(); - - includeDirs.addAll(scannable.getSystemIncludeDirs()); + + Map<Artifact, Artifact> legalOutputPaths = action.getLegalGeneratedScannerFileMap(); + // Deduplicate include directories. This can occur especially with "built-in" and "system" + // include directories because of the way we retrieve them. Duplicate include directories + // really mess up #include_next directives. + Set<PathFragment> includeDirs = new LinkedHashSet<>(action.getIncludeDirs()); + List<PathFragment> quoteIncludeDirs = action.getQuoteIncludeDirs(); + List<String> cmdlineIncludes = action.getCmdlineIncludes(); + + includeDirs.addAll(action.getSystemIncludeDirs()); // Add the system include paths to the list of include paths. for (PathFragment pathFragment : action.getBuiltInIncludeDirectories()) { @@ -139,12 +133,20 @@ public interface IncludeScanner { IncludeScanner scanner = includeScannerSupplier.scannerFor(quoteIncludeDirs, includeDirList); - Artifact mainSource = scannable.getMainIncludeScannerSource(); - Collection<Artifact> sources = scannable.getIncludeScannerSources(); - scanner.process(mainSource, sources, legalOutputPaths, quoteIncludeDirs, - includeDirList, cmdlineIncludes, includes, actionExecutionContext, - action.getGrepIncludes(), scannable.getModularHeaders()); - } + Artifact mainSource = action.getMainIncludeScannerSource(); + Collection<Artifact> sources = action.getIncludeScannerSources(); + scanner.process( + mainSource, + sources, + legalOutputPaths, + quoteIncludeDirs, + includeDirList, + cmdlineIncludes, + includes, + actionExecutionContext, + action.getGrepIncludes(), + action.getModularHeaders()); + } catch (IOException e) { throw new EnvironmentalExecException(e.getMessage()); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LipoContextProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LipoContextProvider.java deleted file mode 100644 index 90d6117cb0..0000000000 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LipoContextProvider.java +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright 2014 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -package com.google.devtools.build.lib.rules.cpp; - -import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; -import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; -import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; -import com.google.devtools.build.lib.vfs.PathFragment; -import java.util.Map; - -/** - * Provides LIPO context information to the LIPO-enabled target configuration. - * - * <p>This is a rollup of the data collected in the LIPO context collector configuration. Each - * target in the LIPO context collector configuration has a {@link TransitiveLipoInfoProvider} which - * is used to transitively collect the data, then the {@code cc_binary} that is referred to in - * {@code --lipo_context} puts the collected data into {@link LipoContextProvider}, of which there - * is only one in any given build. - */ -@Immutable -@AutoCodec -public final class LipoContextProvider implements TransitiveInfoProvider { - private final CcCompilationContext ccCompilationContext; - - private final ImmutableMap<Artifact, IncludeScannable> includeScannables; - private final ImmutableMap<PathFragment, Artifact> sourceArtifactMap; - - @AutoCodec.Instantiator - public LipoContextProvider( - CcCompilationContext ccCompilationContext, - Map<Artifact, IncludeScannable> includeScannables, - Map<PathFragment, Artifact> sourceArtifactMap) { - this.ccCompilationContext = ccCompilationContext; - this.includeScannables = ImmutableMap.copyOf(includeScannables); - this.sourceArtifactMap = ImmutableMap.copyOf(sourceArtifactMap); - } - - /** Returns merged {@code CcCompilationContext} for the whole LIPO subtree. */ - public CcCompilationContext getLipoCcCompilationContext() { - return ccCompilationContext; - } - - /** - * Returns the map from source artifact to the include scannable object representing - * the corresponding FDO source input file. - */ - public ImmutableMap<Artifact, IncludeScannable> getIncludeScannables() { - return includeScannables; - } - - public ImmutableMap<PathFragment, Artifact> getSourceArtifactMap() { - return sourceArtifactMap; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java index 643fb26eb0..f6609ddf87 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java @@ -236,7 +236,7 @@ public final class LtoBackendArtifacts { fdoSupport .getFdoSupport() .buildProfileForLtoBackend( - fdoSupport, featureConfiguration, buildVariablesBuilder, ruleContext)); + fdoSupport, featureConfiguration, buildVariablesBuilder)); if (profileArtifacts.getProfileArtifact() != null) { builder.addInput(profileArtifacts.getProfileArtifact()); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java index 2060f0c582..9b49079b19 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java @@ -52,8 +52,6 @@ import com.google.devtools.build.lib.rules.cpp.CppConfiguration; import com.google.devtools.build.lib.rules.cpp.CppHelper; import com.google.devtools.build.lib.rules.cpp.CppRuleClasses; import com.google.devtools.build.lib.rules.cpp.CppSemantics; -import com.google.devtools.build.lib.rules.cpp.TransitiveLipoInfoProvider; -import com.google.devtools.build.lib.rules.cpp.transitions.LipoContextCollectorTransition; import com.google.devtools.build.lib.rules.proto.ProtoCommon; import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder; import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.ToolchainInvocation; @@ -122,12 +120,7 @@ public abstract class CcProtoAspect extends NativeAspectClass implements Configu .value(PROTO_TOOLCHAIN_LABEL)) .add( attr(CcToolchain.CC_TOOLCHAIN_DEFAULT_ATTRIBUTE_NAME, LABEL) - .value(ccToolchainAttrValue)) - .add( - attr(TransitiveLipoInfoProvider.LIPO_CONTEXT_COLLECTOR, LABEL) - .cfg(LipoContextCollectorTransition.INSTANCE) - .value(CppRuleClasses.LIPO_CONTEXT_COLLECTOR) - .skipPrereqValidatorCheck()); + .value(ccToolchainAttrValue)); return result.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/ContextCollectorOwnerTransition.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/ContextCollectorOwnerTransition.java deleted file mode 100644 index 3fe0e9d0e5..0000000000 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/ContextCollectorOwnerTransition.java +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright 2017 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.rules.cpp.transitions; - -import com.google.devtools.build.lib.analysis.config.BuildConfiguration; -import com.google.devtools.build.lib.analysis.config.BuildOptions; -import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; -import com.google.devtools.build.lib.rules.cpp.CppOptions; -import com.google.devtools.build.lib.rules.cpp.CppOptions.LipoConfigurationState; -import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; - -/** - * Configuration transition that creates the "artifact owner" configuration from the LIPO context - * collector configuration. - * - * <p>The context collector creates C++ output artifacts but doesn't create the actions that - * generate those artifacts (this is what {@link BuildConfiguration#isActionsEnabled()} means). - * Those actions are the responsibility of the target configuration. This transition produces that - * config so artifacts created by the context collector can be associated with the right "owner". - * Also see {@link BuildConfiguration#getArtifactOwnerTransition()}. - * - * <p>This is a no-op for all configurations but the context collector. - */ -public class ContextCollectorOwnerTransition implements PatchTransition { - - @AutoCodec - public static final ContextCollectorOwnerTransition INSTANCE = - new ContextCollectorOwnerTransition(); - - @Override - public BuildOptions patch(BuildOptions options) { - // If this target and its transitive closure don't have C++ options, there's no context - // collector configuration to change. - if (!options.contains(CppOptions.class)) { - return options; - } - if (!options.get(CppOptions.class).isLipoContextCollector()) { - return options; - } - BuildOptions ownerOptions = options.clone(); - ownerOptions.get(CppOptions.class).lipoConfigurationState = LipoConfigurationState.APPLY_LIPO; - return ownerOptions; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/DisableLipoTransition.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/DisableLipoTransition.java deleted file mode 100644 index a69611d9e8..0000000000 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/DisableLipoTransition.java +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright 2015 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.rules.cpp.transitions; - -import com.google.devtools.build.lib.analysis.config.BuildOptions; -import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; -import com.google.devtools.build.lib.rules.cpp.CppOptions; -import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; -import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode; - -/** - * Configuration transition that turns off LIPO/FDO settings. - * - * <p>Has no effect on non-LIPO-enabled configurations or the LIPO context collector configuration. - */ -public final class DisableLipoTransition implements PatchTransition { - - @AutoCodec public static final DisableLipoTransition INSTANCE = new DisableLipoTransition(); - - private DisableLipoTransition() {} - - @Override - public BuildOptions patch(BuildOptions options) { - // If this target and its transitive closure don't have C++ options, there's no - // LIPO context to change. - if (!options.contains(CppOptions.class)) { - return options; - } - CppOptions cppOptions = options.get(CppOptions.class); - - if (cppOptions.getLipoMode() == LipoMode.OFF || cppOptions.isLipoContextCollector()) { - return options; - } - BuildOptions lipoDisabledOptions = options.clone(); - lipoDisabledOptions.get(CppOptions.class).lipoConfigurationState = - CppOptions.LipoConfigurationState.IGNORE_LIPO; - return lipoDisabledOptions; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/EnableLipoTransition.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/EnableLipoTransition.java deleted file mode 100644 index cdd92b9954..0000000000 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/EnableLipoTransition.java +++ /dev/null @@ -1,79 +0,0 @@ -// Copyright 2017 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.rules.cpp.transitions; - -import com.google.devtools.build.lib.analysis.config.BuildOptions; -import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; -import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; -import com.google.devtools.build.lib.rules.cpp.CppOptions; -import com.google.devtools.build.lib.rules.cpp.CppOptions.LipoConfigurationState; -import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; -import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization; -import java.util.Objects; - -/** - * Configuration transition that turns on LIPO/FDO settings for configurations that have them - * disabled. - */ -@Immutable -@AutoCodec -public class EnableLipoTransition implements PatchTransition { - private final Label ruleLabel; - private final int hashCode; - - /** - * Creates a new transition that only triggers on the given rule. This can be used for - * restricting this transition to the LIPO context binary. - */ - public EnableLipoTransition(Label ruleLabel) { - this(ruleLabel, Objects.hashCode(ruleLabel)); - } - - @VisibleForSerialization - @AutoCodec.Instantiator - EnableLipoTransition(Label ruleLabel, int hashCode) { - this.ruleLabel = ruleLabel; - this.hashCode = hashCode; - } - - @Override - public BuildOptions patch(BuildOptions options) { - CppOptions cppOptions = options.get(CppOptions.class); - if (!cppOptions.isDataConfigurationForLipoOptimization() - || !ruleLabel.equals(cppOptions.getLipoContextForBuild())) { - return options; - } - BuildOptions lipoEnabledOptions = options.clone(); - lipoEnabledOptions.get(CppOptions.class).lipoConfigurationState = - LipoConfigurationState.APPLY_LIPO; - return lipoEnabledOptions; - } - - @Override - public boolean equals(Object other) { - if (this == other) { - return true; - } else if (!(other instanceof EnableLipoTransition)) { - return false; - } - return ruleLabel.equals(((EnableLipoTransition) other).ruleLabel); - } - - @Override - public int hashCode() { - return hashCode; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/LipoContextCollectorTransition.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/LipoContextCollectorTransition.java deleted file mode 100644 index 231dcca181..0000000000 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/LipoContextCollectorTransition.java +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright 2017 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.rules.cpp.transitions; - -import com.google.devtools.build.lib.analysis.config.BuildOptions; -import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; -import com.google.devtools.build.lib.rules.cpp.CppOptions; -import com.google.devtools.build.lib.rules.cpp.FdoSupport; - -/** - * Configuration transition that enters "LIPO context collector" mode on a - * "LIPO optimization"-enabled input configuration. - * - * <p>See {@link FdoSupport} for details. - */ -public class LipoContextCollectorTransition implements PatchTransition { - public static final LipoContextCollectorTransition INSTANCE = - new LipoContextCollectorTransition(); - - private LipoContextCollectorTransition() {} - - @Override - public BuildOptions patch(BuildOptions options) { - // If this target and its transitive closure don't have C++ options, there's no - // LIPO context to change. - if (!options.contains(CppOptions.class)) { - return options; - } - CppOptions cppOptions = options.get(CppOptions.class); - if (!cppOptions.isLipoOptimization()) { - return options; - } - BuildOptions collectorOptions = options.clone(); - collectorOptions.get(CppOptions.class).lipoConfigurationState = - CppOptions.LipoConfigurationState.LIPO_CONTEXT_COLLECTOR; - return collectorOptions; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/LipoDataTransitionRuleSet.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/LipoDataTransitionRuleSet.java deleted file mode 100644 index 748f008952..0000000000 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/LipoDataTransitionRuleSet.java +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2018 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -package com.google.devtools.build.lib.rules.cpp.transitions; - -import com.google.common.collect.ImmutableList; -import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; -import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider.RuleSet; - -/** Rule set to deal with LIPO data transitions */ -public final class LipoDataTransitionRuleSet implements RuleSet { - public static final LipoDataTransitionRuleSet INSTANCE = new LipoDataTransitionRuleSet(); - - private LipoDataTransitionRuleSet() { - // Use the static INSTANCE field instead. - } - - @Override - public void init(ConfiguredRuleClassProvider.Builder builder) { - builder.setLipoDataTransition(DisableLipoTransition.INSTANCE); - } - - @Override - public ImmutableList<RuleSet> requires() { - return ImmutableList.of(); - } -} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java index a4eec29cf2..bf14785ca5 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java @@ -507,7 +507,6 @@ public final class SkyframeBuildView { return null; } boolean extendedSanityChecks = config != null && config.extendedSanityChecks(); - boolean allowRegisteringActions = config == null || config.isActionsEnabled(); return new CachingAnalysisEnvironment( artifactFactory, skyframeExecutor.getActionKeyContext(), @@ -515,8 +514,7 @@ public final class SkyframeBuildView { isSystemEnv, extendedSanityChecks, eventHandler, - env, - allowRegisteringActions); + env); } /** diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index ac6e8d1496..f024250102 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -78,7 +78,6 @@ import com.google.devtools.build.lib.analysis.config.HostTransition; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition; import com.google.devtools.build.lib.analysis.config.transitions.NoTransition; -import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget; import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget.DuplicateException; import com.google.devtools.build.lib.cmdline.Label; @@ -1214,19 +1213,13 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { List<BuildConfiguration> topLevelTargetConfigs = getConfigurations(eventHandler, getTopLevelBuildOptions(buildOptions, multiCpu), keepGoing); - // The host configuration inherits the data, not target options. This is so host tools don't - // apply LIPO. BuildConfiguration firstTargetConfig = topLevelTargetConfigs.get(0); - ConfigurationTransition dataTransition = - ((ConfiguredRuleClassProvider) ruleClassProvider).getLipoDataTransition(); - BuildOptions dataOptions = dataTransition != NoTransition.INSTANCE - ? ((PatchTransition) dataTransition).patch(firstTargetConfig.getOptions()) - : firstTargetConfig.getOptions(); + BuildOptions targetOptions = firstTargetConfig.getOptions(); BuildOptions hostOptions = - dataOptions.get(BuildConfiguration.Options.class).useDistinctHostConfiguration - ? HostTransition.INSTANCE.patch(dataOptions) - : dataOptions; + targetOptions.get(BuildConfiguration.Options.class).useDistinctHostConfiguration + ? HostTransition.INSTANCE.patch(targetOptions) + : targetOptions; BuildConfiguration hostConfig = getConfiguration(eventHandler, hostOptions, keepGoing); // TODO(gregce): cache invalid option errors in BuildConfigurationFunction, then use a dedicated diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkUtils.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkUtils.java index 84cec5b823..4f6c0463e7 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkUtils.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkUtils.java @@ -23,7 +23,6 @@ public final class SkylarkUtils { private static class BazelInfo { String toolsRepository; ImmutableMap<String, Class<?>> fragmentNameToClass; - Object lipoDataTransition; } private static final String BAZEL_INFO_KEY = "$bazel"; @@ -67,25 +66,4 @@ public final class SkylarkUtils { public static ImmutableMap<String, Class<?>> getFragmentMap(Environment env) { return getInfo(env).fragmentNameToClass; } - - /** - * Sets the configuration transition to apply to <code>cfg = "data"</code> attributes. - * - * <p>This must be a - * {@link com.google.devtools.build.lib.analysis.config.transitions.PatchTransition}. But that - * class isn't available in <code>lib.syntax</code>, so we can't type-check it here. - */ - public static void setLipoDataTransition(Environment env, Object lipoDataTransition) { - getInfo(env).lipoDataTransition = lipoDataTransition; - } - - /** - * Returns the configuration transition to apply to <code>cfg = "data"</code> attributes. - * - * <p>This is always a - * {@link com.google.devtools.build.lib.analysis.config.transitions.PatchTransition}. - */ - public static Object getLipoDataTransition(Environment env) { - return getInfo(env).lipoDataTransition; - } } |