Skip to content

Conversation

ADD-SP
Copy link
Member

@ADD-SP ADD-SP commented Sep 19, 2025

The fixup of #7621 for better error message. Otherwise, the error message is a bit encrypted.

$ cargo run
error: failed to select a version for `tokio`.
    ... required by package `uring-test v0.1.0 (xxxx)`
versions that meet the requirements `*` (locked to 1.47.1) are: 1.47.1

package `uring-test` depends on `tokio` with feature `io-uring` but `tokio` does not have that feature.
 A required dependency with that name exists, but only optional dependencies can be used as features.


failed to select a version for `tokio` which could resolve this conflict

This is a bit complicated. It seems like it's not worth the complexity, but we still have to do it because we also want to convert --cfg tokio_taskdump into a Cargo feature, and taskdump already has similar user-friendly error messages.

tokio/tokio/src/lib.rs

Lines 477 to 478 in 3b5a15d

#[cfg(all(not(tokio_unstable), tokio_taskdump))]
compile_error!("The `tokio_taskdump` feature requires `--cfg tokio_unstable`.");

@ADD-SP ADD-SP added A-tokio Area: The main tokio crate M-fs Module: tokio/fs labels Sep 19, 2025
@ADD-SP ADD-SP changed the title fs: emit compilation error without 'tokio_unstable' for 'io-uring' fs: emit compilation error without tokio_unstable for io-uring Sep 19, 2025
@ADD-SP ADD-SP force-pushed the add_sp/fs-emit-compile-error branch 18 times, most recently from a579578 to d0c7c35 Compare September 20, 2025 05:49
@ADD-SP ADD-SP marked this pull request as ready for review September 20, 2025 06:14
@ADD-SP ADD-SP requested a review from mox692 September 20, 2025 06:15
/// panic!("This panic will shutdown the runtime.");
/// }).await;
/// }
/// # #[cfg(not(tokio_unstable))]
Copy link
Member Author

@ADD-SP ADD-SP Sep 20, 2025

Choose a reason for hiding this comment

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

I removed it due to this

Click to see the image

/// }).await;
/// })
/// }
/// # #[cfg(not(tokio_unstable))]
Copy link
Member Author

Choose a reason for hiding this comment

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

Signed-off-by: ADD-SP <qiqi.zhang@konghq.com>
@ADD-SP ADD-SP force-pushed the add_sp/fs-emit-compile-error branch from d0c7c35 to 33c603b Compare September 21, 2025 14:23
@mox692
Copy link
Member

mox692 commented Sep 23, 2025

Sorry for the delay, I'll check this tomorrow.

Copy link
Member

@mox692 mox692 left a comment

Choose a reason for hiding this comment

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

I think this change makes CI code cleaner, thanks! I left some comments but mostly lgtm.

Comment on lines 339 to 351
cargo nextest run --all-features
cargo test --doc --all-features
# taskdump is an unstable feature, but it can only be enabled
# by --cfg tokio_taskdump, not by a feature flag, so we can
# use $TOKIO_STABLE_FEATURES here.
cargo nextest run --features $TOKIO_STABLE_FEATURES
cargo test --doc --features $TOKIO_STABLE_FEATURES
Copy link
Member

Choose a reason for hiding this comment

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

Does this part really use the taskdump feature?

Copy link
Member Author

@ADD-SP ADD-SP Sep 24, 2025

Choose a reason for hiding this comment

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

Emm, it looks doesn't use the taskdump feature. Initially, this job was introduced in 9eb3f5b, but is looks not related to the taskdump.

Fixed in 3cee7fd.

Comment on lines -783 to +802
- name: "clippy --all ${{ matrix.rustflags }}"
run: cargo clippy --all --tests --all-features --no-deps
- name: "clippy --all --features ${{ env.TOKIO_STABLE_FEATURES }}"
run: cargo clippy --all --tests --no-deps --features $TOKIO_STABLE_FEATURES
- name: "clippy --all --all-features --unstable"
run: cargo clippy --all --tests --no-deps --all-features
env:
RUSTFLAGS: ${{ matrix.rustflags }}
RUSTFLAGS: --cfg tokio_unstable --cfg tokio_taskdump -Dwarnings
Copy link
Member

Choose a reason for hiding this comment

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

If I understand correctly, this change removed the case where we don't use tokio_unstable?

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a cargo cliipy in the near above, did you mean this?

      - name: "clippy --all --features ${{ env.TOKIO_STABLE_FEATURES }}"
        run: cargo clippy --all --tests --no-deps --features $TOKIO_STABLE_FEATURES

Signed-off-by: ADD-SP <qiqi.zhang@konghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-fs Module: tokio/fs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants