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.

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

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

Merged
merged 2 commits into from
Dec 12, 2024

Conversation

jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Dec 12, 2024

Unfortunately, #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 #134040:

  1. Revert bootstrap: Forward cargo JSON output to stdout, not stderr #134123 which is a fix-forward after bootstrap: print{ln}! -> eprint{ln}! (take 2) #134040.
  2. Revert bootstrap: print{ln}! -> eprint{ln}! (take 2) #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

…eyouxu,clubby789"

This reverts commit c42c248, reversing
changes made to 0f1b827.
…ln, r=jieyouxu"

This reverts commit b282774, reversing
changes made to e0f3db0.
@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 12, 2024
@rustbot
Copy link
Collaborator

rustbot commented Dec 12, 2024

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

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

@Zalathar
Copy link
Contributor

I'm sad about it, but I think this is the right call.

When I fixed the cargo JSON issue, I was hopeful that it would be the last problem. But if new subtleties keep popping up, we have to respect the fact that making this kind of sweeping change to bootstrap and friends is not viable in their current state.

@jieyouxu
Copy link
Member Author

jieyouxu commented Dec 12, 2024

Part of the problem is that bootstrap and friends don't properly lock their stderr and stdout streams, and use println/eprintln naively. I would love to see some cleanup/improvement attempts to make the output logic more robust (properly locking output streams and properly coordinate the interleaving between stdout/stderr). But I think just changing println -> eprintln is proving to be non-viable.

@jieyouxu
Copy link
Member Author

@bors r=@lqd rollup p=6
(contributor experience)

@bors
Copy link
Contributor

bors commented Dec 12, 2024

📌 Commit 3c3512c has been approved by lqd

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 12, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 12, 2024
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`
@bors
Copy link
Contributor

bors commented Dec 12, 2024

⌛ Testing commit 3c3512c with merge 279c65f...

@rust-log-analyzer
Copy link
Collaborator

The job dist-x86_64-apple failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
   Compiling tracing-tree v0.3.1
[RUSTC-TIMING] tracing_tree test:false 2.517
   Compiling rustc_codegen_llvm v0.0.0 (/Users/runner/work/rust/rust/compiler/rustc_codegen_llvm)
[RUSTC-TIMING] rustc_codegen_llvm test:false 62.093
rustc exited with signal: 11 (SIGSEGV)

Caused by:
Caused by:
  process didn't exit successfully: `/Users/runner/work/rust/rust/build/bootstrap/debug/rustc /Users/runner/work/rust/rust/build/bootstrap/debug/rustc --crate-name rustc_codegen_llvm --edition=2021 compiler/rustc_codegen_llvm/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata,link -C opt-level=3 -C embed-bitcode=no -C codegen-units=1 --check-cfg 'cfg(docsrs)' --check-cfg 'cfg(feature, values())' -C metadata=31812fcdce1ed1a6 -C extra-filename=-31812fcdce1ed1a6 --out-dir /Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps --target x86_64-apple-darwin -L dependency=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps -L dependency=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/release/deps --extern bitflags=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/libbitflags-a1ecb01bec05c607.rmeta --extern itertools=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/libitertools-fc8482292e402829.rmeta --extern libc=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/liblibc-71418dd2bae71546.rmeta --extern measureme=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/libmeasureme-65f26e237d9cbc69.rmeta --extern object=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/libobject-b9fdd0c4eec82855.rmeta --extern rustc_demangle=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_demangle-59d4195c10f8ce59.rmeta --extern rustc_abi=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_abi-8c308e15a91667ae.rmeta --extern rustc_ast=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_ast-6307ddaffbb78c2c.rmeta --extern rustc_attr=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_attr-a64f29895039e85d.rmeta --extern rustc_codegen_ssa=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_codegen_ssa-471fc4cdea12e5aa.rmeta --extern rustc_data_structures=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_data_structures-ad6787aabac41fc7.rmeta --extern rustc_errors=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_errors-ae10afa2bd2798f4.rmeta --extern rustc_fluent_macro=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/release/deps/librustc_fluent_macro-e27f644eab9503e4.dylib --extern rustc_fs_util=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_fs_util-1af14c49c485068e.rmeta --extern rustc_hir=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_hir-ddd2bd35821eba08.rmeta --extern rustc_index=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_index-ba2f5cf42e80b6b3.rmeta --extern rustc_llvm=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_llvm-a806c45190fb0592.rmeta --extern rustc_macros=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/release/deps/librustc_macros-21f9064df7350bce.dylib --extern rustc_metadata=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_metadata-8c77a936e16e5034.rmeta --extern rustc_middle=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_middle-8709a3bcf14547b0.rmeta --extern rustc_query_system=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_query_system-531ae3841194b38a.rmeta --extern rustc_sanitizers=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_sanitizers-21a20f1a3f01e840.rmeta --extern rustc_session=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_session-63c14fae59e29aee.rmeta --extern rustc_span=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_span-2d8da5b611828dda.rmeta --extern rustc_symbol_mangling=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_symbol_mangling-595e7538c1ed6229.rmeta --extern rustc_target=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/librustc_target-bcb3e09908ca7102.rmeta --extern serde=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/libserde-632f5b42d8037b7d.rmeta --extern serde_json=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/libserde_json-b239b225d337f1f5.rmeta --extern smallvec=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/libsmallvec-e8983336671b9a36.rmeta --extern tracing=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/deps/libtracing-6a46015c9b81a7be.rmeta --cfg=windows_raw_dylib -Csymbol-mangling-version=v0 -Zunstable-options '--check-cfg=cfg(bootstrap)' '--check-cfg=cfg(llvm_enzyme)' -Zmacro-backtrace -Csplit-debuginfo=unpacked '-Wrustc::internal' '-Drustc::symbol_intern_string_literal' -Wkeyword_idents_2024 -Wunsafe_op_in_unsafe_fn -Zosx-rpath-install-name '-Clink-args=-Wl,-rpath,@loader_path/../lib' -Zon-broken-pipe=kill -Zdylib-lto -Clto=thin -Cembed-bitcode=yes -Z binary-dep-depinfo -L native=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/build/psm-c80cca6b00966791/out -L native=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/build/blake3-dcfcf6f77ce37e35/out -L native=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/build/blake3-dcfcf6f77ce37e35/out -L native=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/build/rustc_llvm-d5c0fad7751ae19c/out -L native=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/llvm/lib` (exit status: 254)
[RUSTC-TIMING] rustc_hir_analysis test:false 291.587
[RUSTC-TIMING] rustc_mir_transform test:false 255.667
[RUSTC-TIMING] rustc_borrowck test:false 337.436
Build completed unsuccessfully in 1:11:49

@bors
Copy link
Contributor

bors commented Dec 12, 2024

💔 Test failed - checks-actions

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

Guessing this is spurious as its a revert
@bors retry

@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 12, 2024
@jieyouxu
Copy link
Member Author

SIGSEGV in building cg_llvm is not what I had in mind.

@jieyouxu jieyouxu added the CI-spurious-x86_64-apple-SIGSEGV-SIGILL CI spurious failure: SIGSEGV/SIGILL while building rustc itself on x86_64-apple label Dec 12, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 12, 2024
…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
@bors bors merged commit 8d2759b into rust-lang:master Dec 12, 2024
6 of 7 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 12, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 12, 2024
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`
@bors
Copy link
Contributor

bors commented Dec 12, 2024

⌛ Testing commit 3c3512c with merge d4025ee...

@jieyouxu jieyouxu deleted the revert-134040 branch December 13, 2024 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc CI-spurious-x86_64-apple-SIGSEGV-SIGILL CI spurious failure: SIGSEGV/SIGILL while building rustc itself on x86_64-apple 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
Development

Successfully merging this pull request may close these issues.

7 participants