diff --git a/examples/ambiguous_deps/BUILD.bazel b/examples/ambiguous_deps/BUILD.bazel new file mode 100644 index 0000000000..0cf4b47890 --- /dev/null +++ b/examples/ambiguous_deps/BUILD.bazel @@ -0,0 +1,40 @@ +load("@rules_cc//cc:defs.bzl", "cc_library") +load("@rules_rust//rust:defs.bzl", "rust_binary") + +# A rust_binary that depends on two native libs with the same name. +# See https://github.com/bazelbuild/rules_rust/issues/840. +rust_binary( + name = "bin_with_same_name_deps", + srcs = ["bin.rs"], + deps = [ + "//ambiguous_deps/x:exc", + "//ambiguous_deps/y:exc", + ], +) + +# A rust_binary that depends on a native library with a name that doesn't +# match the `lib.a` pattern on linux. +rust_binary( + name = "nonstandard_name_bin", + srcs = ["nonstandard_name_bin.rs"], + deps = [":nonstandard_name_intermediate"], +) + +cc_library( + name = "nonstandard_name_cc_lib", + srcs = ["cc_library_with_func.cc"], +) + +genrule( + name = "nonstandard_name_gen", + srcs = [":nonstandard_name_cc_lib"], + outs = ["nonstandard_name_gen.a"], + # Copy the first member (libnonstandard_name_cc_lib.a) from the srcs to the + # output nonstandard_name_gen.a. + cmd = "cp $$(awk '{print $$1}' <<< '$(SRCS)') $@", +) + +cc_library( + name = "nonstandard_name_intermediate", + srcs = [":nonstandard_name_gen.a"], +) diff --git a/examples/ambiguous_deps/bin.rs b/examples/ambiguous_deps/bin.rs new file mode 100644 index 0000000000..8f69444497 --- /dev/null +++ b/examples/ambiguous_deps/bin.rs @@ -0,0 +1,10 @@ +use std::os::raw::c_int; + +extern "C" { + pub fn cx() -> c_int; + pub fn cy() -> c_int; +} + +fn main() { + println!("hi {} {}", unsafe { cx() }, unsafe { cy() }); +} diff --git a/examples/ambiguous_deps/cc_library_with_func.cc b/examples/ambiguous_deps/cc_library_with_func.cc new file mode 100644 index 0000000000..536fa936bf --- /dev/null +++ b/examples/ambiguous_deps/cc_library_with_func.cc @@ -0,0 +1,3 @@ +extern "C" int func() { + return 123; +} diff --git a/examples/ambiguous_deps/nonstandard_name_bin.rs b/examples/ambiguous_deps/nonstandard_name_bin.rs new file mode 100644 index 0000000000..2c07702673 --- /dev/null +++ b/examples/ambiguous_deps/nonstandard_name_bin.rs @@ -0,0 +1,9 @@ +use std::os::raw::c_int; + +extern "C" { + pub fn func() -> c_int; +} + +fn main() { + println!("hi {}", unsafe { func() }); +} diff --git a/examples/ambiguous_deps/x/BUILD.bazel b/examples/ambiguous_deps/x/BUILD.bazel new file mode 100644 index 0000000000..87ae07c712 --- /dev/null +++ b/examples/ambiguous_deps/x/BUILD.bazel @@ -0,0 +1,7 @@ +load("@rules_cc//cc:defs.bzl", "cc_library") + +cc_library( + name = "exc", + srcs = ["exc.cc"], + visibility = ["//ambiguous_deps:__subpackages__"], +) diff --git a/examples/ambiguous_deps/x/exc.cc b/examples/ambiguous_deps/x/exc.cc new file mode 100644 index 0000000000..2b67c3455b --- /dev/null +++ b/examples/ambiguous_deps/x/exc.cc @@ -0,0 +1 @@ +extern "C" int cx() { return 17; } diff --git a/examples/ambiguous_deps/y/BUILD.bazel b/examples/ambiguous_deps/y/BUILD.bazel new file mode 100644 index 0000000000..87ae07c712 --- /dev/null +++ b/examples/ambiguous_deps/y/BUILD.bazel @@ -0,0 +1,7 @@ +load("@rules_cc//cc:defs.bzl", "cc_library") + +cc_library( + name = "exc", + srcs = ["exc.cc"], + visibility = ["//ambiguous_deps:__subpackages__"], +) diff --git a/examples/ambiguous_deps/y/exc.cc b/examples/ambiguous_deps/y/exc.cc new file mode 100644 index 0000000000..903ed08150 --- /dev/null +++ b/examples/ambiguous_deps/y/exc.cc @@ -0,0 +1 @@ +extern "C" int cy() { return 113; } diff --git a/rust/private/clippy.bzl b/rust/private/clippy.bzl index 8525a16aec..d1464e9dc4 100644 --- a/rust/private/clippy.bzl +++ b/rust/private/clippy.bzl @@ -70,7 +70,7 @@ def _clippy_aspect_impl(target, ctx): are_linkstamps_supported = False, ) - compile_inputs, out_dir, build_env_files, build_flags_files, linkstamp_outs = collect_inputs( + compile_inputs, out_dir, build_env_files, build_flags_files, linkstamp_outs, ambiguous_libs = collect_inputs( ctx, ctx.rule.file, ctx.rule.files, @@ -94,6 +94,7 @@ def _clippy_aspect_impl(target, ctx): crate_info = crate_info, dep_info = dep_info, linkstamp_outs = linkstamp_outs, + ambiguous_libs = ambiguous_libs, output_hash = determine_output_hash(crate_info.root, ctx.label), rust_flags = [], out_dir = out_dir, diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index 0cf17c34b2..3da2e67e50 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -23,6 +23,7 @@ load("//rust/private:providers.bzl", _BuildInfo = "BuildInfo") load("//rust/private:stamp.bzl", "is_stamping_enabled") load( "//rust/private:utils.bzl", + "abs", "expand_dict_value_locations", "expand_list_element_locations", "find_cc_toolchain", @@ -328,6 +329,122 @@ def _process_build_scripts( compile_inputs = depset(transitive = [extra_inputs, compile_inputs]) return compile_inputs, out_dir, build_env_file, build_flags_files +def _symlink_for_ambiguous_lib(actions, toolchain, crate_info, lib): + """Constructs a disambiguating symlink for a library dependency. + + Args: + actions (Actions): The rule's context actions object. + toolchain: The Rust toolchain object. + crate_info (CrateInfo): The target crate's info. + lib (File): The library to symlink to. + + Returns: + (File): The disambiguating symlink for the library. + """ + # FIXME: Once the relative order part of the native-link-modifiers rustc + # feature is stable, we should be able to eliminate the need to construct + # symlinks by passing the full paths to the libraries. + # https://github.com/rust-lang/rust/issues/81490. + + # Take the absolute value of hash() since it could be negative. + path_hash = abs(hash(lib.path)) + lib_name = get_lib_name(lib) + + prefix = "lib" + extension = ".a" + if toolchain.os.startswith("windows"): + prefix = "" + extension = ".lib" + + # Ensure the symlink follows the lib.a pattern on Unix-like platforms + # or .lib on Windows. + # Add a hash of the original library path to disambiguate libraries with the same basename. + symlink_name = "{}{}-{}{}".format(prefix, lib_name, path_hash, extension) + + # Add the symlink to a target crate-specific _ambiguous_libs/ subfolder, + # to avoid possible collisions with sibling crates that may depend on the + # same ambiguous libraries. + symlink = actions.declare_file("_ambiguous_libs/" + crate_info.output.basename + "/" + symlink_name) + actions.symlink( + output = symlink, + target_file = lib, + progress_message = "Creating symlink to ambiguous lib: {}".format(lib.path), + ) + return symlink + +def _disambiguate_libs(actions, toolchain, crate_info, dep_info, use_pic): + """Constructs disambiguating symlinks for ambiguous library dependencies. + + The symlinks are all created in a _ambiguous_libs/ subfolder specific to + the target crate to avoid possible collisions with sibling crates that may + depend on the same ambiguous libraries. + + Args: + actions (Actions): The rule's context actions object. + toolchain: The Rust toolchain object. + crate_info (CrateInfo): The target crate's info. + dep_info: (DepInfo): The target crate's dependency info. + use_pic: (boolean): Whether the build should use PIC. + + Returns: + dict[String, File]: A mapping from ambiguous library paths to their + disambiguating symlink. + """ + # FIXME: Once the relative order part of the native-link-modifiers rustc + # feature is stable, we should be able to eliminate the need to construct + # symlinks by passing the full paths to the libraries. + # https://github.com/rust-lang/rust/issues/81490. + + # A dictionary from file paths of ambiguous libraries to the corresponding + # symlink. + ambiguous_libs = {} + + # A dictionary maintaining a mapping from preferred library name to the + # last visited artifact with that name. + visited_libs = {} + for link_input in dep_info.transitive_noncrates.to_list(): + for lib in link_input.libraries: + # FIXME: Dynamic libs are not disambiguated right now, there are + # cases where those have a non-standard name with version (e.g., + # //test/unit/versioned_libs). We hope that the link modifiers + # stabilization will come before we need to make this work. + if _is_dylib(lib): + continue + artifact = get_preferred_artifact(lib, use_pic) + name = get_lib_name(artifact) + + # On Linux-like platforms, normally library base names start with + # `lib`, following the pattern `lib[name].(a|lo)` and we pass + # -lstatic=name. + # On Windows, the base name looks like `name.lib` and we pass + # -lstatic=name. + # FIXME: Under the native-link-modifiers unstable rustc feature, + # we could use -lstatic:+verbatim instead. + needs_symlink_to_standardize_name = ( + (toolchain.os.startswith("linux") or toolchain.os.startswith("mac") or toolchain.os.startswith("darwin")) and + artifact.basename.endswith(".a") and not artifact.basename.startswith("lib") + ) or ( + toolchain.os.startswith("windows") and not artifact.basename.endswith(".lib") + ) + + # Detect cases where we need to disambiguate library dependencies + # by constructing symlinks. + if ( + needs_symlink_to_standardize_name or + # We have multiple libraries with the same name. + (name in visited_libs and visited_libs[name].path != artifact.path) + ): + # Disambiguate the previously visited library (if we just detected + # that it is ambiguous) and the current library. + if name in visited_libs: + old_path = visited_libs[name].path + if old_path not in ambiguous_libs: + ambiguous_libs[old_path] = _symlink_for_ambiguous_lib(actions, toolchain, crate_info, visited_libs[name]) + ambiguous_libs[artifact.path] = _symlink_for_ambiguous_lib(actions, toolchain, crate_info, artifact) + + visited_libs[name] = artifact + return ambiguous_libs + def collect_inputs( ctx, file, @@ -362,7 +479,8 @@ def collect_inputs( - (str): The `OUT_DIR` of the current build info - (File): An optional path to a generated environment file from a `cargo_build_script` target - (depset[File]): All direct and transitive build flag files from the current build info - - (list[File]): Linkstamp outputs. + - (list[File]): Linkstamp outputs + - (dict[String, File]): Ambiguous libs, see `_disambiguate_libs`. """ linker_script = getattr(file, "linker_script") if hasattr(file, "linker_script") else None @@ -374,6 +492,7 @@ def collect_inputs( # the output is rlib. This avoids quadratic behavior where transitive noncrates are # flattened on each transitive rust_library dependency. additional_transitive_inputs = [] + ambiguous_libs = {} if crate_info.type in ("staticlib", "proc-macro"): additional_transitive_inputs = _collect_libs_from_linker_inputs( dep_info.transitive_noncrates.to_list(), @@ -381,11 +500,12 @@ def collect_inputs( ) elif crate_info.type in ("bin", "dylib", "cdylib"): linker_inputs = dep_info.transitive_noncrates.to_list() + ambiguous_libs = _disambiguate_libs(ctx.actions, toolchain, crate_info, dep_info, use_pic) additional_transitive_inputs = _collect_libs_from_linker_inputs(linker_inputs, use_pic) + [ additional_input for linker_input in linker_inputs for additional_input in linker_input.additional_inputs - ] + ] + ambiguous_libs.values() # Compute linkstamps. Use the inputs of the binary as inputs to the # linkstamp action to ensure linkstamps are rebuilt whenever binary inputs @@ -451,7 +571,7 @@ def collect_inputs( build_env_files = [f for f in build_env_files] + [build_env_file] compile_inputs = depset(build_env_files, transitive = [compile_inputs]) - return compile_inputs, out_dir, build_env_files, build_flags_files, linkstamp_outs + return compile_inputs, out_dir, build_env_files, build_flags_files, linkstamp_outs, ambiguous_libs def construct_arguments( ctx, @@ -464,6 +584,7 @@ def construct_arguments( crate_info, dep_info, linkstamp_outs, + ambiguous_libs, output_hash, rust_flags, out_dir, @@ -487,6 +608,7 @@ def construct_arguments( crate_info (CrateInfo): The CrateInfo provider of the target crate dep_info (DepInfo): The DepInfo provider of the target crate linkstamp_outs (list): Linkstamp outputs of native dependencies + ambiguous_libs (dict): Ambiguous libs, see `_disambiguate_libs` output_hash (str): The hashed path of the crate root rust_flags (list): Additional flags to pass to rustc out_dir (str): The path to the output directory for the target Crate. @@ -639,7 +761,7 @@ def construct_arguments( rustc_flags.add("--codegen=linker=" + ld) rustc_flags.add_joined("--codegen", link_args, join_with = " ", format_joined = "link-args=%s") - _add_native_link_flags(rustc_flags, dep_info, linkstamp_outs, crate_info.type, toolchain, cc_toolchain, feature_configuration) + _add_native_link_flags(rustc_flags, dep_info, linkstamp_outs, ambiguous_libs, crate_info.type, toolchain, cc_toolchain, feature_configuration) # These always need to be added, even if not linking this crate. add_crate_link_flags(rustc_flags, dep_info, force_all_deps_direct) @@ -731,7 +853,7 @@ def rustc_compile_action( # Determine if the build is currently running with --stamp stamp = is_stamping_enabled(attr) - compile_inputs, out_dir, build_env_files, build_flags_files, linkstamp_outs = collect_inputs( + compile_inputs, out_dir, build_env_files, build_flags_files, linkstamp_outs, ambiguous_libs = collect_inputs( ctx = ctx, file = ctx.file, files = ctx.files, @@ -756,6 +878,7 @@ def rustc_compile_action( crate_info = crate_info, dep_info = dep_info, linkstamp_outs = linkstamp_outs, + ambiguous_libs = ambiguous_libs, output_hash = output_hash, rust_flags = rust_flags, out_dir = out_dir, @@ -1097,30 +1220,33 @@ def _get_crate_dirname(crate): """ return crate.output.dirname -def _portable_link_flags(lib, use_pic): +def _portable_link_flags(lib, use_pic, ambiguous_libs): + artifact = get_preferred_artifact(lib, use_pic) + if ambiguous_libs and artifact.path in ambiguous_libs: + artifact = ambiguous_libs[artifact.path] if lib.static_library or lib.pic_static_library: return [ - "-lstatic=%s" % get_lib_name(get_preferred_artifact(lib, use_pic)), + "-lstatic=%s" % get_lib_name(artifact), ] elif _is_dylib(lib): return [ - "-ldylib=%s" % get_lib_name(get_preferred_artifact(lib, use_pic)), + "-ldylib=%s" % get_lib_name(artifact), ] return [] -def _make_link_flags_windows(linker_input_and_use_pic): - linker_input, use_pic = linker_input_and_use_pic +def _make_link_flags_windows(linker_input_and_use_pic_and_ambiguous_libs): + linker_input, use_pic, ambiguous_libs = linker_input_and_use_pic_and_ambiguous_libs ret = [] for lib in linker_input.libraries: if lib.alwayslink: ret.extend(["-C", "link-arg=/WHOLEARCHIVE:%s" % get_preferred_artifact(lib, use_pic).path]) else: - ret.extend(_portable_link_flags(lib, use_pic)) + ret.extend(_portable_link_flags(lib, use_pic, ambiguous_libs)) return ret -def _make_link_flags_darwin(linker_input_and_use_pic): - linker_input, use_pic = linker_input_and_use_pic +def _make_link_flags_darwin(linker_input_and_use_pic_and_ambiguous_libs): + linker_input, use_pic, ambiguous_libs = linker_input_and_use_pic_and_ambiguous_libs ret = [] for lib in linker_input.libraries: if lib.alwayslink: @@ -1129,11 +1255,11 @@ def _make_link_flags_darwin(linker_input_and_use_pic): ("link-arg=-Wl,-force_load,%s" % get_preferred_artifact(lib, use_pic).path), ]) else: - ret.extend(_portable_link_flags(lib, use_pic)) + ret.extend(_portable_link_flags(lib, use_pic, ambiguous_libs)) return ret -def _make_link_flags_default(linker_input_and_use_pic): - linker_input, use_pic = linker_input_and_use_pic +def _make_link_flags_default(linker_input_and_use_pic_and_ambiguous_libs): + linker_input, use_pic, ambiguous_libs = linker_input_and_use_pic_and_ambiguous_libs ret = [] for lib in linker_input.libraries: if lib.alwayslink: @@ -1146,20 +1272,23 @@ def _make_link_flags_default(linker_input_and_use_pic): "link-arg=-Wl,--no-whole-archive", ]) else: - ret.extend(_portable_link_flags(lib, use_pic)) + ret.extend(_portable_link_flags(lib, use_pic, ambiguous_libs)) return ret -def _libraries_dirnames(linker_input_and_use_pic): - link_input, use_pic = linker_input_and_use_pic - return [get_preferred_artifact(lib, use_pic).dirname for lib in link_input.libraries] +def _libraries_dirnames(linker_input_and_use_pic_and_ambiguous_libs): + link_input, use_pic, _ = linker_input_and_use_pic_and_ambiguous_libs + + # De-duplicate names. + return depset([get_preferred_artifact(lib, use_pic).dirname for lib in link_input.libraries]).to_list() -def _add_native_link_flags(args, dep_info, linkstamp_outs, crate_type, toolchain, cc_toolchain, feature_configuration): +def _add_native_link_flags(args, dep_info, linkstamp_outs, ambiguous_libs, 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 linkstamp_outs (list): Linkstamp outputs of native dependencies + ambiguous_libs (dict): Ambiguous libs, see `_disambiguate_libs` crate_type: Crate type of the current target toolchain (rust_toolchain): The current `rust_toolchain` cc_toolchain (CcToolchainInfo): The current `cc_toolchain` @@ -1179,9 +1308,15 @@ def _add_native_link_flags(args, dep_info, linkstamp_outs, crate_type, toolchain make_link_flags = _make_link_flags_default # TODO(hlopko): Remove depset flattening by using lambdas once we are on >=Bazel 5.0 - args_and_pic = [(arg, use_pic) for arg in dep_info.transitive_noncrates.to_list()] - args.add_all(args_and_pic, map_each = _libraries_dirnames, uniquify = True, format_each = "-Lnative=%s") - args.add_all(args_and_pic, map_each = make_link_flags) + args_and_pic_and_ambiguous_libs = [(arg, use_pic, ambiguous_libs) for arg in dep_info.transitive_noncrates.to_list()] + args.add_all(args_and_pic_and_ambiguous_libs, map_each = _libraries_dirnames, uniquify = True, format_each = "-Lnative=%s") + if ambiguous_libs: + # If there are ambiguous libs, the disambiguation symlinks to them are + # all created in the same directory. Add it to the library search path. + ambiguous_libs_dirname = ambiguous_libs.values()[0].dirname + args.add("-Lnative={}".format(ambiguous_libs_dirname)) + + args.add_all(args_and_pic_and_ambiguous_libs, map_each = make_link_flags) for linkstamp_out in linkstamp_outs: args.add_all(["-C", "link-arg=%s" % linkstamp_out.path]) diff --git a/rust/private/rustdoc.bzl b/rust/private/rustdoc.bzl index 89fefefb6b..86c2acd9af 100644 --- a/rust/private/rustdoc.bzl +++ b/rust/private/rustdoc.bzl @@ -79,7 +79,7 @@ def rustdoc_compile_action( aliases = crate_info.aliases, ) - compile_inputs, out_dir, build_env_files, build_flags_files, linkstamp_outs = collect_inputs( + compile_inputs, out_dir, build_env_files, build_flags_files, linkstamp_outs, ambiguous_libs = collect_inputs( ctx = ctx, file = ctx.file, files = ctx.files, @@ -109,6 +109,7 @@ def rustdoc_compile_action( crate_info = rustdoc_crate_info, dep_info = dep_info, linkstamp_outs = linkstamp_outs, + ambiguous_libs = ambiguous_libs, output_hash = None, rust_flags = rustdoc_flags, out_dir = out_dir, diff --git a/rust/private/utils.bzl b/rust/private/utils.bzl index bf475bc397..f9abbf9540 100644 --- a/rust/private/utils.bzl +++ b/rust/private/utils.bzl @@ -122,6 +122,19 @@ def get_lib_name(lib): else: return libname +def abs(value): + """Returns the absolute value of a number. + + Args: + value (int): A number. + + Returns: + int: The absolute value of the number. + """ + if value < 0: + return -value + return value + def determine_output_hash(crate_root, label): """Generates a hash of the crate root file's path. @@ -134,9 +147,7 @@ def determine_output_hash(crate_root, label): """ # Take the absolute value of hash() since it could be negative. - h = hash(crate_root.path) + hash(repr(label)) - if h < 0: - h = -h + h = abs(hash(crate_root.path) + hash(repr(label))) return repr(h) def get_preferred_artifact(library_to_link, use_pic):