Skip to content

Failure to generate rustdoc when RUSTFLAGS env var sets -Dwarnings #589

@obi1kenobi

Description

@obi1kenobi

If the RUSTFLAGS="-Dwarnings" env var value is set, cargo-semver-checks will fail to generate rustdoc on crates that have warnings.

Encountered in:
libp2p/rust-libp2p#4932 (comment)
https://github.com/libp2p/rust-libp2p/actions/runs/7003957574/job/19050749994

cargo-semver-checks produces the following error log:

Error: running cargo-doc failed on /home/runner/work/rust-libp2p/rust-libp2p/semver-checks/target/semver-checks/local-libp2p-0_53_1/Cargo.toml:
    Checking cfg-if v1.0.0
   Compiling libc v0.2.150
< ... snip ... >
    Checking libp2p-dns v0.41.1 (/home/runner/work/rust-libp2p/rust-libp2p/transports/dns)
    Checking libp2p-yamux v0.45.0 (/home/runner/work/rust-libp2p/rust-libp2p/muxers/yamux)
error: use of deprecated unit variant `yamux::WindowUpdateMode::OnReceive`: Use `WindowUpdateMode::OnRead` instead.
   --> /home/runner/work/rust-libp2p/rust-libp2p/muxers/yamux/src/lib.rs:[235](https://github.com/libp2p/rust-libp2p/actions/runs/7003957574/job/19050749994#step:4:236):51
    |
235 |         WindowUpdateMode(yamux::WindowUpdateMode::OnReceive)
    |                                                   ^^^^^^^^^
    |
    = note: `-D deprecated` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(deprecated)]`

    Checking libp2p-autonat v0.12.0 (/home/runner/work/rust-libp2p/rust-libp2p/protocols/autonat)
    Checking libp2p-tcp v0.41.0 (/home/runner/work/rust-libp2p/rust-libp2p/transports/tcp)
error: could not compile `libp2p-yamux` (lib) due to previous error

We can't simply unset RUSTFLAGS, because it may contain settings other than -Dwarnings, for example --cfg <something>, that are relevant and required. At the same time, many users deny warnings in CI, so the current behavior offers poor UX by default for those users. We want to offer "pit of success" UX, and here we fall short.

Options:

  1. Print out a warning message if RUSTFLAGS is set to a value that contains -Dwarnings or -D warnings.
    a. This warning can urge users not to set that value, but can be missed when it gets buried behind a long error message from a rustdoc generation failure like in the case above.

  2. Attempt to parse RUSTFLAGS and remove -Dwarnings in its many forms.
    a. Can we really be sure that we're doing this safely and in a way that won't ever go wrong?
    b. If we fail to remove -Dwarnings because it was specified in some way we didn't catch, this can be even more confusing. I generally don't love "magic" like this because it has a tendency to go wrong and cause more issues.

  3. Change the default behavior to unset RUSTFLAGS / remove -Dwarnings from RUSTFLAGS, but offer a flag to switch that behavior off and leave RUSTFLAGS unchanged.
    a. This is more complex to implement, but also seems like it would suit more users' needs both by default and in general.
    b. Should we do this inside cargo-semver-checks itself or just inside our prebuilt GitHub Action?

I'm currently inclined toward 3. and toward implementing it just inside our action, not inside cargo-semver-checks itself. Would love to hear your thoughts!

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-bugCategory: doesn't meet expectationsE-help-wantedCall for participation: Help is requested to fix this issue.E-mentorCall for participation: Mentorship is available for this issue.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions