mesonbuild / meson

The Meson Build System
http://mesonbuild.com
Apache License 2.0
5.35k stars 1.53k forks source link

rust: add new 'object' type crate #11213

Open bluca opened 1 year ago

bluca commented 1 year ago

Allows linking Rust sources in C/C++ programs as static object files, which means the Rust standard libraries/crates are not statically linked in the program, saving a huge amount of installation space. The caller is responsible for providing linkage to the standard library and/or any needed external crate, and for providing allocators stubs if needed.

Fixes https://github.com/mesonbuild/meson/issues/8828

It will be used by systemd: https://github.com/systemd/systemd/pull/19598

codecov[bot] commented 1 year ago

Codecov Report

Merging #11213 (d57da04) into master (0544ffa) will decrease coverage by 2.83%. The diff coverage is n/a.

:exclamation: Current head d57da04 differs from pull request most recent head 759137b. Consider uploading reports for the commit 759137b to get more accurate results

@@            Coverage Diff             @@
##           master   #11213      +/-   ##
==========================================
- Coverage   68.82%   65.99%   -2.84%     
==========================================
  Files         414      207     -207     
  Lines       90124    45075   -45049     
  Branches    21310     9339   -11971     
==========================================
- Hits        62027    29746   -32281     
+ Misses      23438    12981   -10457     
+ Partials     4659     2348    -2311     
Impacted Files Coverage Δ
scripts/clangtidy.py 0.00% <0.00%> (-93.34%) :arrow_down:
modules/cuda.py 0.00% <0.00%> (-69.82%) :arrow_down:
templates/cudatemplates.py 37.50% <0.00%> (-62.50%) :arrow_down:
compilers/cuda.py 19.63% <0.00%> (-45.40%) :arrow_down:
dependencies/cuda.py 19.23% <0.00%> (-43.75%) :arrow_down:
modules/icestorm.py 57.14% <0.00%> (-40.00%) :arrow_down:
compilers/cython.py 43.18% <0.00%> (-38.64%) :arrow_down:
compilers/d.py 38.33% <0.00%> (-18.60%) :arrow_down:
dependencies/coarrays.py 45.00% <0.00%> (-17.50%) :arrow_down:
compilers/mixins/clang.py 52.87% <0.00%> (-13.80%) :arrow_down:
... and 262 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

eli-schwartz commented 1 year ago

Tests fail because the expected error message in failing: 54 wrong static crate type's test.json no longer matches.

bluca commented 1 year ago

should be fixed now

bjorn3 commented 1 year ago

and for providing allocators stubs if needed.

These are unstable and will likely break in the future (be it to provide a stable alternative or to remove this ability entirely. either option would likely be necessary for avoiding conflicts between rust staticlibs compiled by different rustc versions in the absence of a fix for https://github.com/rust-lang/rust/issues/104707). https://github.com/rust-lang/rust/pull/86844 would allow it to eventually work in a stable way, but the response was this:

[...] We don't think, at this time, that we want to constrain all future design space by committing to the default output format (rlib) being something that works with any linker. [...]

so it might not land, or might take a long time to land. In any case cc @joshtriplett so that you are aware of this PR and to correct me in case I misunderstood your comment.

dcbaker commented 1 year ago

I’d really like to hear what Josh has to say, this would be the first step we’ve really taken to provide a feature (AFAIK) that cargo doesn’t provide in some form

bluca commented 1 year ago

and for providing allocators stubs if needed.

These are unstable and will likely break in the future (be it to provide a stable alternative or to remove this ability entirely. either option would likely be necessary for avoiding conflicts between rust staticlibs compiled by different rustc versions in the absence of a fix for rust-lang/rust#104707). rust-lang/rust#86844 would allow it to eventually work in a stable way, but the response was this:

As far as I can see these are already in use in the wild, so that ship has sailed? Anyway we'd just do what the kernel does in that case, if upstream is too dogmatic to provide something useful. I don't think it's really related to this PR anyway, as I'm already using the functionality without any allocator at all?

[...] We don't think, at this time, that we want to constrain all future design space by committing to the default output format (rlib) being something that works with any linker. [...]

so it might not land, or might take a long time to land. In any case cc @joshtriplett so that you are aware of this PR and to correct me in case I misunderstood your comment.

Sorry not sure I follow here, we are not using rlib here, but just plain elf objects, no?

bluca commented 1 year ago

I’d really like to hear what Josh has to say, this would be the first step we’ve really taken to provide a feature (AFAIK) that cargo doesn’t provide in some form

This is in practice just adding a wrapper for '--emit obj' so it can be done with plain rust tools as well, i think?

bjorn3 commented 1 year ago

As far as I can see these are already in use in the wild, so that ship has sailed? Anyway we'd just do what the kernel does in that case, if upstream is too dogmatic to provide something useful.

The rust for linux explicitly only works with a single rustc version. It uses a decent amount of other unstable features either way, so new rustc versions can and do require adapting to changes on the rust for linux side. If you use unstable features, then it is up to you as user to fix any breakage resulting from changes to those unstable features, not up to the rust project. Note that I opened https://github.com/rust-lang/rust/pull/86844 precisely to be able to move rust for linux away from this allocator hack.

Sorry not sure I follow here, we are not using rlib here, but just plain elf objects, no?

I am pretty sure that comment was meant for directly linking anything produced by rustc other than the the libraries produced by the dylib, cdylib or staticlib crate types. This includes both rlibs and --emit obj.

bluca commented 1 year ago

As far as I can see these are already in use in the wild, so that ship has sailed? Anyway we'd just do what the kernel does in that case, if upstream is too dogmatic to provide something useful.

The rust for linux explicitly only works with a single rustc version. It uses a decent amount of other unstable features either way, so new rustc versions can and do require adapting to changes on the rust for linux side. If you use unstable features, then it is up to you as user to fix any breakage resulting from changes to those unstable features, not up to the rust project. Note that I opened rust-lang/rust#86844 precisely to be able to move rust for linux away from this allocator hack.

It's not just the kernel, there's plenty of other projects using similar workarounds to overcome rust's shortcomings, see for example: https://github.com/bazelbuild/rules_rust/blob/main/test/cc_common_link/allocator_library.cc https://sources.debian.org/src/chromium/108.0.5359.124-1/build/rust/std/remap_alloc.cc/ https://sources.debian.org/src/firefox/108.0-2/third_party/libwebrtc/build/rust/std/remap_alloc.c/

Anyway, it's really off-topic, I'll remove that comment from the commit message as it's not useful here.

Sorry not sure I follow here, we are not using rlib here, but just plain elf objects, no?

I am pretty sure that comment was meant for directly linking anything produced by rustc other than the the libraries produced by the dylib, cdylib or staticlib crate types. This includes both rlibs and --emit obj.

Not sure I follow, the internal rlib format is one thing, but an object file is an object file, it's either valid or it isn't...

bluca commented 1 year ago

I’d really like to hear what Josh has to say, this would be the first step we’ve really taken to provide a feature (AFAIK) that cargo doesn’t provide in some form

Btw @dcbaker this PR is implementing your suggestion from https://github.com/systemd/systemd/pull/19598#discussion_r633898326 - unless I've misunderstood those comments?

That sounds doable. we can probably add some "fake" crate types or an option for rust of "force object". It wouldn't surprise me if zig has the same limitation.

If you prefer a "force object" flag instead of the fake crate type, I can implement that instead, I don't mind one way or the other, whichever works best for Meson. Let me know and I can change it.

bjorn3 commented 1 year ago

Sorry not sure I follow here, we are not using rlib here, but just plain elf objects, no?

I am pretty sure that comment was meant for directly linking anything produced by rustc other than the the libraries produced by the dylib, cdylib or staticlib crate types. This includes both rlibs and --emit obj.

Not sure I follow, the internal rlib format is one thing, but an object file is an object file, it's either valid or it isn't...

As far as I understand --emit obj was meant for debugging, not for production usage. In any case several features expect extra object files or libraries to be linked that are not emitted as part of --emit obj. This includes the allocator shim (for liballoc and if the allocator error handler in libstd should panic or abort), libpanic_abort/libpanic_unwind (for panicking with libstd), libcompiler_builtins (most functionality provided by libgcc or compiler-rt, but for example the stack probing function (necessary on x86_64 only right now, but may be extended to more platforms in the futute) has a name unique to rust)

Again it is fine to use this if you accept that it will break occasionally. However I'm worried about encouraging it's use without people getting these warnings by adding it to meson directly.

eli-schwartz commented 1 year ago

Anyway we'd just do what the kernel does in that case, if upstream is too dogmatic to provide something useful.

You mean "wait for rust to mature as a language"? :P

I'm very much an anti-fan of catering to unstable rust. I can kind of sort of swallow the idea that unstable rust exists in the first place (salt is involved), I just don't think I've ever actually been happy to find it used in actual, you know, software. Which people do. All the time.

Building Meson on top of a house of cards feels unfortunate, because we all know that Meson is just going to end up breaking as a result.

IMO if we do implement such a feature at all, it should emit a warning message telling you not to do it.

bluca commented 1 year ago

I don't think emit obj is unstable? Again the whole allocator thread is wildly off topic and unrelated, it has nothing to do with this PR.

joshtriplett commented 1 year ago

As @bjorn3 said, I'd generally expect that --emit obj will tend to not align with the stability people expect of Rust. It is unlikely to go away (we don't typically remove stable compiler options that anyone is using), but there's no long-term guarantee that the result is usable in any particular form other than feeding it back into Rust. We haven't provided such a mechanism yet, and --emit obj isn't that mechanism despite having the apparent input and output types (Rust code -> object) that you might be expecting.

A C equivalent would be something like "expecting you to link libgcc or equivalent yourself to get necessary builtins", but without the expectation that -lgcc (or compiler-specific equivalent) suffices. As an example, the Linux kernel doesn't use the compiler's own libraries, and instead provides its own versions of built-in functions. As a result, new versions of GCC have broken the Linux kernel in a variety of ways; among them, requiring a new builtin that the kernel hasn't implemented yet. The kernel can of course "just" deal with that by adding the new builtin as soon as someone tests the kernel with that new version of GCC, because a large army of people are testing bleeding-edge kernels with bleeding-edge compilers. But that's not a solution that scales well down to smaller projects, so smaller projects often end up linking libgcc or incurring more maintenance effort.

Rust, similarly, doesn't directly guarantee the exact surface area of "what you have to provide to link a Rust object". (I'm not just talking about the standard library here, or even core.) Cargo static or dynamic libraries do provide such a guarantee, insofar as linking it for you; using rustc as a linker similarly provides more guarantees. (Neither of those works for everyone, of course.) And in the future I hope we provide more output types that make such guarantees; for instance, "build this Rust crate with its Rust interface and export that from a library". (That's something several of us are personally pushing towards right now.)

I want to explicitly say that this is a use case we care about. (That's not the same as "have the bandwidth to highly prioritize", but we do care.) I do want it to be easy to link C and Rust together, not just by creating whole libraries, but also by being able to call any C function for which you have a header, or conversely export a C header. And I don't think this should require invoking external tools like bindgen or cbindgen, either; I would like this to be as easy as rustc abc.rs xyz.c or similar. We're not there yet right now, though.

In the meantime, I would suggest that attempting to treat Rust like C and "just" emit an object file and then link it with the system linker will produce disappointment.[^1] And, frankly, I would expect some of that disappointment to end up directed at Rust upstream, perhaps more so than might be warranted. I'd love to avoid that. Or, at the least, I'd prefer the disappointment of "Rust obviously doesn't work exactly like C" to the much larger disappointment of "it seemed like I might be able to treat Rust just like C, but then I did that and later it broke".

[^1]: As noted above, attempting to treat C like C and "just" emit an object file and link it with the system linker will also regularly produce disappointment. The Linux kernel folks have learned to deal with that and drown it in resources, and other embedded developers have learned to live with that and don't blame it on the compiler or otherwise expect it to reliably work.

bluca commented 1 year ago

I'd generally expect that --emit obj will tend to not align with the stability people expect of Rust.

I most certainly have no stability expectations whatsoever from Rustc, quite the opposite actually. So the only expectation I have from --emit obj is that it is recognised as a command line interface, and might emit some sort of compiler object file. Whether it actually works, is usable, or is usable in the same way across versions or requires further hacking, there's no expectation at all. I'm happy to add a warning message in Meson to highlight that in this PR, will send an update shortly.

I want to explicitly say that this is a use case we care about.

Can you provide an alternative that provides the same result and can be used today?

Because from my point of view, you say that, but then at the same time you are attempting to block this without suggesting an alternative, which is frustrating. This PR actually works today, it uses a stable command line interface, and unblocks our use case. It works just fine, as it can be seen from the linked systemd PR. I don't really care if in the future rustc might break it. First of all, that's my problem to deal with, not Meson's, so it is completely irrelevant here - it doesn't matter if it might require additional linking or such things, this PR leaves that to the caller intentionally, so that it's not Meson's problem to solve, it's entirely on the caller and documented as such. Secondly, gccrs was recently merged and at some point will hopefully be usable, and the moment it does we will for sure switch to it, so all those issues due to the unreliable first-party tools will go away. The intention here is to unblock ourselves and get started, given gccrs will take some time yet.

Honestly, this is incredibly frustrating. Do we really have to end up in a place where we are forced to drop attempts to introduce Rust into systemd's codebase not for good reasons, but due to dogmatic vetoes (in unrelated third party projects!) because we aren't drinking the cargo and static linking kool-aids? Let me be 100% clear: we will not do static linking in systemd, so if you veto this, and cannot point to an alternative with the same effect that exists and works today, it means you are killing the attempt to use Rust as well. Is that really what you want?

bluca commented 1 year ago

Building Meson on top of a house of cards feels unfortunate, because we all know that Meson is just going to end up breaking as a result.

IMO if we do implement such a feature at all, it should emit a warning message telling you not to do it.

Added the warning as requested.

jpakkane commented 1 year ago

I don't really care if in the future rustc might break it. First of all, that's my problem to deal with, not Meson's, so it is completely irrelevant here

That is totally understandable from your perspective but it is not so easy for us. Whenever anything that used to work breaks during a Meson build anywhere, we get a bug report about it. This is already the case (I spend a fair bit of time fixing Debian bugs that are not, strictly speaking, our fault). That is the reason why we only want to provide features that we know that we can support. As an example suppose this is merged, some tens of projects start using it and then rustc breaks this functionality (which, AFAUI they can do as it is unstable) and then Debian does a repository rebuild. Then end result is that the Meson package gets tens of RC bugs. Fixing all of that is now on me, I can't reassign them to the rustc compiler because they will just send them back with "you used an unstable interface, not our problem, reassigning" or somesuch. This is not a situation you want to be in, trust me. Been there, done that, got the scars and therapy bill. And that is only one situation for one distro.

If rustc provided functionality to do what is wanted here in a way that is guaranteed to work in the future then yes, we would support it in a hearbeat. As long as they don't, we are in an extremely unfortunate place where somebody is going to be very unhappy.

bluca commented 1 year ago

I don't really care if in the future rustc might break it. First of all, that's my problem to deal with, not Meson's, so it is completely irrelevant here

That is totally understandable from your perspective but it is not so easy for us. Whenever anything that used to work breaks during a Meson build anywhere, we get a bug report about it. This is already the case (I spend a fair bit of time fixing Debian bugs that are not, strictly speaking, our fault). That is the reason why we only want to provide features that we know that we can support. As an example suppose this is merged, some tens of projects start using it and then rustc breaks this functionality (which, AFAUI they can do as it is unstable) and then Debian does a repository rebuild. Then end result is that the Meson package gets tens of RC bugs. Fixing all of that is now on me, I can't reassign them to the rustc compiler because they will just send them back with "you used an unstable interface, not our problem, reassigning" or somesuch. This is not a situation you want to be in, trust me. Been there, done that, got the scars and therapy bill. And that is only one situation for one distro.

If rustc provided functionality to do what is wanted here in a way that is guaranteed to work in the future then yes, we would support it in a hearbeat. As long as they don't, we are in an extremely unfortunate place where somebody is going to be very unhappy.

Given the (now added) warning is in place, in that situation the right thing (even policy-wise) would be for the RC bugs to be assigned to the packages using the unstable functionality, no? Let's be realistic: rustc is never going to provide that guarantee. The rustc developers are dogmatically opposed to it for ideological reasons, so it's just not going to happen. Given a sensible toolchain (gccrs) is at best some years away from being usable, this effectively means the idea of introducing Rust into systemd is dead and buried for good.

Maybe as a compromise, instead of adding direct support for emit obj, I could add a flag that simply removes the emit link and the restriction on using cdylib with static Meson targets? By itself, that would do nothing, so it's not Meson's problem what happens with it. I can then pass emit obj -C prefer-dynamic myself as compiler arguments. This should actually work just as well for me, and remove any responsibility from you. After all, the restriction on matching Meson target to crate type is a safety net that Meson added, and could be overridden. Would this be acceptable?

eli-schwartz commented 1 year ago

Given the (now added) warning is in place, in that situation the right thing (even policy-wise) would be for the RC bugs to be assigned to the packages using the unstable functionality, no?

Yes, that is more or less why I wanted a warning. The idea is to basically communicate whenever this feature is used "The use of this feature means that Meson makes no guarantee anything works. We believe it's likely this is going to break without warning in micro-releases of rust, and if it does, you need to report it to systemd, not us, because all bugs are systemd's responsibility for using unstable features." Modulo bikeshedding about the precise wording.

bjorn3 commented 1 year ago

Let's be realistic: rustc is never going to provide that guarantee. The rustc developers are dogmatically opposed to it for ideological reasons, so it's just not going to happen.

While we care about this use case, the workaround you have to make it work despite missing support in rustc is not something that we want to guarantee will remain working.[^1] Rather I expect that in the future a new mechanism is proposed which we can and do guarantee to remain working with future rustc versions. Said mechanism would not require you to provide an allocator shim yourself. It could be that the allocator shim is no longer necessary at all with this mechanism or that the mechanism allows you to ask rustc to emit an allocator shim working for your particular configuration.

[^1]: For example a fix for the soundness issue in https://github.com/rust-lang/rust/issues/104707 when trying to link multiple --crate-type staticlib compiled by different rustc versions together may require changing the symbol names in the allocator shim to be unique for every rustc version.

bjorn3 commented 1 year ago

In any case the blessed methods for now are --crate-type cdylib (producing a dynamic library) and --crate-type staticlib (producing a .a static library) Note that because of https://github.com/rust-lang/rust/issues/104707 you can only link a single --crate-type staticlib into any executable or dynamic library. As such if you use --crate-type staticlib you will have to make a single super crate with --crate-type staticlib that includes all sub crates.

bluca commented 1 year ago

Given the (now added) warning is in place, in that situation the right thing (even policy-wise) would be for the RC bugs to be assigned to the packages using the unstable functionality, no?

Yes, that is more or less why I wanted a warning. The idea is to basically communicate whenever this feature is used "The use of this feature means that Meson makes no guarantee anything works. We believe it's likely this is going to break without warning in micro-releases of rust, and if it does, you need to report it to systemd, not us, because all bugs are systemd's responsibility for using unstable features." Modulo bikeshedding about the precise wording.

Changed the warning just now as:

Due to the unreliability of Rustc, crate type "object" compiled output might be unstable and require extra work to use.
The use of this feature means that Meson makes no guarantee anything works. We believe it's likely this is going to break without warning in
micro-releases of rust, and if it does, you need to report it to the package making use of it, not Meson, because all bugs are the responsibility of
the package using unstable features.

Let me know if that works or you want something different. Also as mentioned before I can change to a flag and remove emit obj and just remove the check that stops a static target using cdylib, if that's better for you.

bluca commented 1 year ago

Let's be realistic: rustc is never going to provide that guarantee. The rustc developers are dogmatically opposed to it for ideological reasons, so it's just not going to happen.

While we care about this use case, the workaround you have to make it work despite missing support in rustc is not something that we want to guarantee will remain working.1 Rather I expect that in the future a new mechanism is proposed which we can and do guarantee to remain working with future rustc versions. Said mechanism would not require you to provide an allocator shim yourself. It could be that the allocator shim is no longer necessary at all with this mechanism or that the mechanism allows you to ask rustc to emit an allocator shim working for your particular configuration.

Sorry, but I don't quite buy into the whole "jam tomorrow" thing when ideology is in the mix, I'll believe it when I see it, and so far it's been years and there's absolutely nothing to show for it, and it's pretty transparent that it is intentional. Which is fine, it's open source so I don't expect others to work on what I want (but I conversely do expect them not to stand in the way either), hence why I'm going for a different solution (that actually exists and works). Also, I'm not providing any allocator, and things are working just fine (because the system allocator is used), so I really don't care about allocators right now.

In any case the blessed methods for now are --crate-type cdylib (producing a dynamic library) and --crate-type staticlib (producing a .a static library) Note that because of rust-lang/rust#104707 you can only link a single --crate-type staticlib into any executable or dynamic library. As such if you use --crate-type staticlib you will have to make a single super crate with --crate-type staticlib that includes all sub crates.

Those still static link the entire universe, and thus are not usable alternatives to this.

bjorn3 commented 1 year ago

Also, I'm not providing any allocator, and things are working just fine (because the system allocator is used), so I really don't care about allocators right now.

Right, I got your use case mixed up with that of someone else. My bad. I thought this was about compiling and linking libcore and liballoc and the user code as as .o, but in your case libstd is compiled as dylib like usual and then you want to statically link your own code, but not libstd. In that case the allocator shim is indeed already provided by the libstd dylib.

You only need a way to combine --crate-type staticlib with dynamically linking libstd. That should be a much easier problem to solve. I think a small change to the dependency format calculator to allow this combination would be enough and I don't think it paint us in a corner either. This seems to be the logic forbidding it: https://github.com/rust-lang/rust/blob/6a20f7df5755d8c6b68110d2d0391a7b03268e77/compiler/rustc_metadata/src/dependency_format.rs#L128-L146 By the way it seems that we have already relaxed this logic previously to allow dynamically linking libstd for cdylib's in https://github.com/rust-lang/rust/pull/68448. And the original logic seems to originate from when support for mixing of static and dynamic linking for rust crates was first introduced. Before that all crates needed to be either static libraries or dynamic libraries. Maybe this limitation was added as a just in case?

bluca commented 1 year ago

You only need a way to combine --crate-type staticlib with dynamically linking libstd. That should be a much easier problem to solve. I think a small change to the dependency format calculator to allow this combination would be enough and I don't think it paint us in a corner either.

Whether it's easy or hard in theory doesn't really help me. In practice it's not solved and it's not going to be solved, so that still leaves me just as blocked.

joshtriplett commented 1 year ago

Following up and clarifying something:

I was not in any way suggesting it's necessary to use Cargo to build. It's widely recommended, but not necessary. And even if you do use Cargo to build, you certainly don't need to (for instance) use remote crates from crates.io, or similar; you can absolutely use it as a local-only build system. But it's also entirely possible to build using exclusively rustc, and in particular, I would expect any mechanism that has a Cargo option to have a corresponding rustc option. (And some things have rustc options and not Cargo options.)

Let's be realistic: rustc is never going to provide that guarantee. The rustc developers are dogmatically opposed to it for ideological reasons, so it's just not going to happen.

I'm not sure what opposition you're referring to. It seems like you're conflating "haven't done it yet" with "dogmatically opposed to it".

As an example, I am working right now on providing better ways to ship stable dynamic library ABIs using much more of Rust (beyond just the C ABI).

This is not an easy problem. But I am not aware of any ideological opposition to it.

bluca commented 1 year ago

Let's be realistic: rustc is never going to provide that guarantee. The rustc developers are dogmatically opposed to it for ideological reasons, so it's just not going to happen.

I'm not sure what opposition you're referring to. It seems like you're conflating "haven't done it yet" with "dogmatically opposed to it".

As an example, I am working right now on providing better ways to ship stable dynamic library ABIs using much more of Rust (beyond just the C ABI).

This is not an easy problem. But I am not aware of any ideological opposition to it.

I do hope you realize how it looks to make all those claims while at the very same time coming here just to veto the one small change required to make it work.

Because as far as I can see, the answer to the question "if not with this, then how can I do full dynamic linking in a Meson project?" is "jam tomorrow". The problem is, "jam tomorrow" isn't that helpful when you need to eat today. So, let me ask again, how do I get unblocked and make progress today after you vetoed this?

dcbaker commented 1 year ago

As an example, I am working right now on providing better ways to ship stable dynamic library ABIs using much more of Rust (beyond just the C ABI).

This is not an easy problem. But I am not aware of any ideological opposition to it.

As an aside @joshtriplett, thanks for working on this. It's a really big problem for Linux people, and has been one of the biggest hurdles to using Rust in Mesa (that we basically have to vendor rust code to use crates).

It sounds like though that using --emit obj is fine and not on the chopping block?

joshtriplett commented 1 year ago

Suppose I added a -C staticlib-prefer-dynamic today, that acts like prefer-dynamic but does work with --crate-type staticlib. Would that fully address the problem here, insofar as linking against dynamic libraries where available, such that if you provide a .so for Rust's libstd (or a .so for some other crate), we'll automatically use it? Or is there part of the problem that doesn't address? I want to make sure I'm not oversimplifying the problem here.

Also:

Maybe as a compromise, instead of adding direct support for emit obj, I could add a flag that simply removes the emit link and the restriction on using cdylib with static Meson targets? By itself, that would do nothing, so it's not Meson's problem what happens with it. I can then pass emit obj -C prefer-dynamic myself as compiler arguments. This should actually work just as well for me, and remove any responsibility from you. After all, the restriction on matching Meson target to crate type is a safety net that Meson added, and could be overridden. Would this be acceptable?

I can't speak for Meson's preferences here, but I personally think this or something like it would be preferable, for a variety of reasons. In particular, rather than Meson having an established option that relies on behavior from rustc where rustc can't currently guarantee that behavior if it isn't handling linking itself, Meson could instead allow individual projects to pass Rust options through to get the behavior they're wanting to experiment with, without that being the responsibility of Meson. This would allow projects like systemd to do this kind of PoC experimentation (and, ideally, work with upstream on the features they'd need to make it less of a PoC).

(Also, separately, if some path gets taken here that motivates having a stability warning in meson, I'd be happy to help with that warning; for instance, s/micro-releases/minor releases/, as such a change wouldn't occur in a point release. But I do think a path by which the project using meson directly passes the options it wants and takes more direct responsibility for those options would be preferable.)

Would the combination of the above two things work here? Should I make a PR for -C staticlib-prefer-dynamic today?

bluca commented 1 year ago

Suppose I added a -C staticlib-prefer-dynamic today, that acts like prefer-dynamic but does work with --crate-type staticlib. Would that fully address the problem here, insofar as linking against dynamic libraries where available, such that if you provide a .so for Rust's libstd (or a .so for some other crate), we'll automatically use it? Or is there part of the problem that doesn't address? I want to make sure I'm not oversimplifying the problem here.

The end goal/workflow is that the local .rs files get compiled into a static .a which contains only the local code, and not the standard library, crates, and the kitchen sink. Then we can link this .a with other code, and dynamic link with what's necessary. If what you are describing achieves this goal, and if I understood correctly it does, then sure it would work. But note that the linking is not done by rustc here, only the compiling, because there's a bunch of C/C++ code too that is linked together. Ideally the option would not be a "prefer" in the sense that it's best-effort and silently fallback to embedding if it can't work, but instead it fails, so that we don't end up unintentionally shipping gigabytes of binaries because something changed somewhere during an automated archive rebuild, but that's a stretch goal.

Also:

Maybe as a compromise, instead of adding direct support for emit obj, I could add a flag that simply removes the emit link and the restriction on using cdylib with static Meson targets? By itself, that would do nothing, so it's not Meson's problem what happens with it. I can then pass emit obj -C prefer-dynamic myself as compiler arguments. This should actually work just as well for me, and remove any responsibility from you. After all, the restriction on matching Meson target to crate type is a safety net that Meson added, and could be overridden. Would this be acceptable?

I can't speak for Meson's preferences here, but I personally think this or something like it would be preferable, for a variety of reasons. In particular, rather than Meson having an established option that relies on behavior from rustc where rustc can't currently guarantee that behavior if it isn't handling linking itself, Meson could instead allow individual projects to pass Rust options through to get the behavior they're wanting to experiment with, without that being the responsibility of Meson. This would allow projects like systemd to do this kind of PoC experimentation (and, ideally, work with upstream on the features they'd need to make it less of a PoC).

If the Meson mantainers want me to do this change, I can do so. Note that it's not just about passing options through, there's a few things here and there to adjust too, but nothing major.

bluca commented 1 year ago

(Also, separately, if some path gets taken here that motivates having a stability warning in meson, I'd be happy to help with that warning; for instance, s/micro-releases/minor releases/, as such a change wouldn't occur in a point release. But I do think a path by which the project using meson directly passes the options it wants and takes more direct responsibility for those options would be preferable.)

Fixed this in the latest push.

Would the combination of the above two things work here? Should I make a PR for -C staticlib-prefer-dynamic today?

Thanks, if you do, I'm happy to test things out.

dcbaker commented 1 year ago

You can pass rust flags via either the RUSTFLAGS environment variable, -Drust_args, or in the meson.build files themselves via add_project_arguments(..., languages : 'rust'). If that doesn't work, I'll fix that ASAP, that's a pretty big bug and we should backport that fix to 1.0.0.

Another thought I had was that we could add a method to the rust module and have the method always emit a non-fatal warning that it relies on unstable rustc behavior, and that we can't guarantee it will always work, and that the build system maintainer is responsible for the breakage.

Another thing to keep in mind here, is that I'm planning to add support for gcc-rs in the 1.1.0 cycle, so it's quite possible that with gcc 13 and meson 1.1.0 there will be people building your code with gcc rs as well as with rustc.

@jpakkane what are your ghtoughs at this point, I'm happy with the implementation, but I think you had some lingering meta concerns?

bluca commented 1 year ago

Would the combination of the above two things work here? Should I make a PR for -C staticlib-prefer-dynamic today?

Thanks, if you do, I'm happy to test things out.

@joshtriplett any update?

dcbaker commented 1 year ago

There is another option I just thought of, which, actually might be the best option yet, because we should do it anyway:

Normally you can take a built target and call .extract_objects() on it, and get back a list of object files. This is because normally we build targets in two step:

  1. create an object file from each source file
  2. link said object files as appropriate

This doesn't work with Rust because we just order rustc to link a single archive, library, or executable immediately, but there isn't really a good reason that we should do that, and it in fact makes a number of things impossible, including link_whole with a rust generated library (#10722 and #10724), it also means that you can't write really obvious code like:

target = static_archive(
  'target',
  'file.c',
  'file.rs',
  rust_crate_type : 'staticlib'
)

You have to write and link two targets. If we fixed this then you'd be able to do one of two things with Rust code, either:

target = static_archive('target', ''file.c', 'file.rs', link_language : 'c') # force linkage with C to avoid linking rust stdlib, we'll do that later

or:

target = static_archive('rust', 'file.rs', rust_crate_type : 'staticlib')
other_target = static_archive('other target', ..., objects : target.extract_objects())

which should achieve the same result. That's all on my TODO list anyway, so it could find it's way up that list if you thought that would solve the issue.

bluca commented 1 year ago

There is another option I just thought of, which, actually might be the best option yet, because we should do it anyway:

Normally you can take a built target and call .extract_objects() on it, and get back a list of object files. This is because normally we build targets in two step:

  1. create an object file from each source file
  2. link said object files as appropriate

This doesn't work with Rust because we just order rustc to link a single archive, library, or executable immediately, but there isn't really a good reason that we should do that, and it in fact makes a number of things impossible, including link_whole with a rust generated library (#10722 and #10724), it also means that you can't write really obvious code like:

target = static_archive(
  'target',
  'file.c',
  'file.rs',
  rust_crate_type : 'staticlib'
)

You have to write and link two targets. If we fixed this then you'd be able to do one of two things with Rust code, either:

target = static_archive('target', ''file.c', 'file.rs', link_language : 'c') # force linkage with C to avoid linking rust stdlib, we'll do that later

or:

target = static_archive('rust', 'file.rs', rust_crate_type : 'staticlib')
other_target = static_archive('other target', ..., objects : target.extract_objects())

which should achieve the same result. That's all on my TODO list anyway, so it could find it's way up that list if you thought that would solve the issue.

Yeah that sounds great, and should solve my problem too. Do you need help with anything?

dcbaker commented 1 year ago

Just motivation to deal with some very hairy code, lol

bluca commented 1 year ago

You have my sword!

joshtriplett commented 1 year ago

@bluca: @bjorn3 wrote a patch, just needs to get through PR review and then you can try it out in nightly.

bluca commented 1 year ago

@bluca: @bjorn3 wrote a patch, just needs to get through PR review and then you can try it out in nightly.

Do you have a link? Curious to have a look - thanks

bluca commented 1 year ago

@joshtriplett @bjorn3 any news?

bjorn3 commented 1 year ago

We've gotten confirmation from the compiler team that they expect to look at the PR (https://github.com/rust-lang/rust/pull/106560) in their meeting this week. We'll provide updates here after that.