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

Fix occasional failure to generate rustdoc when RUSTFLAGS="-Dwarnings". #643

Merged
merged 2 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 75 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ jobs:
- run-on-libp2p-dcutr-relay
- run-on-libp2p-gossipsub-request-response
- run-on-libp2p-uds
- run-on-libp2p-whose-dependency-has-warnings
- run-on-core-graphics
- run-on-bevy-core
- run-on-bevy-gltf
Expand All @@ -53,6 +54,7 @@ jobs:
echo "run-on-libp2p-dcutr-relay: ${{ needs.run-on-libp2p-dcutr-relay.result }}"
echo "run-on-libp2p-gossipsub-request-response: ${{ needs.run-on-libp2p-gossipsub-request-response.result }}"
echo "run-on-libp2p-uds: ${{ needs.run-on-libp2p-uds.result }}"
echo "run-on-libp2p-whose-dependency-has-warnings: ${{ needs.run-on-libp2p-whose-dependency-has-warnings.result }}"
echo "run-on-core-graphics: ${{ needs.run-on-core-graphics.result }}"
echo "run-on-bevy-core: ${{ needs.run-on-bevy-core.result }}"
echo "run-on-bevy-gltf: ${{ needs.run-on-bevy-gltf.result }}"
Expand Down Expand Up @@ -86,6 +88,8 @@ jobs:
run: exit 1
- if: ${{ needs.run-on-libp2p-uds.result != 'success' }}
run: exit 1
- if: ${{ needs.run-on-libp2p-whose-dependency-has-warnings.result != 'success' }}
run: exit 1
- if: ${{ needs.run-on-core-graphics.result != 'success' }}
run: exit 1
- if: ${{ needs.run-on-bevy-core.result != 'success' }}
Expand Down Expand Up @@ -1518,6 +1522,76 @@ jobs:
key: ${{ runner.os }}-${{ steps.toolchain.outputs.cachekey }}-${{ hashFiles('semver\**\Cargo.lock') }}-${{ github.job }}-rustdoc
path: subject\target\semver-checks\cache

run-on-libp2p-whose-dependency-has-warnings:
# cargo-semver-checks failed to generate rustdoc JSON for a commit near libp2p v0.53.1 because:
# - RUSTFLAGS was set to "-Dwarning" by default by a GitHub Action
# - libp2p has a dependency (libp2p-yamux) whose dependency (yamux) released a newer version,
# which deprecated an item that libp2p-yamux uses
# - cargo-semver-checks added "--cap-lints" to RUSTDOCFLAGS but not RUSTFLAGS
# - generating rustdoc JSON for libp2p involves a rustc check of its dependency, and that
# check obeys RUSTFLAGS, not RUSTDOCFLAGS
#
# https://github.com/obi1kenobi/cargo-semver-checks/issues/589
name: 'Semver: dependency has warnings with "-Dwarnings" (libp2p v0.53.1)'
runs-on: ubuntu-latest
needs:
- build-binary
steps:
- name: Checkout cargo-semver-checks
uses: actions/checkout@v4
with:
persist-credentials: false
path: 'semver'

- name: Checkout libp2p
uses: actions/checkout@v4
with:
persist-credentials: false
repository: 'libp2p/rust-libp2p'
ref: '4759ba8244242628164b7ec9e7dc99d67269e39f'
path: 'subject'

- name: Install rust
id: toolchain
uses: actions-rust-lang/setup-rust-toolchain@v1
with:
cache: false
rustflags: "-Dwarnings" # set explicitly, to ensure we trigger the bug described above

- name: Restore binary
id: cache-binary
uses: actions/cache/restore@v3
with:
path: bins/
key: bins-${{ runner.os }}-${{ github.run_id }}-${{ github.run_attempt }}
fail-on-cache-miss: true

- name: Restore rustdoc
id: cache
uses: actions/cache/restore@v3
with:
key: ${{ runner.os }}-${{ steps.toolchain.outputs.cachekey }}-${{ hashFiles('semver/**/Cargo.lock') }}-${{ github.job }}-rustdoc
path: subject/target/semver-checks/cache

- name: Restore cargo index and rustdoc target dir
uses: Swatinem/rust-cache@v2
with:
workspaces: |
subject/target/semver-checks/local-libp2p-0_53_1

- name: Run semver-checks
run: |
cd semver
../bins/cargo-semver-checks semver-checks check-release --manifest-path="../subject/Cargo.toml" -p libp2p --verbose

- name: Save rustdoc
uses: actions/cache/save@v3
if: steps.cache.outputs.cache-hit != 'true'
with:
key: ${{ runner.os }}-${{ steps.toolchain.outputs.cachekey }}-${{ hashFiles('semver/**/Cargo.lock') }}-${{ github.job }}-rustdoc
path: subject/target/semver-checks/cache


run-on-perf-event-open-sys2:
# cargo-semver-checks failed to find the generated rustdoc JSON
# because the crate used a lib name different from the crate name.
Expand All @@ -1533,7 +1607,7 @@ jobs:
persist-credentials: false
path: 'semver'

- name: Checkout bevy
- name: Checkout phantomical/perf-event
uses: actions/checkout@v4
with:
persist-credentials: false
Expand Down
15 changes: 14 additions & 1 deletion src/rustdoc_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,18 @@ impl RustdocCommand {
let version = crate_source.version()?;
let pkg_spec = format!("{crate_name}@{version}");

// Generating rustdoc JSON for a crate also involves checking that crate's dependencies.
// The check is done by rustc, not rustdoc, so it's subject to RUSTFLAGS not RUSTDOCFLAGS.
// We don't want rustc to fail that check if the user has set RUSTFLAGS="-Dwarnings" here.
// This fixes: https://github.com/obi1kenobi/cargo-semver-checks/issues/589
let rustflags = match std::env::var("RUSTFLAGS") {
Ok(mut prior_rustflags) => {
prior_rustflags.push_str(" --cap-lints=allow");
std::borrow::Cow::Owned(prior_rustflags)
}
Err(_) => std::borrow::Cow::Borrowed("--cap-lints=allow"),
};

// Run the rustdoc generation command on the placeholder crate,
// specifically requesting the rustdoc of *only* the crate specified in `pkg_spec`.
//
Expand All @@ -96,8 +108,9 @@ impl RustdocCommand {
cmd.env("RUSTC_BOOTSTRAP", "1")
.env(
"RUSTDOCFLAGS",
"-Z unstable-options --document-private-items --document-hidden-items --output-format=json --cap-lints allow",
"-Z unstable-options --document-private-items --document-hidden-items --output-format=json --cap-lints=allow",
)
.env("RUSTFLAGS", rustflags.as_ref())
.stdout(std::process::Stdio::null()) // Don't pollute output
.stderr(stderr)
.arg("doc")
Expand Down
Loading