Skip to content
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 rustc-1.68 build environment #1890

Merged
merged 1 commit into from
Apr 27, 2023
Merged

Conversation

ojeda
Copy link
Contributor

@ojeda ojeda commented Apr 18, 2023

Rust 1.68.2 will be the next version supported by the kernel, already submitted to the LKML.

Furthermore, I would like to start tracking the latest version, please see the commit message at [2] and a previous discussion at [3].

Therefore, add rustc-1.68 as a new build environment.

Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]
Link: https://lore.kernel.org/all/[email protected]/ [2]
Link: https://lore.kernel.org/rust-for-linux/CANiq72mT3bVDKdHgaea-6WiZazd8Mvurqmqegbe5JZxVyLR8Yg@mail.gmail.com/ [3]

Rust 1.68.2 will be the next version supported by the kernel,
already submitted to the LKML.

Furthermore, I would like to start tracking the latest version,
please see the commit message at [2] and a previous discussion
at [3].

Therefore, add `rustc-1.68` as a new build environment.

Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]
Link: https://lore.kernel.org/all/[email protected]/ [2]
Link: https://lore.kernel.org/rust-for-linux/CANiq72mT3bVDKdHgaea-6WiZazd8Mvurqmqegbe5JZxVyLR8Yg@mail.gmail.com/ [3]
Signed-off-by: Miguel Ojeda <[email protected]>
@ojeda
Copy link
Contributor Author

ojeda commented Apr 18, 2023

Cc @aliceinwire @10ne1 since they reviewed previous ones.

Cc @mgalka @nuclearcat too, since they managed changes for Rust on kernelci-deploy.

@gctucker Please let me know if such frequent upgrades would be too problematic for KernelCI.

Thanks!

@gctucker
Copy link
Contributor

@gctucker Please let me know if such frequent upgrades would be too problematic for KernelCI.

That's fine, however we need to remember to also add the Docker images for the new toolchain versions. That's currently configured in the kernelci-deplpoy repository, there are scripts run periodically to build them and push them to Docker Hub. I guess that's something to be improved as in principle we could derive the images to build from the YAML config.

@mgalka @nuclearcat Are you OK to take care of updating the scripts for rust-1.68.2? We might also tweak the build configs to use the toolchain on staging (and the main build-configs.yaml might also be tweaked to build the rust tree with the new version but that's up to @ojeda I guess).

@nuclearcat
Copy link
Member

nuclearcat commented Apr 19, 2023

I will take care about that.
Question , can we keep one version or we need to keep old ones? As images of Rust are very large ones and take significant time to build.

@nuclearcat nuclearcat self-assigned this Apr 19, 2023
@gctucker
Copy link
Contributor

can we keep one version or we need to keep old ones?

We only need to build the ones that are being used, and afaik that's one cutting edge and an older one for mainline. So unless @ojeda tells us we need more for some particular reasons, I think we'll only just need 2 images. Basically, once the build configs have been updated to use 1.68 instead of 1.66 then we should update the script to build 1.68 instead of 1.66... That's the kind of thing that could be automated but right now we can just update the configs hand-in-hand.

@ojeda
Copy link
Contributor Author

ojeda commented Apr 19, 2023

So I think we need from 1 to 3 at the same time, depending on how the following align at a given moment:

  • The one in mainline.
  • The one in rust-next if different, until it moves to mainline.
  • The one for rust-for-linux/rust.

The last one is the least important, and I can make the it the same as the first or the second (currently it isn't, but I will fix that), so effectively we only need 2.

Moreover, the branch of the last one is frozen now except for minimizing differences with mainline and will eventually go away, so it should be temporary anyway, i.e. we will only need 1 or 2 after it is gone.

@ojeda
Copy link
Contributor Author

ojeda commented Apr 19, 2023

In particular, 1.66 can go away as soon as I move rust-for-linux/rust to 1.68.

@mgalka @nuclearcat Are you OK to take care of updating the scripts for rust-1.68.2? We might also tweak the build configs to use the toolchain on staging (and the main build-configs.yaml might also be tweaked to build the rust tree with the new version but that's up to @ojeda I guess).

For build-configs.yaml, should that PR be sent in parallel with this one (even if it is not taken until it can be, i.e. until the compiler change lands into the respective place: mainline / rust-next etc.)?

@gctucker
Copy link
Contributor

For build-configs.yaml, should that PR be sent in parallel with this one (even if it is not taken until it can be)?

Yes feel free to send another PR now for that, we can add the staging-skip label to ignore it until the Docker images are available. In principle it only takes a one-line change in the script that builds the images, so we might get results on staging.kernelci.org tomorrow or Friday with the new 1.68 toolchain and in production next week.

@ojeda
Copy link
Contributor Author

ojeda commented Apr 19, 2023

images of Rust are very large ones and take significant time to build.

If --recurse-submodules is one of the causes for that (which may be, since Rust has e.g. LLVM as a submodule), then maybe you could try to remove it -- perhaps @10ne1 remembers why we have the option there, but if we only need the standard library sources of Rust (which is needed to compile the kernel), then we could remove it since they are in the "base" repo, rather than a submodule. We could also only clone the tag we need, rather than the history.

@ojeda
Copy link
Contributor Author

ojeda commented Apr 19, 2023

Yes feel free to send another PR now for that, we can add the staging-skip label to ignore it until the Docker images are available. In principle it only takes a one-line change in the script that builds the images, so we might get results on staging.kernelci.org tomorrow or Friday with the new 1.68 toolchain and in production next week.

So I was asking because if we use 1.68 right now in mainline (or even rust-next), it will break (in general) -- there are changes needed in the kernel side, i.e. I need to synchronize the compiler upgrade and the KernelCI change.

I think you told me a while ago that it could be hard to dynamically pick the building environment. But if that is easy now, the version can be known via scripts/min-tool-version.sh rustc in the kernel tree.

@gctucker
Copy link
Contributor

Yes feel free to send another PR now for that, we can add the staging-skip label to ignore it until the Docker images are available. In principle it only takes a one-line change in the script that builds the images, so we might get results on staging.kernelci.org tomorrow or Friday with the new 1.68 toolchain and in production next week.

So I was asking because if we use 1.68 right now in mainline (or even rust-next), it will break (in general) -- there are changes needed in the kernel side, i.e. I need to synchronize the compiler upgrade and the KernelCI change.

I think you told me a while ago that it could be hard to dynamically pick the building environment. But if that is easy now, the version can be known via scripts/min-tool-version.sh rustc in the kernel tree.

Ah sorry I think I was missing some context, didn't realise there were also some kernel changes related to this. Basically you can choose which toolchain to use with each branch, and if there's backwards compatibility of the code with previous toolchains then once the kernel changes have landed on a branch then the toolchain can be updated in KernelCI for that branch. If there's no backwards compatibility, then you can choose whether you prefer KernelCI builds to fail first with the new toolchain and then get fixed with kernel patches or fail first with the new kernel patches and then get fixed with a toolchain update. We can't really get these things updated exactly at the same time, but maybe that's fine as it should be obvious which kernel version is being built with which toolchain version. I would vote for updating the KernelCI config first to start using the new toolchain and then make sure the kernel patches land to fix any build issues (on rust-next etc.). For mainline it's more about keeping the older 1.62 toolchain working, so quite a different approach there - basically just keep building with the minimal version supported as per the documentation I guess.

@ojeda
Copy link
Contributor Author

ojeda commented Apr 19, 2023

Sorry, I should have made it a bit more clear!

There may or may not be backwards compatibility, i.e. sometimes newer versions work, sometimes they do not (until we can declare a minimum version, that is, but that still is in the future).

I am happy either way. If we update the kernel first, then the error will be obvious since the toolchain is older (and we error on purpose if we see that in the kernel). However, updating KernelCI first allows us to know if the new version actually manages or not to compile the kernel (in the kernel build system, a newer toolchain is just a warning), but if it fails, the error may be a bit less obvious.

For mainline it's more about keeping the older 1.62 toolchain working, so quite a different approach there - basically just keep building with the minimal version supported as per the documentation I guess.

Wouldn't mainline be similar? i.e. I have less control over when Linus lands the PR (compared to me pushing to rust-next), but the minimum will move the same way.

In other words, the Rust version upgrades are currently a global flag day. This is, of course, bad, but it is what we have until Rust gives us stability for some unstable features we need now.

(We considered supporting two versions meanwhile by hand, for instance, but that requires more complexity and duplication, which does not seem worth it since we are still upstreaming core code, i.e. there are not real users yet; and the goal is to avoid this situation to begin with by avoiding the unstable bits. But if it would be a big problem failing some builds for KernelCI, we could reconsider it).

@ojeda
Copy link
Contributor Author

ojeda commented Apr 20, 2023

then the error will be obvious since the toolchain is older (and we error on purpose if we see that in the kernel).

By the way, this is true if there is no reconfiguring; otherwise, Rust gets disabled automatically. Either way, it may be a good idea to check with make LLVM=1 rustavailable and/or inspecting CONFIG_RUST=y in the config -- not sure if we are doing something like that already (or if something else will break if Rust got disabled for any reason, e.g. a test that checks one of the Rust samples got built/ran successfully or similar -- we do this kind of thing in our GitHub CI).

So one more argument for updating KernelCI first.

nuclearcat added a commit to nuclearcat/kernelci-deploy that referenced this pull request Apr 21, 2023
As per kernelci/kernelci-core#1890
we need to start building Rust 1.68.
Update deploy script, first for staging, to test that.

Signed-off-by: Denys Fedoryshchenko <[email protected]>
nuclearcat added a commit to nuclearcat/kernelci-deploy that referenced this pull request Apr 21, 2023
As per kernelci/kernelci-core#1890
we need to start building Rust 1.68.
Update deploy script, first for staging, to test that.

Signed-off-by: Denys Fedoryshchenko <[email protected]>
nuclearcat added a commit to nuclearcat/kernelci-deploy that referenced this pull request Apr 21, 2023
As per kernelci/kernelci-core#1890
we need to start building Rust 1.68.
Update deploy script, first for staging, to test that.

Signed-off-by: Denys Fedoryshchenko <[email protected]>
nuclearcat added a commit to kernelci/kernelci-deploy that referenced this pull request Apr 21, 2023
As per kernelci/kernelci-core#1890
we need to start building Rust 1.68.
Update deploy script, first for staging, to test that.

Signed-off-by: Denys Fedoryshchenko <[email protected]>
@gctucker
Copy link
Contributor

For mainline it's more about keeping the older 1.62 toolchain working, so quite a different approach there - basically just keep building with the minimal version supported as per the documentation I guess.

Wouldn't mainline be similar? i.e. I have less control over when Linus lands the PR (compared to me pushing to rust-next), but the minimum will move the same way.

Right, if there's no stable minimum rustc version for mainline and it's also a moving target then it's a similar issue. I was comparing with Clang which has a minimal version while linux-next is being built with the latest in KernelCI.

Copy link
Contributor

@gctucker gctucker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK so basically I think we can take the approach of updating KernelCI first with a new Rust version, then the kernel code will need to be updated to make it work again.

I see the kernelci/staging-rustc-1.68:kselftest-kernelci image has been built and pushed to Docker Hub so I think we're good to go. The next step of course is to update build-configs.yaml to use rustc-1.68 instead of rustc-1.66, and I guess this could be done now based on the previous discussion.

@gctucker gctucker added this pull request to the merge queue Apr 27, 2023
Merged via the queue into kernelci:main with commit a3d83b9 Apr 27, 2023
@gctucker
Copy link
Contributor

Also I've created #1910 about checking that Rust support is enabled. I think we'll be able to add this kind of custom steps with the new build job templates. We probably don't want to try to enable this with the current builds, although we could with some logic in the Python code if it's really needed.

@ojeda ojeda deleted the rust-1.68.2 branch April 27, 2023 09:49
@ojeda
Copy link
Contributor Author

ojeda commented Apr 27, 2023

Thanks! I will send the PR for the build-configs.yaml then.

ojeda added a commit to ojeda/kernelci-core that referenced this pull request May 8, 2023
This moves both `rust-for-linux_rust-next` and `next`'s Rust variant.

As discussed [1], for this kind of upgrades, we will move KernelCI first
and then I will apply push the branch. The run is expected to fail if
it happens to run between those two moves.

Link: kernelci#1890 (comment) [1]
Signed-off-by: Miguel Ojeda <[email protected]>
gctucker pushed a commit that referenced this pull request May 15, 2023
This moves both `rust-for-linux_rust-next` and `next`'s Rust variant.

As discussed [1], for this kind of upgrades, we will move KernelCI first
and then I will apply push the branch. The run is expected to fail if
it happens to run between those two moves.

Link: #1890 (comment) [1]
Signed-off-by: Miguel Ojeda <[email protected]>
@ojeda ojeda mentioned this pull request Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants