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

bootstrap: print{ln}! -> eprint{ln}! (take 2) #134040

Merged
merged 2 commits into from
Dec 10, 2024
Merged

bootstrap: print{ln}! -> eprint{ln}! (take 2) #134040

merged 2 commits into from
Dec 10, 2024

Conversation

clubby789
Copy link
Contributor

r? @jieyouxu

Reland of #133817 with the print!s changed as well.

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Dec 8, 2024
@rustbot
Copy link
Collaborator

rustbot commented Dec 8, 2024

Some changes occurred in src/tools/compiletest

cc @jieyouxu

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

@bors

This comment was marked as resolved.

Sign in to view
@jieyouxu
Copy link
Member

jieyouxu commented Dec 9, 2024

Rebased to fix merge conflict, no functional changes.

@clubby789 @jieyouxu
compiletest: print{,ln}! -> eprint{,ln}!
a6c4628 
Co-authored-by: Jieyou Xu <jieyouxu@outlook.com>
@jieyouxu
Copy link
Member

jieyouxu commented Dec 9, 2024

Fixed a missing conversion in compiletest

                     if self.config.mode == crate::common::Mode::Ui {
-                        println!("note: by default, ui tests are expected not to compile");
+                        eprintln!("note: by default, ui tests are expected not to compile");
                     }

jieyouxu
jieyouxu approved these changes Dec 9, 2024
View reviewed changes
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks! I did some local testing

  • ./x clean: OK
  • Basic compiler flow: ./x build library: OK
  • Library test flow: ./x test library/alloc: OK
  • Check compiler flow: ./x check compiler: OK
  • Stage 1 ui test flow (no changes): OK
  • Stage 1 ui test failure flow: OK
    • stderr diff changed: OK
    • rebless: OK
  • Stage 1 run-make: OK
  • ./x run src/tools/unicode-table-generator/: OK

AFAICT this looks fine to me.

@jieyouxu
Copy link
Member

jieyouxu commented Dec 9, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 9, 2024

📌 Commit a6c4628 has been approved by jieyouxu

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 Dec 9, 2024
@fmease fmease mentioned this pull request Dec 9, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 10, 2024
@bors
Auto merge of rust-lang#134096 - fmease:rollup-0asgoo8, r=fmease
ff7906b 
Rollup of 9 pull requests

Successful merges:

 - rust-lang#133996 (Move most tests for `-l` and `#[link(..)]` into `tests/ui/link-native-libs`)
 - rust-lang#134012 (Grammar fixes)
 - rust-lang#134032 (docs: better examples for `std::ops::ControlFlow`)
 - rust-lang#134040 (bootstrap: print{ln}! -> eprint{ln}! (take 2))
 - rust-lang#134043 (Add test to check unicode identifier version)
 - rust-lang#134053 (rustdoc: rename `issue-\d+.rs` tests to have meaningful names (part 10))
 - rust-lang#134055 (interpret: clean up deduplicating allocation functions)
 - rust-lang#134073 (dataflow_const_prop: do not eval a ptr address in SwitchInt)
 - rust-lang#134084 (Fix typo in RFC mention 3598 -> 3593)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b282774 into rust-lang:master Dec 10, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 10, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 10, 2024
@rust-timer
Unrolled build for rust-lang#134040
759c6e9 
Rollup merge of rust-lang#134040 - clubby789:bootstrap-eprintln, r=jieyouxu

bootstrap: print{ln}! -> eprint{ln}! (take 2)

r? `@jieyouxu`

Reland of rust-lang#133817 with the `print!`s changed as well.
@Zalathar Zalathar mentioned this pull request Dec 10, 2024
Merged
fmease added a commit to fmease/rust that referenced this pull request Dec 10, 2024
@fmease
Rollup merge of rust-lang#134123 - Zalathar:json-output, r=jieyouxu,c…
c42c248 
…lubby789

bootstrap: Forward cargo JSON output to stdout, not stderr

This fixes the RA errors I've been seeing on proc-macros after the re-landing of rust-lang#134040.

r? clubby789
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 10, 2024
@rust-timer
Unrolled build for rust-lang#134123
85717cd 
Rollup merge of rust-lang#134123 - Zalathar:json-output, r=jieyouxu,clubby789

bootstrap: Forward cargo JSON output to stdout, not stderr

This fixes the RA errors I've been seeing on proc-macros after the re-landing of rust-lang#134040.

r? clubby789
@Zalathar
Copy link
Contributor

This ended up causing problems with rust-analyzer that were (hopefully) fixed by #134123.

This was referenced Dec 12, 2024
Merged
Closed
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Dec 12, 2024
@jieyouxu
Revert "Rollup merge of rust-lang#134040 - clubby789:bootstrap-eprint…
3c3512c 
…ln, r=jieyouxu"

This reverts commit b282774, reversing
changes made to e0f3db0.
@jieyouxu jieyouxu mentioned this pull request Dec 12, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 12, 2024
@bors
Auto merge of rust-lang#134207 - jieyouxu:revert-134040, r=lqd
279c65f 
Revert "bootstrap: print{ln}! -> eprint{ln}! (take 2) rust-lang#134040"

Unfortunately, rust-lang#134040 is proving to have caused more output interleaving problems that are tricky to diagnose and fix, and I think we probably should leave these untouched as bootstrap and compiletest has a bunch of interconnecting parts, and the commands and tools that they exercise do not consistently use stderr/stdout either. This causes hard-to-diagnose output interleaving bugs, which unfortunately degrades contributor experience.

This PR reverts two PRs in order to cleanly revert rust-lang#134040:

1. Revert rust-lang#134123 which is a fix-forward after rust-lang#134040.
2. Revert rust-lang#134040 itself.

I don't regret the initial effort `@clubby789,` and thank you for making the attempts, but I think we need to refrain from touching too many of these at once because some of the interleaving are very non-obvious and we don't have test coverage for.

r? `@clubby789`
cc `@Zalathar`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 12, 2024
@matthiaskrgr
Rollup merge of rust-lang#134207 - jieyouxu:revert-134040, r=lqd
8d2759b 
Revert "bootstrap: print{ln}! -> eprint{ln}! (take 2) rust-lang#134040"

Unfortunately, rust-lang#134040 is proving to have caused more output interleaving problems that are tricky to diagnose and fix, and I think we probably should leave these untouched as bootstrap and compiletest has a bunch of interconnecting parts, and the commands and tools that they exercise do not consistently use stderr/stdout either. This causes hard-to-diagnose output interleaving bugs, which unfortunately degrades contributor experience.

This PR reverts two PRs in order to cleanly revert rust-lang#134040:

1. Revert rust-lang#134123 which is a fix-forward after rust-lang#134040.
2. Revert rust-lang#134040 itself.

I don't regret the initial effort `@clubby789,` and thank you for making the attempts, but I think we need to refrain from touching too many of these at once because some of the interleaving are very non-obvious and we don't have test coverage for.

r? `@clubby789`
cc `@Zalathar`
@matthiaskrgr matthiaskrgr mentioned this pull request Dec 12, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 12, 2024
@bors
Auto merge of rust-lang#134223 - matthiaskrgr:rollup-qy69vqb, r=matth…
d4025ee 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#133122 (Add unpolished, experimental support for AFIDT (async fn in dyn trait))
 - rust-lang#133249 (ABI checks: add support for loongarch)
 - rust-lang#134089 (Use newly added exceptions to non default branch warning)
 - rust-lang#134188 (Bump Fuchsia)
 - rust-lang#134204 (Fix our `llvm::Bool` typedef to be signed, to match `LLVMBool`)
 - rust-lang#134207 (Revert "bootstrap: print{ln}! -> eprint{ln}! (take 2) rust-lang#134040")
 - rust-lang#134214 (rustdoc: fix self cmp)

r? `@ghost`
`@rustbot` modify labels: rollup
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 12, 2024
@rust-timer
Unrolled build for rust-lang#134207
bbf741a 
Rollup merge of rust-lang#134207 - jieyouxu:revert-134040, r=lqd

Revert "bootstrap: print{ln}! -> eprint{ln}! (take 2) rust-lang#134040"

Unfortunately, rust-lang#134040 is proving to have caused more output interleaving problems that are tricky to diagnose and fix, and I think we probably should leave these untouched as bootstrap and compiletest has a bunch of interconnecting parts, and the commands and tools that they exercise do not consistently use stderr/stdout either. This causes hard-to-diagnose output interleaving bugs, which unfortunately degrades contributor experience.

This PR reverts two PRs in order to cleanly revert rust-lang#134040:

1. Revert rust-lang#134123 which is a fix-forward after rust-lang#134040.
2. Revert rust-lang#134040 itself.

I don't regret the initial effort `@clubby789,` and thank you for making the attempts, but I think we need to refrain from touching too many of these at once because some of the interleaving are very non-obvious and we don't have test coverage for.

r? `@clubby789`
cc `@Zalathar`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jieyouxu jieyouxu jieyouxu approved these changes

Assignees

@jieyouxu jieyouxu

Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
1.85.0
Development

Successfully merging this pull request may close these issues.
5 participants
@clubby789 @rustbot @bors @jieyouxu @Zalathar