-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
rust: allow linking with dynamic libstd #14224
base: master
Are you sure you want to change the base?
Conversation
df1552f
to
daf6d4a
Compare
8f12ae2
to
273198e
Compare
273198e
to
4e48662
Compare
# -Z staticlib-prefer-dynamic) are not yet stable; alternatively, | ||
# one could use "--emit obj" (implemented in the pull request at | ||
# https://github.com/mesonbuild/meson/pull/11213) or "--emit rlib" | ||
# (officially not recommended for linking with C programs). |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
for arg in args: | ||
rustc_rpath_args.append('-C') | ||
rustc_rpath_args.append(f'link-arg={arg}:{self.get_target_libdir()}') | ||
rustc_rpath_args.append('link-arg=' + arg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
quasi related, btw: meson currently doesn't have a unified way to specifically pass link args for rust, only these ad-hoc spots #13537
Allow adding extra directories to the rpath. Rust needs this when Rustup is in use. Signed-off-by: Paolo Bonzini <[email protected]>
RustCompiler.build_rpath_args works by appending the directory to the arguments computed by self.linker.build_rpath_args. This does not work if there is no argument to begin with, which happens for example in program crates. Use the new extra_paths argument to force inclusion of the libdir into the rpath of the binary, even in that case. Signed-off-by: Paolo Bonzini <[email protected]>
As an initial implementation, simply adding "-C prefer-dynamic" works for binary crates (as well as dylib and proc-macro that already used it). In the future this could be extended to other crate types. For more information see the comment in the changed file, as well as mesonbuild#8828 and mesonbuild#14215. Signed-off-by: Paolo Bonzini <[email protected]>
4e48662
to
6365f28
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I am a little bit confused about some existing code but that's unrelated to the PR itself.
# add prefer-dynamic if any of the Rust libraries we link | ||
# against are dynamic or this is a dynamic library itself, | ||
# otherwise we'll end up with multiple implementations of libstd. | ||
args += ['-C', 'prefer-dynamic'] | ||
|
||
if isinstance(target, build.SharedLibrary) or has_shared_deps: | ||
if isinstance(target, build.SharedLibrary) or has_shared_deps \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's kinda confusing that it's just checking if the target is a rust shared lib at all (isinstance(target, build.SharedLibrary
): seems like it would add rustc to RPATH for cdylibs without rust_dynamic_std specified? idk though!
As a first step towards fixing #8828, allow linking Rust bin crates with dynamic libstd.
staticlib crates cannot yet do this, but this can be extended later on; in the meanwhile, this PR creates the API with a minimal implementation.