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>

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 @@ def _rust_bindgen_impl(ctx):
     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,17 +125,17 @@ def _rust_bindgen_impl(ctx):
     }
 
     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,
         inputs = depset(
             [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],

rust/private/rustc.bzl

@@ -23,7 +23,7 @@ load(
     "expand_locations",
     "find_cc_toolchain",
     "get_lib_name",
-    "get_libs_for_static_executable",
+    "get_preferred_artifact",
     "relativize",
     "rule_attrs",
 )
@@ -54,9 +54,8 @@ DepInfo = provider(
         "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 @@ def collect_deps(label, deps, proc_macro_deps, aliases, toolchain):
 
     direct_crates = []
     transitive_crates = []
-    transitive_dylibs = []
-    transitive_staticlibs = []
+    transitive_noncrates = []
+    transitive_noncrate_libs = []
     transitive_build_infos = []
     build_info = None
 
@@ -165,26 +164,17 @@ def collect_deps(label, deps, proc_macro_deps, aliases, toolchain):
             ))
 
             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 @@ def collect_deps(label, deps, proc_macro_deps, aliases, toolchain):
     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 @@ def construct_arguments(
             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 @@ def rustc_compile_action(
         ),
     )
 
+    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 rustc_compile_action(
         ),
     ]
 
+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 @@ def _compute_rpaths(toolchain, output_dir, dep_info):
     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 @@ def _get_crate_dirname(crate):
     """
     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

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 @@ def _dirname(path_str):
     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 @@ def _build_rustdoc_flags(dep_info, crate):
     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 []
 

rust/private/utils.bzl

@@ -118,19 +118,7 @@ def determine_output_hash(crate_root):
     """
     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:

test/unit/native_deps/alwayslink.cc

@@ -0,0 +1 @@
+extern "C" int alwayslink() { return 42; }