Skip to content

Conversation

@clux
Copy link
Member

@clux clux commented Jul 2, 2025

Updates to Edition 2024 and bumps MSRV to 1.85 (minimum for Edition 2024).

✔️ Disambiguate IntoFuture

Started complaining in 2024 due to new addition to the prelude, we basically got this error;

compiler output

error[E0034]: multiple applicable items in scope
    --> kube-runtime/src/controller/mod.rs:1663:42
     |
1663 |                     reconciler(obj, ctx).into_future().in_current_span(),
     |                                          ^^^^^^^^^^^ multiple `into_future` found
     |
     = note: candidate #1 is defined in an impl of the trait `std::future::IntoFuture` for the type `F`
     = note: candidate #2 is defined in an impl of the trait `TryFutureExt` for the type `Fut`
     = note: candidate #3 is defined in an impl of the trait `StreamExt` for the type `T`
help: disambiguate the method for candidate #1
     |
1663 -                     reconciler(obj, ctx).into_future().in_current_span(),
1663 +                     std::future::IntoFuture::into_future(reconciler(obj, ctx)).in_current_span(),
     |
help: disambiguate the method for candidate #2
     |
1663 -                     reconciler(obj, ctx).into_future().in_current_span(),
1663 +                     TryFutureExt::into_future(reconciler(obj, ctx)).in_current_span(),
     |
help: disambiguate the method for candidate #3
     |
1663 -                     reconciler(obj, ctx).into_future().in_current_span(),
1663 +                     StreamExt::into_future(reconciler(obj, ctx)).in_current_span(),

Have pinned to the old trait for now.

✔️ Impl Trait

This requires fixing the return-position impl Trait issue in Rust 2024 in one version.rs where we impl Ord from a fn. It is documented in more detail on the edition guide/RPIT. Possibly it is interacting with tail expression temporary scope.

compiler output

warning: `kube-core` (lib) generated 1 warning
error[E0515]: cannot return value referencing local variable `v`
   --> kube-client/src/discovery/apigroup.rs:132:13
    |
131 |             let pri = v.priority();
    |                       - `v` is borrowed here
132 |             Reverse(pri)
    |             ^^^^^^^^^^^^ returns a value referencing data owned by the current function
    |
note: this call may capture more lifetimes than intended, because Rust 2024 has adjusted the `impl Trait` lifetime capture rules
   --> kube-client/src/discovery/apigroup.rs:131:23
    |
131 |             let pri = v.priority();
    |                       ^^^^^^^^^^^^
help: if you can modify this crate, add a precise capturing bound to avoid overcapturing: `+ use<>`
   --> /kube/kube/kube-core/src/version.rs:160:31
    |
160 |     pub fn priority(&self) -> impl Ord {
    |                               ^^^^^^^^

error[E0515]: cannot return value referencing temporary value
   --> kube-client/src/discovery/apigroup.rs:296:48
    |
296 |                 v.sort_by_cached_key(|(ar, _)| Reverse(Version::parse(ar.version.as_str()).priority()));
    |                                                ^^^^^^^^-----------------------------------^^^^^^^^^^^^
    |                                                |       |
    |                                                |       temporary value created here
    |                                                returns a value referencing data owned by the current function

Added use<> thingy on this fn and the other fn and a similar case in remote_command. Had to also add this to fns used by only examples such as the more esoteric streaming logs/websocket stuff.

Small ❓ uncertainty here; there may be more cases as many of these did not show up until i built examples. It's possible we have some untested things that will be affected by this. Might need to do a PSA for it pre-release.

❓ Drop Order Warnings

Warnings that applies to 5 things;

  • self.queue.poll_expired(cx) in scheduler.rs:119 and scheduler.rs:137
  • self.ready_tx.take() in store.rs:133
  • step_trampolined(api, config, state).await in watcher.rs:666
  • while let Some(event) = stream.next().await in reflector/mod.rs:124 and :123
  • store.get(&request.obj_ref) in controller/mod.rs:388

Not sure if this is serious. Maybe this requires extra testing. You only get this output if you try to do cargo fix --edition not if you manually bump. Getting this type of output basically:

compiler output

warning: relative drop order changing in Rust 2024
   --> kube-runtime/src/reflector/mod.rs:124:33
    |
123 | /     stream! {
124 | |         while let Some(event) = stream.next().await {
    | |                        -----    ^^^^^^^^^^^^^^-----
    | |                        |        |             |
    | |                        |        |             this value will be stored in a temporary; let us call it `#1`
    | |                        |        |             `#1` will be dropped later as of Edition 2024
    | |                        |        this value will be stored in a temporary; let us call it `#4`
    | |                        |        up until Edition 2021 `#4` is dropped last but will be dropped earlier in Edition 2024
    | |                        `event` calls a custom destructor
    | |                        `event` will be dropped later as of Edition 2024
125 | |             match event {
126 | |                 Ok(ev) => {
    | |                    --
    | |                    |
    | |                    `ev` calls a custom destructor
    | |                    `ev` will be dropped later as of Edition 2024
127 | |                     writer.apply_watcher_event(&ev);
128 | |                     writer.dispatch_event(&ev).await;
    | |                     --------------------------------
    | |                     |
    | |                     `__awaitee` calls a custom destructor
    | |                     `__awaitee` will be dropped later as of Edition 2024
...   |
133 | |         }
    | |         - now the temporary value is dropped here, before the local variables in the block or statement
warning: relative drop order changing in Rust 2024
   --> kube-runtime/src/scheduler.rs:119:19
    |
119 |               match self.queue.poll_expired(cx) {
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                     |
    |                     this value will be stored in a temporary; let us call it `#2`
    |                     up until Edition 2021 `#2` is dropped last but will be dropped earlier in Edition 2024
120 |                   Poll::Ready(Some(msg)) => {
121 |                       let msg = msg.into_inner();
    |                           ---
    |                           |
    |                           `msg` calls a custom destructor
    |                           `msg` will be dropped later as of Edition 2024
122 |                       let (msg, _) = self.scheduled.remove_entry(&msg).expect(
    |  ____________________________________-
123 | |                         "Expired message was popped from the Scheduler queue, but was not in the metadata map",
124 | |                     );
    | |                     -
    | |                     |
    | |_____________________this value will be stored in a temporary; let us call it `#1`
    |                       `#1` will be dropped later as of Edition 2024
...
132 |           }
    |           - now the temporary value is dropped here, before the local variables in the block or statement
    |
    = warning: this changes meaning in Rust 2024
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/temporary-tail-expr-scope.html>
    = note: most of the time, changing drop order is harmless; inspect the `impl Drop`s for side effects like releasing locks or sending messages

✔️ Watcher Lifetime Conflicts

Some confusing lifetime conflicts seemingly on a simple &str..

compiler output

error: lifetime may not live long enough
   --> kube-runtime/src/watcher.rs:419:9
    |
415 |         &self,
    |         ----- has type `&FullObject<'1, K>`
...
419 |         self.api.watch(wp, version).await.map(StreamExt::boxed)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'static`

error: lifetime may not live long enough
   --> kube-runtime/src/watcher.rs:419:9
    |
416 |         wp: &WatchParams,
    |             - let's call the lifetime of this reference `'2`
...
419 |         self.api.watch(wp, version).await.map(StreamExt::boxed)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'2` must outlive `'static`

error: lifetime may not live long enough
   --> kube-runtime/src/watcher.rs:419:9
    |
417 |         version: &str,
    |                  - let's call the lifetime of this reference `'3`
418 |     ) -> kube_client::Result<BoxStream<'static, kube_client::Result<WatchEvent<Self::Value>>>> {
419 |         self.api.watch(wp, version).await.map(StreamExt::boxed)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'3` must outlive `'static`

Actually turned out to be another RPIT issue in disguise as a result of us doing return impl Stream in a bunch of places. Fixed in client with some help from nat.

✔️ Reserved gen

After being unblocked by schemars 1.0 (which removed the gen export) it was only
2 variables used in internal functions using the newly reserved keyword - fixed in runtime tests

✔️ Tarpaulin

Also trying to unpin tarpaulin since the upstream bug via #1657 seems fixed.

Have unpinned the rust version and bumped tarpaulin

Follow-up fixes from the bump has been to make sure the trybuild tests in derive allow missing docs (which needed an output update).

7 lines down but within what to expect from 4 breaking versions of tarpaulin imo.

@clux clux added this to the next-major milestone Jul 2, 2025
@clux clux added the changelog-change changelog change category for prs label Jul 2, 2025
@codecov
Copy link

codecov bot commented Jul 3, 2025

Codecov Report

Attention: Patch coverage is 77.41935% with 7 lines in your changes missing coverage. Please review.

Project coverage is 75.1%. Comparing base (da44dd0) to head (e857b15).
Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
kube-client/src/api/portforward.rs 0.0% 2 Missing ⚠️
kube-runtime/src/controller/mod.rs 84.7% 2 Missing ⚠️
kube-runtime/src/finalizer.rs 0.0% 2 Missing ⚠️
kube-client/src/api/remote_command.rs 75.0% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1785     +/-   ##
=======================================
- Coverage   76.6%   75.1%   -1.4%     
=======================================
  Files         84      84             
  Lines       7909    7789    -120     
=======================================
- Hits        6052    5849    -203     
- Misses      1857    1940     +83     
Files with missing lines Coverage Δ
kube-client/src/api/core_methods.rs 72.4% <ø> (ø)
kube-client/src/api/subresource.rs 51.8% <100.0%> (-0.9%) ⬇️
kube-client/src/client/mod.rs 75.6% <100.0%> (ø)
kube-core/src/version.rs 96.7% <100.0%> (+2.3%) ⬆️
kube-derive/tests/crd_enum_test.rs 100.0% <ø> (ø)
kube-derive/tests/resource.rs 100.0% <ø> (ø)
kube-derive/tests/test_ui.rs 100.0% <ø> (ø)
kube-runtime/src/reflector/mod.rs 91.4% <100.0%> (-8.6%) ⬇️
kube-runtime/src/utils/predicate.rs 70.5% <100.0%> (-0.4%) ⬇️
kube-runtime/src/watcher.rs 44.6% <ø> (ø)
... and 4 more

... and 38 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@clux
Copy link
Member Author

clux commented Jul 3, 2025

actually green! lint is intentional for now (k8s-openapi pinned to git ref). Updated post above.

@clux clux marked this pull request as ready for review July 3, 2025 14:01
@clux clux requested a review from nightkr July 3, 2025 14:01
This was linked to issues Jul 3, 2025
@clux clux mentioned this pull request Jul 6, 2025
Copy link
Member

@nightkr nightkr left a comment

Choose a reason for hiding this comment

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

LGTM.

I went through the drop order warnings you mention, and none of them should be relevant to us; they're just plain old data that happen to contain heap-allocated stuff.

@clux clux merged commit 06e843b into main Jul 7, 2025
16 of 18 checks passed
@clux clux deleted the rust2024-take2 branch July 7, 2025 08:31
@clux clux mentioned this pull request Sep 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog-change changelog change category for prs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upgrade to edition 2024 Test coverage is broken for Rust 1.83

2 participants