From 6c4b16e19db10176c74d8b12bda7f17ac8e13373 Mon Sep 17 00:00:00 2001 From: Philipp Schrader Date: Wed, 17 Nov 2021 07:41:41 -0800 Subject: [PATCH] Fix _is_shared_library_extension_valid (#14290) Consider libfoo-1.0.so.1.0 valid. Do not fail if the name does not contain a '.', return False instead. (cherry picked from commit 23d096931be9b7247eafa750999dd7feadde14c1) Fixes #14289. Closes #14021. Co-authored-by: Benedek Thaler --- .../builtins_bzl/common/cc/cc_import.bzl | 28 +++++++------ .../cpp/CcImportBaseConfiguredTargetTest.java | 42 +++++++++++++++++++ 2 files changed, 57 insertions(+), 13 deletions(-) diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_import.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_import.bzl index 86e70afa5181ab..5283b97a474e35 100644 --- 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 index 6725a7d46e5212..1ac1fa220baf15 100644 --- 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 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");