Skip to content

Conversation

weihanglo
Copy link
Member

What does this PR try to resolve?

In #12458 we introduced tracing but forgot to consult if it is in tty to enable color logs.

How should we test and review this PR?

cargo tree &> log and check if log file contains any ansi color code.

Additional information

I'll hold off CARGO_LOG respecting --color=always for now.

#9012 is something can be resolved as a whole. tracing_subscriber::reload is also a thing to consider, so that we can configure logging color and output file after loading config.toml

@rustbot
Copy link
Collaborator

rustbot commented Aug 13, 2023

r? @epage

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

@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 13, 2023
@weihanglo
Copy link
Member Author

IIUC, this is aligned with how env_logger handles ASNI color codes: https://github.com/rust-cli/env_logger/blob/bdc25caee0c27fbb1c9c66d6492a5910aaf3ac1d/src/fmt/writer/atty.rs

@weihanglo
Copy link
Member Author

Currently blocked on rust-lang/rust#114804.

While we can switch from main.rs to an empty lib.rs to bypass that, I do think this is a good test to catch this kind of subtle change.

diff --git a/tests/testsuite/future_incompat_report.rs b/tests/testsuite/future_incompat_report.rs
index 9f451a64c..4d2c66d17 100644
--- a/tests/testsuite/future_incompat_report.rs
+++ b/tests/testsuite/future_incompat_report.rs
@@ -164,7 +164,7 @@ fn test_multi_crate() {
                 second-dep = "*"
               "#,
         )
-        .file("src/main.rs", "fn main() {}")
+        .file("src/lib.rs", "")
         .build();
 
     for command in &["build", "check", "rustc", "test"] {

Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

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

r= me when this is able to go through

@weihanglo
Copy link
Member Author

Thank you for the review!

@bors r+

@bors
Copy link
Contributor

bors commented Aug 14, 2023

📌 Commit c07043b has been approved by weihanglo

It is now in the queue for this repository.

@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 Aug 14, 2023
@weihanglo
Copy link
Member Author

@bors r-

@bors bors added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 14, 2023
@bors
Copy link
Contributor

bors commented Aug 14, 2023

⌛ Testing commit c07043b with merge c3136a8...

bors added a commit that referenced this pull request Aug 14, 2023
fix(log): enable ansi color only in terminal
@weihanglo
Copy link
Member Author

@bors r=epage

@bors
Copy link
Contributor

bors commented Aug 14, 2023

📌 Commit c07043b has been approved by epage

It is now in the queue for this repository.

@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-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. labels Aug 14, 2023
@bors
Copy link
Contributor

bors commented Aug 14, 2023

⌛ Testing commit c07043b with merge f137594...

@bors
Copy link
Contributor

bors commented Aug 14, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing f137594 to master...

@bors bors merged commit f137594 into rust-lang:master Aug 14, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 14, 2023
Update cargo

6 commits in 7e9de3f4ec3708f500bec142317895b96131e47c..7c3904d6c3ed54e8a413023519b55a536ad44d5b
2023-08-13 00:47:32 +0000 to 2023-08-14 20:11:43 +0000
- fix(lints): Doctest extraction should respect `[lints]` (rust-lang/cargo#12501)
- test: relax assertions of panic message (again) (rust-lang/cargo#12500)
- doc(unstable): `cargo test` does not provide `--keep-going` (rust-lang/cargo#12492)
- fix(log): enable ansi color only in terminal (rust-lang/cargo#12488)
- Update cargo-yank.md (rust-lang/cargo#12490)
- test: bypass `rustc --test` impl details for `-Zfuture-incompat-test` (rust-lang/cargo#12491)

r? `@ghost`
@weihanglo weihanglo deleted the ansi-color-log branch August 16, 2023 18:56
@ehuss ehuss added this to the 1.73.0 milestone Aug 22, 2023
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Apr 7, 2024
Update cargo

6 commits in 7e9de3f4ec3708f500bec142317895b96131e47c..7c3904d6c3ed54e8a413023519b55a536ad44d5b
2023-08-13 00:47:32 +0000 to 2023-08-14 20:11:43 +0000
- fix(lints): Doctest extraction should respect `[lints]` (rust-lang/cargo#12501)
- test: relax assertions of panic message (again) (rust-lang/cargo#12500)
- doc(unstable): `cargo test` does not provide `--keep-going` (rust-lang/cargo#12492)
- fix(log): enable ansi color only in terminal (rust-lang/cargo#12488)
- Update cargo-yank.md (rust-lang/cargo#12490)
- test: bypass `rustc --test` impl details for `-Zfuture-incompat-test` (rust-lang/cargo#12491)

r? `@ghost`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
Update cargo

6 commits in 7e9de3f4ec3708f500bec142317895b96131e47c..7c3904d6c3ed54e8a413023519b55a536ad44d5b
2023-08-13 00:47:32 +0000 to 2023-08-14 20:11:43 +0000
- fix(lints): Doctest extraction should respect `[lints]` (rust-lang/cargo#12501)
- test: relax assertions of panic message (again) (rust-lang/cargo#12500)
- doc(unstable): `cargo test` does not provide `--keep-going` (rust-lang/cargo#12492)
- fix(log): enable ansi color only in terminal (rust-lang/cargo#12488)
- Update cargo-yank.md (rust-lang/cargo#12490)
- test: bypass `rustc --test` impl details for `-Zfuture-incompat-test` (rust-lang/cargo#12491)

r? `@ghost`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli Area: Command-line interface, option parsing, etc. A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants