Skip to content

Rust 1.73 #2124

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

Merged
merged 5 commits into from
Oct 23, 2023
Merged

Rust 1.73 #2124

merged 5 commits into from
Oct 23, 2023

Conversation

ojeda
Copy link
Contributor

@ojeda ojeda commented Oct 5, 2023

Rust 1.73.0 will likely be the next version supported by the kernel [1].

Therefore, add it as a new build environment.

In addition, use Clang 17 instead of 16 too, since it is the latest and matches the internal LLVM version used by Rust 1.73.0.

Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]

@ojeda
Copy link
Contributor Author

ojeda commented Oct 5, 2023

I based this one on top of #2068 (i.e. the previous upgrade).

@gctucker
Copy link
Contributor

gctucker commented Oct 6, 2023

@aliceinwire I think we need to first have 1.72.1 tested and merged, then this one rebased and tested before it gets merged so it probably won't be ready until next week. It's good to approve changes in they look OK in principle but in this case it's not sufficient to have the PR merged.

@aliceinwire
Copy link
Member

@aliceinwire I think we need to first have 1.72.1 tested and merged, then this one rebased and tested before it gets merged so it probably won't be ready until next week. It's good to approve changes in they look OK in principle but in this case it's not sufficient to have the PR merged.

you are talking about this PR #2068
I will give it a look now, thanks for remember

@aliceinwire
Copy link
Member

aliceinwire commented Oct 6, 2023

why this PR #2068 as the label staging-skip?

@gctucker
Copy link
Contributor

gctucker commented Oct 6, 2023

Maybe ask on the actual PR #2068 :) Also, like I wrote in a previous comment I'm planning to go through this and have it merged today if there's no blockers. It's good to have another pair of eyes (we never have too many reviewers!) but I also don't want us to be doing the same work twice.

@aliceinwire
Copy link
Member

Maybe ask on the actual PR #2068 :) Also, like I wrote in a previous comment I'm planning to go through this and have it merged today if there's no blockers. It's good to have another pair of eyes (we never have too many reviewers!) but I also don't want us to be doing the same work twice.

ok

@nuclearcat
Copy link
Member

Temporary setting staging-skip, cause it is breaking staging:

Traceback (most recent call last):
  File "/data/kernelci-deploy-checkout/kernelci-core/./kci", line 29, in <module>
    kernelci.cli.call(args.command, sys.argv[2:])
  File "/data/kernelci-deploy-checkout/kernelci-core/kernelci/cli/__init__.py", line 53, in call
    _COMMANDS[name](args)
  File "/data/kernelci-deploy-checkout/kernelci-core/kernelci/cli/docker.py", line 216, in main
    sub_main("docker", globals(), args)
  File "/data/kernelci-deploy-checkout/kernelci-core/kernelci/cli/base.py", line 793, in sub_main
    configs = kernelci.config.load(opts.get_yaml_configs())
  File "/data/kernelci-deploy-checkout/kernelci-core/kernelci/config/__init__.py", line 199, in load
    return load_data(data)
  File "/data/kernelci-deploy-checkout/kernelci-core/kernelci/config/__init__.py", line 174, in load_data
    config.update(kernelci.config.build.from_yaml(data, filters))
  File "/data/kernelci-deploy-checkout/kernelci-core/kernelci/config/build.py", line 462, in from_yaml
    build_configs = {
  File "/data/kernelci-deploy-checkout/kernelci-core/kernelci/config/build.py", line 463, in <dictcomp>
    name: BuildConfig.load_from_yaml(
  File "/data/kernelci-deploy-checkout/kernelci-core/kernelci/config/build.py", line 399, in load_from_yaml
    variants = [
  File "/data/kernelci-deploy-checkout/kernelci-core/kernelci/config/build.py", line 400, in <listcomp>
    BuildVariant.load_from_yaml(variant, name, fragments, b_envs)
  File "/data/kernelci-deploy-checkout/kernelci-core/kernelci/config/build.py", line 314, in load_from_yaml
    kw['build_environment'] = build_environments[kw['build_environment']]
KeyError: 'rustc-1.71'

@nuclearcat nuclearcat added the staging-skip Don't test automatically on staging.kernelci.org label Oct 6, 2023
@ojeda
Copy link
Contributor Author

ojeda commented Oct 6, 2023

Does it mean the other repository (kernelci-deploy) needs to be updated first? (I see it the "rustc-1.71" there). Should I generally send a PR like kernelci/kernelci-deploy#96 or kernelci/kernelci-deploy#97 together with these ones?

Or is it something that is manually done on your side when you go to merge this one?

Thanks!

@ojeda
Copy link
Contributor Author

ojeda commented Oct 6, 2023

Ah, I just saw #2068 (comment). I guess Guillaume is talking about the errors from ~summer, rather than just the (trivial) update of the key. In any case, I am happy to open PRs to both repositories and link them when I send these updates.

@gctucker
Copy link
Contributor

gctucker commented Oct 6, 2023

Yeah please just leave that with me for 1.72.1 and if something needs to be done again for 1.73 and all future version updates I can put a comment here and maybe you can take care of it yourself?

@ojeda
Copy link
Contributor Author

ojeda commented Oct 6, 2023

Yeah, sounds good -- thanks a lot!

@gctucker gctucker removed the staging-skip Don't test automatically on staging.kernelci.org label Oct 9, 2023
@gctucker
Copy link
Contributor

gctucker commented Oct 9, 2023

@ojeda Please rebase again now that #2068 has been merged.

Then here's a checklist for upgrading the rustc toolchain based on the 1.72 reviews:

  • First PR, YAML config and Docker image template updates in the kernelci-core repo:
    • Update the rustc build environment version in build-configs.yaml e.g.
  rustc-1.73:
    cc: clang
    cc_version: 16
    arch_params:
      x86_64: *x86_64_params
  • Update the rest of build-configs.yaml with the new rustc build environment name (mainline, next, rust-for-linux entries...)
  • Replace Docker image template file with a matching name e.g. config/docker/rustc-1.73.jinja2
  • Second PR, staging config update in in the kernelci-deploy repo:
    • Bump the rustc version used in build-configs-staging.yaml by updating the patches as per this commit:
      kernelci/kernelci-deploy@ac47c72
      Note: These patches are generated by hand with git format-patch. It's pretty straightforward but this process is not well documented yet, I'll add another comment about it here.
    • Update the staging.kernelci.org script that builds the Docker images as per this commit:
      kernelci/kernelci-deploy@f26278b
  • Ask a sysadmin to create the kernelci/staging-rustc-x.yz and kernelci/rustc-x.yz repositories on Docker hub
  • The second PR needs to be merged in order to get the first PR tested on staging. Typically, kernelci-deploy changes for staging don't really need any review so this should be merged very quickly unless something is obviously wrong with it.
  • Third PR, production config update in kernelci-deploy
  • Once the first 2 PRs have been merged after checking the results on staging, the third one can be merged so the new toolchain will be used in production as part of the following Monday update.

This checklist assumes there's only one rustc version being covered at a time. Presumably this is always going to be the case until a minimal version has been declared in the same way that we have clang-11 for mainline, at which point we might want to start testing both the minimal supported rustc version and the cutting edge one. Is that right, and how far in the future is this expected to happen?

@gctucker
Copy link
Contributor

gctucker commented Oct 9, 2023

Pseudo-documentation for updating the patch series used on staging:

$ cd kernelci-deploy
./pending.py kernelci-core
$ cd checkout/kernelci-core
$ git log --oneline | head -10
d393efe96 STAGING use [KernelCI staging] in LAVA jobs
4a151bfeb STAGING use staging JWT in kubernetes template
086398bdf STAGING use staging branch for buildroot with reduced arch list
8bbf91eee STAGING use staging.kernelci.org branch for test definitions
b818d93bf STAGING use kernelci/staging-qemu Docker image
0c5761309 STAGING ensure no public bisection email reports are sent
a13347326 STAGING add build-configs-staging.yaml
48915a1a0 Merge branch 'legacy-cli' of https://github.com/gctucker/kernelci-core into HEAD
23b7e1731 Merge branch 'update_isar_cip_rootfs' of https://github.com/aliceinwire/kernelci-core into HEAD
084f23cf5 Merge branch 'kci-update-user' of https://github.com/JenySadadia/kernelci-core into HEAD
$ git rebase -i 48915a1a0 --onto 48915a1a0
edit a13347326 STAGING add build-configs-staging.yaml
pick 0c5761309 STAGING ensure no public bisection email reports are sent
pick b818d93bf STAGING use kernelci/staging-qemu Docker image
pick 8bbf91eee STAGING use staging.kernelci.org branch for test definitions
pick 086398bdf STAGING use staging branch for buildroot with reduced arch list
pick 4a151bfeb STAGING use staging JWT in kubernetes template
pick d393efe96 STAGING use [KernelCI staging] in LAVA jobs
$ vim config/core/build-configs-staging.yaml
$ git commit -a --amend -C HEAD
$ git rebase --continue
$ mkdir patches
$ git format-patch 48915a1a0f73
0001-STAGING-add-build-configs-staging.yaml.patch
0002-STAGING-ensure-no-public-bisection-email-reports-are.patch
0003-STAGING-use-kernelci-staging-qemu-Docker-image.patch
0004-STAGING-use-staging.kernelci.org-branch-for-test-def.patch
0005-STAGING-use-staging-branch-for-buildroot-with-reduce.patch
0006-STAGING-use-staging-JWT-in-kubernetes-template.patch
0007-STAGING-use-KernelCI-staging-in-LAVA-jobs.patch
$ cd ../../..
$ git checkout -b staging-update
Switched to a new branch 'staging-update'
$ git rm -rf patches/kernelci-core/staging.kernelci.org
$ mkdir -p patches/kernelci-core
$ mv checkout/kernelci-core/patches patches/kernelci-core/staging.kernelci.org
$ git add patches/kernelci-core/staging.kernelci.org
$ git diff --stat --cached
 patches/kernelci-core/staging.kernelci.org/0001-STAGING-add-build-configs-staging.yaml.patch               | 8 ++++----
 patches/kernelci-core/staging.kernelci.org/0002-STAGING-ensure-no-public-bisection-email-reports-are.patch | 2 +-
 patches/kernelci-core/staging.kernelci.org/0003-STAGING-use-kernelci-staging-qemu-Docker-image.patch       | 2 +-
 patches/kernelci-core/staging.kernelci.org/0004-STAGING-use-staging.kernelci.org-branch-for-test-def.patch | 2 +-
 patches/kernelci-core/staging.kernelci.org/0005-STAGING-use-staging-branch-for-buildroot-with-reduce.patch | 2 +-
 patches/kernelci-core/staging.kernelci.org/0006-STAGING-use-staging-JWT-in-kubernetes-template.patch       | 2 +-
 patches/kernelci-core/staging.kernelci.org/0007-STAGING-use-KernelCI-staging-in-LAVA-jobs.patch            | 2 +-
 7 files changed, 10 insertions(+), 10 deletions(-)
$ git commit -s
# ...

Then push the branch to your GitHub fork and create a PR as usual.

@gctucker
Copy link
Contributor

gctucker commented Oct 9, 2023

And yes we know about quilt ;)

The last part after the rebase could easily be scripted I think, and the rest is kind of expected to be done by hand anyway. Maybe a first scripted procedure could get to the point of doing the interactive rebase, then manual editing, then automated patch updates. That's an improvement someone could work on I guess. In fact there might already be a GitHub issue about it somewhere.

ojeda added 5 commits October 9, 2023 11:35
Rust 1.73.0 will likely be the next version supported by the kernel [1].

Therefore, add it as a new build environment.

In addition, use Clang 17 instead of 16 too, since it is the latest
and matches the internal LLVM version used by Rust 1.73.0.

Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]
Signed-off-by: Miguel Ojeda <[email protected]>
@ojeda
Copy link
Contributor Author

ojeda commented Oct 9, 2023

Rebased, thanks a ton for all that work (and the nicely explained instructions too -- I can try to follow them for Rust 1.74 and see how it goes).

This checklist assumes there's only one rustc version being covered at a time. Presumably this is always going to be the case until a minimal version has been declared in the same way that we have clang-11 for mainline, at which point we might want to start testing both the minimal supported rustc version and the cutting edge one. Is that right, and how far in the future is this expected to happen?

That is right, and indeed, when that point comes, testing minimum and maximum supported versions would be ideal. Regarding timing, It is hard to predict, since it depends on many factors (e.g. ongoing discussion with the Rust project) plus a few major distributions told us they are OK supporting a fixed version, which relaxed the pressure a bit and in turn allows us to reconsider features. So I would say at least a year (e.g. we may discuss it in LPC and, say, decide to try to achieve it for the next-next LTS).

@gctucker
Copy link
Contributor

gctucker commented Oct 9, 2023

Thanks for the update, sounds good.

About 1.73, please note that the checklist isn't quite complete yet :) The 1.72 changes only just landed so it's not regular timing, but if you want to give it a go and add the missing bits that would be great. Otherwise let us know and we'll take care of it for 1.73, then let you try 1.74.

@ojeda
Copy link
Contributor Author

ojeda commented Oct 15, 2023

Done! Please see kernelci/kernelci-deploy#103 and kernelci/kernelci-deploy#104.

@gctucker
Copy link
Contributor

Sorry I created a Docker Hub repository called staging-rust-1.73 yesterday instead of staging-rustc-1.73 (missing c). That's fixed now, so hopefully we'll get some results on staging today.

@nuclearcat
Copy link
Member

@nuclearcat nuclearcat added this pull request to the merge queue Oct 23, 2023
Merged via the queue into kernelci:main with commit a40e506 Oct 23, 2023
@ojeda ojeda deleted the rust-1.73 branch December 14, 2023 21:06
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.

4 participants