Add basic support for building Rust#299
Conversation
nathanchance
left a comment
There was a problem hiding this comment.
I will review this more in depth either later this week or early next week but one initial comment from a brief read.
nathanchance
left a comment
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? |
nathanchance
left a comment
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.
|
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
|
nathanchance
left a comment
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). |
Yes, that could be doable. If these are static libraries though (since we turn off the dynamic linking of LLVM libraries), we would not actually need to distribute them if they get linked into the Rust binaries.
No worries! I can try to mess around with it next week if I can get through the rest of my TODO efficiently. |
This should be all that is needed to make Rust work when using diff --git a/tc_build/llvm.py b/tc_build/llvm.py
index 182a20f..ad932c8 100644
--- a/tc_build/llvm.py
+++ b/tc_build/llvm.py
@@ -438,13 +438,20 @@ class LLVMSlimBuilder(LLVMBuilder):
runtime_distribution_components = []
if llvm_build_tools:
distribution_components += [
+ 'llc',
'llvm-ar',
+ 'llvm-as',
+ 'llvm-cov',
+ 'llvm-dis',
+ 'llvm-link',
'llvm-nm',
'llvm-objcopy',
'llvm-objdump',
'llvm-ranlib',
'llvm-readelf',
+ 'llvm-size',
'llvm-strip',
+ 'opt',
]
if self.project_is_enabled('bolt'):
distribution_components.append('bolt')
@@ -452,6 +459,8 @@ class LLVMSlimBuilder(LLVMBuilder):
distribution_components += ['clang', 'clang-resource-headers']
if self.project_is_enabled('lld'):
distribution_components.append('lld')
+ if self.project_is_enabled('polly'):
+ distribution_components.append('PollyISL')
if build_compiler_rt:
distribution_components.append('llvm-profdata')
if self.llvm_major_version >= LLVM_VER_FOR_RUNTIMES: |
This reverts commit 744315b. This was fine at the time that it was committed but now we want to use the output of build-llvm.py to build Rust from source and that does not work too well with thin archives. Signed-off-by: Nathan Chancellor <nathan@kernel.org>
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>
Currently, the distribution created by LLVMSlimBuilder is not very flexible, even though it may be used in two different but similar manners: creating a slim toolchain for LLVM to build itself or creating a slim toolchain for building the Linux kernel. While there is a lot of overlap between the two, there may not be much overlap for other purposes that may come up in the future, like building Rust. Introduce the concept of distribution profiles, which allow customizing the distribution based on use case. Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Allow the user to customize the final stage's distribution based on the available profiles. Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Building Rust requires many more components that can significantly increase the size of a distribution installation. Add a Rust distribution profile so that these things can be included only when a user cares about building Rust. Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
|
@ojeda Alright, I think I got everything into a mergeable state locally, which I pushed to: https://github.com/nathanchance/tc-build/commits/build-rust-py I sought to address the concerns I had with |
|
I like it! Sometimes the best way forward is to offer options when there is no "good for everyone/everything" solution. I hope it doesn't introduce too much burden on you for testing, but I guess the set of components will not need a lot of changes. I just tested your branch and did In case it helps, I am pushing your branch here. But, of course, please feel free to create a new PR if you prefer. I added the old typo fix at the bottom that I had here. I also changed the example in Thanks a lot! |
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> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Link: https://blog.rust-lang.org/inside-rust/2025/10/16/renaming-the-default-branch-of-rust-lang-rust/ Signed-off-by: Nathan Chancellor <nathan@kernel.org>
By default, configure shortens its output but it is worth printing all of the information to allow the user to verify everything is correct. Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Actually, to avoid changing most of your commit hashes, I have put it at the top instead. |
No, I don't think it is burdensome at all. If anything, I have been wanting to do something like that for some time. I don't anticipate that list changing much but if it does, we can always re-evaluate.
Excellent, thanks for testing and confirming that the defaults still work.
Nope, this works great I think.
Ah yes, sorry that I lost that!
LGTM. I think I am going to merge this up. If there are any problems, we can follow up with fixes on top of main. Thanks again for this, I hope that it is helpful for folks going forward. |
|
It will be definitely be helpful! :) Thanks a lot Nathan -- I will send a message to the mailing lists and Zulip about it so that others are aware. |
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)