From 76b8c1fe7e3902d61da63badef8ded11ee1c2c3e Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Tue, 9 Aug 2022 15:22:04 +0200 Subject: [PATCH 1/2] Also apply get_lib_name correctly to the C++ runtime libraries --- rust/private/rustc.bzl | 27 ++++++++++++++----------- rust/private/utils.bzl | 45 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 56 insertions(+), 16 deletions(-) diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index 97b8a17220..9c027dd81b 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -425,7 +425,7 @@ def _symlink_for_ambiguous_lib(actions, toolchain, crate_info, lib): # Take the absolute value of hash() since it could be negative. path_hash = abs(hash(lib.path)) - lib_name = get_lib_name(lib, for_windows = toolchain.os.startswith("windows")) + lib_name = get_lib_name_for_windows(lib) if toolchain.os.startswith("windows") else get_lib_name(lib) prefix = "lib" extension = ".a" @@ -488,7 +488,7 @@ def _disambiguate_libs(actions, toolchain, crate_info, dep_info, use_pic): if _is_dylib(lib): continue artifact = get_preferred_artifact(lib, use_pic) - name = get_lib_name(artifact, for_windows = toolchain.os.startswith("windows")) + name = get_lib_name_for_windows(lib) if toolchain.os.startswith("windows") else get_lib_name(lib) # On Linux-like platforms, normally library base names start with # `lib`, following the pattern `lib[name].(a|lo)` and we pass @@ -1523,7 +1523,7 @@ def _get_crate_dirname(crate): """ return crate.output.dirname -def _portable_link_flags(lib, use_pic, ambiguous_libs, for_windows = False): +def _portable_link_flags(lib, use_pic, ambiguous_libs, lib_name, for_windows): artifact = get_preferred_artifact(lib, use_pic) if ambiguous_libs and artifact.path in ambiguous_libs: artifact = ambiguous_libs[artifact.path] @@ -1561,14 +1561,14 @@ def _portable_link_flags(lib, use_pic, ambiguous_libs, for_windows = False): artifact.basename.startswith("libtest-") or artifact.basename.startswith("libstd-") or artifact.basename.startswith("test-") or artifact.basename.startswith("std-") ): - return ["-lstatic=%s" % get_lib_name(artifact, for_windows)] + return ["-lstatic=%s" % lib_name(artifact)] return [ - "-lstatic=%s" % get_lib_name(artifact, for_windows), - "-Clink-arg=-l%s" % (get_lib_name(artifact) if not for_windows else artifact.basename), + "-lstatic=%s" % lib_name(artifact), + "-Clink-arg=-l%s" % (lib_name(artifact) if not for_windows else artifact.basename), ] elif _is_dylib(lib): return [ - "-ldylib=%s" % get_lib_name(artifact, for_windows), + "-ldylib=%s" % lib_name(artifact), ] return [] @@ -1580,7 +1580,7 @@ def _make_link_flags_windows(linker_input_and_use_pic_and_ambiguous_libs): 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, ambiguous_libs, for_windows = True)) + ret.extend(_portable_link_flags(lib, use_pic, ambiguous_libs, get_lib_name_for_windows, for_windows = True)) return ret def _make_link_flags_darwin(linker_input_and_use_pic_and_ambiguous_libs): @@ -1593,7 +1593,7 @@ def _make_link_flags_darwin(linker_input_and_use_pic_and_ambiguous_libs): ("link-arg=-Wl,-force_load,%s" % get_preferred_artifact(lib, use_pic).path), ]) else: - ret.extend(_portable_link_flags(lib, use_pic, ambiguous_libs)) + ret.extend(_portable_link_flags(lib, use_pic, ambiguous_libs, get_lib_name, for_windows = False)) return ret def _make_link_flags_default(linker_input_and_use_pic_and_ambiguous_libs): @@ -1610,7 +1610,7 @@ def _make_link_flags_default(linker_input_and_use_pic_and_ambiguous_libs): "link-arg=-Wl,--no-whole-archive", ]) else: - ret.extend(_portable_link_flags(lib, use_pic, ambiguous_libs)) + ret.extend(_portable_link_flags(lib, use_pic, ambiguous_libs, get_lib_name, for_windows = False)) return ret def _libraries_dirnames(linker_input_and_use_pic_and_ambiguous_libs): @@ -1639,10 +1639,13 @@ def _add_native_link_flags(args, dep_info, linkstamp_outs, ambiguous_libs, crate if toolchain.os == "windows": make_link_flags = _make_link_flags_windows + map_to_lib_name = get_lib_name_for_windows elif toolchain.os.startswith("mac") or toolchain.os.startswith("darwin"): make_link_flags = _make_link_flags_darwin + map_to_lib_name = get_lib_name else: make_link_flags = _make_link_flags_default + map_to_lib_name = get_lib_name # TODO(hlopko): Remove depset flattening by using lambdas once we are on >=Bazel 5.0 args_and_pic_and_ambiguous_libs = [(arg, use_pic, ambiguous_libs) for arg in dep_info.transitive_noncrates.to_list()] @@ -1668,7 +1671,7 @@ def _add_native_link_flags(args, dep_info, linkstamp_outs, ambiguous_libs, crate ) args.add_all( cc_toolchain.dynamic_runtime_lib(feature_configuration = feature_configuration), - map_each = get_lib_name, + map_each = map_to_lib_name, format_each = "-ldylib=%s", ) else: @@ -1681,7 +1684,7 @@ def _add_native_link_flags(args, dep_info, linkstamp_outs, ambiguous_libs, crate ) args.add_all( cc_toolchain.static_runtime_lib(feature_configuration = feature_configuration), - map_each = get_lib_name, + map_each = map_to_lib_name, format_each = "-lstatic=%s", ) diff --git a/rust/private/utils.bzl b/rust/private/utils.bzl index e342b3bef8..c99c7932b4 100644 --- a/rust/private/utils.bzl +++ b/rust/private/utils.bzl @@ -97,12 +97,11 @@ def _path_parts(path): path_parts = path.split("/") return [part for part in path_parts if part != "."] -def get_lib_name(lib, for_windows = False): - """Returns the name of a library artifact, eg. libabc.a -> abc +def get_lib_name(lib): + """Returns the name of a library artifact. Args: lib (File): A library file - for_windows: Whether we're building on Windows. Returns: str: The name of the library @@ -126,11 +125,49 @@ def get_lib_name(lib, for_windows = False): # The library name is now everything minus the extension. libname = ".".join(comps[:-1]) - if libname.startswith("lib") and not for_windows: + if libname.startswith("lib"): return libname[3:] else: return libname +# TODO: Could we remove this function in favor of a "windows" parameter in the +# above function? It looks like currently lambdas cannot accept local parameters +# so the following doesn't work: +# args.add_all( +# cc_toolchain.dynamic_runtime_lib(feature_configuration = feature_configuration), +# map_each = lambda x: get_lib_name(x, for_windows = toolchain.os.startswith("windows)), +# format_each = "-ldylib=%s", +# ) +def get_lib_name_for_windows(lib): + """Returns the name of a library artifact for Windows builds. + + Args: + lib (File): A library file + + Returns: + str: The name of the library + """ + # On macos and windows, dynamic/static libraries always end with the + # extension and potential versions will be before the extension, and should + # be part of the library name. + # On linux, the version usually comes after the extension. + # So regardless of the platform we want to find the extension and make + # everything left to it the library name. + + # Search for the extension - starting from the right - by removing any + # trailing digit. + comps = lib.basename.split(".") + for comp in reversed(comps): + if comp.isdigit(): + comps.pop() + else: + break + + # The library name is now everything minus the extension. + libname = ".".join(comps[:-1]) + + return libname + def abs(value): """Returns the absolute value of a number. From 110c77bd5878852b208a78f2d01f5168fbe3a51d Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Tue, 9 Aug 2022 15:43:02 +0200 Subject: [PATCH 2/2] Fix build failures --- rust/private/rustc.bzl | 31 ++++++------- rust/private/utils.bzl | 2 +- .../versioned_libs_unit_test.bzl | 44 +++++++++---------- 3 files changed, 39 insertions(+), 38 deletions(-) diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index 9c027dd81b..70193d33dc 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -27,7 +27,8 @@ load( "expand_dict_value_locations", "expand_list_element_locations", "find_cc_toolchain", - "get_lib_name", + "get_lib_name_default", + "get_lib_name_for_windows", "get_preferred_artifact", "is_exec_configuration", "make_static_lib_symlink", @@ -425,7 +426,7 @@ def _symlink_for_ambiguous_lib(actions, toolchain, crate_info, lib): # Take the absolute value of hash() since it could be negative. path_hash = abs(hash(lib.path)) - lib_name = get_lib_name_for_windows(lib) if toolchain.os.startswith("windows") else get_lib_name(lib) + lib_name = get_lib_name_for_windows(lib) if toolchain.os.startswith("windows") else get_lib_name_default(lib) prefix = "lib" extension = ".a" @@ -488,7 +489,7 @@ def _disambiguate_libs(actions, toolchain, crate_info, dep_info, use_pic): if _is_dylib(lib): continue artifact = get_preferred_artifact(lib, use_pic) - name = get_lib_name_for_windows(lib) if toolchain.os.startswith("windows") else get_lib_name(lib) + name = get_lib_name_for_windows(artifact) if toolchain.os.startswith("windows") else get_lib_name_default(artifact) # On Linux-like platforms, normally library base names start with # `lib`, following the pattern `lib[name].(a|lo)` and we pass @@ -1523,7 +1524,7 @@ def _get_crate_dirname(crate): """ return crate.output.dirname -def _portable_link_flags(lib, use_pic, ambiguous_libs, lib_name, for_windows): +def _portable_link_flags(lib, use_pic, ambiguous_libs, get_lib_name, for_windows): artifact = get_preferred_artifact(lib, use_pic) if ambiguous_libs and artifact.path in ambiguous_libs: artifact = ambiguous_libs[artifact.path] @@ -1561,14 +1562,14 @@ def _portable_link_flags(lib, use_pic, ambiguous_libs, lib_name, for_windows): artifact.basename.startswith("libtest-") or artifact.basename.startswith("libstd-") or artifact.basename.startswith("test-") or artifact.basename.startswith("std-") ): - return ["-lstatic=%s" % lib_name(artifact)] + return ["-lstatic=%s" % get_lib_name(artifact)] return [ - "-lstatic=%s" % lib_name(artifact), - "-Clink-arg=-l%s" % (lib_name(artifact) if not for_windows else artifact.basename), + "-lstatic=%s" % get_lib_name(artifact), + "-Clink-arg=-l%s" % (get_lib_name(artifact) if not for_windows else artifact.basename), ] elif _is_dylib(lib): return [ - "-ldylib=%s" % lib_name(artifact), + "-ldylib=%s" % get_lib_name(artifact), ] return [] @@ -1593,7 +1594,7 @@ def _make_link_flags_darwin(linker_input_and_use_pic_and_ambiguous_libs): ("link-arg=-Wl,-force_load,%s" % get_preferred_artifact(lib, use_pic).path), ]) else: - ret.extend(_portable_link_flags(lib, use_pic, ambiguous_libs, get_lib_name, for_windows = False)) + ret.extend(_portable_link_flags(lib, use_pic, ambiguous_libs, get_lib_name_default, for_windows = False)) return ret def _make_link_flags_default(linker_input_and_use_pic_and_ambiguous_libs): @@ -1610,7 +1611,7 @@ def _make_link_flags_default(linker_input_and_use_pic_and_ambiguous_libs): "link-arg=-Wl,--no-whole-archive", ]) else: - ret.extend(_portable_link_flags(lib, use_pic, ambiguous_libs, get_lib_name, for_windows = False)) + ret.extend(_portable_link_flags(lib, use_pic, ambiguous_libs, get_lib_name_default, for_windows = False)) return ret def _libraries_dirnames(linker_input_and_use_pic_and_ambiguous_libs): @@ -1639,13 +1640,13 @@ def _add_native_link_flags(args, dep_info, linkstamp_outs, ambiguous_libs, crate if toolchain.os == "windows": make_link_flags = _make_link_flags_windows - map_to_lib_name = get_lib_name_for_windows + get_lib_name = get_lib_name_for_windows elif toolchain.os.startswith("mac") or toolchain.os.startswith("darwin"): make_link_flags = _make_link_flags_darwin - map_to_lib_name = get_lib_name + get_lib_name = get_lib_name_default else: make_link_flags = _make_link_flags_default - map_to_lib_name = get_lib_name + get_lib_name = get_lib_name_default # TODO(hlopko): Remove depset flattening by using lambdas once we are on >=Bazel 5.0 args_and_pic_and_ambiguous_libs = [(arg, use_pic, ambiguous_libs) for arg in dep_info.transitive_noncrates.to_list()] @@ -1671,7 +1672,7 @@ def _add_native_link_flags(args, dep_info, linkstamp_outs, ambiguous_libs, crate ) args.add_all( cc_toolchain.dynamic_runtime_lib(feature_configuration = feature_configuration), - map_each = map_to_lib_name, + map_each = get_lib_name, format_each = "-ldylib=%s", ) else: @@ -1684,7 +1685,7 @@ def _add_native_link_flags(args, dep_info, linkstamp_outs, ambiguous_libs, crate ) args.add_all( cc_toolchain.static_runtime_lib(feature_configuration = feature_configuration), - map_each = map_to_lib_name, + map_each = get_lib_name, format_each = "-lstatic=%s", ) diff --git a/rust/private/utils.bzl b/rust/private/utils.bzl index c99c7932b4..583dc6e5ee 100644 --- a/rust/private/utils.bzl +++ b/rust/private/utils.bzl @@ -97,7 +97,7 @@ def _path_parts(path): path_parts = path.split("/") return [part for part in path_parts if part != "."] -def get_lib_name(lib): +def get_lib_name_default(lib): """Returns the name of a library artifact. Args: diff --git a/test/unit/versioned_libs/versioned_libs_unit_test.bzl b/test/unit/versioned_libs/versioned_libs_unit_test.bzl index 0a3052510b..1ede046104 100644 --- a/test/unit/versioned_libs/versioned_libs_unit_test.bzl +++ b/test/unit/versioned_libs/versioned_libs_unit_test.bzl @@ -3,32 +3,32 @@ load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest") # buildifier: disable=bzl-visibility -load("//rust/private:utils.bzl", "get_lib_name") +load("//rust/private:utils.bzl", "get_lib_name_default", "get_lib_name_for_windows") def _produced_expected_lib_name_test_impl(ctx): env = unittest.begin(ctx) - asserts.equals(env, "python", get_lib_name(struct(basename = "libpython.dylib"))) - asserts.equals(env, "python", get_lib_name(struct(basename = "libpython.so"))) - asserts.equals(env, "python", get_lib_name(struct(basename = "libpython.a"))) - asserts.equals(env, "python", get_lib_name(struct(basename = "python.dll"))) - asserts.equals(env, "python", get_lib_name(struct(basename = "python.lib"))) - - asserts.equals(env, "python3", get_lib_name(struct(basename = "libpython3.dylib"))) - asserts.equals(env, "python3.8", get_lib_name(struct(basename = "libpython3.8.dylib"))) - asserts.equals(env, "python3", get_lib_name(struct(basename = "libpython3.a"))) - asserts.equals(env, "python3.8", get_lib_name(struct(basename = "libpython3.8.a"))) - - asserts.equals(env, "python38", get_lib_name(struct(basename = "python38.dll"))) - asserts.equals(env, "python38m", get_lib_name(struct(basename = "python38m.dll"))) - - asserts.equals(env, "python", get_lib_name(struct(basename = "libpython.so.3"))) - asserts.equals(env, "python", get_lib_name(struct(basename = "libpython.so.3.8"))) - asserts.equals(env, "python", get_lib_name(struct(basename = "libpython.so.3.8.0"))) - asserts.equals(env, "python", get_lib_name(struct(basename = "libpython.a.3"))) - asserts.equals(env, "python", get_lib_name(struct(basename = "libpython.a.3.8"))) - asserts.equals(env, "python", get_lib_name(struct(basename = "libpython.a.3.8.0"))) - asserts.equals(env, "python-3.8.0", get_lib_name(struct(basename = "libpython-3.8.0.so.3.8.0"))) + asserts.equals(env, "python", get_lib_name_default(struct(basename = "libpython.dylib"))) + asserts.equals(env, "python", get_lib_name_default(struct(basename = "libpython.so"))) + asserts.equals(env, "python", get_lib_name_default(struct(basename = "libpython.a"))) + asserts.equals(env, "python", get_lib_name_for_windows(struct(basename = "python.dll"))) + asserts.equals(env, "python", get_lib_name_for_windows(struct(basename = "python.lib"))) + + asserts.equals(env, "python3", get_lib_name_default(struct(basename = "libpython3.dylib"))) + asserts.equals(env, "python3.8", get_lib_name_default(struct(basename = "libpython3.8.dylib"))) + asserts.equals(env, "python3", get_lib_name_default(struct(basename = "libpython3.a"))) + asserts.equals(env, "python3.8", get_lib_name_default(struct(basename = "libpython3.8.a"))) + + asserts.equals(env, "python38", get_lib_name_for_windows(struct(basename = "python38.dll"))) + asserts.equals(env, "python38m", get_lib_name_for_windows(struct(basename = "python38m.dll"))) + + asserts.equals(env, "python", get_lib_name_default(struct(basename = "libpython.so.3"))) + asserts.equals(env, "python", get_lib_name_default(struct(basename = "libpython.so.3.8"))) + asserts.equals(env, "python", get_lib_name_default(struct(basename = "libpython.so.3.8.0"))) + asserts.equals(env, "python", get_lib_name_default(struct(basename = "libpython.a.3"))) + asserts.equals(env, "python", get_lib_name_default(struct(basename = "libpython.a.3.8"))) + asserts.equals(env, "python", get_lib_name_default(struct(basename = "libpython.a.3.8.0"))) + asserts.equals(env, "python-3.8.0", get_lib_name_default(struct(basename = "libpython-3.8.0.so.3.8.0"))) return unittest.end(env)