-
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: add new 'object' type crate #11213
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ 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
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Tests fail because the expected error message in |
should be fixed now |
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:
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. |
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 |
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?
Sorry not sure I follow here, we are not using rlib here, but just plain elf objects, no? |
This is in practice just adding a wrapper for '--emit obj' so it can be done with plain rust tools as well, i think? |
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.
I am pretty sure that comment was meant for directly linking anything produced by rustc other than the the libraries produced by the |
It's not just the kernel, there's plenty of other projects using similar workarounds to overcome rust's shortcomings, see for example: Anyway, it's really off-topic, I'll remove that comment from the commit message as it's not useful here.
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... |
Btw @dcbaker this PR is implementing your suggestion from systemd/systemd#19598 (comment) - unless I've misunderstood those comments?
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. |
As far as I understand 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. |
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. |
I don't think |
As @bjorn3 said, I'd generally expect that A C equivalent would be something like "expecting you to link libgcc or equivalent yourself to get necessary builtins", but without the expectation that 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 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". Footnotes
|
I most certainly have no stability expectations whatsoever from Rustc, quite the opposite actually. So the only expectation I have from
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. 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? |
Added the warning as requested. |
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? Maybe as a compromise, instead of adding direct support for |
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. |
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. Footnotes
|
Changed the warning just now as:
Let me know if that works or you want something different. Also as mentioned before I can change to a flag and remove |
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.
Those still static link the entire universe, and thus are not usable alternatives to this. |
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 |
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. |
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.)
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? |
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 |
Suppose I added a Also:
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, Would the combination of the above two things work here? Should I make a PR for |
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.
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. |
Fixed this in the latest push.
Thanks, if you do, I'm happy to test things out. |
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.
You can pass rust flags via either the 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? |
@joshtriplett any update? |
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
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 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? |
Just motivation to deal with some very hairy code, lol |
You have my sword! |
@joshtriplett @bjorn3 any news? |
We've gotten confirmation from the compiler team that they expect to look at the PR (rust-lang/rust#106560) in their meeting this week. We'll provide updates here after that. |
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 #8828
It will be used by systemd: systemd/systemd#19598