Skipping linker flag for versioned dylib on Darwin

.bazelci/presubmit.yml

@@ -3,12 +3,16 @@ tasks:
   ubuntu1804_bazel400:
     platform: ubuntu1804
     bazel: 4.2.0 # test minimum supported version of bazel
+    shell_commands:
+      - tests/core/cgo/generate_imported_dylib.sh
     build_targets:
     - "//..."
     test_targets:
     - "//..."
   ubuntu2004:
     # enable some unflipped incompatible flags on this platform to ensure we don't regress.
+    shell_commands:
+      - tests/core/cgo/generate_imported_dylib.sh
     build_flags:
     - "--incompatible_load_proto_rules_from_bzl"
     - "--incompatible_restrict_string_escapes"
@@ -22,11 +26,15 @@ tasks:
     test_targets:
     - "//..."
   macos:
+    shell_commands:
+      - tests/core/cgo/generate_imported_dylib.sh
     build_targets:
     - "//..."
     test_targets:
     - "//..."
   rbe_ubuntu1604:
+    shell_commands:
+      - tests/core/cgo/generate_imported_dylib.sh
     build_targets:
     - "//..."
     test_flags:

.gitignore

@@ -1,2 +1,3 @@
 /bazel-*
 /tests/core/cgo/libimported.*
+/tests/core/cgo/libversioned.*

go/private/common.bzl

@@ -173,7 +173,7 @@ def has_shared_lib_extension(path):
     Matches filenames of shared libraries, with or without a version number extension.
     """
     return (has_simple_shared_lib_extension(path) or
-            has_versioned_shared_lib_extension(path))
+            get_versioned_shared_lib_extension(path))
 
 def has_simple_shared_lib_extension(path):
     """
@@ -183,21 +183,21 @@ def has_simple_shared_lib_extension(path):
         return True
     return False
 
-def has_versioned_shared_lib_extension(path):
-    """Returns whether the path appears to be an versioned .so or .dylib file."""
+def get_versioned_shared_lib_extension(path):
+    """If appears to be an versioned .so or .dylib file, return the extension; otherwise empty"""
     parts = path.split("/")[-1].split(".")
     if not parts[-1].isdigit():
-        return False
+        return ""
     for i in range(len(parts) - 1, 0, -1):
         if not parts[i].isdigit():
-            if parts[i] == "dylib" or parts[i] == "so":
-                return True
+            if i != 0 and (parts[i] == "dylib" or parts[i] == "so"):
+                return ".".join(parts[i:])
 
-            # somehting like foo.bar.1.2
-            return False
+            # somehting like foo.bar.1.2 or dylib.1.2
+            return ""
 
     # something like 1.2.3, or so.1.2, or dylib.1.2, or foo.1.2
-    return False
+    return ""
 
 MINIMUM_BAZEL_VERSION = "4.2.0"
 

go/private/rules/cgo.bzl

@@ -14,8 +14,8 @@
 
 load(
     "//go/private:common.bzl",
+    "get_versioned_shared_lib_extension",
     "has_simple_shared_lib_extension",
-    "has_versioned_shared_lib_extension",
     "hdr_exts",
 )
 load(
@@ -119,25 +119,30 @@ def cgo_configure(go, srcs, cdeps, cppopts, copts, cxxopts, clinkopts):
                 # If both static and dynamic variants are available, Bazel will only give
                 # us the static variant. We'll get one file for each transitive dependency,
                 # so the same file may appear more than once.
-                if (lib.basename.startswith("lib") and
-                    has_simple_shared_lib_extension(lib.basename)):
-                    # If the loader would be able to find the library using rpaths,
-                    # use -L and -l instead of hard coding the path to the library in
-                    # the binary. This gives users more flexibility. The linker will add
-                    # rpaths later. We can't add them here because they are relative to
-                    # the binary location, and we don't know where that is.
-                    libname = lib.basename[len("lib"):lib.basename.rindex(".")]
-                    clinkopts.extend(["-L", lib.dirname, "-l", libname])
-                    inputs_direct.append(lib)
-                elif (lib.basename.startswith("lib") and
-                      has_versioned_shared_lib_extension(lib.basename)):
-                    # With a versioned shared library, we must use the full filename,
-                    # otherwise the library will not be found by the linker.
-                    libname = ":%s" % lib.basename
-                    clinkopts.extend(["-L", lib.dirname, "-l", libname])
-                    inputs_direct.append(lib)
-                else:
-                    lib_opts.append(lib.path)
+                if lib.basename.startswith("lib"):
+                    if has_simple_shared_lib_extension(lib.basename):
+                        # If the loader would be able to find the library using rpaths,
+                        # use -L and -l instead of hard coding the path to the library in
+                        # the binary. This gives users more flexibility. The linker will add
+                        # rpaths later. We can't add them here because they are relative to
+                        # the binary location, and we don't know where that is.
+                        libname = lib.basename[len("lib"):lib.basename.rindex(".")]
+                        clinkopts.extend(["-L", lib.dirname, "-l", libname])
+                        inputs_direct.append(lib)
+                        continue
+                    extension = get_versioned_shared_lib_extension(lib.basename)
+                    if extension.startswith("so"):
+                        # With a versioned .so file, we must use the full filename,
+                        # otherwise the library will not be found by the linker.
+                        libname = ":%s" % lib.basename
+                        clinkopts.extend(["-L", lib.dirname, "-l", libname])
+                        inputs_direct.append(lib)
+                        continue
+                    elif extension.startswith("dylib"):
+                        # With a versioned .dylib file, we can only symlink it as a simple .dylib
+                        # file and treat it as an unversioned one.
+                        continue
+                lib_opts.append(lib.path)
             clinkopts.extend(cc_link_flags)
 
         elif hasattr(d, "objc"):

tests/core/cgo/BUILD.bazel

@@ -151,22 +151,24 @@ cc_binary(
     tags = ["manual"],
 )
 
+# //tests/core/cgo:generate_imported_dylib.sh must be run first
 go_test(
     name = "versioned_dylib_test",
     srcs = ["dylib_test.go"],
     embed = [":versioned_dylib_client"],
     rundir = ".",
-    tags = ["manual"],  # //tests/core/cgo:generate_imported_dylib.sh must be run first
+    target_compatible_with = select({
+        "@platforms//os:osx": [],
+        "@platforms//os:linux": [],
+        "//conditions:default": ["@platforms//:incompatible"],
+    }),
 )
 
 go_library(
     name = "versioned_dylib_client",
     srcs = ["dylib_client.go"],
     cdeps = select({
-        # This test exists just for versioned `.so`s on Linux,
-        # but we can reuse the above test's dylib so it passes on darwin,
-        # where filename suffixes are not used for library version.
-        "@io_bazel_rules_go//go/platform:darwin": [":darwin_imported_dylib"],
+        "@io_bazel_rules_go//go/platform:darwin": [":darwin_imported_versioned_dylib"],
         "//conditions:default": [":linux_imported_versioned_dylib"],
         # TODO(jayconrod): Support windows, skip others.
     }),
@@ -177,7 +179,16 @@ go_library(
 
 cc_import(
     name = "linux_imported_versioned_dylib",
-    shared_library = "libimported.so.2",
+    shared_library = "libversioned.so.2",
+    tags = ["manual"],
+)
+
+cc_library(
+    name = "darwin_imported_versioned_dylib",
+    srcs = [
+        "libversioned.dylib",
+        "libversioned.dylib.2",
+    ],
     tags = ["manual"],
 )
 

tests/core/cgo/generate_imported_dylib.sh

@@ -7,10 +7,16 @@ cd "$(dirname "$0")"
 case "$(uname -s)" in
   Linux*)
     cc -shared -o libimported.so imported.c
-    cc -shared -o libimported.so.2 imported.c
+    cc -shared -o libversioned.so.2 imported.c
     ;;
   Darwin*)
     cc -shared -Wl,-install_name,@rpath/libimported.dylib -o libimported.dylib imported.c
+    # Oracle Instant Client was distributed as libclntsh.dylib.12.1 (https://www.oracle.com/database/technologies/instant-client/macos-intel-x86-downloads.html),
+    # even though the standard name should be libclntsh.12.1.dylib, according to
+    # "Mac OS X For Unix Geeks", 4th Edition, Chapter 11
+    cc -shared -Wl,-install_name,@rpath/libversioned.dylib.2 -o libversioned.dylib.2 imported.c
+    # We need a symlink to load dylib on Darwin
+    ln -s libversioned.dylib.2 libversioned.dylib
     ;;
   *)
     echo "Unsupported OS: $(uname -s)" >&2