From 9fce7603ffd4f63b5b7e819e6699a603acda965c Mon Sep 17 00:00:00 2001 From: Cal Peyser Date: Tue, 26 Jul 2016 18:39:13 +0000 Subject: Linker outputs can optionally be configured from the CROSSTOOL. Introduces infrastructure to allow other artifact categories (such as debug symbols or compiler outputs) to be defined in other changes. -- MOS_MIGRATED_REVID=128495797 --- .../build/lib/rules/cpp/ArtifactCategory.java | 162 ++++++++++++++++++++ .../devtools/build/lib/rules/cpp/CcBinary.java | 5 +- .../devtools/build/lib/rules/cpp/CcLibrary.java | 28 ++-- .../build/lib/rules/cpp/CcLibraryHelper.java | 14 +- .../build/lib/rules/cpp/CcToolchainFeatures.java | 169 ++++++++++++++++----- .../devtools/build/lib/rules/cpp/CppHelper.java | 6 +- .../devtools/build/lib/rules/cpp/CppModel.java | 68 ++++++++- .../google/devtools/build/lib/rules/cpp/Link.java | 37 ++++- src/main/protobuf/crosstool_config.proto | 18 ++- src/test/java/com/google/devtools/build/lib/BUILD | 1 + .../build/lib/packages/util/MockCcSupport.java | 21 +++ .../rules/cpp/CcLibraryConfiguredTargetTest.java | 45 ++++++ 12 files changed, 502 insertions(+), 72 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/rules/cpp/ArtifactCategory.java diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/ArtifactCategory.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/ArtifactCategory.java new file mode 100644 index 0000000000..04036faf6f --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/ArtifactCategory.java @@ -0,0 +1,162 @@ +// Copyright 2016 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.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.vfs.PathFragment; + +/** + * A category of artifacts that are candidate input/output to an action, for which the toolchain can + * select a single artifact. + */ +public enum ArtifactCategory { + STATIC_LIBRARY { + @Override + public String getCategoryName() { + return STATIC_LIBRARY_CATEGORY_NAME; + } + + @Override + public ImmutableMap getTemplateVariables(RuleContext ruleContext) { + return ImmutableMap.of("base_name", ruleContext.getLabel().getName()); + } + }, + + PIC_STATIC_LIBRARY { + @Override + public String getCategoryName() { + return PIC_STATIC_LIBRARY_CATEGORY_NAME; + } + + @Override + public ImmutableMap getTemplateVariables(RuleContext ruleContext) { + return ImmutableMap.of("base_name", ruleContext.getLabel().getName()); + } + }, + + ALWAYS_LINK_STATIC_LIBRARY { + @Override + public String getCategoryName() { + return ALWAYS_LINK_STATIC_LIBRARY_CATEGORY_NAME; + } + + @Override + public ImmutableMap getTemplateVariables(RuleContext ruleContext) { + return ImmutableMap.of("base_name", ruleContext.getLabel().getName()); + } + }, + + ALWAYS_LINK_PIC_STATIC_LIBRARY { + @Override + public String getCategoryName() { + return ALWAYS_LINK_PIC_STATIC_LIBRARY_CATEGORY_NAME; + } + + @Override + public ImmutableMap getTemplateVariables(RuleContext ruleContext) { + return ImmutableMap.of("base_name", ruleContext.getLabel().getName()); + } + }, + + DYNAMIC_LIBRARY { + @Override + public String getCategoryName() { + return DYNAMIC_LIBRARY_CATEGORY_NAME; + } + + @Override + public ImmutableMap getTemplateVariables(RuleContext ruleContext) { + return ImmutableMap.of("base_name", ruleContext.getLabel().getName()); + } + }, + + EXECUTABLE { + @Override + public String getCategoryName() { + return EXECUTABLE_CATEGORY_NAME; + } + + @Override + public ImmutableMap getTemplateVariables(RuleContext ruleContext) { + return ImmutableMap.of("base_name", ruleContext.getLabel().getName()); + } + }, + + INTERFACE { + @Override + public String getCategoryName() { + return INTERFACE_CATEGORY_NAME; + } + + @Override + public ImmutableMap getTemplateVariables(RuleContext ruleContext) { + return ImmutableMap.of("base_name", ruleContext.getLabel().getName()); + } + }, + + DEBUG_SYMBOLS { + @Override + public String getCategoryName() { + return DEBUG_SYMBOL_CATEGORY_NAME; + } + + @Override + public ImmutableMap getTemplateVariables(RuleContext ruleContext) { + return ImmutableMap.of(); + } + }; + + /** Error for template evaluation failure. */ + @VisibleForTesting + public static final String TEMPLATE_EVAL_FAILURE = + "Error evaluating file name pattern for artifact category %s: %s"; + + @VisibleForTesting public static final String STATIC_LIBRARY_CATEGORY_NAME = "static_library"; + + @VisibleForTesting + public static final String PIC_STATIC_LIBRARY_CATEGORY_NAME = "pic_static_library"; + + @VisibleForTesting + public static final String ALWAYS_LINK_STATIC_LIBRARY_CATEGORY_NAME = "alwayslink_static_library"; + + @VisibleForTesting + public static final String ALWAYS_LINK_PIC_STATIC_LIBRARY_CATEGORY_NAME = + "alwayslink_pic_static_library"; + + @VisibleForTesting public static final String DYNAMIC_LIBRARY_CATEGORY_NAME = "dynamic_library"; + @VisibleForTesting public static final String EXECUTABLE_CATEGORY_NAME = "executable"; + @VisibleForTesting public static final String INTERFACE_CATEGORY_NAME = "interface_library"; + private static final String DEBUG_SYMBOL_CATEGORY_NAME = "debug_symbol"; + + /** Returns the name of the category. */ + public abstract String getCategoryName(); + + /** Returns an artifact given a templated name. */ + public Artifact getArtifactForName(String artifactName, RuleContext ruleContext) { + PathFragment name = + new PathFragment(ruleContext.getLabel().getName()).replaceName(artifactName); + return ruleContext.getPackageRelativeArtifact( + name, ruleContext.getConfiguration().getBinDirectory()); + } + + /** + * Returns a map of candidate template variables to their values. For example, the entry (foo, + * bar) indicates that the crosstool artifact name "library_%{foo}.extension" should be templated + * to "library_bar.extension". + */ + public abstract ImmutableMap getTemplateVariables(RuleContext ruleContext); +} 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 f202ce123c..eef2b319de 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 @@ -156,8 +156,9 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { return CcBinary.init(semantics, context, /*fake =*/ false, /*isTest =*/ false); } - public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleContext, boolean fake, - boolean isTest) throws InterruptedException { + public static ConfiguredTarget init( + CppSemantics semantics, RuleContext ruleContext, boolean fake, boolean isTest) + throws InterruptedException, RuleErrorException { ruleContext.checkSrcsSamePackage(true); FeatureConfiguration featureConfiguration = CcCommon.configureFeatures(ruleContext); CcCommon common = new CcCommon(ruleContext); 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 e7db2a5273..896ead8c13 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 @@ -106,12 +106,16 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { return builder.build(); } - public static void init(CppSemantics semantics, RuleContext ruleContext, - RuleConfiguredTargetBuilder targetBuilder, LinkTargetType linkType, + public static void init( + CppSemantics semantics, + RuleContext ruleContext, + RuleConfiguredTargetBuilder targetBuilder, + LinkTargetType linkType, boolean neverLink, boolean linkStatic, boolean collectLinkstamp, - boolean addDynamicRuntimeInputArtifactsToRunfiles) { + boolean addDynamicRuntimeInputArtifactsToRunfiles) + throws RuleErrorException { FeatureConfiguration featureConfiguration = CcCommon.configureFeatures(ruleContext); final CcCommon common = new CcCommon(ruleContext); PrecompiledFiles precompiledFiles = new PrecompiledFiles(ruleContext); @@ -186,19 +190,19 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { // doesn't support it, then register an action which complains when triggered, // which only happens when some rule explicitly depends on the dynamic library. if (!createDynamicLibrary && !supportsDynamicLinker) { - Artifact solibArtifact = CppHelper.getLinkedArtifact( - ruleContext, LinkTargetType.DYNAMIC_LIBRARY); + Artifact solibArtifact = + CppHelper.getLinuxLinkedArtifact(ruleContext, LinkTargetType.DYNAMIC_LIBRARY); ruleContext.registerAction(new FailAction(ruleContext.getActionOwner(), ImmutableList.of(solibArtifact), "Toolchain does not support dynamic linking")); } else if (!createDynamicLibrary && ruleContext.attributes().isConfigurable("srcs", BuildType.LABEL_LIST)) { - // If "srcs" is configurable, the .so output is always declared because the logic that - // determines implicit outs doesn't know which value of "srcs" will ultimately get chosen. Here, - // where we *do* have the correct value, it may not contain any source files to generate an - // .so with. If that's the case, register a fake generating action to prevent a "no generating - // action for this artifact" error. - Artifact solibArtifact = CppHelper.getLinkedArtifact( - ruleContext, LinkTargetType.DYNAMIC_LIBRARY); + // If "srcs" is configurable, the .so output is always declared because the logic that + // determines implicit outs doesn't know which value of "srcs" will ultimately get chosen. + // Here, where we *do* have the correct value, it may not contain any source files to + // generate an .so with. If that's the case, register a fake generating action to prevent + // a "no generating action for this artifact" error. + Artifact solibArtifact = + CppHelper.getLinuxLinkedArtifact(ruleContext, LinkTargetType.DYNAMIC_LIBRARY); ruleContext.registerAction(new FailAction(ruleContext.getActionOwner(), ImmutableList.of(solibArtifact), "configurable \"srcs\" triggers an implicit .so output " + "even though there are no sources to compile in this configuration")); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java index 401fefd881..40b1b8b7df 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java @@ -39,6 +39,7 @@ 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.packages.BuildType; +import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariablesExtension; import com.google.devtools.build.lib.rules.cpp.CppConfiguration.HeadersCheckingMode; @@ -134,7 +135,6 @@ public final class CcLibraryHelper { Link.LinkTargetType.ALWAYS_LINK_PIC_STATIC_LIBRARY.getActionName(), Link.LinkTargetType.EXECUTABLE.getActionName())); - private final FileTypeSet sourceTypeSet; private final Set actionConfigSet; @@ -851,8 +851,10 @@ public final class CcLibraryHelper { /** * Create the C++ compile and link actions, and the corresponding C++-related providers. + * + * @throws RuleErrorException */ - public Info build() { + public Info build() throws RuleErrorException { // Fail early if there is no lipo context collector on the rule - otherwise we end up failing // in lipo optimization. Preconditions.checkState( @@ -1038,14 +1040,16 @@ public final class CcLibraryHelper { if (ruleContext.attributes().get("alwayslink", Type.BOOLEAN)) { archiveFile.add( - CppHelper.getLinkedArtifact(ruleContext, Link.LinkTargetType.ALWAYS_LINK_STATIC_LIBRARY)); + CppHelper.getLinuxLinkedArtifact( + ruleContext, Link.LinkTargetType.ALWAYS_LINK_STATIC_LIBRARY)); } else { - archiveFile.add(CppHelper.getLinkedArtifact(ruleContext, Link.LinkTargetType.STATIC_LIBRARY)); + archiveFile.add( + CppHelper.getLinuxLinkedArtifact(ruleContext, Link.LinkTargetType.STATIC_LIBRARY)); } if (CppRuleClasses.shouldCreateDynamicLibrary(ruleContext.attributes())) { dynamicLibrary.add( - CppHelper.getLinkedArtifact(ruleContext, Link.LinkTargetType.DYNAMIC_LIBRARY)); + CppHelper.getLinuxLinkedArtifact(ruleContext, Link.LinkTargetType.DYNAMIC_LIBRARY)); } outputGroups.put("archive", archiveFile.build()); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java index b48e03a3f7..f935b4faac 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java @@ -26,10 +26,13 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; 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.CToolchain.Tool; import java.io.IOException; import java.io.ObjectInputStream; import java.io.Serializable; @@ -67,7 +70,7 @@ public class CcToolchainFeatures implements Serializable { /** * Thrown when a flag value cannot be expanded under a set of build variables. - * + * *

This happens for example when a flag references a variable that is not provided by the * action, or when a flag group references multiple variables of sequence type. */ @@ -76,13 +79,17 @@ public class CcToolchainFeatures implements Serializable { super(message); } } - + + /** Error message thrown when a toolchain does not provide a required artifact_name_pattern. */ + public static final String MISSING_ARTIFACT_NAME_PATTERN_ERROR_TEMPLATE = + "Toolchain must provide artifact_name_pattern for category %s"; + /** * A piece of a single string value. - * - *

A single value can contain a combination of text and variables (for example - * "-f %{var1}/%{var2}"). We split the string into chunks, where each chunk represents either a - * text snippet, or a variable that is to be replaced. + * + *

A single value can contain a combination of text and variables (for example "-f + * %{var1}/%{var2}"). We split the string into chunks, where each chunk represents either a text + * snippet, or a variable that is to be replaced. */ interface StringChunk { @@ -146,22 +153,21 @@ public class CcToolchainFeatures implements Serializable { /** * Parser for toolchain string values. - * + * *

A string value contains a snippet of text supporting variable expansion. For example, a - * string value "-f %{var1}/%{var2}" will expand the values of the variables "var1" and "var2" - * in the corresponding places in the string. - * - *

The {@code StringValueParser} takes a string and parses it into a list of - * {@link StringChunk} objects, where each chunk represents either a snippet of text or a - * variable to be expanded. In the above example, the resulting chunks would be - * ["-f ", var1, "/", var2]. - * + * string value "-f %{var1}/%{var2}" will expand the values of the variables "var1" and "var2" in + * the corresponding places in the string. + * + *

The {@code StringValueParser} takes a string and parses it into a list of {@link + * StringChunk} objects, where each chunk represents either a snippet of text or a variable to be + * expanded. In the above example, the resulting chunks would be ["-f ", var1, "/", var2]. + * *

In addition to the list of chunks, the {@link StringValueParser} also provides the set of * variables necessary for the expansion of this flag via {@link #getUsedVariables}. - * + * *

To get a literal percent character, "%%" can be used in the string. */ - private static class StringValueParser { + static class StringValueParser { private final String value; @@ -173,22 +179,18 @@ public class CcToolchainFeatures implements Serializable { private final ImmutableList.Builder chunks = ImmutableList.builder(); private final ImmutableSet.Builder usedVariables = ImmutableSet.builder(); - private StringValueParser(String value) throws InvalidConfigurationException { + StringValueParser(String value) throws InvalidConfigurationException { this.value = value; parse(); } - /** - * @return the parsed chunks for this string. - */ - private ImmutableList getChunks() { + /** @return the parsed chunks for this string. */ + ImmutableList getChunks() { return chunks.build(); } - /** - * @return all variable names needed to expand this string. - */ - private ImmutableSet getUsedVariables() { + /** @return all variable names needed to expand this string. */ + ImmutableSet getUsedVariables() { return usedVariables.build(); } @@ -402,14 +404,15 @@ public class CcToolchainFeatures implements Serializable { /** * Expands all flags in this group and adds them to {@code commandLine}. - * + * *

The flags of the group will be expanded either: + * *

    - *
  • once, if there is no variable of sequence type in any of the group's flags, or
    • - *
  • for each element in the sequence, if there is one variable of sequence type within - * the flags.
    • + *
  • once, if there is no variable of sequence type in any of the group's flags, or + *
  • for each element in the sequence, if there is one variable of sequence type within the + * flags. *
- * + * *

Having more than a single variable of sequence type in a single flag group is not * supported. */ @@ -633,6 +636,7 @@ public class CcToolchainFeatures implements Serializable { this.configName = actionConfig.getConfigName(); this.actionName = actionConfig.getActionName(); this.tools = actionConfig.getToolList(); + ImmutableList.Builder flagSetBuilder = ImmutableList.builder(); for (CToolchain.FlagSet flagSet : actionConfig.getFlagSetList()) { if (!flagSet.getActionList().isEmpty()) { @@ -694,6 +698,57 @@ public class CcToolchainFeatures implements Serializable { } } } + + /** A description of how artifacts of a certain type are named. */ + @Immutable + private static class ArtifactNamePattern { + + private final ArtifactCategory artifactCategory; + private final ImmutableSet variables; + private final ImmutableList chunks; + + private ArtifactNamePattern(CToolchain.ArtifactNamePattern artifactNamePattern) + throws InvalidConfigurationException { + + ArtifactCategory foundCategory = null; + for (ArtifactCategory artifactCategory : ArtifactCategory.values()) { + if (artifactNamePattern.getCategoryName().equals(artifactCategory.getCategoryName())) { + foundCategory = artifactCategory; + } + } + if (foundCategory == null) { + throw new ExpansionException( + String.format( + "Artifact category %s not recognized", artifactNamePattern.getCategoryName())); + } + this.artifactCategory = foundCategory; + + StringValueParser parser = new StringValueParser(artifactNamePattern.getPattern()); + this.variables = parser.getUsedVariables(); + this.chunks = parser.getChunks(); + } + + /** Returns the ArtifactCategory for this ArtifactNamePattern. */ + ArtifactCategory getArtifactCategory() { + return this.artifactCategory; + } + + /** + * Returns the artifact name that pattern selects for a given rule. + */ + public String getArtifactName(RuleContext ruleContext) { + StringBuilder result = new StringBuilder(); + Variables.View artifactNameVariables = + new Variables.Builder() + .addAllVariables(artifactCategory.getTemplateVariables(ruleContext)) + .build() + .getView(variables); + for (StringChunk chunk : chunks) { + chunk.expand(artifactNameVariables.getVariables(), result); + } + return result.toString(); + } + } /** * Configured build variables usable by the toolchain configuration. @@ -1132,6 +1187,9 @@ public class CcToolchainFeatures implements Serializable { return actionConfig.getTool(enabledFeatureNames); } } + + /** All artifact name patterns defined in this feature configuration. */ + private final ImmutableList artifactNamePatterns; /** * All features and action configs in the order in which they were specified in the configuration. @@ -1194,6 +1252,7 @@ public class CcToolchainFeatures implements Serializable { * * @param toolchain the toolchain configuration as specified by the user. * @throws InvalidConfigurationException if the configuration has logical errors. + * @throws ArtifactNamePatternNotProvidedException */ @VisibleForTesting public CcToolchainFeatures(CToolchain toolchain) throws InvalidConfigurationException { @@ -1219,7 +1278,7 @@ public class CcToolchainFeatures implements Serializable { selectablesByName.put(actionConfig.getName(), actionConfig); actionConfigsByActionName.put(actionConfig.getActionName(), actionConfig); } - + this.selectables = selectablesBuilder.build(); this.selectablesByName = ImmutableMap.copyOf(selectablesByName); @@ -1228,6 +1287,14 @@ public class CcToolchainFeatures implements Serializable { this.actionConfigsByActionName = actionConfigsByActionName.build(); + ImmutableList.Builder artifactNamePatternsBuilder = + ImmutableList.builder(); + for (CToolchain.ArtifactNamePattern artifactNamePattern : + toolchain.getArtifactNamePatternList()) { + artifactNamePatternsBuilder.add(new ArtifactNamePattern(artifactNamePattern)); + } + this.artifactNamePatterns = artifactNamePatternsBuilder.build(); + // Next, we build up all forward references for 'implies' and 'requires' edges. ImmutableMultimap.Builder implies = ImmutableMultimap.builder(); @@ -1382,11 +1449,43 @@ public class CcToolchainFeatures implements Serializable { return featureNames; } + /** + * Returns the artifact selected by the toolchain for the given action type and action category, + * or null if the category is not supported by the action config. + */ + Artifact getArtifactForCategory(ArtifactCategory artifactCategory, RuleContext ruleContext) + throws ExpansionException { + ArtifactNamePattern patternForCategory = null; + for (ArtifactNamePattern artifactNamePattern : artifactNamePatterns) { + if (artifactNamePattern.getArtifactCategory() == artifactCategory) { + patternForCategory = artifactNamePattern; + } + } + if (patternForCategory == null) { + throw new ExpansionException( + String.format( + MISSING_ARTIFACT_NAME_PATTERN_ERROR_TEMPLATE, artifactCategory.getCategoryName())); + } + + String templatedName = patternForCategory.getArtifactName(ruleContext); + return artifactCategory.getArtifactForName(templatedName, ruleContext); + } + + /** Returns true if the toolchain defines an ArtifactNamePattern for the given category. */ + boolean hasPatternForArtifactCategory(ArtifactCategory artifactCategory) { + for (ArtifactNamePattern artifactNamePattern : artifactNamePatterns) { + if (artifactNamePattern.getArtifactCategory() == artifactCategory) { + return true; + } + } + return false; + } + /** * Implements the feature selection algorithm. - * - *

Feature selection is done by first enabling all features reachable by an 'implies' edge, - * and then iteratively pruning features that have unmet requirements. + * + *

Feature selection is done by first enabling all features reachable by an 'implies' edge, and + * then iteratively pruning features that have unmet requirements. */ private class FeatureSelection { 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 4a20c22d82..b3367f4048 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 @@ -324,10 +324,8 @@ public class CppHelper { src.getRoot()); } - /** - * Returns the linked artifact. - */ - public static Artifact getLinkedArtifact(RuleContext ruleContext, LinkTargetType linkType) { + /** Returns the linked artifact for linux. */ + public static Artifact getLinuxLinkedArtifact(RuleContext ruleContext, LinkTargetType linkType) { PathFragment name = new PathFragment(ruleContext.getLabel().getName()); if (linkType != LinkTargetType.EXECUTABLE) { name = name.replaceName("lib" + name.getBaseName() + linkType.getExtension()); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java index 38aa8b9299..13866e81d5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.FailAction; import com.google.devtools.build.lib.analysis.AnalysisEnvironment; import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleContext; @@ -26,7 +27,9 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.packages.BuildType; +import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.rules.cpp.CcCompilationOutputs.Builder; +import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.ExpansionException; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariablesExtension; import com.google.devtools.build.lib.rules.cpp.CppCompileAction.DotdFile; @@ -736,6 +739,49 @@ public final class CppModel { } } + /** + * Returns the linked artifact resulting from a linking of the given type. Consults the feature + * configuration to obtain an action_config that provides the artifact. If the feature + * configuration provides no artifact, uses a default. + * + *

We cannot assume that the feature configuration contains an action_config for the link + * action, because the linux link action depends on hardcoded values in + * LinkCommandLine.getRawLinkArgv(), which are applied on the condition that an action_config is + * not present. + * TODO(b/30393154): Assert that the given link action has an action_config. + * + * @throws RuleErrorException + */ + private Artifact getLinkedArtifact(LinkTargetType linkTargetType) throws RuleErrorException { + Artifact result = null; + Artifact linuxDefault = CppHelper.getLinuxLinkedArtifact(ruleContext, linkTargetType); + CcToolchainFeatures toolchain = CppHelper.getToolchain(ruleContext).getFeatures(); + + if (toolchain.hasPatternForArtifactCategory(linkTargetType.getLinkerOutput())) { + try { + result = toolchain.getArtifactForCategory(linkTargetType.getLinkerOutput(), ruleContext); + } catch (ExpansionException e) { + ruleContext.throwWithRuleError(e.getMessage()); + } + } else { + return linuxDefault; + } + // If the linked artifact is not the linux default, then a FailAction is generated for the + // linux default to satisfy the requirement of the implicit output. + // TODO(b/30132703): Remove the implicit outputs of cc_library. + if (!result.equals(linuxDefault)) { + ruleContext.registerAction( + new FailAction( + ruleContext.getActionOwner(), + ImmutableList.of(linuxDefault), + String.format( + "the given toolchain supports creation of %s instead of %s", + linuxDefault.getExecPathString(), result.getExecPathString()))); + } + + return result; + } + /** * Constructs the C++ linker actions. It generally generates two actions, one for a static library * and one for a dynamic library. If PIC is required for shared libraries, but not for binaries, @@ -745,8 +791,11 @@ public final class CppModel { * can be used for linking, but doesn't contain any executable code. This increases the number of * cache hits for link actions. Call {@link #setAllowInterfaceSharedObjects(boolean)} to enable * this behavior. + * + * @throws RuleErrorException */ - public CcLinkingOutputs createCcLinkActions(CcCompilationOutputs ccOutputs) { + public CcLinkingOutputs createCcLinkActions(CcCompilationOutputs ccOutputs) + throws RuleErrorException { // For now only handle static links. Note that the dynamic library link below ignores linkType. // TODO(bazel-team): Either support non-static links or move this check to setLinkType(). Preconditions.checkState(linkType.isStaticLibraryLink(), "can only handle static links"); @@ -775,7 +824,11 @@ public final class CppModel { // // Presumably, it is done this way because the .a file is an implicit output of every cc_library // rule, so we can't use ".pic.a" that in the always-PIC case. - Artifact linkedArtifact = CppHelper.getLinkedArtifact(ruleContext, linkType); + + // If the crosstool is configured to select an output artifact, we use that selection. + // Otherwise, we use linux defaults. + Artifact linkedArtifact = getLinkedArtifact(linkType); + CppLinkAction maybePicAction = newLinkActionBuilder(linkedArtifact) .addNonLibraryInputs(ccOutputs.getObjectFiles(usePicForBinaries)) @@ -796,7 +849,10 @@ public final class CppModel { ? LinkTargetType.ALWAYS_LINK_PIC_STATIC_LIBRARY : LinkTargetType.PIC_STATIC_LIBRARY; - Artifact picArtifact = CppHelper.getLinkedArtifact(ruleContext, picLinkType); + // If the crosstool is configured to select an output artifact, we use that selection. + // Otherwise, we use linux defaults. + Artifact picArtifact = getLinkedArtifact(picLinkType); + CppLinkAction picAction = newLinkActionBuilder(picArtifact) .addNonLibraryInputs(ccOutputs.getObjectFiles(true)) @@ -817,7 +873,9 @@ public final class CppModel { // Create dynamic library. Artifact soImpl; if (soImplArtifact == null) { - soImpl = CppHelper.getLinkedArtifact(ruleContext, LinkTargetType.DYNAMIC_LIBRARY); + // If the crosstool is configured to select an output artifact, we use that selection. + // Otherwise, we use linux defaults. + soImpl = getLinkedArtifact(LinkTargetType.DYNAMIC_LIBRARY); } else { soImpl = soImplArtifact; } @@ -826,7 +884,7 @@ public final class CppModel { Artifact soInterface = null; if (cppConfiguration.useInterfaceSharedObjects() && allowInterfaceSharedObjects) { soInterface = - CppHelper.getLinkedArtifact(ruleContext, LinkTargetType.INTERFACE_DYNAMIC_LIBRARY); + CppHelper.getLinuxLinkedArtifact(ruleContext, LinkTargetType.INTERFACE_DYNAMIC_LIBRARY); sonameLinkopts = ImmutableList.of("-Wl,-soname=" + SolibSymlinkAction.getDynamicLibrarySoname(soImpl.getRootRelativePath(), false)); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java index 38ab7e8ccf..8826b0acb7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java @@ -94,34 +94,50 @@ public abstract class Link { */ public enum LinkTargetType { /** A normal static archive. */ - STATIC_LIBRARY(".a", true, "c++-link-static-library"), + STATIC_LIBRARY(".a", true, "c++-link-static-library", ArtifactCategory.STATIC_LIBRARY), /** A static archive with .pic.o object files (compiled with -fPIC). */ - PIC_STATIC_LIBRARY(".pic.a", true, "c++-link-pic-static-library"), + PIC_STATIC_LIBRARY( + ".pic.a", true, "c++-link-pic-static-library", ArtifactCategory.PIC_STATIC_LIBRARY), /** An interface dynamic library. */ - INTERFACE_DYNAMIC_LIBRARY(".ifso", false, "c++-link-interface-dynamic-library"), + INTERFACE_DYNAMIC_LIBRARY( + ".ifso", false, "c++-link-interface-dynamic-library", ArtifactCategory.INTERFACE), /** A dynamic library. */ - DYNAMIC_LIBRARY(".so", false, "c++-link-dynamic-library"), + DYNAMIC_LIBRARY(".so", false, "c++-link-dynamic-library", ArtifactCategory.DYNAMIC_LIBRARY), /** A static archive without removal of unused object files. */ - ALWAYS_LINK_STATIC_LIBRARY(".lo", true, "c++-link-alwayslink-static-library"), + ALWAYS_LINK_STATIC_LIBRARY( + ".lo", + true, + "c++-link-alwayslink-static-library", + ArtifactCategory.ALWAYS_LINK_STATIC_LIBRARY), /** A PIC static archive without removal of unused object files. */ - ALWAYS_LINK_PIC_STATIC_LIBRARY(".pic.lo", true, "c++-link-alwayslink-pic-static-library"), + ALWAYS_LINK_PIC_STATIC_LIBRARY( + ".pic.lo", + true, + "c++-link-alwayslink-pic-static-library", + ArtifactCategory.ALWAYS_LINK_PIC_STATIC_LIBRARY), /** An executable binary. */ - EXECUTABLE("", false, "c++-link-executable"); + EXECUTABLE("", false, "c++-link-executable", ArtifactCategory.EXECUTABLE); private final String extension; private final boolean staticLibraryLink; private final String actionName; + private final ArtifactCategory linkerOutput; - private LinkTargetType(String extension, boolean staticLibraryLink, String actionName) { + private LinkTargetType( + String extension, + boolean staticLibraryLink, + String actionName, + ArtifactCategory linkerOutput) { this.extension = extension; this.staticLibraryLink = staticLibraryLink; this.actionName = actionName; + this.linkerOutput = linkerOutput; } public String getExtension() { @@ -132,6 +148,11 @@ public abstract class Link { return staticLibraryLink; } + /** Returns an {@code ArtifactCategory} identifying the artifact type this link action emits. */ + public ArtifactCategory getLinkerOutput() { + return linkerOutput; + } + /** * The name of a link action with this LinkTargetType, for the purpose of crosstool feature * selection. diff --git a/src/main/protobuf/crosstool_config.proto b/src/main/protobuf/crosstool_config.proto index bab618c2b0..0ce4984b8f 100644 --- a/src/main/protobuf/crosstool_config.proto +++ b/src/main/protobuf/crosstool_config.proto @@ -208,6 +208,20 @@ message CToolchain { repeated string execution_requirement = 3; } + // The name for an artifact of a given category of input or output artifacts + // to an action. + message ArtifactNamePattern { + // The category of artifacts that this selection applies to. This field + // is compared against a list of categories defined in bazel. Example + // categories include "linked_output" or "debug_symbols". An error is thrown + // if no category is matched. + required string category_name = 1; + // The pattern for creating the artifact for this selection. The given + // pattern is templated by bazel and then used to create an artifact in + // a target-specific directory in bazel-bin. + required string pattern = 2; + } + // An action config corresponds to a blaze action, and allows selection of // a tool based on activated features. Action configs come in two varieties: // automatic (the blaze action will exist whether or not the action config @@ -217,6 +231,7 @@ message CToolchain { // Action config activation occurs by the same semantics as features: a // feature can 'require' or 'imply' an action config in the same way that it // would another feature. + // Next ID: 8 message ActionConfig { // The name other features will use to activate this action config. Can // be the same as action_name. @@ -261,6 +276,7 @@ message CToolchain { repeated Feature feature = 50; repeated ActionConfig action_config = 53; + repeated ArtifactNamePattern artifact_name_pattern = 54; // The unique identifier of the toolchain within the crosstool release. It // must be possible to use this as a directory name in a path. @@ -401,7 +417,7 @@ message CToolchain { // why they are recorded here. repeated string debian_extra_requires = 33; - // Next free id: 54 + // Next free id: 55 } message ToolPath { diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index 85e6514a35..55f7f5f0e1 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD @@ -527,6 +527,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/build/lib:vfs", "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/rules/cpp", "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/common/options", "//src/main/protobuf:build_java_proto", diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java index 53e90d2a0c..a3c5af2bd7 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java @@ -209,6 +209,27 @@ public abstract class MockCcSupport { + " }" + "}"; + public static final String STATIC_LINK_AS_LIB_CONFIGURATION = + "" + + "artifact_name_pattern {" + + " category_name: 'static_library'" + + " pattern: '%{base_name}.lib'" + + "}"; + + public static final String STATIC_LINK_AS_DOT_A_CONFIGURATION = + "" + + "artifact_name_pattern {" + + " category_name: 'static_library'" + + " pattern: 'lib%{base_name}.a'" + + "}"; + + public static final String STATIC_LINK_BAD_TEMPLATE_CONFIGURATION = + "" + + "artifact_name_pattern {" + + " category_name: 'static_library'" + + " pattern: 'foo%{bad_variable}bar'" + + "}"; + /** Filter to remove implicit dependencies of C/C++ rules. */ private final Predicate