diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index b182cfacae..5ddfcde1b8 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -154,8 +154,12 @@ platforms: - "-@test_chdir_remote//sub:go_default_test" - "-//tests/core/cgo:dylib_client" - "-//tests/core/cgo:dylib_test" + - "-//tests/core/cgo:generated_dylib_client" + - "-//tests/core/cgo:generated_dylib_test" - "-//tests/core/cgo:versioned_dylib_client" - "-//tests/core/cgo:versioned_dylib_test" + - "-//tests/core/cgo:generated_versioned_dylib_client" + - "-//tests/core/cgo:generated_versioned_dylib_test" - "-//tests/core/cross:proto_test" - "-//tests/core/go_embed_data:go_default_test" - "-//tests/core/go_embed_data:go_default_library" @@ -264,10 +268,14 @@ platforms: - "-@org_golang_x_tools//cmd/splitdwarf/internal/macho:macho_test" - "-@test_chdir_remote//sub:go_default_test" - "-//tests:buildifier_test" # requires bash - - "-//tests/core/cgo:dylib_test" - "-//tests/core/cgo:dylib_client" - - "-//tests/core/cgo:versioned_dylib_test" + - "-//tests/core/cgo:dylib_test" + - "-//tests/core/cgo:generated_dylib_client" + - "-//tests/core/cgo:generated_dylib_test" - "-//tests/core/cgo:versioned_dylib_client" + - "-//tests/core/cgo:versioned_dylib_test" + - "-//tests/core/cgo:generated_versioned_dylib_client" + - "-//tests/core/cgo:generated_versioned_dylib_test" - "-//tests/core/coverage:coverage_test" - "-//tests/core/cross:proto_test" - "-//tests/core/go_binary:go_default_test" diff --git a/.gitignore b/.gitignore index a6ef824c1f..c98766f173 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ /bazel-* +/tests/core/cgo/libimported.* diff --git a/go/private/actions/link.bzl b/go/private/actions/link.bzl index 1fd472aee8..5a44dc0809 100644 --- a/go/private/actions/link.bzl +++ b/go/private/actions/link.bzl @@ -128,9 +128,10 @@ def emit_link( # the bazel execroot. Most binaries are only dynamically linked against # system libraries though. cgo_rpaths = sorted(collections.uniq([ - rpath.flag(go, d, relative_to = executable) + f for d in archive.cgo_deps.to_list() if has_shared_lib_extension(d.basename) + for f in rpath.flags(go, d, executable = executable) ])) extldflags.extend(cgo_rpaths) diff --git a/go/private/rpath.bzl b/go/private/rpath.bzl index b5b7ad46ee..53435e39b9 100644 --- a/go/private/rpath.bzl +++ b/go/private/rpath.bzl @@ -17,29 +17,43 @@ load( "paths", ) -def _rpath(go, library, relative_to = None): - """Returns the rpath of a library, possibly relative to another file.""" - if not relative_to: - return paths.dirname(library.short_path) +def _rpath(go, library, executable = None): + """Returns the potential rpaths of a library, possibly relative to another file.""" + if not executable: + return [paths.dirname(library.short_path)] - # Go back to the workspace root from the executable file - depth = relative_to.short_path.count("/") - back_to_root = paths.join(*([".."] * depth)) origin = go.mode.goos == "darwin" and "@loader_path" or "$ORIGIN" - # Then walk back to the library's short path - return paths.join(origin, back_to_root, paths.dirname(library.short_path)) + # Accomodate for two kinds of executable paths. + rpaths = [] + + # 1. Where the executable is inside its own .runfiles directory. + # This is the case for generated libraries as well as remote builds. + # a) go back to the workspace root from the executable file in .runfiles + depth = executable.short_path.count("/") + back_to_root = paths.join(*([".."] * depth)) + + # b) then walk back to the library's short path + rpaths.append(paths.join(origin, back_to_root, paths.dirname(library.short_path))) + + # 2. Where the executable is outside the .runfiles directory: + # This is the case for local pre-built libraries, as well as local + # generated libraries. + runfiles_dir = paths.basename(executable.short_path) + ".runfiles" + rpaths.append(paths.join(origin, runfiles_dir, go._ctx.workspace_name, paths.dirname(library.short_path))) + + return rpaths -def _flag(go, *args, **kwargs): - """Returns the linker flag rpath for a library.""" - return "-Wl,-rpath," + _rpath(go, *args, **kwargs) +def _flags(go, *args, **kwargs): + """Returns the rpath linker flags for a library.""" + return ["-Wl,-rpath," + p for p in _rpath(go, *args, **kwargs)] def _install_name(f): """Returns the install name for a dylib on macOS.""" return f.short_path rpath = struct( - flag = _flag, + flags = _flags, install_name = _install_name, rpath = _rpath, ) diff --git a/go/private/rules/binary.bzl b/go/private/rules/binary.bzl index c7fed7fe23..525169d272 100644 --- a/go/private/rules/binary.bzl +++ b/go/private/rules/binary.bzl @@ -55,7 +55,7 @@ def new_cc_import( alwayslink = False, linkopts = []): if dynamic_library: - linkopts = linkopts + [rpath.flag(go, dynamic_library)] + linkopts = linkopts + rpath.flags(go, dynamic_library) return CcInfo( compilation_context = cc_common.create_compilation_context( defines = defines, diff --git a/tests/core/cgo/BUILD.bazel b/tests/core/cgo/BUILD.bazel index fd8dfec486..d205044cc3 100644 --- a/tests/core/cgo/BUILD.bazel +++ b/tests/core/cgo/BUILD.bazel @@ -73,6 +73,7 @@ go_test( srcs = ["dylib_test.go"], embed = [":dylib_client"], rundir = ".", + tags = ["manual"], # //tests/core/cgo:generate_imported_dylib.sh must be run first ) go_library( @@ -85,6 +86,7 @@ go_library( }), cgo = True, importpath = "github.com/bazelbuild/rules_go/tests/core/cgo/dylib", + tags = ["manual"], ) cc_import( @@ -93,22 +95,53 @@ cc_import( tags = ["manual"], ) +cc_import( + name = "linux_imported_dylib", + shared_library = ":libimported.so", + tags = ["manual"], +) + +go_test( + name = "generated_dylib_test", + srcs = ["dylib_test.go"], + embed = [":generated_dylib_client"], + rundir = ".", +) + +go_library( + name = "generated_dylib_client", + srcs = ["dylib_client.go"], + cdeps = select({ + "@io_bazel_rules_go//go/platform:darwin": [":darwin_imported_generated_dylib"], + "//conditions:default": [":linux_imported_generated_dylib"], + # TODO(jayconrod): Support windows, skip others. + }), + cgo = True, + importpath = "github.com/bazelbuild/rules_go/tests/core/cgo/dylib", +) + +cc_import( + name = "darwin_imported_generated_dylib", + shared_library = ":libimported_generated.dylib", + tags = ["manual"], +) + cc_binary( - name = "libimported.dylib", + name = "libimported_generated.dylib", srcs = ["imported.c"], - linkopts = ["-Wl,-install_name,@rpath/libimported.dylib"], + linkopts = ["-Wl,-install_name,@rpath/libimported_generated.dylib"], linkshared = True, tags = ["manual"], ) cc_import( - name = "linux_imported_dylib", - shared_library = ":libimported.so", + name = "linux_imported_generated_dylib", + shared_library = ":libimported_generated.so", tags = ["manual"], ) cc_binary( - name = "libimported.so", + name = "libimported_generated.so", srcs = ["imported.c"], linkshared = True, tags = ["manual"], @@ -119,6 +152,7 @@ go_test( srcs = ["dylib_test.go"], embed = [":versioned_dylib_client"], rundir = ".", + tags = ["manual"], # //tests/core/cgo:generate_imported_dylib.sh must be run first ) go_library( @@ -134,17 +168,46 @@ go_library( }), cgo = True, importpath = "github.com/bazelbuild/rules_go/tests/core/cgo/dylib", + tags = ["manual"], ) -cc_library( +cc_import( name = "linux_imported_versioned_dylib", - srcs = [":libimported.so.2"], + shared_library = "libimported.so.2", + tags = ["manual"], +) + +go_test( + name = "generated_versioned_dylib_test", + srcs = ["dylib_test.go"], + embed = [":generated_versioned_dylib_client"], + rundir = ".", +) + +go_library( + name = "generated_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_generated_dylib"], + "//conditions:default": [":linux_imported_generated_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_generated_versioned_dylib", + srcs = [":libimported_generated.so.2"], linkstatic = False, tags = ["manual"], ) cc_binary( - name = "libimported.so.2", + name = "libimported_generated.so.2", srcs = ["imported.c"], linkshared = True, tags = ["manual"], diff --git a/tests/core/cgo/README.rst b/tests/core/cgo/README.rst index 34e971635a..a385cab383 100644 --- a/tests/core/cgo/README.rst +++ b/tests/core/cgo/README.rst @@ -10,18 +10,23 @@ opts_test Checks that different sets of options are passed to C and C++ sources in a ``go_library`` with ``cgo = True``. -dylib_test ----------- +(generated_)?(versioned_)?dylib_test +------------------------------------ Checks that Go binaries can link against dynamic C libraries. Some libraries (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 ----------- +The non ``generated_`` tests are manual. The ``generate_imported_dylib.sh`` +script must be run before running the tests themselves. -Checks that Go binaries can link against dynamic C libraries that are only -available as a versioned shared library, like ``libfoo.so.1``. +The ``generated_`` variants check that Go binaries can link against dynamic C +libraries that are generated by another rule, rather than being included in the +source tree. + +The ``versioned_`` variants check that Go binaries can link against dynamic C +libraries that are only available as a versioned shared library, like +``libfoo.so.1``, as used on Linux. cc_libs_test ------------ diff --git a/tests/core/cgo/generate_imported_dylib.sh b/tests/core/cgo/generate_imported_dylib.sh new file mode 100755 index 0000000000..5c272ef444 --- /dev/null +++ b/tests/core/cgo/generate_imported_dylib.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash + +set -exo pipefail + +cd "$(dirname "$0")" + +case "$(uname -s)" in + Linux*) + cc -shared -o libimported.so imported.c + cc -shared -o libimported.so.2 imported.c + ;; + Darwin*) + cc -shared -Wl,-install_name,@rpath/libimported.dylib -o libimported.dylib imported.c + ;; + *) + echo "Unsupported OS: $(uname -s)" >&2 + exit 1 +esac