Allow dependencies on versioned shared libraries (#2066)
Fixes #2037
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
index be80f40..e14014d 100644
--- a/.bazelci/presubmit.yml
+++ b/.bazelci/presubmit.yml
@@ -165,6 +165,8 @@
- "-@test_chdir_remote//sub:go_default_test"
- "-//tests/core/cgo:dylib_client"
- "-//tests/core/cgo:dylib_test"
+ - "-//tests/core/cgo:versioned_dylib_client"
+ - "-//tests/core/cgo:versioned_dylib_test"
- "-//tests/core/coverage:coverage_test_test"
- "-//tests/core/coverage:cross_cover_test_test"
- "-//tests/core/cross:cross_test"
@@ -323,6 +325,8 @@
- "-//tests/core/cgo:opts"
- "-//tests/core/cgo:dylib_test"
- "-//tests/core/cgo:dylib_client"
+ - "-//tests/core/cgo:versioned_dylib_test"
+ - "-//tests/core/cgo:versioned_dylib_client"
- "-//tests/core/cgo:cc_libs_test"
- "-//tests/core/cgo:pure"
- "-//tests/core/cgo:cc_srcs"
diff --git a/go/private/actions/binary.bzl b/go/private/actions/binary.bzl
index a5adc62..3e98f5a 100644
--- a/go/private/actions/binary.bzl
+++ b/go/private/actions/binary.bzl
@@ -21,7 +21,7 @@
load(
"@io_bazel_rules_go//go/private:common.bzl",
"ARCHIVE_EXTENSION",
- "SHARED_LIB_EXTENSIONS",
+ "has_shared_lib_extension",
)
def emit_binary(
@@ -61,7 +61,7 @@
cgo_dynamic_deps = [
d
for d in archive.cgo_deps.to_list()
- if any([d.basename.endswith(ext) for ext in SHARED_LIB_EXTENSIONS])
+ if has_shared_lib_extension(d.basename)
]
runfiles = go._ctx.runfiles(files = cgo_dynamic_deps).merge(archive.runfiles)
diff --git a/go/private/actions/link.bzl b/go/private/actions/link.bzl
index 27b5cfe..9350237 100644
--- a/go/private/actions/link.bzl
+++ b/go/private/actions/link.bzl
@@ -18,8 +18,8 @@
)
load(
"@io_bazel_rules_go//go/private:common.bzl",
- "SHARED_LIB_EXTENSIONS",
"as_iterable",
+ "has_shared_lib_extension",
)
load(
"@io_bazel_rules_go//go/private:mode.bzl",
@@ -115,7 +115,7 @@
cgo_dynamic_deps = [
d
for d in archive.cgo_deps.to_list()
- if any([d.basename.endswith(ext) for ext in SHARED_LIB_EXTENSIONS])
+ if has_shared_lib_extension(d.basename)
]
cgo_rpaths = []
for d in cgo_dynamic_deps:
diff --git a/go/private/common.bzl b/go/private/common.bzl
index f301822..4c5dc83 100644
--- a/go/private/common.bzl
+++ b/go/private/common.bzl
@@ -153,6 +153,48 @@
"darwin": ".dylib",
}.get(goos, ".so")
+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))
+
+def has_simple_shared_lib_extension(path):
+ """
+ Matches filenames of shared libraries, without a version number extension.
+ """
+ if any([path.endswith(ext) for ext in SHARED_LIB_EXTENSIONS]):
+ return True
+ return False
+
+def has_versioned_shared_lib_extension(path):
+ """
+ Matches shared libraries with version names in the extension, i.e.
+ libmylib.so.2 or libmylib.so.2.10.
+ """
+ if not path[-1].isdigit():
+ return False
+
+ so_location = path.rfind(".so")
+ # Version extensions are only allowed for .so files
+ if so_location == -1:
+ return False
+ last_dot = so_location
+ for i in range(so_location + 3, len(path)):
+ if path[i] == ".":
+ if i - last_dot > 1:
+ last_dot = i
+ else:
+ return False
+ elif not path[i].isdigit():
+ return False
+
+ if last_dot == len(path):
+ return False
+
+ return True
+
MINIMUM_BAZEL_VERSION = "0.8.0"
def as_list(v):
diff --git a/go/private/rules/cgo.bzl b/go/private/rules/cgo.bzl
index dd6595f..7631f31 100644
--- a/go/private/rules/cgo.bzl
+++ b/go/private/rules/cgo.bzl
@@ -22,10 +22,11 @@
)
load(
"@io_bazel_rules_go//go/private:common.bzl",
- "SHARED_LIB_EXTENSIONS",
"as_iterable",
"as_list",
"as_set",
+ "has_simple_shared_lib_extension",
+ "has_versioned_shared_lib_extension",
"join_srcs",
"pkg_dir",
"split_srcs",
@@ -141,7 +142,7 @@
# 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
- any([lib.basename.endswith(ext) for ext in SHARED_LIB_EXTENSIONS])):
+ 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
@@ -150,6 +151,13 @@
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)
clinkopts.extend(cc_link_flags(d))
@@ -345,7 +353,7 @@
# 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
- any([lib.basename.endswith(ext) for ext in SHARED_LIB_EXTENSIONS])):
+ 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
@@ -353,6 +361,12 @@
# the binary location, and we don't know where that is.
libname = lib.basename[len("lib"):lib.basename.rindex(".")]
linkopts.extend(["-L", lib.dirname, "-l", libname])
+ 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
+ linkopts.extend(["-L", lib.dirname, "-l", libname])
else:
linkopts.append(lib.path)
linkopts.extend(cc_link_flags(d))
diff --git a/go/private/skylib/lib/new_sets.bzl b/go/private/skylib/lib/new_sets.bzl
index b6fb99d..3e203c1 100644
--- a/go/private/skylib/lib/new_sets.bzl
+++ b/go/private/skylib/lib/new_sets.bzl
@@ -20,7 +20,7 @@
values in the set can be retrieved using `sets.to_list(my_set)`.
"""
-load(":dicts.bzl", "dicts")
+load(":skylib/lib/dicts.bzl", "dicts")
def _make(elements = None):
"""Creates a new set.
diff --git a/go/private/skylib/lib/unittest.bzl b/go/private/skylib/lib/unittest.bzl
index 11af89f..9ab2857 100644
--- a/go/private/skylib/lib/unittest.bzl
+++ b/go/private/skylib/lib/unittest.bzl
@@ -19,8 +19,8 @@
assertions used to within tests.
"""
-load(":sets.bzl", "sets")
-load(":new_sets.bzl", new_sets = "sets")
+load(":skylib/lib/sets.bzl", "sets")
+load(":skylib/lib/new_sets.bzl", new_sets = "sets")
def _make(impl, attrs = None):
"""Creates a unit test rule from its implementation function.
diff --git a/tests/core/README.rst b/tests/core/README.rst
index 4c54d57..7204dff 100644
--- a/tests/core/README.rst
+++ b/tests/core/README.rst
@@ -21,6 +21,7 @@
* `race instrumentation <race/README.rst>`_
* `stdlib functionality <stdlib/README.rst>`_
* `Basic go_binary functionality <go_binary/README.rst>`_
+* `Starlark unit tests <starlark/README.rst>`_
* `coverage functionality <coverage/README.rst>`_
* `Import maps <importmap/README.rst>`_
* `Basic go_path functionality <go_path/README.rst>`_
diff --git a/tests/core/cgo/BUILD.bazel b/tests/core/cgo/BUILD.bazel
index 1b8a0e2..39275dd 100644
--- a/tests/core/cgo/BUILD.bazel
+++ b/tests/core/cgo/BUILD.bazel
@@ -68,6 +68,42 @@
)
go_test(
+ name = "versioned_dylib_test",
+ srcs = ["dylib_test.go"],
+ embed = [":versioned_dylib_client"],
+ rundir = ".",
+)
+
+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"],
+ "//conditions:default": [":linux_imported_versioned_dylib"],
+ # TODO(jayconrod): Support windows, skip others.
+ }),
+ cgo = True,
+ importpath = "github.com/bazelbuild/rules_go/tests/core/cgo/dylib",
+)
+
+cc_library(
+ name = "linux_imported_versioned_dylib",
+ srcs = [":libimported.so.2"],
+ linkstatic = False,
+ tags = ["manual"],
+)
+
+cc_binary(
+ name = "libimported.so.2",
+ srcs = ["imported.c"],
+ linkshared = True,
+ tags = ["manual"],
+)
+
+go_test(
name = "cc_libs_test",
srcs = [
"cc_libs_darwin_test.go",
diff --git a/tests/core/cgo/README.rst b/tests/core/cgo/README.rst
index 15a7f31..98a2f96 100644
--- a/tests/core/cgo/README.rst
+++ b/tests/core/cgo/README.rst
@@ -14,6 +14,12 @@
(especially those provided with ``cc_import``) may only have dynamic versions,
and we should be able to link against them and find them at run-time.
+dylib_test
+----------
+
+Checks that Go binaries can link against dynamic C libraries that are only
+available as a versioned shared library, like ``libfoo.so.1``.
+
cc_libs_test
------------
diff --git a/tests/core/starlark/BUILD.bazel b/tests/core/starlark/BUILD.bazel
new file mode 100644
index 0000000..6d36a25
--- /dev/null
+++ b/tests/core/starlark/BUILD.bazel
@@ -0,0 +1,3 @@
+load(":common_tests.bzl", "common_test_suite")
+
+common_test_suite()
diff --git a/tests/core/starlark/README.rst b/tests/core/starlark/README.rst
new file mode 100644
index 0000000..e1f8df7
--- /dev/null
+++ b/tests/core/starlark/README.rst
@@ -0,0 +1,9 @@
+Starlark unit tests
+=======================
+
+common_test_suite
+---------
+
+Checks that ``has_shared_lib_extension`` from ``//go/private:common.bzl``
+correctly matches shared library filenames, which may optionally have a version
+number at the end.
diff --git a/tests/core/starlark/common_tests.bzl b/tests/core/starlark/common_tests.bzl
new file mode 100644
index 0000000..11eff53
--- /dev/null
+++ b/tests/core/starlark/common_tests.bzl
@@ -0,0 +1,38 @@
+load("@io_bazel_rules_go//go/private:common.bzl", "has_shared_lib_extension")
+load("@io_bazel_rules_go//go/private:skylib/lib/unittest.bzl", "asserts", "unittest")
+
+def _versioned_shared_libraries_test(ctx):
+ env = unittest.begin(ctx)
+
+ # See //src/test/java/com/google/devtools/build/lib/rules/cpp:CppFileTypesTest.java
+ # for the corresponding native C++ rules tests.
+ asserts.true(env, has_shared_lib_extension("somelibrary.so"))
+ asserts.true(env, has_shared_lib_extension("somelibrary.so.2"))
+ asserts.true(env, has_shared_lib_extension("somelibrary.so.20"))
+ asserts.true(env, has_shared_lib_extension("somelibrary.so.20.2"))
+ asserts.true(env, has_shared_lib_extension("a/somelibrary.so.2"))
+ asserts.true(env, has_shared_lib_extension("somelibrary✅.so.2"))
+ asserts.true(env, has_shared_lib_extension("somelibrary✅.so.2.1"))
+ asserts.false(env, has_shared_lib_extension("somelibrary.so.e"))
+ asserts.false(env, has_shared_lib_extension("xx.1"))
+ asserts.false(env, has_shared_lib_extension("somelibrary.so.2e"))
+ asserts.false(env, has_shared_lib_extension("somelibrary.so.e2"))
+ asserts.false(env, has_shared_lib_extension("somelibrary.so.20.e2"))
+ asserts.false(env, has_shared_lib_extension("somelibrary.a.2"))
+ asserts.false(env, has_shared_lib_extension("somelibrary.a..2"))
+ asserts.false(env, has_shared_lib_extension("somelibrary.so.2."))
+ asserts.false(env, has_shared_lib_extension("somelibrary.so."))
+ asserts.false(env, has_shared_lib_extension("somelibrary.so.2🚫"))
+ asserts.false(env, has_shared_lib_extension("somelibrary.so.🚫2"))
+ asserts.false(env, has_shared_lib_extension("somelibrary.so🚫.2.0"))
+
+ return unittest.end(env)
+
+versioned_shared_libraries_test = unittest.make(_versioned_shared_libraries_test)
+
+def common_test_suite():
+ """Creates the test targets and test suite for common.bzl tests."""
+ unittest.suite(
+ "common_tests",
+ versioned_shared_libraries_test,
+ )
