diff options
21 files changed, 520 insertions, 163 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 09e4f212e4..eb4913ba5f 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 @@ -13,22 +13,24 @@ // limitations under the License package com.google.devtools.build.lib.rules.cpp; +import com.google.common.collect.ImmutableList; + /** * 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("lib", ".a"), - ALWAYSLINK_STATIC_LIBRARY("lib", ".lo"), - DYNAMIC_LIBRARY("lib", ".so"), - EXECUTABLE("", ""), - INTERFACE_LIBRARY("lib", ".ifso"), + STATIC_LIBRARY("lib", ".a", ".lib"), + ALWAYSLINK_STATIC_LIBRARY("lib", ".lo", ".lo.lib"), + DYNAMIC_LIBRARY("lib", ".so", ".dylib", ".dll"), + EXECUTABLE("", "", ".exe"), + INTERFACE_LIBRARY("lib", ".ifso", ".tbd", ".if.lib"), PIC_FILE("", ".pic"), INCLUDED_FILE_LIST("", ".d"), - OBJECT_FILE("", ".o"), + OBJECT_FILE("", ".o", ".obj"), PIC_OBJECT_FILE("", ".pic.o"), CPP_MODULE("", ".pcm"), - GENERATED_ASSEMBLY("", ".s"), + GENERATED_ASSEMBLY("", ".s", ".asm"), PROCESSED_HEADER("", ".processed"), GENERATED_HEADER("", ".h"), PREPROCESSED_C_SOURCE("", ".i"), @@ -40,10 +42,21 @@ public enum ArtifactCategory { private final String defaultPrefix; private final String defaultExtension; + // The extensions allowed for this artifact name pattern, Bazel should recognized them as + // corresponding file type in CppFileTypes.java + final ImmutableList<String> allowedExtensions; - ArtifactCategory(String prefix, String extension) { - this.defaultPrefix = prefix; - this.defaultExtension = extension; + ArtifactCategory( + String defaultPrefix, + String defaultExtension, + String... extraAllowedExtensions) { + this.defaultPrefix = defaultPrefix; + this.defaultExtension = defaultExtension; + this.allowedExtensions = + new ImmutableList.Builder<String>() + .add(defaultExtension) + .add(extraAllowedExtensions) + .build(); } /** Returns the name of the category. */ @@ -58,4 +71,8 @@ public enum ArtifactCategory { public String getDefaultExtension() { return defaultExtension; } + + public ImmutableList<String> getAllowedExtensions() { + return allowedExtensions; + } } 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 a9ccb7a46a..334035c29d 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 @@ -60,7 +60,6 @@ import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; import com.google.devtools.build.lib.rules.cpp.Link.LinkingMode; import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink; import com.google.devtools.build.lib.syntax.Type; -import com.google.devtools.build.lib.util.OsUtils; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; @@ -199,12 +198,16 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { // linkopt "-shared", which causes the result of linking to be a shared // library. In this case, the name of the executable target should end // in ".so" or "dylib" or ".dll". + Artifact binary; PathFragment binaryPath = PathFragment.create(ruleContext.getTarget().getName()); if (!isLinkShared(ruleContext)) { - binaryPath = PathFragment.create(binaryPath.getPathString() + OsUtils.executableExtension()); + binary = + CppHelper.getLinkedArtifact( + ruleContext, ccToolchain, ruleContext.getConfiguration(), LinkTargetType.EXECUTABLE); + } else { + binary = ruleContext.getBinArtifact(binaryPath); } - Artifact binary = ruleContext.getBinArtifact(binaryPath); if (isLinkShared(ruleContext) && !CppFileTypes.SHARED_LIBRARY.matches(binary.getFilename()) && !CppFileTypes.VERSIONED_SHARED_LIBRARY.matches(binary.getFilename())) { @@ -361,7 +364,11 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { // If we are using a toolchain supporting interface library and targeting Windows, we build // the interface library with the link action and add it to `interface_output` output group. if (CppHelper.useInterfaceSharedObjects(cppConfiguration, ccToolchain)) { - interfaceLibrary = ruleContext.getRelatedArtifact(binary.getRootRelativePath(), ".ifso"); + interfaceLibrary = CppHelper.getLinkedArtifact( + ruleContext, + ccToolchain, + ruleContext.getConfiguration(), + LinkTargetType.INTERFACE_DYNAMIC_LIBRARY); linkActionBuilder.setInterfaceOutput(interfaceLibrary); linkActionBuilder.addActionOutput(interfaceLibrary); } 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 f429affa17..f5d8e50eb7 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 @@ -218,12 +218,18 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { if (!createDynamicLibrary && !supportsDynamicLinker) { ImmutableList.Builder<Artifact> dynamicLibraries = ImmutableList.builder(); dynamicLibraries.add( - CppHelper.getLinuxLinkedArtifact( - ruleContext, ruleContext.getConfiguration(), LinkTargetType.NODEPS_DYNAMIC_LIBRARY)); + CppHelper.getLinkedArtifact( + ruleContext, + ccToolchain, + ruleContext.getConfiguration(), + LinkTargetType.NODEPS_DYNAMIC_LIBRARY)); if (CppHelper.useInterfaceSharedObjects(ccToolchain.getCppConfiguration(), ccToolchain)) { dynamicLibraries.add( - CppHelper.getLinuxLinkedArtifact( - ruleContext, ruleContext.getConfiguration(), LinkTargetType.INTERFACE_DYNAMIC_LIBRARY)); + CppHelper.getLinkedArtifact( + ruleContext, + ccToolchain, + ruleContext.getConfiguration(), + LinkTargetType.INTERFACE_DYNAMIC_LIBRARY)); } ruleContext.registerAction(new FailAction(ruleContext.getActionOwner(), dynamicLibraries.build(), "Toolchain does not support dynamic linking")); @@ -236,12 +242,18 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { // a "no generating action for this artifact" error. ImmutableList.Builder<Artifact> dynamicLibraries = ImmutableList.builder(); dynamicLibraries.add( - CppHelper.getLinuxLinkedArtifact( - ruleContext, ruleContext.getConfiguration(), LinkTargetType.NODEPS_DYNAMIC_LIBRARY)); + CppHelper.getLinkedArtifact( + ruleContext, + ccToolchain, + ruleContext.getConfiguration(), + LinkTargetType.NODEPS_DYNAMIC_LIBRARY)); if (CppHelper.useInterfaceSharedObjects(ccToolchain.getCppConfiguration(), ccToolchain)) { dynamicLibraries.add( - CppHelper.getLinuxLinkedArtifact( - ruleContext, ruleContext.getConfiguration(), LinkTargetType.INTERFACE_DYNAMIC_LIBRARY)); + CppHelper.getLinkedArtifact( + ruleContext, + ccToolchain, + ruleContext.getConfiguration(), + LinkTargetType.INTERFACE_DYNAMIC_LIBRARY)); } ruleContext.registerAction(new FailAction(ruleContext.getActionOwner(), dynamicLibraries.build(), "configurable \"srcs\" triggers an implicit .so output " 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 2ef3fecd4b..b2294e45f6 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 @@ -578,21 +578,24 @@ public final class CcLinkingHelper { * output groups. */ private void addLinkerOutputArtifacts( - Map<String, NestedSet<Artifact>> outputGroups, CcCompilationOutputs ccOutputs) { + Map<String, NestedSet<Artifact>> outputGroups, CcCompilationOutputs ccOutputs) + throws RuleErrorException { NestedSetBuilder<Artifact> archiveFile = new NestedSetBuilder<>(Order.STABLE_ORDER); NestedSetBuilder<Artifact> dynamicLibrary = new NestedSetBuilder<>(Order.STABLE_ORDER); if (ruleContext.attributes().get("alwayslink", Type.BOOLEAN)) { archiveFile.add( - CppHelper.getLinuxLinkedArtifact( + CppHelper.getLinkedArtifact( ruleContext, + ccToolchain, configuration, Link.LinkTargetType.ALWAYS_LINK_STATIC_LIBRARY, linkedArtifactNameSuffix)); } else { archiveFile.add( - CppHelper.getLinuxLinkedArtifact( + CppHelper.getLinkedArtifact( ruleContext, + ccToolchain, configuration, Link.LinkTargetType.STATIC_LIBRARY, linkedArtifactNameSuffix)); @@ -600,8 +603,9 @@ public final class CcLinkingHelper { if (!ruleContext.attributes().get("linkstatic", Type.BOOLEAN) && !ccOutputs.isEmpty()) { dynamicLibrary.add( - CppHelper.getLinuxLinkedArtifact( + CppHelper.getLinkedArtifact( ruleContext, + ccToolchain, configuration, Link.LinkTargetType.NODEPS_DYNAMIC_LIBRARY, linkedArtifactNameSuffix)); @@ -609,8 +613,9 @@ public final class CcLinkingHelper { if (CppHelper.useInterfaceSharedObjects(ccToolchain.getCppConfiguration(), ccToolchain) && emitInterfaceSharedObjects) { dynamicLibrary.add( - CppHelper.getLinuxLinkedArtifact( + CppHelper.getLinkedArtifact( ruleContext, + ccToolchain, configuration, LinkTargetType.INTERFACE_DYNAMIC_LIBRARY, linkedArtifactNameSuffix)); @@ -855,8 +860,9 @@ public final class CcLinkingHelper { if (CppHelper.useInterfaceSharedObjects(cppConfiguration, ccToolchain) && emitInterfaceSharedObjects) { soInterface = - CppHelper.getLinuxLinkedArtifact( + CppHelper.getLinkedArtifact( ruleContext, + ccToolchain, configuration, LinkTargetType.INTERFACE_DYNAMIC_LIBRARY, linkedArtifactNameSuffix); @@ -1002,10 +1008,6 @@ public final class CcLinkingHelper { */ private Artifact getLinkedArtifact(LinkTargetType linkTargetType) throws RuleErrorException { Artifact result = null; - Artifact linuxDefault = - CppHelper.getLinuxLinkedArtifact( - ruleContext, configuration, linkTargetType, linkedArtifactNameSuffix); - try { String maybePicName = ruleContext.getLabel().getName() + linkedArtifactNameSuffix; if (linkTargetType.picness() == Picness.PIC) { @@ -1032,6 +1034,9 @@ public final class CcLinkingHelper { // 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. + Artifact linuxDefault = + CppHelper.getLinuxLinkedArtifact( + ruleContext, configuration, linkTargetType, linkedArtifactNameSuffix); if (!result.equals(linuxDefault)) { ruleContext.registerAction( new FailAction( 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 e6a53e362e..80dca05475 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 @@ -38,6 +38,7 @@ import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec. import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; import com.google.devtools.build.lib.util.Pair; +import com.google.devtools.build.lib.util.StringUtil; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain; import java.io.IOException; @@ -954,6 +955,17 @@ public class CcToolchainFeatures implements Serializable { "Invalid toolchain configuration: Artifact category %s not recognized", artifactNamePattern.getCategoryName())); } + + String extension = artifactNamePattern.getExtension(); + if (!foundCategory.getAllowedExtensions().contains(extension)) { + throw new InvalidConfigurationException( + String.format( + "Unrecognized file extension '%s', allowed extensions are %s," + + " please check artifact_name_pattern configuration for %s in your CROSSTOOL.", + extension, + StringUtil.joinEnglishList(foundCategory.getAllowedExtensions(), "or", "'"), + foundCategory.getCategoryName())); + } this.artifactCategory = foundCategory; this.prefix = artifactNamePattern.getPrefix(); this.extension = artifactNamePattern.getExtension(); @@ -964,6 +976,14 @@ public class CcToolchainFeatures implements Serializable { return this.artifactCategory; } + public String getPrefix() { + return this.prefix; + } + + public String getExtension() { + return this.extension; + } + /** Returns the artifact name that this pattern selects. */ public String getArtifactName(String baseName) { return prefix + baseName + extension; @@ -1451,6 +1471,27 @@ public class CcToolchainFeatures implements Serializable { .getChild(patternForCategory.getArtifactName(output.getBaseName())).getPathString(); } + /** + * Returns the artifact name extension selected by the toolchain for the given artifact category. + * + * @throws InvalidConfigurationException if the category is not supported by the action config. + */ + String getArtifactNameExtensionForCategory(ArtifactCategory artifactCategory) + throws InvalidConfigurationException { + ArtifactNamePattern patternForCategory = null; + for (ArtifactNamePattern artifactNamePattern : artifactNamePatterns) { + if (artifactNamePattern.getArtifactCategory() == artifactCategory) { + patternForCategory = artifactNamePattern; + } + } + if (patternForCategory == null) { + throw new InvalidConfigurationException( + String.format( + MISSING_ARTIFACT_NAME_PATTERN_ERROR_TEMPLATE, artifactCategory.getCategoryName())); + } + return patternForCategory.getExtension(); + } + /** Returns true if the toolchain defines an ArtifactNamePattern for the given category. */ boolean hasPatternForArtifactCategory(ArtifactCategory artifactCategory) { for (ArtifactNamePattern artifactNamePattern : artifactNamePatterns) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java index 0c7fa7d795..6c31c3a0bc 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java @@ -96,8 +96,13 @@ public final class CppFileTypes { @Override public boolean apply(String path) { + if (PIC_ARCHIVE.matches(path) + || ALWAYS_LINK_LIBRARY.matches(path) + || path.endsWith(".if.lib")) { + return false; + } for (String ext : extensions) { - if (path.endsWith(ext) && !PIC_ARCHIVE.matches(path)) { + if (path.endsWith(ext)) { return true; } } @@ -117,12 +122,13 @@ public final class CppFileTypes { @Override public boolean apply(String path) { - return path.endsWith(ext) && !ALWAYS_LINK_PIC_LIBRARY.matches(path); + return (path.endsWith(ext) && !ALWAYS_LINK_PIC_LIBRARY.matches(path)) + || path.endsWith(".lo.lib"); } @Override public List<String> getExtensions() { - return ImmutableList.of(ext); + return ImmutableList.of(ext, ".lo.lib"); } }; @@ -133,12 +139,12 @@ public final class CppFileTypes { @Override public boolean apply(String path) { - return path.endsWith(ext) && !PIC_OBJECT_FILE.matches(path); + return (path.endsWith(ext) && !PIC_OBJECT_FILE.matches(path)) || path.endsWith(".obj"); } @Override public List<String> getExtensions() { - return ImmutableList.of(ext); + return ImmutableList.of(ext, ".obj"); } }; 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 07082ad5fc..b22ec96649 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 @@ -621,18 +621,49 @@ public class CppHelper { } /** - * Returns the linked artifact for linux. + * Returns the linked artifact. * * @param ruleContext the ruleContext to be used to scope the artifact * @param config the configuration to be used to scope the artifact * @param linkType the type of artifact, used to determine extension */ - public static Artifact getLinuxLinkedArtifact( - RuleContext ruleContext, BuildConfiguration config, LinkTargetType linkType) { - return getLinuxLinkedArtifact(ruleContext, config, linkType, ""); + public static Artifact getLinkedArtifact( + RuleContext ruleContext, + CcToolchainProvider ccToolchain, + BuildConfiguration config, + LinkTargetType linkType) + throws RuleErrorException { + return getLinkedArtifact( + ruleContext, ccToolchain, config, linkType, /* linkedArtifactNameSuffix= */ ""); + } + + /** Returns the linked artifact with the given suffix. */ + public static Artifact getLinkedArtifact( + RuleContext ruleContext, + CcToolchainProvider ccToolchain, + BuildConfiguration config, + LinkTargetType linkType, + String linkedArtifactNameSuffix) + throws RuleErrorException { + PathFragment name = PathFragment.create(ruleContext.getLabel().getName()); + try { + name = + name.replaceName( + getArtifactNameForCategory( + ruleContext, + ccToolchain, + linkType.getLinkerOutput(), + name.getBaseName() + + linkedArtifactNameSuffix + + linkType.getPicExtensionWhenApplicable())); + } catch (RuleErrorException e) { + ruleContext.throwWithRuleError("Cannot get linked artifact name: " + e.getMessage()); + } + + return ruleContext.getPackageRelativeArtifact( + name, config.getBinDirectory(ruleContext.getRule().getRepository())); } - /** Returns the linked artifact with the given suffix for linux. */ public static Artifact getLinuxLinkedArtifact( RuleContext ruleContext, BuildConfiguration config, @@ -641,7 +672,10 @@ public class CppHelper { PathFragment name = PathFragment.create(ruleContext.getLabel().getName()); if (linkType != LinkTargetType.EXECUTABLE) { name = name.replaceName( - "lib" + name.getBaseName() + linkedArtifactNameSuffix + linkType.getExtension()); + "lib" + name.getBaseName() + + linkedArtifactNameSuffix + + linkType.getPicExtensionWhenApplicable() + + linkType.getDefaultExtension()); } return ruleContext.getPackageRelativeArtifact( @@ -969,7 +1003,7 @@ public class CppHelper { objectDir.getRelative(outputName), sourceTreeArtifact.getRoot()); } - static String getArtifactNameForCategory( + public static String getArtifactNameForCategory( RuleContext ruleContext, CcToolchainProvider toolchain, ArtifactCategory category, diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java index f7cd57da1d..24167f01f2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java @@ -52,6 +52,7 @@ import com.google.devtools.build.lib.rules.cpp.LibrariesToLinkCollector.Collecte import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; import com.google.devtools.build.lib.rules.cpp.Link.LinkerOrArchiver; import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink; +import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; @@ -992,30 +993,34 @@ public class CppLinkActionBuilder { ImmutableSet<Artifact> expandedLinkerArtifactsNoLinkstamps = Sets.difference(expandedLinkerArtifacts, linkstampObjectArtifacts).immutableCopy(); - CcToolchainVariables variables = - LinkBuildVariables.setupVariables( - getLinkType().linkerOrArchiver().equals(LinkerOrArchiver.LINKER), - configuration, - output, - paramFile, - thinltoParamFile, - thinltoMergedObjectFile, - mustKeepDebug, - symbolCounts, - toolchain, - featureConfiguration, - useTestOnlyFlags, - isLtoIndexing, - toolchain.getInterfaceSoBuilder(), - interfaceOutput, - ltoOutputRootPrefix, - defFile, - fdoSupport, - collectedLibrariesToLink.getRuntimeLibrarySearchDirectories(), - collectedLibrariesToLink.getLibrariesToLink(), - collectedLibrariesToLink.getLibrarySearchDirectories()); - - buildVariablesBuilder.addAllNonTransitive(variables); + try { + CcToolchainVariables variables = + LinkBuildVariables.setupVariables( + getLinkType().linkerOrArchiver().equals(LinkerOrArchiver.LINKER), + configuration, + output, + paramFile, + thinltoParamFile, + thinltoMergedObjectFile, + mustKeepDebug, + symbolCounts, + toolchain, + featureConfiguration, + useTestOnlyFlags, + isLtoIndexing, + toolchain.getInterfaceSoBuilder(), + interfaceOutput, + ltoOutputRootPrefix, + defFile, + fdoSupport, + collectedLibrariesToLink.getRuntimeLibrarySearchDirectories(), + collectedLibrariesToLink.getLibrariesToLink(), + collectedLibrariesToLink.getLibrarySearchDirectories()); + buildVariablesBuilder.addAllNonTransitive(variables); + } catch (EvalException e) { + ruleContext.ruleError(e.getLocalizedMessage()); + } + for (VariablesExtension extraVariablesExtension : variablesExtensions) { extraVariablesExtension.addVariables(buildVariablesBuilder); } 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 76018356ea..42d13bbb66 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 @@ -96,7 +96,6 @@ public abstract class Link { public enum LinkTargetType { /** A normal static archive. */ STATIC_LIBRARY( - ".a", LinkerOrArchiver.ARCHIVER, "c++-link-static-library", Picness.NOPIC, @@ -105,7 +104,6 @@ public abstract class Link { /** An objc static archive. */ OBJC_ARCHIVE( - ".a", LinkerOrArchiver.ARCHIVER, "objc-archive", Picness.NOPIC, @@ -114,7 +112,6 @@ public abstract class Link { /** An objc fully linked static archive. */ OBJC_FULLY_LINKED_ARCHIVE( - ".a", LinkerOrArchiver.ARCHIVER, "objc-fully-link", Picness.NOPIC, @@ -123,7 +120,6 @@ public abstract class Link { /** An objc executable. */ OBJC_EXECUTABLE( - "", LinkerOrArchiver.LINKER, "objc-executable", Picness.NOPIC, @@ -132,7 +128,6 @@ public abstract class Link { /** An objc executable that includes objc++/c++ source. */ OBJCPP_EXECUTABLE( - "", LinkerOrArchiver.LINKER, "objc++-executable", Picness.NOPIC, @@ -141,7 +136,6 @@ public abstract class Link { /** A static archive with .pic.o object files (compiled with -fPIC). */ PIC_STATIC_LIBRARY( - ".pic.a", LinkerOrArchiver.ARCHIVER, "c++-link-static-library", Picness.PIC, @@ -150,7 +144,6 @@ public abstract class Link { /** An interface dynamic library. */ INTERFACE_DYNAMIC_LIBRARY( - ".ifso", LinkerOrArchiver.LINKER, "c++-link-dynamic-library", Picness.NOPIC, // Actually PIC but it's not indicated in the file name @@ -159,7 +152,6 @@ public abstract class Link { /** A dynamic library built from cc_library srcs. */ NODEPS_DYNAMIC_LIBRARY( - ".so", LinkerOrArchiver.LINKER, "c++-link-nodeps-dynamic-library", Picness.NOPIC, // Actually PIC but it's not indicated in the file name @@ -167,7 +159,6 @@ public abstract class Link { Executable.NOT_EXECUTABLE), /** A transitive dynamic library used for distribution. */ DYNAMIC_LIBRARY( - ".so", LinkerOrArchiver.LINKER, "c++-link-dynamic-library", Picness.NOPIC, // Actually PIC but it's not indicated in the file name @@ -176,7 +167,6 @@ public abstract class Link { /** A static archive without removal of unused object files. */ ALWAYS_LINK_STATIC_LIBRARY( - ".lo", LinkerOrArchiver.ARCHIVER, "c++-link-static-library", Picness.NOPIC, @@ -185,7 +175,6 @@ public abstract class Link { /** A PIC static archive without removal of unused object files. */ ALWAYS_LINK_PIC_STATIC_LIBRARY( - ".pic.lo", LinkerOrArchiver.ARCHIVER, "c++-link-static-library", Picness.PIC, @@ -194,14 +183,12 @@ public abstract class Link { /** An executable binary. */ EXECUTABLE( - "", LinkerOrArchiver.LINKER, "c++-link-executable", Picness.NOPIC, // Picness is not indicate in the file name ArtifactCategory.EXECUTABLE, Executable.EXECUTABLE); - private final String extension; private final LinkerOrArchiver linkerOrArchiver; private final String actionName; private final ArtifactCategory linkerOutput; @@ -209,13 +196,11 @@ public abstract class Link { private final Executable executable; LinkTargetType( - String extension, LinkerOrArchiver linkerOrArchiver, String actionName, Picness picness, ArtifactCategory linkerOutput, Executable executable) { - this.extension = extension; this.linkerOrArchiver = linkerOrArchiver; this.actionName = actionName; this.linkerOutput = linkerOutput; @@ -230,8 +215,12 @@ public abstract class Link { return picness; } - public String getExtension() { - return extension; + public String getPicExtensionWhenApplicable() { + return picness == Picness.PIC ? ".pic" : ""; + } + + public String getDefaultExtension() { + return linkerOutput.getDefaultExtension(); } public LinkerOrArchiver linkerOrArchiver() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java index 34d39c3690..de384b6bf9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java @@ -17,8 +17,10 @@ import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.SequenceBuilder; +import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.vfs.PathFragment; /** Enum covering all build variables we create for all various {@link CppLinkAction}. */ @@ -111,7 +113,7 @@ public enum LinkBuildVariables { FdoSupportProvider fdoSupport, Iterable<String> runtimeLibrarySearchDirectories, SequenceBuilder librariesToLink, - Iterable<String> librarySearchDirectories) { + Iterable<String> librarySearchDirectories) throws EvalException { CcToolchainVariables.Builder buildVariables = new CcToolchainVariables.Builder(); // symbol counting @@ -183,11 +185,17 @@ public enum LinkBuildVariables { configuration.getBinDirectory().getExecPathString() + ";" + configuration.getBinDirectory().getExecPath().getRelative(ltoOutputRootPrefix)); + String objectFileExtension; + try { + objectFileExtension = ccToolchainProvider.getFeatures() + .getArtifactNameExtensionForCategory(ArtifactCategory.OBJECT_FILE); + } catch (InvalidConfigurationException e) { + throw new EvalException(null, "artifact name pattern for object_file must be specified", e); + } buildVariables.addStringVariable( THINLTO_OBJECT_SUFFIX_REPLACE.getVariableName(), Iterables.getOnlyElement(CppFileTypes.LTO_INDEXING_OBJECT_FILE.getExtensions()) - + ";" - + Iterables.getOnlyElement(CppFileTypes.OBJECT_FILE.getExtensions())); + + ";" + objectFileExtension); if (thinltoMergedObjectFile != null) { buildVariables.addStringVariable( THINLTO_MERGED_OBJECT_FILE.getVariableName(), @@ -226,4 +234,4 @@ public enum LinkBuildVariables { fdoSupport.getFdoSupport().getLinkOptions(featureConfiguration, buildVariables); return buildVariables.build(); } -}
\ No newline at end of file +} 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 4cc1620711..1b18643978 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 @@ -478,7 +478,7 @@ public abstract class MockCcSupport { + "artifact_name_pattern {" + " category_name: 'static_library'" + " prefix: 'lib'" - + " extension: '.tweaked.a'" + + " extension: '.lib'" + "}"; public static final String STATIC_LINK_AS_DOT_A_CONFIGURATION = diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java index 32821eec44..f94c047667 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java @@ -45,7 +45,6 @@ import com.google.devtools.build.lib.rules.cpp.transitions.LipoDataTransitionRul import com.google.devtools.build.lib.rules.platform.PlatformRules; import com.google.devtools.build.lib.rules.repository.CoreWorkspaceRules; import com.google.devtools.build.lib.util.FileType; -import com.google.devtools.build.lib.util.OsUtils; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.PathFragment; @@ -134,7 +133,7 @@ public class CcCommonTest extends BuildViewTestCase { public void testEmptyBinary() throws Exception { ConfiguredTarget emptybin = getConfiguredTarget("//empty:emptybinary"); assertThat(baseNamesOf(getFilesToBuild(emptybin))) - .isEqualTo("emptybinary" + OsUtils.executableExtension()); + .isEqualTo("emptybinary"); } private List<String> getCopts(String target) throws Exception { 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 dd864bc08c..6dfc222072 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 @@ -17,6 +17,7 @@ package com.google.devtools.build.lib.rules.cpp; import static com.google.common.collect.Iterables.getOnlyElement; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.fail; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; @@ -29,6 +30,7 @@ import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.OutputGroupInfo; +import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.test.InstrumentedFilesProvider; import com.google.devtools.build.lib.analysis.util.AnalysisMock; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; @@ -37,6 +39,7 @@ import com.google.devtools.build.lib.packages.util.MockCcSupport; import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; import com.google.devtools.build.lib.util.FileType; +import com.google.devtools.build.lib.util.StringUtil; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig; import com.google.protobuf.TextFormat; @@ -64,15 +67,34 @@ public class CcLibraryConfiguredTargetTest extends BuildViewTestCase { @Before public final void createFiles() throws Exception { - scratch.file("hello/BUILD", - "cc_library(name = 'hello',", - " srcs = ['hello.cc'])", - "cc_library(name = 'hello_static',", - " srcs = ['hello.cc'],", - " linkstatic = 1)"); - scratch.file("hello/hello.cc", - "#include <stdio.h>", - "int hello_world() { printf(\"Hello, world!\\n\"); }"); + scratch.file( + "hello/BUILD", + "cc_library(", + " name = 'hello',", + " srcs = ['hello.cc'],", + ")", + "cc_library(", + " name = 'hello_static',", + " srcs = ['hello.cc'],", + " linkstatic = 1,", + ")", + "cc_library(", + " name = 'hello_alwayslink',", + " srcs = ['hello.cc'],", + " alwayslink = 1,", + ")", + "cc_binary(", + " name = 'hello_bin',", + " srcs = ['hello_main.cc'],", + ")"); + scratch.file( + "hello/hello.cc", + "#include <stdio.h>", + "int hello_world() { printf(\"Hello, world!\\n\"); }"); + scratch.file( + "hello/hello_main.cc", + "#include <stdio.h>", + "int main() { printf(\"Hello, world!\\n\"); }"); } private CppCompileAction getCppCompileAction(String label) throws Exception { @@ -100,6 +122,38 @@ public class CcLibraryConfiguredTargetTest extends BuildViewTestCase { .isNull(); } + public void checkWrongExtensionInArtifactNamePattern( + String categoryName, ImmutableList<String> correctExtensions) throws Exception { + AnalysisMock.get() + .ccSupport() + .setupCrosstool( + mockToolsConfig, + MockCcSupport.COPY_DYNAMIC_LIBRARIES_TO_BINARY_CONFIGURATION, + MockCcSupport.TARGETS_WINDOWS_CONFIGURATION, + "supports_interface_shared_objects: true", + "artifact_name_pattern {" + + " category_name: '" + + categoryName + + "'" + + " prefix: ''" + + " extension: '.wrong_ext'" + + "}"); + + try { + useConfiguration(); + fail("Should fail"); + } catch (InvalidConfigurationException e) { + assertThat(e) + .hasMessageThat() + .contains( + String.format( + "Unrecognized file extension '.wrong_ext', allowed " + + "extensions are %s, please check artifact_name_pattern configuration for %s " + + "in your CROSSTOOL.", + StringUtil.joinEnglishList(correctExtensions, "or", "'"), categoryName)); + } + } + @Test public void testDefinesAndMakeVariables() throws Exception { ConfiguredTarget l = scratchConfiguredTarget("a", "l", @@ -347,7 +401,7 @@ public class CcLibraryConfiguredTargetTest extends BuildViewTestCase { useConfiguration("--features=" + Link.LinkTargetType.STATIC_LIBRARY.getActionName()); ConfiguredTarget hello = getConfiguredTarget("//hello:hello"); Artifact archive = - FileType.filter(getFilesToBuild(hello), FileType.of(".tweaked.a")).iterator().next(); + FileType.filter(getFilesToBuild(hello), FileType.of(".lib")).iterator().next(); CppLinkAction action = (CppLinkAction) getGeneratingAction(archive); @@ -363,12 +417,123 @@ public class CcLibraryConfiguredTargetTest extends BuildViewTestCase { "artifact_name_pattern {" + " category_name: 'object_file'" + " prefix: ''" - + " extension: '.test.o'" + + " extension: '.obj'" + "}"); useConfiguration(); ConfiguredTarget hello = getConfiguredTarget("//hello:hello"); - assertThat(artifactByPath(getFilesToBuild(hello), ".a", ".test.o")).isNotNull(); + assertThat(artifactByPath(getFilesToBuild(hello), ".a", ".obj")).isNotNull(); + } + + @Test + public void testWindowsFileNamePatternsCanBeSpecifiedInToolchain() throws Exception { + AnalysisMock.get() + .ccSupport() + .setupCrosstool( + mockToolsConfig, + MockCcSupport.COPY_DYNAMIC_LIBRARIES_TO_BINARY_CONFIGURATION, + MockCcSupport.TARGETS_WINDOWS_CONFIGURATION, + "needsPic: false", + "supports_interface_shared_objects: true", + "artifact_name_pattern {" + + " category_name: 'object_file'" + + " prefix: ''" + + " extension: '.obj'" + + "}", + "artifact_name_pattern {" + + " category_name: 'static_library'" + + " prefix: ''" + + " extension: '.lib'" + + "}", + "artifact_name_pattern {" + + " category_name: 'alwayslink_static_library'" + + " prefix: ''" + + " extension: '.lo.lib'" + + "}", + "artifact_name_pattern {" + + " category_name: 'executable'" + + " prefix: ''" + + " extension: '.exe'" + + "}", + "artifact_name_pattern {" + + " category_name: 'dynamic_library'" + + " prefix: ''" + + " extension: '.dll'" + + "}", + "artifact_name_pattern {" + + " category_name: 'interface_library'" + + " prefix: ''" + + " extension: '.if.lib'" + + "}"); + + useConfiguration(); + + ConfiguredTarget hello = getConfiguredTarget("//hello:hello"); + Artifact helloObj = getBinArtifact("_objs/hello/hello/hello.obj", "//hello:hello"); + CppCompileAction helloObjAction = (CppCompileAction) getGeneratingAction(helloObj); + assertThat(helloObjAction).isNotNull(); + + Artifact helloLib = + FileType.filter(getFilesToBuild(hello), CppFileTypes.ARCHIVE).iterator().next(); + assertThat(helloLib.getExecPathString()).endsWith("hello.lib"); + + ConfiguredTarget helloAlwaysLink = getConfiguredTarget("//hello:hello_alwayslink"); + Artifact helloLibAlwaysLink = + FileType.filter(getFilesToBuild(helloAlwaysLink), CppFileTypes.ALWAYS_LINK_LIBRARY) + .iterator() + .next(); + assertThat(helloLibAlwaysLink.getExecPathString()).endsWith("hello_alwayslink.lo.lib"); + + ConfiguredTarget helloBin = getConfiguredTarget("//hello:hello_bin"); + Artifact helloBinExe = getFilesToBuild(helloBin).iterator().next(); + assertThat(helloBinExe.getExecPathString()).endsWith("hello_bin.exe"); + + assertThat( + artifactsToStrings( + getOutputGroup(hello, CcLinkingHelper.DYNAMIC_LIBRARY_OUTPUT_GROUP_NAME))) + .containsExactly("bin hello/hello.dll", "bin hello/hello.if.lib"); + } + + @Test + public void testWrongObjectFileArtifactNamePattern() throws Exception { + checkWrongExtensionInArtifactNamePattern( + "object_file", + ArtifactCategory.OBJECT_FILE.getAllowedExtensions()); + } + + @Test + public void testWrongStaticLibraryArtifactNamePattern() throws Exception { + checkWrongExtensionInArtifactNamePattern( + "static_library", + ArtifactCategory.STATIC_LIBRARY.getAllowedExtensions()); + } + + @Test + public void testWrongAlwayslinkStaticLibraryArtifactNamePattern() throws Exception { + checkWrongExtensionInArtifactNamePattern( + "alwayslink_static_library", + ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY.getAllowedExtensions()); + } + + @Test + public void testWrongExecutableArtifactNamePattern() throws Exception { + checkWrongExtensionInArtifactNamePattern( + "executable", + ArtifactCategory.EXECUTABLE.getAllowedExtensions()); + } + + @Test + public void testWrongDynamicLibraryArtifactNamePattern() throws Exception { + checkWrongExtensionInArtifactNamePattern( + "dynamic_library", + ArtifactCategory.DYNAMIC_LIBRARY.getAllowedExtensions()); + } + + @Test + public void testWrongInterfaceLibraryArtifactNamePattern() throws Exception { + checkWrongExtensionInArtifactNamePattern( + "interface_library", + ArtifactCategory.INTERFACE_LIBRARY.getAllowedExtensions()); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java index a780425d10..f8c29e00e8 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java @@ -1742,7 +1742,7 @@ public class CcToolchainFeaturesTest extends FoundationTestCase { "artifact_name_pattern {", "category_name: 'static_library'", "prefix: 'foo'", - "extension: 'bar'}"); + "extension: '.a'}"); toolchainFeatures.getArtifactNameForCategory(ArtifactCategory.DYNAMIC_LIBRARY, "output_name"); fail("Should throw InvalidConfigurationException."); } catch (InvalidConfigurationException e) { @@ -1754,4 +1754,32 @@ public class CcToolchainFeaturesTest extends FoundationTestCase { ArtifactCategory.DYNAMIC_LIBRARY.toString().toLowerCase())); } } + + @Test + public void testGetArtifactNameExtensionForCategory() throws Exception { + CcToolchainFeatures toolchainFeatures = + buildFeatures( + "artifact_name_pattern {", + " category_name: 'object_file'", + " prefix: ''", + " extension: '.obj'", + "}", + "artifact_name_pattern {", + " category_name: 'executable'", + " prefix: ''", + " extension: ''", + "}", + "artifact_name_pattern {", + " category_name: 'static_library'", + " prefix: ''", + " extension: '.a'", + "}"); + assertThat(toolchainFeatures.getArtifactNameExtensionForCategory(ArtifactCategory.OBJECT_FILE)) + .isEqualTo(".obj"); + assertThat(toolchainFeatures.getArtifactNameExtensionForCategory(ArtifactCategory.EXECUTABLE)) + .isEmpty(); + assertThat( + toolchainFeatures.getArtifactNameExtensionForCategory(ArtifactCategory.STATIC_LIBRARY)) + .isEqualTo(".a"); + } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java index fda3b53507..4c3680d974 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java @@ -53,7 +53,6 @@ import com.google.devtools.build.lib.rules.cpp.Link.LinkerOrArchiver; import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink; import com.google.devtools.build.lib.testutil.TestUtils; import com.google.devtools.build.lib.util.OS; -import com.google.devtools.build.lib.util.OsUtils; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; @@ -179,8 +178,7 @@ public class CppLinkActionTest extends BuildViewTestCase { scratch.file("x/some-other-dir/qux.so"); ConfiguredTarget configuredTarget = getConfiguredTarget("//x:foo"); - CppLinkAction linkAction = (CppLinkAction) getGeneratingAction(configuredTarget, "x/foo" - + OsUtils.executableExtension()); + CppLinkAction linkAction = (CppLinkAction) getGeneratingAction(configuredTarget, "x/foo"); List<String> arguments = linkAction.getLinkCommandLine().arguments(); @@ -206,7 +204,7 @@ public class CppLinkActionTest extends BuildViewTestCase { ConfiguredTarget configuredTarget = getConfiguredTarget("//x:foo"); CppLinkAction linkAction = (CppLinkAction) - getGeneratingAction(configuredTarget, "x/foo" + OsUtils.executableExtension()); + getGeneratingAction(configuredTarget, "x/foo"); Iterable<? extends VariableValue> runtimeLibrarySearchDirectories = linkAction @@ -238,9 +236,8 @@ public class CppLinkActionTest extends BuildViewTestCase { ConfiguredTarget configuredTarget = getConfiguredTarget("//x:a"); String cpu = CrosstoolConfigurationHelper.defaultCpu(); - String extension = OsUtils.executableExtension(); CppLinkAction linkAction = - (CppLinkAction) getGeneratingAction(configuredTarget, "x/a" + extension); + (CppLinkAction) getGeneratingAction(configuredTarget, "x/a"); assertThat(artifactsToStrings(linkAction.getInputs())) .contains("bin _solib_" + cpu + "/libx_Sliba.ifso"); assertThat(linkAction.getArguments()) @@ -251,7 +248,7 @@ public class CppLinkActionTest extends BuildViewTestCase { .contains("bin _solib_" + cpu + "/libx_Sliba.so"); configuredTarget = getConfiguredTarget("//x:b"); - linkAction = (CppLinkAction) getGeneratingAction(configuredTarget, "x/b" + extension); + linkAction = (CppLinkAction) getGeneratingAction(configuredTarget, "x/b"); assertThat(artifactsToStrings(linkAction.getInputs())).contains("bin x/_objs/b/x/a.pic.o"); runfilesProvider = configuredTarget.getProvider(RunfilesProvider.class); assertThat(artifactsToStrings(runfilesProvider.getDefaultRunfiles().getArtifacts())) @@ -809,7 +806,7 @@ public class CppLinkActionTest extends BuildViewTestCase { for (LinkTargetType linkType : targetTypesToTest) { scratch.deleteFile("dummyRuleContext/BUILD"); - Artifact output = scratchArtifact("output." + linkType.getExtension()); + Artifact output = scratchArtifact("output." + linkType.getDefaultExtension()); CppLinkActionBuilder builder = createLinkBuilder( diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java index bc463b09ab..874bf638a0 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java @@ -25,7 +25,6 @@ import com.google.devtools.build.lib.packages.util.MockCcSupport; import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.LibraryToLinkValue; import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.VariableValue; import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; -import com.google.devtools.build.lib.util.OsUtils; import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; @@ -106,7 +105,7 @@ public class LinkBuildVariablesTest extends LinkBuildVariablesTestCase { CcToolchainVariables variables = getLinkBuildVariables(target, Link.LinkTargetType.EXECUTABLE); String variableValue = getVariableValue(variables, LinkBuildVariables.LINKER_PARAM_FILE.getVariableName()); - assertThat(variableValue).matches(".*bin/x/bin" + OsUtils.executableExtension() + "-2.params$"); + assertThat(variableValue).matches(".*bin/x/bin" + "-2.params$"); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java index 5bc6ce32ae..262cb7de5d 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java @@ -22,7 +22,6 @@ import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.util.OsUtils; import org.junit.Test; import org.junit.runner.RunWith; @@ -56,7 +55,7 @@ public class ProtoLangToolchainTest extends BuildViewTestCase { assertThat(toolchain.commandLine()).isEqualTo("cmd-line"); assertThat(toolchain.pluginExecutable().getExecutable().getRootRelativePathString()) - .isEqualTo("x/plugin" + OsUtils.executableExtension()); + .isEqualTo("x/plugin"); TransitiveInfoCollection runtimes = toolchain.runtime(); assertThat(runtimes.getLabel()).isEqualTo(Label.parseAbsolute("//x:runtime")); diff --git a/src/test/py/bazel/bazel_windows_cpp_test.py b/src/test/py/bazel/bazel_windows_cpp_test.py index 71d8231dc2..1b85d5f1df 100644 --- a/src/test/py/bazel/bazel_windows_cpp_test.py +++ b/src/test/py/bazel/bazel_windows_cpp_test.py @@ -140,8 +140,8 @@ class BazelWindowsCppTest(test_base.TestBase): # TODO(pcloudy): change suffixes to .lib and .dll after making DLL # extensions correct on # Windows. - import_library = os.path.join(bazel_bin, 'libA.ifso') - shared_library = os.path.join(bazel_bin, 'libA.so') + import_library = os.path.join(bazel_bin, 'A.if.lib') + shared_library = os.path.join(bazel_bin, 'A.dll') def_file = os.path.join(bazel_bin, 'A.def') self.assertTrue(os.path.exists(import_library)) self.assertTrue(os.path.exists(shared_library)) @@ -161,8 +161,8 @@ class BazelWindowsCppTest(test_base.TestBase): # TODO(pcloudy): change suffixes to .lib and .dll after making DLL # extensions correct on # Windows. - import_library = os.path.join(bazel_bin, 'libB.ifso') - shared_library = os.path.join(bazel_bin, 'libB.so') + import_library = os.path.join(bazel_bin, 'B.if.lib') + shared_library = os.path.join(bazel_bin, 'B.dll') def_file = os.path.join(bazel_bin, 'B.def') self.assertTrue(os.path.exists(import_library)) self.assertTrue(os.path.exists(shared_library)) @@ -176,7 +176,7 @@ class BazelWindowsCppTest(test_base.TestBase): '--features=no_windows_export_all_symbols' ]) self.AssertExitCode(exit_code, 1, stderr) - self.assertIn('output \'libB.ifso\' was not created', ''.join(stderr)) + self.assertIn('output \'B.if.lib\' was not created', ''.join(stderr)) def testBuildCcBinaryWithDependenciesDynamicallyLinked(self): self.createProjectFiles() @@ -191,15 +191,15 @@ class BazelWindowsCppTest(test_base.TestBase): # extensions correct on # Windows. # a_import_library - self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'libA.ifso'))) + self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'A.if.lib'))) # a_shared_library - self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'libA.so'))) + self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'A.dll'))) # a_def_file self.assertFalse(os.path.exists(os.path.join(bazel_bin, 'A.def'))) # b_import_library - self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'libB.ifso'))) + self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'B.if.lib'))) # b_shared_library - self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'libB.so'))) + self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'B.dll'))) # b_def_file self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'B.def'))) # c_exe @@ -220,11 +220,11 @@ class BazelWindowsCppTest(test_base.TestBase): exit_code, _, stderr = self.RunBazel(['build', '//main:main']) self.AssertExitCode(exit_code, 0, stderr) - # Test if libA.so and libB.so are copied to the directory of main.exe + # Test if A.dll and B.dll are copied to the directory of main.exe main_bin = os.path.join(bazel_bin, 'main/main.exe') self.assertTrue(os.path.exists(main_bin)) - self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'main/libA.so'))) - self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'main/libB.so'))) + self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'main/A.dll'))) + self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'main/B.dll'))) # Run the binary to see if it runs successfully exit_code, stdout, stderr = self.RunProgram([main_bin]) @@ -245,12 +245,12 @@ class BazelWindowsCppTest(test_base.TestBase): ]) # //main:main depends on both //lib:A and //:A, - # their dlls are both called libA.so, + # their dlls are both called A.dll, # so there should be a conflict error exit_code, _, stderr = self.RunBazel(['build', '//main:main']) self.AssertExitCode(exit_code, 1, stderr) self.assertIn( - 'ERROR: file \'main/libA.so\' is generated by these conflicting ' + 'ERROR: file \'main/A.dll\' is generated by these conflicting ' 'actions:', ''.join(stderr)) def testBuildDifferentCcBinariesDependOnConflictDLLs(self): @@ -282,7 +282,7 @@ class BazelWindowsCppTest(test_base.TestBase): exit_code, _, stderr = self.RunBazel(['build', '//main:other_main']) self.AssertExitCode(exit_code, 1, stderr) self.assertIn( - 'ERROR: file \'main/libA.so\' is generated by these conflicting ' + 'ERROR: file \'main/A.dll\' is generated by these conflicting ' 'actions:', ''.join(stderr)) def testDynamicLinkingMSVCRT(self): @@ -337,7 +337,7 @@ class BazelWindowsCppTest(test_base.TestBase): 'main/BUILD', [ 'cc_binary(', - ' name = "main.so",', + ' name = "main.dll",', ' srcs = ["main.cc"],', ' deps = ["//:B"],', # Transitively depends on //:A ' linkstatic = 1,' @@ -348,19 +348,19 @@ class BazelWindowsCppTest(test_base.TestBase): bazel_bin = self.getBazelInfo('bazel-bin') exit_code, _, stderr = self.RunBazel([ - 'build', '//main:main.so', '--output_groups=default,interface_library' + 'build', '//main:main.dll', '--output_groups=default,interface_library' ]) self.AssertExitCode(exit_code, 0, stderr) - main_library = os.path.join(bazel_bin, 'main/main.so') - main_interface = os.path.join(bazel_bin, 'main/main.ifso') - def_file = os.path.join(bazel_bin, 'main/main.so.def') + main_library = os.path.join(bazel_bin, 'main/main.dll') + main_interface = os.path.join(bazel_bin, 'main/main.dll.if.lib') + def_file = os.path.join(bazel_bin, 'main/main.dll.def') self.assertTrue(os.path.exists(main_library)) self.assertTrue(os.path.exists(main_interface)) self.assertTrue(os.path.exists(def_file)) - # libA.so and libB.so should not be copied. - self.assertFalse(os.path.exists(os.path.join(bazel_bin, 'main/libA.so'))) - self.assertFalse(os.path.exists(os.path.join(bazel_bin, 'main/libB.so'))) + # A.dll and B.dll should not be copied. + self.assertFalse(os.path.exists(os.path.join(bazel_bin, 'main/A.dll'))) + self.assertFalse(os.path.exists(os.path.join(bazel_bin, 'main/B.dll'))) self.AssertFileContentContains(def_file, 'hello_A') self.AssertFileContentContains(def_file, 'hello_B') self.AssertFileContentContains(def_file, 'hello_C') @@ -371,7 +371,7 @@ class BazelWindowsCppTest(test_base.TestBase): 'main/BUILD', [ 'cc_binary(', - ' name = "main.so",', + ' name = "main.dll",', ' srcs = ["main.cc"],', ' deps = ["//:B"],', # Transitively depends on //:A ' linkstatic = 0,' @@ -382,19 +382,19 @@ class BazelWindowsCppTest(test_base.TestBase): bazel_bin = self.getBazelInfo('bazel-bin') exit_code, _, stderr = self.RunBazel([ - 'build', '//main:main.so', '--output_groups=default,interface_library' + 'build', '//main:main.dll', '--output_groups=default,interface_library' ]) self.AssertExitCode(exit_code, 0, stderr) - main_library = os.path.join(bazel_bin, 'main/main.so') - main_interface = os.path.join(bazel_bin, 'main/main.ifso') - def_file = os.path.join(bazel_bin, 'main/main.so.def') + main_library = os.path.join(bazel_bin, 'main/main.dll') + main_interface = os.path.join(bazel_bin, 'main/main.dll.if.lib') + def_file = os.path.join(bazel_bin, 'main/main.dll.def') self.assertTrue(os.path.exists(main_library)) self.assertTrue(os.path.exists(main_interface)) self.assertTrue(os.path.exists(def_file)) - # libA.so and libB.so should be copied. - self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'main/libA.so'))) - self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'main/libB.so'))) + # A.dll and B.dll should be copied. + self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'main/A.dll'))) + self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'main/B.dll'))) # hello_A and hello_B should not be exported. self.AssertFileContentNotContains(def_file, 'hello_A') self.AssertFileContentNotContains(def_file, 'hello_B') @@ -406,7 +406,7 @@ class BazelWindowsCppTest(test_base.TestBase): 'main/BUILD', [ 'cc_binary(', - ' name = "main.so",', + ' name = "main.dll",', ' srcs = ["main.cc"],', ' deps = ["//:B"],', # Transitively depends on //:A ' linkstatic = 1,' @@ -416,12 +416,12 @@ class BazelWindowsCppTest(test_base.TestBase): bazel_bin = self.getBazelInfo('bazel-bin') exit_code, _, stderr = self.RunBazel( - ['build', '//main:main.so', '--output_groups=def_file']) + ['build', '//main:main.dll', '--output_groups=def_file']) self.AssertExitCode(exit_code, 0, stderr) # Although windows_export_all_symbols is not specified for this target, # we should still be able to get the DEF file by def_file output group. - def_file = os.path.join(bazel_bin, 'main/main.so.def') + def_file = os.path.join(bazel_bin, 'main/main.dll.def') self.assertTrue(os.path.exists(def_file)) self.AssertFileContentContains(def_file, 'hello_A') self.AssertFileContentContains(def_file, 'hello_B') @@ -441,7 +441,6 @@ class BazelWindowsCppTest(test_base.TestBase): self.ScratchFile('WORKSPACE') self.ScratchFile('lib.cc', ['void hello() {}']) self.ScratchFile('my_lib.def', [ - 'LIBRARY liblib.so', 'EXPORTS', ' ?hello@@YAXXZ', ]) @@ -453,7 +452,7 @@ class BazelWindowsCppTest(test_base.TestBase): ')', '', 'cc_binary(', - ' name = "lib.so",', + ' name = "lib_dy.dll",', ' srcs = ["lib.cc"],', ' win_def_file = "my_lib.def",', ' linkshared = 1,', @@ -468,13 +467,13 @@ class BazelWindowsCppTest(test_base.TestBase): ]) bazel_bin = self.getBazelInfo('bazel-bin') - lib_ifso = os.path.join(bazel_bin, 'liblib.ifso') + lib_if = os.path.join(bazel_bin, 'lib.if.lib') lib_def = os.path.join(bazel_bin, 'lib.def') - self.assertTrue(os.path.exists(lib_ifso)) + self.assertTrue(os.path.exists(lib_if)) self.assertFalse(os.path.exists(lib_def)) # Test specifying DEF file in cc_binary - exit_code, _, stderr = self.RunBazel(['build', '//:lib.so', '-s']) + exit_code, _, stderr = self.RunBazel(['build', '//:lib_dy.dll', '-s']) self.AssertExitCode(exit_code, 0, stderr) self.assertIn('/DEF:my_lib.def', ''.join(stderr)) @@ -483,9 +482,9 @@ class BazelWindowsCppTest(test_base.TestBase): self.ScratchFile('BUILD', [ 'cc_import(', ' name = "a_import",', - ' static_library = "A.a",', - ' shared_library = "A.so",', - ' interface_library = "A.ifso",', + ' static_library = "A.lib",', + ' shared_library = "A.dll",', + ' interface_library = "A.if.lib",', ' hdrs = ["a.h"],', ' alwayslink = 1,', ')', diff --git a/src/test/shell/bazel/bazel_windows_example_test.sh b/src/test/shell/bazel/bazel_windows_example_test.sh index b36d30148e..45215dee23 100755 --- a/src/test/shell/bazel/bazel_windows_example_test.sh +++ b/src/test/shell/bazel/bazel_windows_example_test.sh @@ -63,11 +63,15 @@ function assert_binary_run_from_subdir() { # function test_cpp() { local cpp_pkg=examples/cpp - assert_build_output ./bazel-bin/${cpp_pkg}/libhello-lib.a ${cpp_pkg}:hello-world - assert_build_output ./bazel-bin/${cpp_pkg}/hello-world.pdb ${cpp_pkg}:hello-world --output_groups=pdb_file - assert_build_output ./bazel-bin/${cpp_pkg}/hello-world.pdb -c dbg ${cpp_pkg}:hello-world --output_groups=pdb_file + assert_build_output \ + ./bazel-bin/${cpp_pkg}/hello-lib.lib ${cpp_pkg}:hello-world + assert_build_output ./bazel-bin/${cpp_pkg}/hello-world.pdb \ + ${cpp_pkg}:hello-world --output_groups=pdb_file + assert_build_output ./bazel-bin/${cpp_pkg}/hello-world.pdb -c dbg \ + ${cpp_pkg}:hello-world --output_groups=pdb_file assert_build -c opt ${cpp_pkg}:hello-world --output_groups=pdb_file - test -f ./bazel-bin/${cpp_pkg}/hello-world.pdb && fail "PDB file should not be generated in OPT mode" + test -f ./bazel-bin/${cpp_pkg}/hello-world.pdb \ + && fail "PDB file should not be generated in OPT mode" assert_build ${cpp_pkg}:hello-world ./bazel-bin/${cpp_pkg}/hello-world foo >& $TEST_log \ || fail "./bazel-bin/${cpp_pkg}/hello-world foo execution failed" diff --git a/tools/cpp/CROSSTOOL.tpl b/tools/cpp/CROSSTOOL.tpl index 38df97f323..02ff6617c9 100644 --- a/tools/cpp/CROSSTOOL.tpl +++ b/tools/cpp/CROSSTOOL.tpl @@ -169,6 +169,12 @@ toolchain { target_cpu: "x64_windows" target_system_name: "local" + artifact_name_pattern { + category_name: 'executable' + prefix: '' + extension: '.exe' + } + %{msys_x64_mingw_content} linking_mode_flags { mode: DYNAMIC } @@ -279,6 +285,42 @@ toolchain { name: "no_legacy_features" } + artifact_name_pattern { + category_name: 'object_file' + prefix: '' + extension: '.obj' + } + + artifact_name_pattern { + category_name: 'static_library' + prefix: '' + extension: '.lib' + } + + artifact_name_pattern { + category_name: 'alwayslink_static_library' + prefix: '' + extension: '.lo.lib' + } + + artifact_name_pattern { + category_name: 'executable' + prefix: '' + extension: '.exe' + } + + artifact_name_pattern { + category_name: 'dynamic_library' + prefix: '' + extension: '.dll' + } + + artifact_name_pattern { + category_name: 'interface_library' + prefix: '' + extension: '.if.lib' + } + # Suppress startup banner. feature { name: "nologo" diff --git a/tools/cpp/windows_cc_configure.bzl b/tools/cpp/windows_cc_configure.bzl index 605760d817..a22cca1959 100644 --- a/tools/cpp/windows_cc_configure.bzl +++ b/tools/cpp/windows_cc_configure.bzl @@ -56,6 +56,7 @@ def _get_escaped_windows_msys_crosstool_content(repository_ctx, use_mingw = Fals ' tool_path { name: "cpp" path: "%s%s/bin/cpp" }\n' % (escaped_msys_root, prefix) + ' tool_path { name: "dwp" path: "%s%s/bin/dwp" }\n' % (escaped_msys_root, prefix) + ' tool_path { name: "gcc" path: "%s%s/bin/gcc" }\n' % (escaped_msys_root, prefix) + + ' artifact_name_pattern { category_name: "executable" prefix: "" extension: ".exe"}\n' + ' cxx_flag: "-std=gnu++0x"\n' + ' linker_flag: "-lstdc++"\n' + ' cxx_builtin_include_directory: "%s%s/"\n' % (escaped_msys_root, prefix) + |