Apply get_lib_name correctly to the C++ runtime libraries (#1508)

#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`.

Author: scentini

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(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 @@ 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(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, 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 @@ 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" % 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 @@ 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 +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))
+            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))
+            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 @@ def _add_native_link_flags(args, dep_info, linkstamp_outs, ambiguous_libs, crate
 
     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()]

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_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 @@ 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.
 

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)