Fix _is_shared_library_extension_valid

erenon · copybara-github · commit 23d096931be9 · 2021-11-03T09:41:36.000-07:00
Consider libfoo-1.0.so.1.0 valid. Do not fail if the name does not contain a '.', return False instead. Closes #14021. PiperOrigin-RevId: 407357609
diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_import.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_import.bzl
@@ -44,19 +44,21 @@ def _is_shared_library_extension_valid(shared_library_name):
         shared_library_name.endswith(".dylib")):
         return True
 
-    # validate against the regex "^.+\.so(\.\d\w*)+$" for versioned .so files
-    parts = shared_library_name.split(".")
-    extension = parts[1]
-    if extension != "so":
-        return False
-    version_parts = parts[2:]
-    for part in version_parts:
-        if not part[0].isdigit():
-            return False
-        for c in part[1:].elems():
-            if not (c.isalnum() or c == "_"):
-                return False
-    return True
+    # validate agains the regex "^.+\\.((so)|(dylib))(\\.\\d\\w*)+$",
+    # must match VERSIONED_SHARED_LIBRARY.
+    for ext in (".so.", ".dylib."):
+        name, _, version = shared_library_name.rpartition(ext)
+        if name and version:
+            version_parts = version.split(".")
+            for part in version_parts:
+                if not part[0].isdigit():
+                    return False
+                for c in part[1:].elems():
+                    if not (c.isalnum() or c == "_"):
+                        return False
+            return True
+
+    return False
 
 def _perform_error_checks(
         system_provided,
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcImportBaseConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcImportBaseConfiguredTargetTest.java
@@ -216,6 +216,35 @@ public void testCcImportWithVersionedSharedLibrary() throws Exception {
         .containsExactly("bin _solib_k8/_U_S_Sa_Cfoo___Ua/libfoo.so.1ab2.1_a2");
   }
 
+  @Test
+  public void testCcImportWithVersionedSharedLibraryWithDotInTheName() throws Exception {
+    useConfiguration("--cpu=k8");
+
+    ConfiguredTarget target =
+        scratchConfiguredTarget(
+            "a",
+            "foo",
+            starlarkImplementationLoadStatement,
+            "cc_import(name = 'foo', shared_library = 'libfoo.qux.so.1ab2.1_a2')");
+
+    Artifact dynamicLibrary =
+        target
+            .get(CcInfo.PROVIDER)
+            .getCcLinkingContext()
+            .getLibraries()
+            .getSingleton()
+            .getResolvedSymlinkDynamicLibrary();
+    Iterable<Artifact> dynamicLibrariesForRuntime =
+        target
+            .get(CcInfo.PROVIDER)
+            .getCcLinkingContext()
+            .getDynamicLibrariesForRuntime(/* linkingStatically= */ false);
+    assertThat(artifactsToStrings(ImmutableList.of(dynamicLibrary)))
+        .containsExactly("src a/libfoo.qux.so.1ab2.1_a2");
+    assertThat(artifactsToStrings(dynamicLibrariesForRuntime))
+        .containsExactly("bin _solib_k8/_U_S_Sa_Cfoo___Ua/libfoo.qux.so.1ab2.1_a2");
+  }
+
   @Test
   public void testCcImportWithInvalidVersionedSharedLibrary() throws Exception {
     checkError(
@@ -229,6 +258,19 @@ public void testCcImportWithInvalidVersionedSharedLibrary() throws Exception {
         ")");
   }
 
+  @Test
+  public void testCcImportWithInvalidSharedLibraryNoExtension() throws Exception {
+    checkError(
+        "a",
+        "foo",
+        "does not produce any cc_import shared_library files " + "(expected",
+        starlarkImplementationLoadStatement,
+        "cc_import(",
+        "  name = 'foo',",
+        "  shared_library = 'libfoo',",
+        ")");
+  }
+
   @Test
   public void testCcImportWithInterfaceSharedLibrary() throws Exception {
     useConfiguration("--cpu=k8");
