-
Notifications
You must be signed in to change notification settings - Fork 98
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
config: add rust build configurations #1468
Conversation
10ne1
commented
Oct 12, 2022
•
edited
Loading
edited
00b7dce
to
a7147e0
Compare
Testing it, might take a while, i will report progress here. |
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.
In general it looks good. I'd probably go with 2 separate names for the .config files (mentioned above)
a7147e0
to
22f6c42
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.
Some changes are required to make the image build by kernelci Jenkins.
Suggested by Michal in pull request kernelci#1468 Co-authored-by: Michał Gałka <[email protected]>
Suggested by Michal in pull request kernelci#1468 Co-authored-by: Michał Gałka <[email protected]> Signed-off-by: Adrian Ratiu <[email protected]>
bee6a19
to
02f2b0b
Compare
See also the email thread with @ojeda from January 2022 https://groups.io/g/kernelci/message/1336 |
Thanks for the Cc!
The current commits seem to be using the standalone installers, not |
Tested builds on staging: Builds OK |
02f2b0b
to
f4f2c20
Compare
f4f2c20
to
bc83a3e
Compare
Hi @ojeda If you are ok with the current state of the PR can you please leave a comment so I can add a reviewed-by tag for you? (I also updated the commit msg & PR description to mention the offline install method). |
I'm seeing some problems with rust on staging:
Several rust builds failed. |
bc83a3e
to
40dc7d2
Compare
@nuclearcat thanks for the heads-up, I've forgot to add the new ${CARGO_HOME}/bin to the path, so bindgen couldn't be found. It's fixed now. |
I wrote a nit above and also I left a comment a few days ago above on minimizing the Other than those, it looks good to me! Reviewed-by: Miguel Ojeda [email protected] One final note: the Thanks a lot for this! |
40dc7d2
to
8a6323d
Compare
It seems build still failed recently on staging due kfselftest fragment missing (not sure why):
|
@nuclearcat This fails because the rust configurations do not depend on kselftest config fragment generation, but apparently kselftest is used by the staging tests? Why do they do that? We could fix it the following way, but I don't think it makes sense because kselftests should not be required for this PR.
Then the following works:
But again, I repeat, kselftest should not be required. Is there any way to avoid it? To drop that hardcoded |
@nuclearcat @mgalka Gentle ping. Any thoughts about dropping the unnecessary +kselftest from the staging run |
I don't know why rust:
tree: mainline
branch: 'master'
variants:
rustc-1.62:
build_environment: rustc-1.62
fragments: [rust, rust-samples]
architectures:
x86_64: &x86_64_arch_nokselftest
base_defconfig: 'x86_64_defconfig'
extra_configs:
- 'allmodconfig'
- 'allnoconfig'
- 'x86_64_defconfig+x86-chromebook'
- 'x86_64_defconfig+x86-chromebook+amdgpu'
fragments: [amdgpu, crypto, ima, x86_kvm_guest, x86-chromebook]
rust-for-linux:
tree: rust-for-linux
branch: 'rust'
variants:
rustc-1.62:
build_environment: rustc-1.62
fragments: [rust, rust-for-linux-samples]
architectures:
x86_64: *x86_64_arch_nokselftest
I'm giving a try to at the moment on staging. |
Well staging build configurations are pretty arbitrary and aim at getting the highest tests / build ratio. Building with kselftest fragment enabled is required to run kselftests, and we need that on staging. There's no point building without kselftest on staging as that's not adding any real coverage. If we can't build kselftest with rust then we just need to fix that in the rust build config options, no need to disable kselftest builds when rust is not enabled. |
Starting with the upcoming v6.1, mainline Linux has merged the initial Rust infrastructure so this adds some configs for build testing it together with some sample modules. The kernel requires a specific version of rustc, so we add the rustc-1.62 build environment which derives from clang-15, since a C compiler is still required to build the kernel and the supported kernel version is 15 (we might bump this later). Obviously GCC can be used as well but for now testing all the toolchain combinations does not add significant value. In the future more toolchain combinations can be used as needed. The official "offline" toolchain installation method is used as documented at [1] with sha256sum and because some distros like Debian stable might not provide up to date toolchain and crates to keep up with the mainline kernel. Only the x86_64 architecture is supported by the kernel for now. We also add the Rust-for-Linux kernel maintainer trees which contain additional modules and bindings. [1] https://forge.rust-lang.org/infra/other-installation-methods.html Reviewed-by: Miguel Ojeda <[email protected]> Co-authored-by: Michał Gałka <[email protected]> Signed-off-by: Adrian Ratiu <[email protected]>
8a6323d
to
1bf07b9
Compare
Thanks for the context, that is what I was missing, so we do have to hardcode something either way to pass the staging tests. Between the choice of the two hardcoding options - the rust build config options (eg @mgalka do you agree? EDIT: I've pushed the change adding the keselftest fragment. |
I'm fine with it. Giving it a try on staging. |
Tested OK on staging. A build which previously failed, works fine now. https://bot.staging.kernelci.org/job/kernel-build/95009/console |
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.
LGTM
Thanks LGTM |
PR kernelci#1468 added the mainline rust build in a separate `rust` build configuration instead of adding to the already existing mainline config which can cause unnecessary builds. Move it in the proper place. Suggested-by: Guillaume Tucker <[email protected]> Signed-off-by: Adrian Ratiu <[email protected]>
PR kernelci#1468 added the mainline rust build in a separate `rust` build configuration instead of adding to the already existing mainline config which can cause unnecessary builds. Move it in the proper place. Suggested-by: Guillaume Tucker <[email protected]> Signed-off-by: Adrian Ratiu <[email protected]>
PR kernelci#1468 added the mainline rust build in a separate `rust` build configuration instead of adding to the already existing mainline config which can cause unnecessary builds. Move it in the proper place. Suggested-by: Guillaume Tucker <[email protected]> Reviewed-by: Reviewed-by: Miguel Ojeda <[email protected]> Signed-off-by: Adrian Ratiu <[email protected]>
PR kernelci#1468 added the mainline rust build in a separate `rust` build configuration instead of adding to the already existing mainline config which can cause unnecessary builds. Move it in the proper place. Fixes: 43875d0 ("config: add rust build configurations") Suggested-by: Guillaume Tucker <[email protected]> Reviewed-by: Miguel Ojeda <[email protected]> Reviewed-by: Alice Ferrazzi <[email protected]> Signed-off-by: Adrian Ratiu <[email protected]>
PR kernelci#1468 added the mainline rust build in a separate `rust` build configuration instead of adding to the already existing mainline config which can cause unnecessary builds. Move it in the proper place. While at it also change the x86_64_arch anchor to build just the base x86_64_defconfig to avoid unnecessary allmodconfig rebuilds. Fixes: 43875d0 ("config: add rust build configurations") Suggested-by: Guillaume Tucker <[email protected]> Reviewed-by: Miguel Ojeda <[email protected]> Reviewed-by: Alice Ferrazzi <[email protected]> Signed-off-by: Adrian Ratiu <[email protected]>
PR #1468 added the mainline rust build in a separate `rust` build configuration instead of adding to the already existing mainline config which can cause unnecessary builds. Move it in the proper place. While at it also change the x86_64_arch anchor to build just the base x86_64_defconfig to avoid unnecessary allmodconfig rebuilds. Fixes: 43875d0 ("config: add rust build configurations") Suggested-by: Guillaume Tucker <[email protected]> Reviewed-by: Miguel Ojeda <[email protected]> Reviewed-by: Alice Ferrazzi <[email protected]> Signed-off-by: Adrian Ratiu <[email protected]>