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

Jump to bottom

Print colored warnings when build script panics #8017

Merged
merged 1 commit into from
Mar 18, 2020
Merged

Print colored warnings when build script panics #8017

merged 1 commit into from
Mar 18, 2020

Conversation

aleksator
Copy link
Contributor

Fixes #3672

@rust-highfive
Copy link

r? @Eh2406

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 18, 2020
@Eh2406
Copy link
Contributor

Eh2406 commented Mar 18, 2020

@bors r? @ehuss

@rust-highfive rust-highfive assigned ehuss and unassigned Eh2406 Mar 18, 2020
ehuss
ehuss reviewed Mar 18, 2020
View reviewed changes
src/cargo/core/compiler/custom_build.rs Outdated
Comment on lines 458 to 465
library_paths: Vec::new(),
library_links: Vec::new(),
linker_args: Vec::new(),
cfgs: Vec::new(),
env: Vec::new(),
metadata: Vec::new(),
rerun_if_changed: Vec::new(),
rerun_if_env_changed: Vec::new(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe derive Default on BuildOutput, and then use default() here to avoid specifying all these fields.

src/cargo/core/compiler/custom_build.rs Outdated
metadata_hash,
warnings_in_case_of_panic,
);
return output.map(|_| ());
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using a closure here, perhaps do something like:

if let Err(e) = output {
    …
    return Err(e);
}

Just seems a slightly clearer to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, this was actually the first version I wrote, but changed it to this one to avoid needless wrapping/unwrapping. Will change back

@aleksator aleksator force-pushed the 3672_color_build_script_warnings_on_panic branch from 04d880b to 590c803 Compare March 18, 2020 19:09
@ehuss
Copy link
Contributor

ehuss commented Mar 18, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Mar 18, 2020

📌 Commit 590c803 has been approved by ehuss

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 18, 2020
@bors
Copy link
Contributor

bors commented Mar 18, 2020

⌛ Testing commit 590c803 with merge 03d7154...

@bors
Copy link
Contributor

bors commented Mar 18, 2020

☀️ Test successful - checks-azure
Approved by: ehuss
Pushing 03d7154 to master...

@bors bors merged commit 03d7154 into rust-lang:master Mar 18, 2020
@aleksator aleksator deleted the 3672_color_build_script_warnings_on_panic branch March 19, 2020 09:41
@ehuss ehuss mentioned this pull request Mar 25, 2020
Merged
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 25, 2020
@Dylan-DPC
Rollup merge of rust-lang#70395 - ehuss:update-cargo, r=ehuss
108125a 
Update cargo.

8 commits in 7019b3ed3d539db7429d10a343b69be8c426b576..8a0d4d9c9abc74fd670353094387d62028b40ae9
2020-03-17 21:02:00 +0000 to 2020-03-24 17:57:04 +0000
- Re-implement proc-macro feature decoupling. (rust-lang/cargo#8028)
- Remove unused transitive dependencies: miniz_oxide, adler32 (rust-lang/cargo#8023)
- Fix bug with -Zfeatures=dev_dep and `check --profile=test`. (rust-lang/cargo#8027)
- Remove Config from CompileOptions. (rust-lang/cargo#8021)
- Add `rustless.org` to documented blocklist. (rust-lang/cargo#7922)
- Print colored warnings when build script panics (rust-lang/cargo#8017)
- Do not supply --crate-version flag to rustdoc if present in RUSTDOCFLAGS (rust-lang/cargo#8014)
- Add proc-macro to index, and new feature resolver. (rust-lang/cargo#8003)
Centril added a commit to Centril/rust that referenced this pull request Mar 26, 2020
@Centril
Rollup merge of rust-lang#70395 - ehuss:update-cargo, r=ehuss
16e9d3f 
Update cargo.

8 commits in 7019b3ed3d539db7429d10a343b69be8c426b576..8a0d4d9c9abc74fd670353094387d62028b40ae9
2020-03-17 21:02:00 +0000 to 2020-03-24 17:57:04 +0000
- Re-implement proc-macro feature decoupling. (rust-lang/cargo#8028)
- Remove unused transitive dependencies: miniz_oxide, adler32 (rust-lang/cargo#8023)
- Fix bug with -Zfeatures=dev_dep and `check --profile=test`. (rust-lang/cargo#8027)
- Remove Config from CompileOptions. (rust-lang/cargo#8021)
- Add `rustless.org` to documented blocklist. (rust-lang/cargo#7922)
- Print colored warnings when build script panics (rust-lang/cargo#8017)
- Do not supply --crate-version flag to rustdoc if present in RUSTDOCFLAGS (rust-lang/cargo#8014)
- Add proc-macro to index, and new feature resolver. (rust-lang/cargo#8003)
@ehuss ehuss added this to the 1.44.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ehuss ehuss ehuss left review comments

Assignees

@ehuss ehuss

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.44.0
Development

Successfully merging this pull request may close these issues.

Print & color build script warnings, even when build script panics
5 participants
@aleksator @rust-highfive @Eh2406 @ehuss @bors