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.

Sign up for GitHub

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(doctest): warn only once if doctest xcompile is skipped #14920

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

fix(doctest): warn only once if doctest xcompile is skipped #14920

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4639cf5
as per issue/comment https://github.com/rust-lang/cargo/issues/12118#…
progressive-galib Dec 11, 2024
d3e21b0
Merge branch 'rust-lang:master' into galib-xcompile-doctest-warning-i…
progressive-galib Dec 12, 2024
2e446e5
Merge branch 'rust-lang:master' into galib-xcompile-doctest-warning-i…
progressive-galib Dec 12, 2024
e758536
warns once if only even if multiple doctests are skipped
progressive-galib Dec 12, 2024
13071f5
added a fullstop in the end of a warning and modified the test for my…
progressive-galib Dec 12, 2024
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
4 changes: 4 additions & 0 deletions src/cargo/ops/cargo_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ fn run_doc_tests(
CompileKind::Host => {}
CompileKind::Target(target) => {
if target.short_name() != compilation.host {

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the contribution.

We need a test for this warning to make sure it doesn't emit excessive warnings.

#12118 (comment)

Copy link
Author

@progressive-galib progressive-galib Dec 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

currently it will show every time a doctest is passed over (i.e skipped)

else we are gonna need to use something like,

pub static ref WARNONCE: Mutex<bool> = Mutex::new(false);

(btw i really like lazy_static crate but i kinda admire low package count of a self hosted compiler)

Copy link
Member

Choose a reason for hiding this comment

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

This part of cargo is single-threaded, so perhaps a simple variable here serves the purpose.

cargo/src/cargo/ops/cargo_test.rs

Line 181 in 94d274d

Copy link
Author

Choose a reason for hiding this comment

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

done, pushed it with without locally building.... man as someone working with very slow computer gotta say i just 💖 your CI/CD set up.

Copy link
Author

Choose a reason for hiding this comment

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

done, all tests have passed

gctx.shell().concise(|shell| {
shell.warn("skipping cross-compilation doctest(s), use --verbose to see the full list in detail".to_string())
})?;
// Skip doctests, -Zdoctest-xcompile not enabled.
gctx.shell().verbose(|shell| {
shell.note(format!(
Expand Down
Loading