Add basic support for building Rust #299
Conversation
There was a problem hiding this comment.
I took this for a spin today and I think this is pretty good shape for a v1 but I will try to do an actual hard review over the next couple of days if I am able to.
I did work on my idea I suggested earlier and pushed that just to see if it worked and looked cleaner and I think it does: https://github.com/nathanchance/tc-build/commits/rust-review
Thanks! I agree it looks better (sorry, I should have replied above). Should I put your commit in this PR and apply the fixup? |
There was a problem hiding this comment.
Thanks! I agree it looks better (sorry, I should have replied above). Should I put your commit in this PR and apply the fixup?
No worries! Yes, I think that would be good.
I am not sure how far you would want to modify the LLVM side or the
do_llvm()step, so I decided to send this with the failing CI so that you see it.
We should probably fix the CI configuration since it is custom tailored but should we also put an informative message in build-rust.py if these files cannot be found in the provided LLVM install folder?
Couple more minor comments but I think I am pretty happy with this, at least as an initial revision. If you felt like any of the “nice to have”s in the initial PR message were worth following up on in the future, feel free to open issues here for tracking.
We will be adding Rust support in the future, so pull out common git repository management code from LLVMSourceManager that can be shared with a RustSourceManager. Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
There a lot of options missing and it does not do any fancy kind of build. But it is a start, and it is already useful, e.g. Peter could have used it to test the new KCFI arity flag that requires LLVM 21 but upstream Rust still uses LLVM 20. I took the approach that the new script only takes care of building Rust provided an existing LLVM, which seemed simple and clear. Thus add the basic infrastructure, plus a bit of documentation. Add it to the CI, too. I successfully built it in a clean Debian 12 and Fedora 41. Cc: Peter Zijlstra <peterz@infradead.org> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
|
v2: The biggest change is that instead of writing a
The Moreover, I reviewed the rest of the
A few other notes:
Finally, I modified the LLVM side a bit to unbreak the CI. It turns out we will need some changes on the LLVM (please see the
|
There was a problem hiding this comment.
I wanted to get this fully reviewed and flushed out today but I ran out of time so I am just posting what I have and coming back to it later because I should not have left you hanging for so long.
I was able to test this on AArch64 and it worked fine until I got an error due to an old copy of binutils in my path so I am rerunning it now.
I want to run some measurements on how much the default install size would change if we just enabled the things that --library would need and just did not give an option to turn it off.
| '--release-description', self.vendor_string, | ||
| '--disable-docs', | ||
| '--enable-locked-deps', | ||
| '--tools', 'cargo,clippy,rustdoc,rustfmt,src', |
There was a problem hiding this comment.
If you think this list will ever grow or want to be customizable, consider using a separate list variable then ','.join(...).
|
No worries at all, I understand, and it is not easy to review and test this kind of PR. Great to hear at least the arm64 is not completely broken. Thanks for checking if we could do |
Better than not completely broken now :)
While I do use tc-build for the kernel.org toolchains, I use a custom I think that it would be reasonable to add those to the distributions list by default, as it only adds about 300 build targets in my testing and I would expect people using this to have a reasonable amount of space for build and install artifacts when the space requirements for the source is already rather large. We could make it easier on people doing local builds if we could utilize thin archives in certain circumstances since my measurements show significant savings but I understand it might be tricky to safely do. Additionally, we might be missing some things in the distribution list, as I get the following error when I run This has probably not been noticed because we run just the |
🎉
Yeah, doubling doesn't sound good. What about the LLVM+Rust toolchains? If we eventually start providing the combined ones with this, then I assume we can eventually drop the two tables and just provide one table (since it will be exactly matching), thus we would save some space there at least.
Good catch, thanks! I can take a look (it might take me some time). |
There a lot of options missing and it does not do any fancy kind of build. But it is a start, and it is already useful, e.g. Peter could have used it to test the new KCFI arity flag that requires LLVM 21 but upstream Rust still uses LLVM 20. It could also be useful to potentially/eventually have LLVM+Rust builds in kernel.org that use the exact same LLVM build.
I took the approach that the new script only takes care of building Rust provided an existing LLVM, which seemed simple and clear.
Thus add the basic infrastructure, plus a bit of documentation. Add it to the CI, too.
I successfully built it in a clean Debian 12 and Fedora 41.
A few notes:
The script works on its own, but the CI does not pass because
llvm-config --bindirdoes not work with the set of flags used in CI for LLVM -- we are missing at leastllvm-configin the distribution components of the LLVM step, but then also headers likellvm/Config/llvm-config.h.I am not sure how far you would want to modify the LLVM side or the
do_llvm()step, so I decided to send this with the failing CI so that you see it.If
LLVM_INSTALL_UTILSis enabled (to getFileCheckinstalled, which is used by the Rust build system for testing), then we can leavecodegen-teststo its default (enabled) in the Rust configuration.But perhaps we want to leave those tests anyway as the equivalent of the
check-*s in LLVM, i.e. disabled by default even if the Rust configuration enables them by default, and let the user enable them.Rust uses submodules and so on and manages them on its own if allowed, so I just did that. I didn't add/test shallow clones, so I didn't add them.
This does nothing about the LLVM patches that Rust may carry (until they get upstreamed). One could use
--rust-folderto point to a checkout of the branch that Rust uses, and perhaps we should add some documentation about it, or even provide an option in./build-llvm.pyto build such a branch, but I didn't try/test anything related to that yet.We probably want to offer eventually
--quiet-x,--no-locked-deps,--tools,--codegen-tests,--docs, etc.After this, we could add a
--build-bindgenhere or perhaps a new./build-bindgen.py, so that we can easily get a full toolchain for the kernel. It is really just a one liner once one has the Rust toolchain, so it is not too important.Some refactoring could be done so that both
./build-llvm.pyand./build-rust.pyreuse certain bits, e.g.show_install_info().I tried to follow the LLVM script as closely as possible and "copy" the style etc., but there may be mistakes -- please feel free to take this and modify it as you see fit, of course.
I hope this helps!
Cc: Peter Zijlstra (by email)