rustc: correctly handle alwayslink staticlibs (#606)
* test: add test that demonstrates missing alwayslink libraries
* refactor: inline get_libs_for_static_executable into rustc logic
We then move the function to its only remaining caller in bindgen. I
didn't inline it there because it's used in a few different ways and
cleaning it up felt like more work than it was immediately worth.
* refactor: make DepInfo carry around LibraryToLink and not File data
We need this information to implement support for alwayslink, and it
cascaded around the codebase more than a little. The next change will
implement always link, which is currently left as a stub so that this
change has no functional changes.
* rustc: remove awkward file-ending check for _is_dylib
Instead we just verify we're not going to link anything static. This
removes the need for a toolchain in _is_dylib, which will allow
further cleanups in a moment.
* rustc: remove unused toolchain= argument to _make_link_flags
This allows us to use a more efficient construction to add linker
arguments to the command line.
* rustc: add support for alwayslink library dependencies
This follows up on the previous commit's refactoring and actually adds
alwayslink support.
Fixes #325.
* tests: add coverage for a cdylib with alwayslink
Makes sure that cdylibs also correctly pick up their alwayslink
dependencies.
* rustc: tote around LinkerInput for noncrate deps
This lets us avoid reifying a depset a little longer, which is nice.
Co-authored-by: Augie Fackler <augie@google.com>
diff --git a/bindgen/bindgen.bzl b/bindgen/bindgen.bzl
index 7644f78..9265f9f 100644
--- a/bindgen/bindgen.bzl
+++ b/bindgen/bindgen.bzl
@@ -16,7 +16,20 @@
load("//rust:rust.bzl", "rust_library")
# buildifier: disable=bzl-visibility
-load("//rust/private:utils.bzl", "find_toolchain", "get_libs_for_static_executable")
+load("//rust/private:utils.bzl", "find_toolchain", "get_preferred_artifact")
+
+# TODO(hlopko): use the more robust logic from rustc.bzl also here, through a reasonable API.
+def _get_libs_for_static_executable(dep):
+ """find the libraries used for linking a static executable.
+
+ Args:
+ dep (Target): A cc_library target.
+
+ Returns:
+ depset: A depset[File]
+ """
+ linker_inputs = dep[CcInfo].linking_context.linker_inputs.to_list()
+ return depset([get_preferred_artifact(lib) for li in linker_inputs for lib in li.libraries])
def rust_bindgen_library(
name,
@@ -84,7 +97,7 @@
output = ctx.outputs.out
# libclang should only have 1 output file
- libclang_dir = get_libs_for_static_executable(libclang).to_list()[0].dirname
+ libclang_dir = _get_libs_for_static_executable(libclang).to_list()[0].dirname
include_directories = cc_lib[CcInfo].compilation_context.includes.to_list()
quote_include_directories = cc_lib[CcInfo].compilation_context.quote_includes.to_list()
system_include_directories = cc_lib[CcInfo].compilation_context.system_includes.to_list()
@@ -112,7 +125,7 @@
}
if libstdcxx:
- env["LD_LIBRARY_PATH"] = ":".join([f.dirname for f in get_libs_for_static_executable(libstdcxx).to_list()])
+ env["LD_LIBRARY_PATH"] = ":".join([f.dirname for f in _get_libs_for_static_executable(libstdcxx).to_list()])
ctx.actions.run(
executable = bindgen_bin,
@@ -120,9 +133,9 @@
[header],
transitive = [
cc_lib[CcInfo].compilation_context.headers,
- get_libs_for_static_executable(libclang),
+ _get_libs_for_static_executable(libclang),
] + [
- get_libs_for_static_executable(libstdcxx),
+ _get_libs_for_static_executable(libstdcxx),
] if libstdcxx else [],
),
outputs = [unformatted_output],
diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl
index 7686edd..d71c629 100644
--- a/rust/private/rustc.bzl
+++ b/rust/private/rustc.bzl
@@ -23,7 +23,7 @@
"expand_locations",
"find_cc_toolchain",
"get_lib_name",
- "get_libs_for_static_executable",
+ "get_preferred_artifact",
"relativize",
"rule_attrs",
)
@@ -54,9 +54,8 @@
"direct_crates": "depset[CrateInfo]",
"transitive_build_infos": "depset[BuildInfo]",
"transitive_crates": "depset[CrateInfo]",
- "transitive_dylibs": "depset[File]",
"transitive_libs": "List[File]: All transitive dependencies, not filtered by type.",
- "transitive_staticlibs": "depset[File]",
+ "transitive_noncrates": "depset[LinkerInput]: All transitive dependencies that aren't crates.",
},
)
@@ -149,8 +148,8 @@
direct_crates = []
transitive_crates = []
- transitive_dylibs = []
- transitive_staticlibs = []
+ transitive_noncrates = []
+ transitive_noncrate_libs = []
transitive_build_infos = []
build_info = None
@@ -165,26 +164,17 @@
))
transitive_crates.append(depset([dep[rust_common.crate_info]], transitive = [dep[DepInfo].transitive_crates]))
- transitive_dylibs.append(dep[DepInfo].transitive_dylibs)
- transitive_staticlibs.append(dep[DepInfo].transitive_staticlibs)
+ transitive_noncrates.append(dep[DepInfo].transitive_noncrates)
+ transitive_noncrate_libs.append(depset(dep[DepInfo].transitive_libs))
transitive_build_infos.append(dep[DepInfo].transitive_build_infos)
elif CcInfo in dep:
# This dependency is a cc_library
# TODO: We could let the user choose how to link, instead of always preferring to link static libraries.
- libs = get_libs_for_static_executable(dep)
-
- transitive_dylibs.append(depset([
- lib
- for lib in libs.to_list()
- # Dynamic libraries may have a version number nowhere, or before (macos) or after (linux) the extension.
- if lib.basename.endswith(toolchain.dylib_ext) or lib.basename.split(".", 2)[1] == toolchain.dylib_ext[1:]
- ]))
- transitive_staticlibs.append(depset([
- lib
- for lib in libs.to_list()
- if lib.basename.endswith(toolchain.staticlib_ext)
- ]))
+ linker_inputs = dep[CcInfo].linking_context.linker_inputs.to_list()
+ libs = [get_preferred_artifact(lib) for li in linker_inputs for lib in li.libraries]
+ transitive_noncrate_libs.append(depset(libs))
+ transitive_noncrates.append(depset(dep[CcInfo].linking_context.linker_inputs))
elif BuildInfo in dep:
if build_info:
fail("Several deps are providing build information, only one is allowed in the dependencies", "deps")
@@ -196,18 +186,17 @@
transitive_crates_depset = depset(transitive = transitive_crates)
transitive_libs = depset(
[c.output for c in transitive_crates_depset.to_list()],
- transitive = transitive_staticlibs + transitive_dylibs,
+ transitive = transitive_noncrate_libs,
)
return (
DepInfo(
direct_crates = depset(direct_crates),
transitive_crates = transitive_crates_depset,
- transitive_dylibs = depset(
- transitive = transitive_dylibs,
+ transitive_noncrates = depset(
+ transitive = transitive_noncrates,
order = "topological", # dylib link flag ordering matters.
),
- transitive_staticlibs = depset(transitive = transitive_staticlibs),
transitive_libs = transitive_libs.to_list(),
transitive_build_infos = depset(transitive = transitive_build_infos),
dep_env = build_info.dep_env if build_info else None,
@@ -498,7 +487,7 @@
args.add("--codegen=linker=" + ld)
args.add_joined("--codegen", link_args, join_with = " ", format_joined = "link-args=%s")
- _add_native_link_flags(args, dep_info, crate_type, cc_toolchain, feature_configuration)
+ _add_native_link_flags(args, dep_info, crate_type, toolchain, cc_toolchain, feature_configuration)
# These always need to be added, even if not linking this crate.
add_crate_link_flags(args, dep_info)
@@ -611,8 +600,10 @@
),
)
+ dylibs = [get_preferred_artifact(lib) for linker_input in dep_info.transitive_noncrates.to_list() for lib in linker_input.libraries if _is_dylib(lib)]
+
runfiles = ctx.runfiles(
- files = dep_info.transitive_dylibs.to_list() + getattr(ctx.files, "data", []),
+ files = dylibs + getattr(ctx.files, "data", []),
collect_data = True,
)
@@ -631,6 +622,9 @@
),
]
+def _is_dylib(dep):
+ return not bool(dep.static_library or dep.pic_static_library)
+
def establish_cc_info(ctx, crate_info, toolchain, cc_toolchain, feature_configuration):
"""If the produced crate is suitable yield a CcInfo to allow for interop with cc rules
@@ -754,18 +748,24 @@
Returns:
depset: A set of relative paths from the output directory to each dependency
"""
- if not dep_info.transitive_dylibs:
+ preferreds = [get_preferred_artifact(lib) for linker_input in dep_info.transitive_noncrates.to_list() for lib in linker_input.libraries]
+
+ # TODO(augie): I don't understand why this can't just be filtering on
+ # _is_dylib(lib), but doing that causes failures on darwin and windows
+ # examples that otherwise work.
+ dylibs = [lib for lib in preferreds if lib.basename.endswith(toolchain.dylib_ext) or lib.basename.split(".", 2)[1] == toolchain.dylib_ext[1:]]
+ if not dylibs:
return depset([])
if toolchain.os != "linux":
fail("Runtime linking is not supported on {}, but found {}".format(
toolchain.os,
- dep_info.transitive_dylibs,
+ dep_info.transitive_noncrates,
))
# Multiple dylibs can be present in the same directory, so deduplicate them.
return depset([
relativize(lib_dir, output_dir)
- for lib_dir in _get_dir_names(dep_info.transitive_dylibs.to_list())
+ for lib_dir in _get_dir_names(dylibs)
])
def _get_dir_names(files):
@@ -821,25 +821,78 @@
"""
return crate.output.dirname
-def _add_native_link_flags(args, dep_info, crate_type, cc_toolchain, feature_configuration):
+def _portable_link_flags(lib):
+ if lib.static_library or lib.pic_static_library:
+ return ["-lstatic=%s" % get_lib_name(get_preferred_artifact(lib))]
+ elif _is_dylib(lib):
+ return ["-ldylib=%s" % get_lib_name(get_preferred_artifact(lib))]
+ return []
+
+def _make_link_flags_windows(linker_input):
+ ret = []
+ for lib in linker_input.libraries:
+ if lib.alwayslink:
+ ret.extend(["-C", "link-arg=/WHOLEARCHIVE:%s" % get_preferred_artifact(lib).path])
+ else:
+ ret.extend(_portable_link_flags(lib))
+ return ret
+
+def _make_link_flags_darwin(linker_input):
+ ret = []
+ for lib in linker_input.libraries:
+ if lib.alwayslink:
+ ret.extend([
+ "-C",
+ ("link-arg=-Wl,-force_load,%s" % get_preferred_artifact(lib).path),
+ ])
+ else:
+ ret.extend(_portable_link_flags(lib))
+ return ret
+
+def _make_link_flags_default(linker_input):
+ ret = []
+ for lib in linker_input.libraries:
+ if lib.alwayslink:
+ ret.extend([
+ "-C",
+ "link-arg=-Wl,--whole-archive",
+ "-C",
+ ("link-arg=%s" % get_preferred_artifact(lib).path),
+ "-C",
+ "link-arg=-Wl,--no-whole-archive",
+ ])
+ else:
+ ret.extend(_portable_link_flags(lib))
+ return ret
+
+def _libraries_dirnames(linker_input):
+ return [get_preferred_artifact(lib).dirname for lib in linker_input.libraries]
+
+def _add_native_link_flags(args, dep_info, crate_type, toolchain, cc_toolchain, feature_configuration):
"""Adds linker flags for all dependencies of the current target.
Args:
args (Args): The Args struct for a ctx.action
dep_info (DepInfo): Dependency Info provider
crate_type: Crate type of the current target
+ toolchain (rust_toolchain): The current `rust_toolchain`
cc_toolchain (CcToolchainInfo): The current `cc_toolchain`
feature_configuration (FeatureConfiguration): feature configuration to use with cc_toolchain
"""
- native_libs = depset(transitive = [dep_info.transitive_dylibs, dep_info.transitive_staticlibs])
- args.add_all(native_libs, map_each = _get_dirname, uniquify = True, format_each = "-Lnative=%s")
+ args.add_all(dep_info.transitive_noncrates, map_each = _libraries_dirnames, uniquify = True, format_each = "-Lnative=%s")
if crate_type in ["lib", "rlib"]:
return
- args.add_all(dep_info.transitive_dylibs, map_each = get_lib_name, format_each = "-ldylib=%s")
- args.add_all(dep_info.transitive_staticlibs, map_each = get_lib_name, format_each = "-lstatic=%s")
+ if toolchain.os == "windows":
+ make_link_flags = _make_link_flags_windows
+ elif toolchain.os.startswith("mac") or toolchain.os.startswith("darwin"):
+ make_link_flags = _make_link_flags_darwin
+ else:
+ make_link_flags = _make_link_flags_default
+
+ args.add_all(dep_info.transitive_noncrates, map_each = make_link_flags)
if crate_type in ["dylib", "cdylib"]:
# For shared libraries we want to link C++ runtime library dynamically
diff --git a/rust/private/rustdoc_test.bzl b/rust/private/rustdoc_test.bzl
index fbaf7d5..b407900 100644
--- a/rust/private/rustdoc_test.bzl
+++ b/rust/private/rustdoc_test.bzl
@@ -15,7 +15,7 @@
# buildifier: disable=module-docstring
load("//rust/private:common.bzl", "rust_common")
load("//rust/private:rustc.bzl", "DepInfo")
-load("//rust/private:utils.bzl", "find_toolchain", "get_lib_name")
+load("//rust/private:utils.bzl", "find_toolchain", "get_lib_name", "get_preferred_artifact")
def _rust_doc_test_impl(ctx):
"""The implementation for the `rust_doc_test` rule
@@ -79,7 +79,7 @@
return "/".join(path_str.split("/")[:-1])
def _build_rustdoc_flags(dep_info, crate):
- """Constructs the rustdoc script used to test `crate`.
+ """Constructs the rustdoc script used to test `crate`.
Args:
dep_info (DepInfo): The DepInfo provider
@@ -99,10 +99,16 @@
link_flags += ["--extern=" + c.name + "=" + c.dep.output.short_path for c in d.direct_crates.to_list()]
link_search_flags += ["-Ldependency={}".format(_dirname(c.output.short_path)) for c in d.transitive_crates.to_list()]
- link_flags += ["-ldylib=" + get_lib_name(lib) for lib in d.transitive_dylibs.to_list()]
- link_search_flags += ["-Lnative={}".format(_dirname(lib.short_path)) for lib in d.transitive_dylibs.to_list()]
- link_flags += ["-lstatic=" + get_lib_name(lib) for lib in d.transitive_staticlibs.to_list()]
- link_search_flags += ["-Lnative={}".format(_dirname(lib.short_path)) for lib in d.transitive_staticlibs.to_list()]
+ # TODO(hlopko): use the more robust logic from rustc.bzl also here, through a reasonable API.
+ for lib_to_link in dep_info.transitive_noncrates.to_list():
+ is_static = bool(lib_to_link.static_library or lib_to_link.pic_static_library)
+ f = get_preferred_artifact(lib_to_link)
+ if not is_static:
+ link_flags.append("-ldylib=" + get_lib_name(f))
+ else:
+ link_flags.append("-lstatic=" + get_lib_name(f))
+ link_flags.append("-Lnative={}".format(_dirname(f.short_path)))
+ link_search_flags.append("-Lnative={}".format(_dirname(f.short_path)))
edition_flags = ["--edition={}".format(crate.edition)] if crate.edition != "2015" else []
diff --git a/rust/private/utils.bzl b/rust/private/utils.bzl
index 5bd6259..06ed842 100644
--- a/rust/private/utils.bzl
+++ b/rust/private/utils.bzl
@@ -118,19 +118,7 @@
"""
return repr(hash(crate_root.path))
-def get_libs_for_static_executable(dep):
- """find the libraries used for linking a static executable.
-
- Args:
- dep (Target): A cc_library target.
-
- Returns:
- depset: A depset[File]
- """
- linker_inputs = dep[CcInfo].linking_context.linker_inputs.to_list()
- return depset([_get_preferred_artifact(lib) for li in linker_inputs for lib in li.libraries])
-
-def _get_preferred_artifact(library_to_link):
+def get_preferred_artifact(library_to_link):
"""Get the first available library to link from a LibraryToLink object.
Args:
diff --git a/test/unit/native_deps/alwayslink.cc b/test/unit/native_deps/alwayslink.cc
new file mode 100644
index 0000000..bd98ce0
--- /dev/null
+++ b/test/unit/native_deps/alwayslink.cc
@@ -0,0 +1 @@
+extern "C" int alwayslink() { return 42; }
diff --git a/test/unit/native_deps/native_deps_test.bzl b/test/unit/native_deps/native_deps_test.bzl
index 8086d74..5803f77 100644
--- a/test/unit/native_deps/native_deps_test.bzl
+++ b/test/unit/native_deps/native_deps_test.bzl
@@ -28,7 +28,7 @@
prefix = prefix,
suffix = suffix,
args = action.argv,
- )
+ ),
)
def _lib_has_no_native_libs_test_impl(ctx):
@@ -103,11 +103,75 @@
_assert_argv_contains(env, action, "-lstatic=native_dep")
return analysistest.end(env)
+def _extract_linker_args(argv):
+ return [a for a in argv if a.startswith("link-arg=") or a.startswith("link-arg=-l") or a.startswith("-l") or a.endswith(".lo") or a.endswith(".o")]
+
+def _bin_has_native_dep_and_alwayslink_test_impl(ctx):
+ env = analysistest.begin(ctx)
+ tut = analysistest.target_under_test(env)
+ actions = analysistest.target_actions(env)
+ action = actions[0]
+
+ if ctx.target_platform_has_constraint(ctx.attr._macos_constraint[platform_common.ConstraintValueInfo]):
+ want = [
+ "-lstatic=native_dep",
+ "link-arg=-Wl,-force_load,bazel-out/darwin-fastbuild/bin/test/unit/native_deps/libalwayslink.lo",
+ ]
+ elif ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo]):
+ want = [
+ "-lstatic=native_dep",
+ "link-arg=/WHOLEARCHIVE:bazel-out/x64_windows-fastbuild/bin/test/unit/native_deps/alwayslink.lo.lib",
+ ]
+ else:
+ want = [
+ "-lstatic=native_dep",
+ "link-arg=-Wl,--whole-archive",
+ "link-arg=bazel-out/k8-fastbuild/bin/test/unit/native_deps/libalwayslink.lo",
+ "link-arg=-Wl,--no-whole-archive",
+ ]
+ asserts.equals(env, want, _extract_linker_args(action.argv))
+ return analysistest.end(env)
+
+def _cdylib_has_native_dep_and_alwayslink_test_impl(ctx):
+ env = analysistest.begin(ctx)
+ tut = analysistest.target_under_test(env)
+ actions = analysistest.target_actions(env)
+ action = actions[0]
+
+ if ctx.target_platform_has_constraint(ctx.attr._macos_constraint[platform_common.ConstraintValueInfo]):
+ want = [
+ "-lstatic=native_dep",
+ "link-arg=-Wl,-force_load,bazel-out/darwin-fastbuild/bin/test/unit/native_deps/libalwayslink.lo",
+ ]
+ elif ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo]):
+ want = [
+ "-lstatic=native_dep",
+ "link-arg=/WHOLEARCHIVE:bazel-out/x64_windows-fastbuild/bin/test/unit/native_deps/alwayslink.lo.lib",
+ ]
+ else:
+ want = [
+ "-lstatic=native_dep",
+ "link-arg=-Wl,--whole-archive",
+ "link-arg=bazel-out/k8-fastbuild/bin/test/unit/native_deps/libalwayslink.lo",
+ "link-arg=-Wl,--no-whole-archive",
+ ]
+ asserts.equals(env, want, _extract_linker_args(action.argv))
+ return analysistest.end(env)
+
+
rlib_has_no_native_libs_test = analysistest.make(_rlib_has_no_native_libs_test_impl)
staticlib_has_native_libs_test = analysistest.make(_staticlib_has_native_libs_test_impl)
cdylib_has_native_libs_test = analysistest.make(_cdylib_has_native_libs_test_impl)
proc_macro_has_native_libs_test = analysistest.make(_proc_macro_has_native_libs_test_impl)
bin_has_native_libs_test = analysistest.make(_bin_has_native_libs_test_impl)
+bin_has_native_dep_and_alwayslink_test = analysistest.make(_bin_has_native_dep_and_alwayslink_test_impl, attrs = {
+ "_macos_constraint": attr.label(default = Label("@platforms//os:macos")),
+ "_windows_constraint": attr.label(default = Label("@platforms//os:windows")),
+})
+cdylib_has_native_dep_and_alwayslink_test = analysistest.make(_cdylib_has_native_libs_test_impl, attrs = {
+ "_macos_constraint": attr.label(default = Label("@platforms//os:macos")),
+ "_windows_constraint": attr.label(default = Label("@platforms//os:windows")),
+})
def _native_dep_test():
rust_library(
@@ -141,11 +205,29 @@
deps = [":native_dep"],
)
+ rust_binary(
+ name = "bin_has_native_dep_and_alwayslink",
+ srcs = ["bin_using_native_dep.rs"],
+ deps = [":native_dep", ":alwayslink"],
+ )
+
cc_library(
name = "native_dep",
srcs = ["native_dep.cc"],
)
+ cc_library(
+ name = "alwayslink",
+ srcs = ["alwayslink.cc"],
+ alwayslink = 1,
+ )
+
+ rust_shared_library(
+ name = "cdylib_has_native_dep_and_alwayslink",
+ srcs = ["lib_using_native_dep.rs"],
+ deps = [":native_dep", ":alwayslink"],
+ )
+
rlib_has_no_native_libs_test(
name = "rlib_has_no_native_libs_test",
target_under_test = ":rlib_has_no_native_dep",
@@ -166,6 +248,14 @@
name = "bin_has_native_libs_test",
target_under_test = ":bin_has_native_dep",
)
+ bin_has_native_dep_and_alwayslink_test(
+ name = "bin_has_native_dep_and_alwayslink_test",
+ target_under_test = ":bin_has_native_dep_and_alwayslink",
+ )
+ cdylib_has_native_dep_and_alwayslink_test(
+ name = "cdylib_has_native_dep_and_alwayslink_test",
+ target_under_test = ":cdylib_has_native_dep_and_alwayslink",
+ )
def native_deps_test_suite(name):
"""Entry-point macro called from the BUILD file.
@@ -183,5 +273,7 @@
":cdylib_has_native_libs_test",
":proc_macro_has_native_libs_test",
":bin_has_native_libs_test",
+ ":bin_has_native_dep_and_alwayslink_test",
+ ":cdylib_has_native_dep_and_alwayslink_test",
],
)
