Apply get_lib_name correctly to the C++ runtime libraries (#1508)
https://github.com/bazelbuild/rules_rust/pull/1500 added an additional `for_windows` parameter to `get_lib_name`. I missed the fact that we also pass that function to `map_each` here: https://github.com/bazelbuild/rules_rust/blob/main/rust/private/rustc.bzl#L1671
and as such, this code does not always work correctly (we don't get to pass the `for_windows` parameter, and internally at Google it ended up evaluating to `True` on Linux builds).
I tried to avoid flattening the `cc_toolchain.dynamic_runtime_lib` and `cc_toolchain.static_runtime_lib` depsets by using a lambda:
```
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",
)
```
However it looks like such usage of lambdas is not allowed:
```
Error in add_all: to avoid unintended retention of analysis data structures,
the map_each function (declared at ...) must be declared by a top-level def statement
```
So instead of `get_lib_name` we now have `get_lib_name_default` and `get_lib_name_for_windows`.diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl
index 97b8a17..70193d3 100644
--- a/rust/private/rustc.bzl
+++ b/rust/private/rustc.bzl
@@ -27,7 +27,8 @@
"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 @@
# 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_default(lib)
prefix = "lib"
extension = ".a"
@@ -488,7 +489,7 @@
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(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 @@
"""
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, 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 @@
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" % get_lib_name(artifact)]
return [
- "-lstatic=%s" % get_lib_name(artifact, for_windows),
+ "-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" % get_lib_name(artifact, for_windows),
+ "-ldylib=%s" % get_lib_name(artifact),
]
return []
@@ -1580,7 +1581,7 @@
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 +1594,7 @@
("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_default, for_windows = False))
return ret
def _make_link_flags_default(linker_input_and_use_pic_and_ambiguous_libs):
@@ -1610,7 +1611,7 @@
"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_default, for_windows = False))
return ret
def _libraries_dirnames(linker_input_and_use_pic_and_ambiguous_libs):
@@ -1639,10 +1640,13 @@
if toolchain.os == "windows":
make_link_flags = _make_link_flags_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
+ get_lib_name = get_lib_name_default
else:
make_link_flags = _make_link_flags_default
+ 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()]
diff --git a/rust/private/utils.bzl b/rust/private/utils.bzl
index e342b3b..583dc6e 100644
--- a/rust/private/utils.bzl
+++ b/rust/private/utils.bzl
@@ -97,12 +97,11 @@
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_default(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 @@
# 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.
diff --git a/test/unit/versioned_libs/versioned_libs_unit_test.bzl b/test/unit/versioned_libs/versioned_libs_unit_test.bzl
index 0a30525..1ede046 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, "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(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, "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(struct(basename = "python38.dll")))
- asserts.equals(env, "python38m", get_lib_name(struct(basename = "python38m.dll")))
+ 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(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.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)
