From 64a4b529a12bdd65e32b6f79eb204588e4ff5e91 Mon Sep 17 00:00:00 2001 From: Krasimir Georgiev Date: Tue, 23 Jul 2024 15:11:56 +0200 Subject: [PATCH] rust_stdlib_filegroup: update to support srcs from another package (#2735) The `rust_stdlib_filegroup` rule would sometimes break when passing `.rlib`-s originating from another package to `srcs`. The root cause is the logic inside it that creates `.a` -> `.rlib` symlinks, where the `.a` symlink is declared sibling of the `.rlib`. Bazel only allows symlink siblings to be in the same package as the rule. This updates the logic to detect this and create an intermediary `.rlib` under the rule's package, allowing the `.a` to be symlinked to that. --------- Co-authored-by: scentini --- rust/private/rustc.bzl | 2 +- rust/private/utils.bzl | 16 ++++++++++++- rust/toolchain.bzl | 2 +- test/unit/toolchain/subpackage/BUILD.bazel | 11 +++++++++ test/unit/toolchain/toolchain_test.bzl | 27 ++++++++++++++++++++++ 5 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 test/unit/toolchain/subpackage/BUILD.bazel diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index dd3d816cac..5012678362 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -1616,7 +1616,7 @@ def establish_cc_info(ctx, attr, crate_info, toolchain, cc_toolchain, feature_co # bazel hard-codes a check for endswith((".a", ".pic.a", # ".lib")) in create_library_to_link, so we work around that # by creating a symlink to the .rlib with a .a extension. - dot_a = make_static_lib_symlink(ctx.actions, crate_info.output) + dot_a = make_static_lib_symlink(ctx.label.package, ctx.actions, crate_info.output) # TODO(hlopko): handle PIC/NOPIC correctly library_to_link = cc_common.create_library_to_link( diff --git a/rust/private/utils.bzl b/rust/private/utils.bzl index 57a3fe7a7e..5bd2c92d56 100644 --- a/rust/private/utils.bzl +++ b/rust/private/utils.bzl @@ -420,7 +420,7 @@ def dedent(doc_string): block = " " * space_count return "\n".join([line.replace(block, "", 1).rstrip() for line in lines]) -def make_static_lib_symlink(actions, rlib_file): +def make_static_lib_symlink(ctx_package, actions, rlib_file): """Add a .a symlink to an .rlib file. The name of the symlink is derived from the of the .rlib file as follows: @@ -432,17 +432,31 @@ def make_static_lib_symlink(actions, rlib_file): * `crateb.rlib` is `libcrateb.a`. Args: + ctx_package (string): The rule's context package name. actions (actions): The rule's context actions object. rlib_file (File): The file to symlink, which must end in .rlib. Returns: The symlink's File. """ + if not rlib_file.basename.endswith(".rlib"): fail("file is not an .rlib: ", rlib_file.basename) basename = rlib_file.basename[:-5] if not basename.startswith("lib"): basename = "lib" + basename + + # The .a symlink below is created as a sibling to the .rlib file. + # Bazel doesn't allow creating a symlink outside of the rule's package, + # so if the .rlib file comes from a different package, first symlink it + # to the rule's package. The name of the new .rlib symlink is derived + # as the name of the original .rlib relative to its package. + if rlib_file.owner.package != ctx_package: + new_path = rlib_file.short_path.removeprefix(rlib_file.owner.package).removeprefix("/") + new_rlib_file = actions.declare_file(new_path) + actions.symlink(output = new_rlib_file, target_file = rlib_file) + rlib_file = new_rlib_file + dot_a = actions.declare_file(basename + ".a", sibling = rlib_file) actions.symlink(output = dot_a, target_file = rlib_file) return dot_a diff --git a/rust/toolchain.bzl b/rust/toolchain.bzl index ae3fcefebb..8f900bd058 100644 --- a/rust/toolchain.bzl +++ b/rust/toolchain.bzl @@ -54,7 +54,7 @@ def _rust_stdlib_filegroup_impl(ctx): # # alloc depends on the allocator_library if it's configured, but we # do that later. - dot_a_files = [make_static_lib_symlink(ctx.actions, f) for f in std_rlibs] + dot_a_files = [make_static_lib_symlink(ctx.label.package, ctx.actions, f) for f in std_rlibs] alloc_files = [f for f in dot_a_files if "alloc" in f.basename and "std" not in f.basename] between_alloc_and_core_files = [f for f in dot_a_files if "compiler_builtins" in f.basename] diff --git a/test/unit/toolchain/subpackage/BUILD.bazel b/test/unit/toolchain/subpackage/BUILD.bazel new file mode 100644 index 0000000000..0c8ace8c97 --- /dev/null +++ b/test/unit/toolchain/subpackage/BUILD.bazel @@ -0,0 +1,11 @@ +genrule( + name = "dummy_rlib", + outs = ["core.rlib"], + cmd = "touch $@", +) + +filegroup( + name = "std_libs_srcs", + srcs = [":dummy_rlib"], + visibility = ["//test/unit/toolchain:__subpackages__"], +) diff --git a/test/unit/toolchain/toolchain_test.bzl b/test/unit/toolchain/toolchain_test.bzl index d4056f1424..d8e0effb1a 100644 --- a/test/unit/toolchain/toolchain_test.bzl +++ b/test/unit/toolchain/toolchain_test.bzl @@ -47,9 +47,26 @@ def _toolchain_location_expands_linkflags_impl(ctx): return analysistest.end(env) +def _std_libs_support_srcs_outside_package_test_impl(ctx): + env = analysistest.begin(ctx) + tut = analysistest.target_under_test(env) + actions = analysistest.target_actions(env) + + symlinks = [a for a in actions if a.mnemonic == "Symlink"] + asserts.equals(env, 2, len(symlinks)) + + rlib_symlink = symlinks[0].outputs.to_list()[0] + asserts.equals(env, tut.label.package + "/core.rlib", rlib_symlink.short_path) + + a_symlink = symlinks[1].outputs.to_list()[0] + asserts.equals(env, tut.label.package + "/libcore.a", a_symlink.short_path) + + return analysistest.end(env) + toolchain_specifies_target_triple_test = analysistest.make(_toolchain_specifies_target_triple_test_impl) toolchain_specifies_target_json_test = analysistest.make(_toolchain_specifies_target_json_test_impl) toolchain_location_expands_linkflags_test = analysistest.make(_toolchain_location_expands_linkflags_impl) +std_libs_support_srcs_outside_package_test = analysistest.make(_std_libs_support_srcs_outside_package_test_impl) def _define_test_targets(): native.filegroup( @@ -61,6 +78,11 @@ def _define_test_targets(): srcs = [":stdlib_srcs"], ) + rust_stdlib_filegroup( + name = "std_libs_with_srcs_outside_package", + srcs = ["//test/unit/toolchain/subpackage:std_libs_srcs"], + ) + native.filegroup( name = "target_json", srcs = ["toolchain-test-triple.json"], @@ -166,6 +188,10 @@ def toolchain_test_suite(name): name = "toolchain_location_expands_linkflags_test", target_under_test = ":rust_location_expand_toolchain", ) + std_libs_support_srcs_outside_package_test( + name = "std_libs_support_srcs_outside_package_test", + target_under_test = ":std_libs_with_srcs_outside_package", + ) native.test_suite( name = name, @@ -174,5 +200,6 @@ def toolchain_test_suite(name): ":toolchain_specifies_target_json_test", ":toolchain_specifies_inline_target_json_test", ":toolchain_location_expands_linkflags_test", + ":std_libs_support_srcs_outside_package_test", ], )