Skip to content

Do not pass --Clink-arg=-l for libstd and libtest #1500

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Aug 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions rust/private/rustc.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
lib_name = get_lib_name(lib, for_windows = toolchain.os.startswith("windows"))

prefix = "lib"
extension = ".a"
Expand Down Expand Up @@ -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)
name = get_lib_name(artifact, for_windows = toolchain.os.startswith("windows"))

# On Linux-like platforms, normally library base names start with
# `lib`, following the pattern `lib[name].(a|lo)` and we pass
Expand Down Expand Up @@ -1523,7 +1523,7 @@ def _get_crate_dirname(crate):
"""
return crate.output.dirname

def _portable_link_flags(lib, use_pic, ambiguous_libs):
def _portable_link_flags(lib, use_pic, ambiguous_libs, for_windows = False):
artifact = get_preferred_artifact(lib, use_pic)
if ambiguous_libs and artifact.path in ambiguous_libs:
artifact = ambiguous_libs[artifact.path]
Expand All @@ -1544,13 +1544,31 @@ def _portable_link_flags(lib, use_pic, ambiguous_libs):
# and adding references to these symlinks in the native section A.
# We rely in the behavior of -Clink-arg to put the linker args
# at the end of the linker invocation constructed by rustc.

# We skip adding `-Clink-arg=-l` for libstd and libtest from the standard library, as
# these two libraries are present both as an `.rlib` and a `.so` format.
# On linux, Rustc adds a -Bdynamic to the linker command line before the libraries specified
# with `-Clink-arg`, which leads to us linking against the `.so`s but not putting the
# corresponding value to the runtime library search paths, which results in a
# "cannot open shared object file: No such file or directory" error at exectuion time.
# We can fix this by adding a `-Clink-arg=-Bstatic` on linux, but we don't have that option for
# macos. The proper solution for this issue would be to remove `libtest-{hash}.so` and `libstd-{hash}.so`
# from the toolchain. However, it is not enough to change the toolchain's `rust_std_{...}` filegroups
# here: https://github.com/bazelbuild/rules_rust/blob/a9d5d894ad801002d007b858efd154e503796b9f/rust/private/repository_utils.bzl#L144
# because rustc manages to escape the sandbox and still finds them at linking time.
# We need to modify the repository rules to erase those files completely.
if "lib/rustlib" in artifact.path and (
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),
"-Clink-arg=-l%s" % get_lib_name(artifact),
"-lstatic=%s" % get_lib_name(artifact, for_windows),
"-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),
"-ldylib=%s" % get_lib_name(artifact, for_windows),
]

return []
Expand All @@ -1562,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))
ret.extend(_portable_link_flags(lib, use_pic, ambiguous_libs, for_windows = True))
return ret

def _make_link_flags_darwin(linker_input_and_use_pic_and_ambiguous_libs):
Expand Down
5 changes: 3 additions & 2 deletions rust/private/utils.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,12 @@ 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(lib, for_windows = False):
"""Returns the name of a library artifact, eg. libabc.a -> abc

Args:
lib (File): A library file
for_windows: Whether we're building on Windows.

Returns:
str: The name of the library
Expand All @@ -125,7 +126,7 @@ def get_lib_name(lib):
# The library name is now everything minus the extension.
libname = ".".join(comps[:-1])

if libname.startswith("lib"):
if libname.startswith("lib") and not for_windows:
return libname[3:]
else:
return libname
Expand Down
26 changes: 26 additions & 0 deletions test/native_deps/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
load("@rules_cc//cc:defs.bzl", "cc_library")
load(
"@rules_rust//rust:defs.bzl",
"rust_library",
"rust_test",
)

rust_library(
name = "transitive",
srcs = ["transitive.rs"],
edition = "2018",
)

cc_library(
name = "direct",
srcs = ["direct.cc"],
hdrs = ["direct.h"],
deps = ["transitive"],
)

rust_test(
name = "main",
srcs = ["main.rs"],
edition = "2018",
deps = ["direct"],
)
6 changes: 6 additions & 0 deletions test/native_deps/direct.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include "direct.h"

RustStruct MakeRustStruct() {
RustStruct result;
return result;
}
4 changes: 4 additions & 0 deletions test/native_deps/direct.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
struct RustStruct{};

extern "C" RustStruct MakeRustStruct();

3 changes: 3 additions & 0 deletions test/native_deps/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
println!("Done!");
}
2 changes: 2 additions & 0 deletions test/native_deps/transitive.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#[repr(C)]
pub struct RustStruct {}
Empty file added test/native_deps/user.rs
Empty file.
32 changes: 20 additions & 12 deletions test/unit/native_deps/native_deps_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ def _cdylib_has_native_libs_test_impl(ctx):
assert_argv_contains_prefix_suffix(env, action, "-Lnative=", "/native_deps")
assert_argv_contains(env, action, "--crate-type=cdylib")
assert_argv_contains(env, action, "-lstatic=native_dep")
assert_argv_contains(env, action, "-Clink-arg=-lnative_dep")
native_link_arg = "-Clink-arg=-lnative_dep.lib" if ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo]) else "-Clink-arg=-lnative_dep"
assert_argv_contains(env, action, native_link_arg)
assert_argv_contains_prefix(env, action, "--codegen=linker=")
return analysistest.end(env)

Expand All @@ -41,22 +42,20 @@ def _staticlib_has_native_libs_test_impl(ctx):
assert_argv_contains_prefix_suffix(env, action, "-Lnative=", "/native_deps")
assert_argv_contains(env, action, "--crate-type=staticlib")
assert_argv_contains(env, action, "-lstatic=native_dep")
assert_argv_contains(env, action, "-Clink-arg=-lnative_dep")
native_link_arg = "-Clink-arg=-lnative_dep.lib" if ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo]) else "-Clink-arg=-lnative_dep"
assert_argv_contains(env, action, native_link_arg)
assert_argv_contains_prefix(env, action, "--codegen=linker=")
return analysistest.end(env)

def _proc_macro_has_native_libs_test_impl(ctx):
env = analysistest.begin(ctx)
tut = analysistest.target_under_test(env)
if ctx.configuration.coverage_enabled:
asserts.equals(env, 2, len(tut.actions))
else:
asserts.equals(env, 1, len(tut.actions))
action = tut.actions[0]
assert_argv_contains_prefix_suffix(env, action, "-Lnative=", "/native_deps")
assert_argv_contains(env, action, "--crate-type=proc-macro")
assert_argv_contains(env, action, "-lstatic=native_dep")
assert_argv_contains(env, action, "-Clink-arg=-lnative_dep")
native_link_arg = "-Clink-arg=-lnative_dep.lib" if ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo]) else "-Clink-arg=-lnative_dep"
assert_argv_contains(env, action, native_link_arg)
assert_argv_contains_prefix(env, action, "--codegen=linker=")
return analysistest.end(env)

Expand All @@ -66,7 +65,8 @@ def _bin_has_native_libs_test_impl(ctx):
action = tut.actions[0]
assert_argv_contains_prefix_suffix(env, action, "-Lnative=", "/native_deps")
assert_argv_contains(env, action, "-lstatic=native_dep")
assert_argv_contains(env, action, "-Clink-arg=-lnative_dep")
native_link_arg = "-Clink-arg=-lnative_dep.lib" if ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo]) else "-Clink-arg=-lnative_dep"
assert_argv_contains(env, action, native_link_arg)
assert_argv_contains_prefix(env, action, "--codegen=linker=")
return analysistest.end(env)

Expand Down Expand Up @@ -143,10 +143,18 @@ def _cdylib_has_native_dep_and_alwayslink_test_impl(ctx):
return analysistest.end(env)

rlib_has_no_native_libs_test = analysistest.make(_rlib_has_no_native_libs_test_impl)
staticlib_has_native_libs_test = analysistest.make(_staticlib_has_native_libs_test_impl)
cdylib_has_native_libs_test = analysistest.make(_cdylib_has_native_libs_test_impl)
proc_macro_has_native_libs_test = analysistest.make(_proc_macro_has_native_libs_test_impl)
bin_has_native_libs_test = analysistest.make(_bin_has_native_libs_test_impl)
staticlib_has_native_libs_test = analysistest.make(_staticlib_has_native_libs_test_impl, attrs = {
"_windows_constraint": attr.label(default = Label("@platforms//os:windows")),
})
cdylib_has_native_libs_test = analysistest.make(_cdylib_has_native_libs_test_impl, attrs = {
"_windows_constraint": attr.label(default = Label("@platforms//os:windows")),
})
proc_macro_has_native_libs_test = analysistest.make(_proc_macro_has_native_libs_test_impl, attrs = {
"_windows_constraint": attr.label(default = Label("@platforms//os:windows")),
})
bin_has_native_libs_test = analysistest.make(_bin_has_native_libs_test_impl, attrs = {
"_windows_constraint": attr.label(default = Label("@platforms//os:windows")),
})
bin_has_native_dep_and_alwayslink_test = analysistest.make(_bin_has_native_dep_and_alwayslink_test_impl, attrs = {
"_macos_constraint": attr.label(default = Label("@platforms//os:macos")),
"_windows_constraint": attr.label(default = Label("@platforms//os:windows")),
Expand Down