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

Add log-backtrace option to show backtraces along with logging #104645

Merged
merged 1 commit into from
Jan 14, 2023

Conversation

yukiomoto
Copy link
Contributor

according to #90698, I added a compiler option, -Zlog-backtrace=filter, where filter is a module name, to show backtraces for logging without rebuilding.

resolve #90698

@rustbot
Copy link
Collaborator

rustbot commented Nov 20, 2022

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @oli-obk (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 20, 2022
@yukiomoto yukiomoto force-pushed the log-backtrace-option branch from 68323b1 to 64dd4b3 Compare November 20, 2022 17:25
@rust-log-analyzer

This comment has been minimized.

@yukiomoto yukiomoto force-pushed the log-backtrace-option branch from 64dd4b3 to 0fc4fb6 Compare November 21, 2022 03:48
@oli-obk
Copy link
Contributor

oli-obk commented Nov 21, 2022

hmm... seems like there were two initiatives for this that didn't know about each other. There's already #103993 by @yaahc.

@bors
Copy link
Contributor

bors commented Dec 7, 2022

☔ The latest upstream changes (presumably #105271) made this pull request unmergeable. Please resolve the merge conflicts.

@apiraino
Copy link
Contributor

apiraino commented Jan 5, 2023

hello, can this PR be closed (or somehow merged) into #103993?

I'll let the author decide! thanks.

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 5, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Jan 11, 2023

I apologize, I misunderstood what was going on here, this is a very different PR from the other one. This lgtm. Just needs a rebase, then we can land it.

@rustbot author

@yukiomoto
Copy link
Contributor Author

@oli-obk @apiraino
I'm sorry for the very late reply and thank you for the comments !
I'll rebase it soon.

@yukiomoto yukiomoto force-pushed the log-backtrace-option branch from 0fc4fb6 to 5135eac Compare January 11, 2023 15:23
Comment on lines 234 to 237
init_rustc_env_logger_with_backtrace_option(&config.opts.unstable_opts.log_backtrace);

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we'll stop getting logs before parsing command line flags, but that seems ok to me. The only alternative I see would be to use an env var instead of a command line flag, but I'm not sure if that matters much, so let's land it, we can always adjust it later if someone has a stronger opinion about it

@oli-obk
Copy link
Contributor

oli-obk commented Jan 11, 2023

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jan 11, 2023

📌 Commit 5135eac has been approved by oli-obk

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: This is awaiting some action (such as code changes or more information) from the author. labels Jan 11, 2023
Noratrieb added a commit to Noratrieb/rust that referenced this pull request Jan 11, 2023
…oli-obk

Add log-backtrace option to show backtraces along with logging

according to rust-lang#90698, I added a compiler option, `-Zlog-backtrace=filter`, where `filter` is a module name, to show backtraces for logging without rebuilding.

resolve rust-lang#90698
@Noratrieb
Copy link
Member

I think this PR broke PR CI in the rollup #106725 (https://github.com/rust-lang/rust/actions/runs/3894414159/jobs/6648375548#step:26:80428), even though it passed CI here

Can you rebase and see whether it breaks here as well after it?
@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 11, 2023
@yukiomoto
Copy link
Contributor Author

hmm, I think I already rebased this branch onto b22c152 (current master).

@Noratrieb
Copy link
Member

Noratrieb commented Jan 11, 2023

Weird, creating a new rollup (#106730) with the same PRs except this one made it pass (and the original panic was in rustc_log), so it's highly likely that something doesn't like this PR. Maybe there's some weird conflict between the PRs in there and this one, I suggest you try rebasing again when the rollup is merged.

@yukiomoto
Copy link
Contributor Author

It looks like the PR CI passed here because it got skipped.
Okay, I'll rebase after the rollup is merged. Thank you.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 12, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Jan 12, 2023

Ah this seems to break tools. You can run clippy and miri tests directly via x.py test clippy and x.py test miri. Ideally a fix would happen just inside the rustc crates, so that tools don't have to change.

@yukiomoto
Copy link
Contributor Author

Aww I'm sorry this happens over and over!
Thank you, I'll try to run clippy and miri tests directly and fix the problem.

@yukiomoto yukiomoto force-pushed the log-backtrace-option branch from 6c986b7 to 4e2a356 Compare January 12, 2023 19:26
@yukiomoto
Copy link
Contributor Author

The problem was that the logger initialization gets called more than once in some tests. I fixed it so the logger initialization process wouldn't be called more than once.

I tested with clappy and miri.

@yukiomoto
Copy link
Contributor Author

@rustbot label -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 12, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Jan 13, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Jan 13, 2023

📌 Commit 4e2a356 has been approved by oli-obk

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 Jan 13, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 13, 2023
…oli-obk

Add log-backtrace option to show backtraces along with logging

according to rust-lang#90698, I added a compiler option, `-Zlog-backtrace=filter`, where `filter` is a module name, to show backtraces for logging without rebuilding.

resolve rust-lang#90698
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 13, 2023
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#104645 (Add log-backtrace option to show backtraces along with logging)
 - rust-lang#106465 (Bump `IMPLIED_BOUNDS_ENTAILMENT` to Deny + ReportNow)
 - rust-lang#106489 (Fix linker detection for linker (drivers) with a version postfix (e.g. clang-12 instead of clang))
 - rust-lang#106585 (When suggesting writing a fully qualified path probe for appropriate types)
 - rust-lang#106641 (Provide help on closures capturing self causing borrow checker errors)
 - rust-lang#106678 (Warn when using panic-strategy abort for proc-macro crates)
 - rust-lang#106701 (Fix `mpsc::SyncSender` spinning behavior)
 - rust-lang#106793 (Normalize test output more thoroughly)
 - rust-lang#106797 (riscv: Fix ELF header flags)
 - rust-lang#106813 (Remove redundant session field)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 96bb02f into rust-lang:master Jan 14, 2023
@rustbot rustbot added this to the 1.68.0 milestone Jan 14, 2023
@@ -231,6 +231,10 @@ fn run_compiler(
registry: diagnostics_registry(),
};

if !tracing::dispatcher::has_been_set() {
init_rustc_env_logger_with_backtrace_option(&config.opts.unstable_opts.log_backtrace);
}
Copy link
Member

Choose a reason for hiding this comment

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

This change is somewhat unfortunate for Miri, which was relying on run_compiler not initializing the logger... see rust-lang/miri#2778. (That was a hack, but with the fact that logger filters cannot be changed at runtime, it's hard to avoid such hacks.)

compiler-errors added a commit to compiler-errors/rust that referenced this pull request Feb 9, 2023
Replace a command line flag with an env var to allow tools to initialize the tracing loggers at their own discretion

fixes rust-lang/miri#2778

this was introduced in rust-lang#104645, so this PR reverts the flag-part and uses an env var instead.
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Feb 9, 2023
Replace a command line flag with an env var to allow tools to initialize the tracing loggers at their own discretion

fixes rust-lang/miri#2778

this was introduced in rust-lang#104645, so this PR reverts the flag-part and uses an env var instead.
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Feb 9, 2023
Replace a command line flag with an env var to allow tools to initialize the tracing loggers at their own discretion

fixes rust-lang/miri#2778

this was introduced in rust-lang#104645, so this PR reverts the flag-part and uses an env var instead.
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Feb 9, 2023
Replace a command line flag with an env var to allow tools to initialize the tracing loggers at their own discretion

fixes rust-lang#2778

this was introduced in rust-lang/rust#104645, so this PR reverts the flag-part and uses an env var instead.
celinval added a commit to model-checking/kani that referenced this pull request Mar 8, 2023
Upgrade our toolchain to `nightly-2023-01-23`. The changes here are related to the following changes:
- rust-lang/rust#104986
- rust-lang/rust#105657
- rust-lang/rust#105603
- rust-lang/rust#105613
- rust-lang/rust#105977
- rust-lang/rust#104645
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a way to give backtraces for logging
8 participants