From 0284e822b276e7d1909aafb5e3732467d946d5ae Mon Sep 17 00:00:00 2001 From: Lukacs Berki Date: Fri, 5 Aug 2016 09:21:42 +0000 Subject: Avoid hard-coded extensions for compilation outputs. The multi-layered transformation (base -> base.pic -> base.pic.pcm -> base.pic.pcm.d) is kinda ugly, but is preferable to having 22 separate, mostly orthogonal artifact categories. It's actually untested what happens if we put these new name patterns into the CROSSTOOL file. -- MOS_MIGRATED_REVID=129423055 --- .../build/lib/rules/cpp/ArtifactCategory.java | 169 ++++----------------- .../build/lib/rules/cpp/CcToolchainFeatures.java | 14 +- .../lib/rules/cpp/CppCompileActionBuilder.java | 50 +++--- .../build/lib/rules/cpp/CppConfiguration.java | 27 +++- .../devtools/build/lib/rules/cpp/CppHelper.java | 25 ++- .../devtools/build/lib/rules/cpp/CppModel.java | 155 ++++++++++--------- .../google/devtools/build/lib/rules/cpp/Link.java | 7 +- .../rules/cpp/CcLibraryConfiguredTargetTest.java | 15 ++ 8 files changed, 209 insertions(+), 253 deletions(-) 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 index 04036faf6f..df58dbf9e2 100644 --- 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 @@ -10,153 +10,48 @@ // 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. +// 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"; + STATIC_LIBRARY("lib%{base_name}.a"), + PIC_STATIC_LIBRARY("lib%{base_name}.pic.a"), + ALWAYSLINK_STATIC_LIBRARY("lib%{base_name}.lo"), + ALWAYSLINK_PIC_STATIC_LIBRARY("lib%{base_name}.pic.lo"), + DYNAMIC_LIBRARY("lib%{base_name}.so"), + EXECUTABLE("%{base_name}"), + INTERFACE_LIBRARY("lib%{base_name}.ifso"), + PIC_FILE("%{output_name}.pic"), + INCLUDED_FILE_LIST("%{output_name}.d"), + OBJECT_FILE("%{output_name}.o"), + CPP_MODULE("%{output_name}.pcm"), + GENERATED_ASSEMBLY("%{output_name}.s"), + PROCESSED_HEADER("%{output_name}.processed"), + GENERATED_HEADER("%{output_name}.h"), + PREPROCESSED_C_SOURCE("%{output_name}.i"), + PREPROCESSED_CPP_SOURCE("%{output_name}.ii"), + COVERAGE_DATA_FILE("%{output_name}.gcno"); + + private final String defaultPattern; + + private ArtifactCategory(String defaultPattern) { + this.defaultPattern = defaultPattern; + } - @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"; + private ArtifactCategory() { + this.defaultPattern = null; + } /** 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()); + public String getCategoryName() { + return this.toString().toLowerCase(); } - /** - * 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); + public String getDefaultPattern() { + return defaultPattern; + } } 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 b6f2716f88..7f372b5d91 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,7 +26,6 @@ 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; @@ -735,11 +734,13 @@ public class CcToolchainFeatures implements Serializable { /** * Returns the artifact name that pattern selects for a given rule. */ - public String getArtifactName(RuleContext ruleContext) { + public String getArtifactName(RuleContext ruleContext, Map extraVariables) { StringBuilder result = new StringBuilder(); + PathFragment nameFragment = new PathFragment(ruleContext.getLabel().getName()); Variables.View artifactNameVariables = new Variables.Builder() - .addAllVariables(artifactCategory.getTemplateVariables(ruleContext)) + .addVariable("base_name", nameFragment.getBaseName()) + .addAllVariables(extraVariables) .build() .getView(variables); for (StringChunk chunk : chunks) { @@ -1452,8 +1453,8 @@ public class CcToolchainFeatures implements Serializable { * 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 { + String getArtifactNameForCategory(ArtifactCategory artifactCategory, RuleContext ruleContext, + Map extraVariables) throws ExpansionException { ArtifactNamePattern patternForCategory = null; for (ArtifactNamePattern artifactNamePattern : artifactNamePatterns) { if (artifactNamePattern.getArtifactCategory() == artifactCategory) { @@ -1466,8 +1467,7 @@ public class CcToolchainFeatures implements Serializable { MISSING_ARTIFACT_NAME_PATTERN_ERROR_TEMPLATE, artifactCategory.getCategoryName())); } - String templatedName = patternForCategory.getArtifactName(ruleContext); - return artifactCategory.getArtifactForName(templatedName, ruleContext); + return patternForCategory.getArtifactName(ruleContext, extraVariables); } /** Returns true if the toolchain defines an ArtifactNamePattern for the given category. */ 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 015d76d6a9..83607914ee 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 @@ -14,7 +14,6 @@ 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.Predicate; import com.google.common.base.Predicates; @@ -24,7 +23,6 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.actions.Root; 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; @@ -37,9 +35,7 @@ import com.google.devtools.build.lib.rules.cpp.CppCompileAction.DotdFile; import com.google.devtools.build.lib.rules.cpp.CppCompileAction.SpecialInputsHandler; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.util.Preconditions; -import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; - import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -376,8 +372,30 @@ public class CppCompileActionBuilder { return this; } - public CppCompileActionBuilder setOutputFile(Artifact outputFile) { + public CppCompileActionBuilder setOutputsForTesting(Artifact outputFile, Artifact dotdFile) { this.outputFile = outputFile; + this.dotdFile = dotdFile == null ? null : new DotdFile(dotdFile); + return this; + } + + public CppCompileActionBuilder setOutputs( + ArtifactCategory outputCategory, String outputName, boolean generateDotd) { + this.outputFile = CppHelper.getCompileOutputArtifact( + ruleContext, CppHelper.getCompileArtifactName(ruleContext, outputCategory, outputName)); + if (generateDotd) { + String dotdFileName = CppHelper.getDotdFileName(ruleContext, outputCategory, outputName); + if (configuration.getFragment(CppConfiguration.class).getInmemoryDotdFiles()) { + // Just set the path, no artifact is constructed + dotdFile = new DotdFile( + configuration.getBinDirectory().getExecPath() + .getRelative(CppHelper.getObjDirectory(ruleContext.getLabel())) + .getRelative(dotdFileName)); + } else { + dotdFile = new DotdFile(CppHelper.getCompileOutputArtifact(ruleContext, dotdFileName)); + } + } else { + dotdFile = null; + } return this; } @@ -404,28 +422,6 @@ public class CppCompileActionBuilder { return this; } - @VisibleForTesting - public CppCompileActionBuilder setDotdFileForTesting(Artifact dotdFile) { - this.dotdFile = new DotdFile(dotdFile); - return this; - } - - public CppCompileActionBuilder setDotdFile(PathFragment outputName, String extension) { - if (CppFileTypes.mustProduceDotdFile(outputName.toString())) { - if (configuration.getFragment(CppConfiguration.class).getInmemoryDotdFiles()) { - // Just set the path, no artifact is constructed - PathFragment file = FileSystemUtils.replaceExtension(outputName, extension); - Root root = configuration.getBinDirectory(); - dotdFile = new DotdFile(root.getExecPath().getRelative(file)); - } else { - dotdFile = new DotdFile(ruleContext.getRelatedArtifact(outputName, extension)); - } - } else { - dotdFile = null; - } - return this; - } - public DotdFile getDotdFile() { return this.dotdFile; } 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 1ba5a38807..eb077e95ab 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 @@ -49,6 +49,7 @@ 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.CToolchain.ActionConfig; +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.common.options.OptionsParsingException; @@ -58,6 +59,7 @@ import java.io.Serializable; import java.util.ArrayList; import java.util.EnumSet; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -694,6 +696,27 @@ public class CppConfiguration extends BuildConfiguration.Fragment { // feature configuration, and all crosstools have been converted. private CToolchain addLegacyFeatures(CToolchain toolchain) { CToolchain.Builder toolchainBuilder = CToolchain.newBuilder(); + toolchainBuilder.mergeFrom(toolchain); + + Set definedCategories = new HashSet<>(); + for (ArtifactNamePattern pattern : toolchainBuilder.getArtifactNamePatternList()) { + try { + definedCategories.add(ArtifactCategory.valueOf(pattern.getCategoryName().toUpperCase())); + } catch (IllegalArgumentException e) { + // Invalid category name, will be detected later. + continue; + } + } + + for (ArtifactCategory category : ArtifactCategory.values()) { + if (!definedCategories.contains(category) && category.getDefaultPattern() != null) { + toolchainBuilder.addArtifactNamePattern(ArtifactNamePattern.newBuilder() + .setCategoryName(category.toString().toLowerCase()) + .setPattern(category.getDefaultPattern()) + .build()); + } + } + ImmutableSet.Builder featuresBuilder = ImmutableSet.builder(); for (CToolchain.Feature feature : toolchain.getFeatureList()) { featuresBuilder.add(feature.getName()); @@ -701,8 +724,9 @@ public class CppConfiguration extends BuildConfiguration.Fragment { Set features = featuresBuilder.build(); if (features.contains(CppRuleClasses.NO_LEGACY_FEATURES)) { // The toolchain requested to not get any legacy features enabled. - return toolchain; + return toolchainBuilder.build(); } + try { if (!linkActionsAreConfigured(toolchain)) { @@ -960,7 +984,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment { // Can only happen if we change the proto definition without changing our configuration above. throw new RuntimeException(e); } - toolchainBuilder.mergeFrom(toolchain); return toolchainBuilder.build(); } 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 9233bd4d3e..8981256edc 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 @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.rules.cpp; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; @@ -47,13 +48,11 @@ import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.Preconditions; 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.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; - import javax.annotation.Nullable; /** @@ -605,4 +604,26 @@ public class CppHelper { builder.add(StaticallyLinkedMarkerProvider.class, new StaticallyLinkedMarkerProvider(true)); } } + + static Artifact getCompileOutputArtifact(RuleContext ruleContext, String outputName) { + PathFragment objectDir = getObjDirectory(ruleContext.getLabel()); + return ruleContext.getDerivedArtifact(objectDir.getRelative(outputName), + ruleContext.getConfiguration().getBinDirectory()); + } + + static String getCompileArtifactName(RuleContext ruleContext, ArtifactCategory category, + String base) { + return getToolchain(ruleContext).getFeatures().getArtifactNameForCategory( + category, ruleContext, ImmutableMap.of("output_name", base)); + } + + static String getDotdFileName(RuleContext ruleContext, ArtifactCategory outputCategory, + String outputName) { + String baseName = outputCategory == ArtifactCategory.OBJECT_FILE + || outputCategory == ArtifactCategory.PROCESSED_HEADER + ? outputName + : getCompileArtifactName(ruleContext, outputCategory, outputName); + + return getCompileArtifactName(ruleContext, ArtifactCategory.INCLUDED_FILE_LIST, baseName); + } } 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 13866e81d5..6086c36d55 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 @@ -61,6 +61,7 @@ import javax.annotation.Nullable; public final class CppModel { private final CppSemantics semantics; private final RuleContext ruleContext; + private final CcToolchainFeatures features; private final BuildConfiguration configuration; private final CppConfiguration cppConfiguration; @@ -90,6 +91,7 @@ public final class CppModel { this.semantics = semantics; configuration = ruleContext.getConfiguration(); cppConfiguration = ruleContext.getFragment(CppConfiguration.class); + features = CppHelper.getToolchain(ruleContext).getFeatures(); } private Artifact getDwoFile(Artifact outputFile) { @@ -487,12 +489,10 @@ public final class CppModel { CcCompilationOutputs.Builder result = new CcCompilationOutputs.Builder(); Preconditions.checkNotNull(context); AnalysisEnvironment env = ruleContext.getAnalysisEnvironment(); - PathFragment objectDir = CppHelper.getObjDirectory(ruleContext.getLabel()); - + if (shouldProvideHeaderModules()) { Artifact moduleMapArtifact = context.getCppModuleMap().getArtifact(); Label moduleMapLabel = Label.parseAbsoluteUnchecked(context.getCppModuleMap().getName()); - PathFragment outputName = getObjectOutputPath(moduleMapArtifact, objectDir); CppCompileActionBuilder builder = initializeCompileAction(moduleMapArtifact, moduleMapLabel, /*forInterface=*/ true); @@ -500,28 +500,31 @@ public final class CppModel { // - the compiled source file is the module map // - it creates a header module (.pcm file). createSourceAction( - outputName, + FileSystemUtils.removeExtension(semantics.getEffectiveSourcePath(moduleMapArtifact)) + .getPathString(), result, env, moduleMapArtifact, builder, - ".pcm", - ".pcm.d", + ArtifactCategory.CPP_MODULE, /*addObject=*/ false, /*enableCoverage=*/ false, /*generateDwo=*/ false, + CppFileTypes.mustProduceDotdFile(moduleMapArtifact.getFilename()), ImmutableMap.of()); } for (CppSource source : sourceFiles) { Artifact sourceArtifact = source.getSource(); Label sourceLabel = source.getLabel(); - PathFragment outputName = getObjectOutputPath(sourceArtifact, objectDir); + String outputName = FileSystemUtils.removeExtension( + semantics.getEffectiveSourcePath(sourceArtifact)).getPathString(); CppCompileActionBuilder builder = initializeCompileAction(sourceArtifact, sourceLabel, /*forInterface=*/ false); if (CppFileTypes.CPP_HEADER.matches(source.getSource().getExecPath())) { - createHeaderAction(outputName, result, env, builder); + createHeaderAction(outputName, result, env, builder, + CppFileTypes.mustProduceDotdFile(sourceArtifact.getFilename())); } else { createSourceAction( outputName, @@ -529,11 +532,11 @@ public final class CppModel { env, sourceArtifact, builder, - ".o", - ".d", + ArtifactCategory.OBJECT_FILE, /*addObject=*/ true, isCodeCoverageEnabled(), /*generateDwo=*/ cppConfiguration.useFission(), + CppFileTypes.mustProduceDotdFile(sourceArtifact.getFilename()), source.getBuildVariables()); } } @@ -542,11 +545,13 @@ public final class CppModel { return compilationOutputs; } - private void createHeaderAction(PathFragment outputName, Builder result, AnalysisEnvironment env, - CppCompileActionBuilder builder) { + private void createHeaderAction(String outputName, Builder result, AnalysisEnvironment env, + CppCompileActionBuilder builder, boolean generateDotd) { + String outputNameBase = CppHelper.getCompileArtifactName(ruleContext, + ArtifactCategory.GENERATED_HEADER, outputName); + builder - .setOutputFile(ruleContext.getRelatedArtifact(outputName, ".h.processed")) - .setDotdFile(outputName, ".h.d") + .setOutputs(ArtifactCategory.PROCESSED_HEADER, outputNameBase, generateDotd) // If we generate pic actions, we prefer the header actions to use the pic artifacts. .setPicMode(this.getGeneratePicActions()); setupCompileBuildVariables( @@ -565,16 +570,16 @@ public final class CppModel { } private void createSourceAction( - PathFragment outputName, + String outputName, CcCompilationOutputs.Builder result, AnalysisEnvironment env, Artifact sourceArtifact, CppCompileActionBuilder builder, - String outputExtension, - String dependencyFileExtension, + ArtifactCategory outputCategory, boolean addObject, boolean enableCoverage, boolean generateDwo, + boolean generateDotd, Map sourceSpecificBuildVariables) { PathFragment ccRelativeName = semantics.getEffectiveSourcePath(sourceArtifact); if (cppConfiguration.isLipoOptimization()) { @@ -592,18 +597,22 @@ public final class CppModel { Preconditions.checkState(generatePicAction || generateNoPicAction); if (fake) { boolean usePic = !generateNoPicAction; - createFakeSourceAction(outputName, result, env, builder, outputExtension, - dependencyFileExtension, addObject, ccRelativeName, sourceArtifact.getExecPath(), usePic); + createFakeSourceAction(outputName, result, env, builder, outputCategory, addObject, + ccRelativeName, sourceArtifact.getExecPath(), usePic, generateDotd); } else { // Create PIC compile actions (same as non-PIC, but use -fPIC and // generate .pic.o, .pic.d, .pic.gcno instead of .o, .d, .gcno.) if (generatePicAction) { - Artifact outputFile = ruleContext.getRelatedArtifact(outputName, ".pic" + outputExtension); + String picOutputBase = CppHelper.getCompileArtifactName(ruleContext, + ArtifactCategory.PIC_FILE, outputName); CppCompileActionBuilder picBuilder = copyAsPicBuilder( - builder, outputName, outputFile, dependencyFileExtension); - Artifact gcnoFile = - enableCoverage ? ruleContext.getRelatedArtifact(outputName, ".pic.gcno") : null; - Artifact dwoFile = generateDwo ? getDwoFile(outputFile) : null; + builder, picOutputBase, outputCategory, generateDotd); + String gcnoFileName = CppHelper.getCompileArtifactName(ruleContext, + ArtifactCategory.COVERAGE_DATA_FILE, picOutputBase); + Artifact gcnoFile = enableCoverage + ? CppHelper.getCompileOutputArtifact(ruleContext, gcnoFileName) + : null; + Artifact dwoFile = generateDwo ? getDwoFile(picBuilder.getOutputFile()) : null; setupCompileBuildVariables( picBuilder, @@ -617,7 +626,7 @@ public final class CppModel { if (maySaveTemps) { result.addTemps( createTempsActions(sourceArtifact, outputName, picBuilder, /*usePic=*/true, - ccRelativeName)); + /*generateDotd=*/ generateDotd, ccRelativeName)); } picBuilder.setGcnoFile(gcnoFile); @@ -644,15 +653,16 @@ public final class CppModel { } if (generateNoPicAction) { - Artifact noPicOutputFile = ruleContext.getRelatedArtifact(outputName, outputExtension); - builder - .setOutputFile(noPicOutputFile) - .setDotdFile(outputName, dependencyFileExtension); + Artifact noPicOutputFile = CppHelper.getCompileOutputArtifact(ruleContext, + CppHelper.getCompileArtifactName(ruleContext, outputCategory, outputName)); + builder.setOutputs(outputCategory, outputName, generateDotd); + String gcnoFileName = CppHelper.getCompileArtifactName(ruleContext, + ArtifactCategory.COVERAGE_DATA_FILE, outputName); // Create non-PIC compile actions Artifact gcnoFile = !cppConfiguration.isLipoOptimization() && enableCoverage - ? ruleContext.getRelatedArtifact(outputName, ".gcno") + ? CppHelper.getCompileOutputArtifact(ruleContext, gcnoFileName) : null; Artifact noPicDwoFile = generateDwo ? getDwoFile(noPicOutputFile) : null; @@ -673,6 +683,7 @@ public final class CppModel { outputName, builder, /*usePic=*/ false, + /*generateDotd*/ generateDotd, ccRelativeName)); } @@ -701,23 +712,26 @@ public final class CppModel { } } - private void createFakeSourceAction(PathFragment outputName, CcCompilationOutputs.Builder result, - AnalysisEnvironment env, CppCompileActionBuilder builder, String outputExtension, - String dependencyFileExtension, boolean addObject, PathFragment ccRelativeName, - PathFragment execPath, boolean usePic) { - if (usePic) { - outputExtension = ".pic" + outputExtension; - dependencyFileExtension = ".pic" + dependencyFileExtension; - } - Artifact outputFile = ruleContext.getRelatedArtifact(outputName, outputExtension); - PathFragment tempOutputName = - FileSystemUtils.replaceExtension( - outputFile.getExecPath(), ".temp" + outputExtension, outputExtension); + String getOutputNameBaseWith(String base, boolean usePic) { + return usePic + ? CppHelper.getCompileArtifactName(ruleContext, ArtifactCategory.PIC_FILE, base) + : base; + } + + private void createFakeSourceAction(String outputName, CcCompilationOutputs.Builder result, + AnalysisEnvironment env, CppCompileActionBuilder builder, + ArtifactCategory outputCategory, boolean addObject, PathFragment ccRelativeName, + PathFragment execPath, boolean usePic, boolean generateDotd) { + String outputNameBase = getOutputNameBaseWith(outputName, usePic); + String tempOutputName = ruleContext.getConfiguration().getBinFragment() + .getRelative(CppHelper.getObjDirectory(ruleContext.getLabel())) + .getRelative(CppHelper.getCompileArtifactName(ruleContext, outputCategory, + getOutputNameBaseWith(outputName + ".temp", usePic))) + .getPathString(); builder .setPicMode(usePic) - .setOutputFile(outputFile) - .setDotdFile(outputName, dependencyFileExtension) - .setTempOutputFile(tempOutputName); + .setOutputs(outputCategory, outputNameBase, generateDotd) + .setTempOutputFile(new PathFragment(tempOutputName)); setupCompileBuildVariables( builder, @@ -755,17 +769,18 @@ public final class CppModel { 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; + try { + String templatedName = features.getArtifactNameForCategory( + linkTargetType.getLinkerOutput(), ruleContext, ImmutableMap.of()); + PathFragment artifactFragment = new PathFragment(ruleContext.getLabel().getName()) + .getParentDirectory().getRelative(templatedName); + result = ruleContext.getPackageRelativeArtifact( + artifactFragment, ruleContext.getConfiguration().getBinDirectory()); + } catch (ExpansionException e) { + ruleContext.throwWithRuleError(e.getMessage()); } + // 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. @@ -953,13 +968,6 @@ public final class CppModel { .addNonLibraryInputs(context.getTransitiveCompilationPrerequisites()); } - /** - * Returns the output artifact path relative to the object directory. - */ - private PathFragment getObjectOutputPath(Artifact source, PathFragment objectDirectory) { - return objectDirectory.getRelative(semantics.getEffectiveSourcePath(source)); - } - /** * Creates a basic cpp compile action builder for source file. Configures options, * crosstool inputs, output and dotd file names, compilation context and copts. @@ -978,12 +986,12 @@ public final class CppModel { * changing output and dotd file names. */ private CppCompileActionBuilder copyAsPicBuilder(CppCompileActionBuilder builder, - PathFragment outputName, Artifact outputFile, String dependencyFileExtension) { + String outputName, ArtifactCategory outputCategory, + boolean generateDotd) { CppCompileActionBuilder picBuilder = new CppCompileActionBuilder(builder); picBuilder .setPicMode(true) - .setOutputFile(outputFile) - .setDotdFile(outputName, ".pic" + dependencyFileExtension); + .setOutputs(outputCategory, outputName, generateDotd); return picBuilder; } @@ -991,8 +999,9 @@ public final class CppModel { /** * Create the actions for "--save_temps". */ - private ImmutableList createTempsActions(Artifact source, PathFragment outputName, - CppCompileActionBuilder builder, boolean usePic, PathFragment ccRelativeName) { + private ImmutableList createTempsActions(Artifact source, String outputName, + CppCompileActionBuilder builder, boolean usePic, boolean generateDotd, + PathFragment ccRelativeName) { if (!cppConfiguration.getSaveTemps()) { return ImmutableList.of(); } @@ -1005,13 +1014,13 @@ public final class CppModel { return ImmutableList.of(); } - String iExt = isCFile ? ".i" : ".ii"; - String picExt = usePic ? ".pic" : ""; + ArtifactCategory category = isCFile + ? ArtifactCategory.PREPROCESSED_C_SOURCE : ArtifactCategory.PREPROCESSED_CPP_SOURCE; + + String outputArtifactNameBase = getOutputNameBaseWith(outputName, usePic); CppCompileActionBuilder dBuilder = new CppCompileActionBuilder(builder); - dBuilder - .setOutputFile(ruleContext.getRelatedArtifact(outputName, picExt + iExt)) - .setDotdFile(outputName, picExt + iExt + ".d"); + dBuilder.setOutputs(category, outputArtifactNameBase, generateDotd); setupCompileBuildVariables( dBuilder, usePic, @@ -1025,9 +1034,7 @@ public final class CppModel { ruleContext.registerAction(dAction); CppCompileActionBuilder sdBuilder = new CppCompileActionBuilder(builder); - sdBuilder - .setOutputFile(ruleContext.getRelatedArtifact(outputName, picExt + ".s")) - .setDotdFile(outputName, picExt + ".s.d"); + sdBuilder.setOutputs(ArtifactCategory.GENERATED_ASSEMBLY, outputArtifactNameBase, generateDotd); setupCompileBuildVariables( sdBuilder, usePic, 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 8826b0acb7..1e3e313ab0 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 @@ -21,7 +21,6 @@ import com.google.devtools.build.lib.collect.CollectionUtils; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink; import com.google.devtools.build.lib.util.FileTypeSet; - import java.util.Iterator; /** @@ -102,7 +101,7 @@ public abstract class Link { /** An interface dynamic library. */ INTERFACE_DYNAMIC_LIBRARY( - ".ifso", false, "c++-link-interface-dynamic-library", ArtifactCategory.INTERFACE), + ".ifso", false, "c++-link-interface-dynamic-library", ArtifactCategory.INTERFACE_LIBRARY), /** A dynamic library. */ DYNAMIC_LIBRARY(".so", false, "c++-link-dynamic-library", ArtifactCategory.DYNAMIC_LIBRARY), @@ -112,14 +111,14 @@ public abstract class Link { ".lo", true, "c++-link-alwayslink-static-library", - ArtifactCategory.ALWAYS_LINK_STATIC_LIBRARY), + ArtifactCategory.ALWAYSLINK_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", - ArtifactCategory.ALWAYS_LINK_PIC_STATIC_LIBRARY), + ArtifactCategory.ALWAYSLINK_PIC_STATIC_LIBRARY), /** An executable binary. */ EXECUTABLE("", false, "c++-link-executable", ArtifactCategory.EXECUTABLE); diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java index 9842db86e0..1c6f7548b3 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java @@ -292,6 +292,21 @@ public class CcLibraryConfiguredTargetTest extends BuildViewTestCase { assertThat(action.getArgv()).contains(archive.getExecPathString()); } + @Test + public void testObjectFileNamesCanBeSpecifiedInToolchain() throws Exception { + AnalysisMock.get() + .ccSupport() + .setupCrosstool(mockToolsConfig, + "artifact_name_pattern {" + + " category_name: 'object_file'" + + " pattern: '%{base_name}.obj'" + + "}"); + + useConfiguration(); + ConfiguredTarget hello = getConfiguredTarget("//hello:hello"); + assertThat(artifactByPath(getFilesToBuild(hello), ".a", "hello.obj")).isNotNull(); + } + @Test public void testArtifactSelectionBaseNameTemplating() throws Exception { AnalysisMock.get() -- cgit v1.2.3