Skip to content

rustbuild: drop color handling #54323

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Sep 22, 2018
Merged

rustbuild: drop color handling #54323

merged 1 commit into from
Sep 22, 2018

Conversation

Keruspe
Copy link
Contributor

@Keruspe Keruspe commented Sep 18, 2018

Let cargo handle that for us

Fixes #54322

Needs a beta backport

Let cargo handle that for us

Signed-off-by: Marc-Antoine Perennou <Marc-Antoine@Perennou.com>
@rust-highfive
Copy link
Contributor

r? @aturon

(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 Sep 18, 2018
@eddyb
Copy link
Member

eddyb commented Sep 18, 2018

r? @alexcrichton cc @rust-lang/infra

@rust-highfive rust-highfive assigned alexcrichton and unassigned aturon Sep 18, 2018
@pietroalbini pietroalbini added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Sep 18, 2018
@Mark-Simulacrum
Copy link
Member

Hm, was it confirmed that this actually fixes things? This was originally done due to --message-format=json stealing rustc stdout/err and causing noncolorful output in 305f526.

@kennytm
Copy link
Member

kennytm commented Sep 18, 2018

probably we don't really want the ANSI color for rustc in the CI since we all read the raw log anyway 😛

@Keruspe
Copy link
Contributor Author

Keruspe commented Sep 18, 2018

It fixes bootstrapping using cargo from the latest beta here

@alexcrichton
Copy link
Member

@Keruspe can you check to make sure that ./x.py build still prints colors from both Cargo and the compiler? (when error messages happen)

@Mark-Simulacrum
Copy link
Member

Based on the issue it looks like the logic may have been actually upstreamed into cargo, could you track down/cite the relevant Cargo PR? I think that would help make sure the logic is in fact equivalent.

@Keruspe
Copy link
Contributor Author

Keruspe commented Sep 19, 2018

@alexcrichton seems to work fine here, I get colors from both cargo and rustc (still testing with cargo from the beta branch).

@Mark-Simulacrum seems to be rust-lang/cargo@f1c783b

@pnkfelix pnkfelix added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label Sep 20, 2018
@Mark-Simulacrum
Copy link
Member

@Keruspe Can you confirm that this works with beta Cargo?

Based on the Cargo commits I believe this should be fine so @bors r+

@bors
Copy link
Collaborator

bors commented Sep 20, 2018

📌 Commit 2a45057 has been approved by Mark-Simulacrum

@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 Sep 20, 2018
@Keruspe
Copy link
Contributor Author

Keruspe commented Sep 20, 2018 via email

@Mark-Simulacrum Mark-Simulacrum added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Sep 20, 2018
@pietroalbini
Copy link
Member

@bors p=1 (needs to be backported)

@alexcrichton
Copy link
Member

@bors: p=0

we've got a lot of backports in the queue right now and this one's pretty minor, so I think it's safe if we hold off on this to land naturally

kennytm added a commit to kennytm/rust that referenced this pull request Sep 21, 2018
rustbuild: drop color handling

Let cargo handle that for us

Fixes rust-lang#54322

Needs a beta backport
bors added a commit that referenced this pull request Sep 21, 2018
Rollup of 10 pull requests

Successful merges:

 - #53652 (define copy_within on slices)
 - #54261 (Make `dyn` a keyword in the 2018 edition)
 - #54317 (Implement the dbg!(..) macro)
 - #54323 (rustbuild: drop color handling)
 - #54371 (add -Zui-testing to rustdoc)
 - #54374 (Make 'proc_macro::MultiSpan' public.)
 - #54402 (Use no_default_libraries for all NetBSD flavors)
 - #54412 (add applicability to span_suggestion call)
 - #54413 (Add UI test for deref recursion limit printing twice)
 - #54422 (Simplify slice's first(_mut) and last(_mut) with get)
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Sep 22, 2018
rustbuild: drop color handling

Let cargo handle that for us

Fixes rust-lang#54322

Needs a beta backport
bors added a commit that referenced this pull request Sep 22, 2018
Rollup of 16 pull requests

Successful merges:

 - #53652 (define copy_within on slices)
 - #54261 (Make `dyn` a keyword in the 2018 edition)
 - #54280 (remove (more) CAS API from Atomic* types where not natively supported)
 - #54323 (rustbuild: drop color handling)
 - #54350 (Support specifying edition in doc test)
 - #54370 (Improve handling of type bounds in `bit_set.rs`.)
 - #54371 (add -Zui-testing to rustdoc)
 - #54374 (Make 'proc_macro::MultiSpan' public.)
 - #54402 (Use no_default_libraries for all NetBSD flavors)
 - #54409 (Detect `for _ in in bar {}` typo)
 - #54412 (add applicability to span_suggestion call)
 - #54413 (Add UI test for deref recursion limit printing twice)
 - #54415 (parser: Tweak function parameter parsing to avoid rollback on succesfull path)
 - #54420 (Compress `Liveness` data some more.)
 - #54422 (Simplify slice's first(_mut) and last(_mut) with get)
 - #54446 (Unify christianpoveda's emails)

Failed merges:

 - #54058 (Introduce the partition_dedup/by/by_key methods for slices)

r? @ghost
bors added a commit that referenced this pull request Sep 22, 2018
Rollup of 16 pull requests

Successful merges:

 - #53652 (define copy_within on slices)
 - #54261 (Make `dyn` a keyword in the 2018 edition)
 - #54280 (remove (more) CAS API from Atomic* types where not natively supported)
 - #54323 (rustbuild: drop color handling)
 - #54350 (Support specifying edition in doc test)
 - #54370 (Improve handling of type bounds in `bit_set.rs`.)
 - #54371 (add -Zui-testing to rustdoc)
 - #54374 (Make 'proc_macro::MultiSpan' public.)
 - #54402 (Use no_default_libraries for all NetBSD flavors)
 - #54409 (Detect `for _ in in bar {}` typo)
 - #54412 (add applicability to span_suggestion call)
 - #54413 (Add UI test for deref recursion limit printing twice)
 - #54415 (parser: Tweak function parameter parsing to avoid rollback on succesfull path)
 - #54420 (Compress `Liveness` data some more.)
 - #54422 (Simplify slice's first(_mut) and last(_mut) with get)
 - #54446 (Unify christianpoveda's emails)

Failed merges:

 - #54058 (Introduce the partition_dedup/by/by_key methods for slices)

r? @ghost
@bors bors merged commit 2a45057 into rust-lang:master Sep 22, 2018
@pietroalbini pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Sep 22, 2018
bors added a commit that referenced this pull request Sep 22, 2018
[beta] Rollup backports

Merged and approved:

* #54323: rustbuild: drop color handling
* #54265: avoid leaking host details in proc macro metadata decoding

r? @ghost
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants