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

Setting -Dwarnings breaks feature detection in build.rs #5373

Closed
piodul opened this issue Jan 13, 2023 · 3 comments · Fixed by #5374
Closed

Setting -Dwarnings breaks feature detection in build.rs #5373

piodul opened this issue Jan 13, 2023 · 3 comments · Fixed by #5374
Assignees
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug.

Comments

@piodul
Copy link

piodul commented Jan 13, 2023

I'm not sure whether this is considered a bug due to custom RUSTFLAGS, but treating warnings as errors in CI doesn't sound uncommon.

Version

cargo tree | grep tokio

test-tokio-fail v0.1.0 (/tmp/test-tokio-fail)
└── tokio v1.24.1

Platform

Linux <cut> 6.0.17-200.fc36.x86_64 #1 SMP PREEMPT_DYNAMIC Wed Jan 4 16:00:03 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Description

Steps to reproduce:

rustup toolchain install 1.59.0
cargo new --bin test-tokio-fail
cd ./test-tokio-fail/
echo "tokio = \"=1.24.0\"" >> Cargo.toml # or 1.24.1, same issue

# We are treating warnings as errors in the CI
export RUSTFLAGS="-Dwarnings"
# Alternatively:
# export RUSTFLAGS="--cfg tokio_no_atomic_u64"

cargo +1.59.0 check

I get the following error:

   Compiling tokio v1.24.0
error[E0432]: unresolved import `crate::util::once_cell`
 --> /home/piodul/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.24.0/src/loom/std/atomic_u64_static_once_cell.rs:3:18
  |
3 | use crate::util::once_cell::OnceCell;
  |                  ^^^^^^^^^ could not find `once_cell` in `util`

For more information about this error, try `rustc --explain E0432`.
error: could not compile `tokio` due to previous error

My short investigation led me to the tokio's build.rs file. It looks like -Dwarnings affects the autocfg probe here, seen when running cargo check with high verbosity (-vv):

     Running `/tmp/test-tokio-fail/target/debug/build/tokio-0b45242da01fd5fb/build-script-build`
[tokio 1.24.0] error: unused import: `std::sync::atomic::AtomicU64 as _`
[tokio 1.24.0]  --> <anon>:3:9
[tokio 1.24.0]   |
[tokio 1.24.0] 3 |     use std::sync::atomic::AtomicU64 as _;
[tokio 1.24.0]   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[tokio 1.24.0]   |
[tokio 1.24.0]   = note: `-D unused-imports` implied by `-D warnings`
[tokio 1.24.0] 
[tokio 1.24.0] error: aborting due to previous error
[tokio 1.24.0] 
[tokio 1.24.0] cargo:rustc-cfg=tokio_no_target_has_atomic
[tokio 1.24.0] cargo:rustc-cfg=tokio_no_const_mutex_new
[tokio 1.24.0] cargo:rustc-cfg=tokio_no_atomic_u64

Because of this, cargo:rustc-cfg=tokio_no_atomic_u64 is emitted in addition to others (I get the same effect with RUSTFLAGS="--cfg tokio_no_atomic_u64"). This cfg seems to be key to trigger the error.

@piodul piodul added A-tokio Area: The main tokio crate C-bug Category: This is a bug. labels Jan 13, 2023
@Darksonn Darksonn changed the title Tokio 1.24 fails to build with Rust 1.59.0 and RUSTFLAGS=-Dwarnings Setting -Dwarnings breaks feature detection in build.rs Jan 13, 2023
@Darksonn
Copy link
Contributor

Darksonn commented Jan 13, 2023

Thank you for reporting this. Setting -Dwarnings shouldn't break the feature detection in our build.rs file.

The specific compilation failure you ran into was fixed in 1.24.1 (I hope so), but the problem persists — it will incorrectly detect features even if you upgrade, leading to less efficient code.

@piodul
Copy link
Author

piodul commented Jan 13, 2023

The specific compilation failure you ran into was fixed in 1.24.1 (I hope so)

I'm afraid it wasn't... If I specify =1.24.1 in the reproducer then I get exactly the same failure:

    Checking tokio v1.24.1
error[E0432]: unresolved import `crate::util::once_cell`
 --> /home/piodul/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.24.1/src/loom/std/atomic_u64_static_once_cell.rs:3:18
  |
3 | use crate::util::once_cell::OnceCell;
  |                  ^^^^^^^^^ could not find `once_cell` in `util`

For more information about this error, try `rustc --explain E0432`.
error: could not compile `tokio` due to previous error

@taiki-e
Copy link
Member

taiki-e commented Jan 13, 2023

This is because util::once_cell is available only when rt, signal, or process is enabled, but atomic_u64_static_once_cell.rs is compiled unconditionally.

#[cfg(any(feature = "rt", feature = "signal", feature = "process"))]
pub(crate) mod once_cell;

mod atomic_u32;
mod atomic_u64;

Can be reproduced with:

RUSTFLAGS='--cfg tokio_no_atomic_u64 --cfg tokio_no_const_mutex_new' cargo hack build -p tokio --no-dev-deps

@taiki-e taiki-e self-assigned this Jan 13, 2023
carllerche pushed a commit that referenced this issue Jan 14, 2023
Fixes #5373
Closes #5358 

- Add check for no_atomic_u64 & no_const_mutex_new (condition to atomic_u64_static_once_cell.rs is compiled)
- Allow unused_imports in TARGET_ATOMIC_U64_PROBE. I also tested other *_PROBE and found no other errors triggered by -D warning.
- Fix cfg of util::once_cell module
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 C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants