CROSSTOOL: Make C++ compiled and linked artifact name pattern configurable

This change makes Bazel respect artifact name patterns specified in
CROSSTOOL.

Users cannot specify any arbitrary name pattern, it must ends with allowed
extensions. For example, for dynamic library, it can only ends with .so,
.dylib or .dll, otherwise Bazel throws an error.

Change-Id: I21d9e6fa7c3a282e1a9b8ff29679b00925cddb33
PiperOrigin-RevId: 197553413

7 files changed, 215 insertions, 28 deletions

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"));