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

Upgrade to tracing-subscriber 0.2.13 #77678

Merged
merged 1 commit into from
Oct 8, 2020
Merged

Upgrade to tracing-subscriber 0.2.13 #77678

merged 1 commit into from
Oct 8, 2020

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Oct 7, 2020

The primary motivation is to get the changes from
tokio-rs/tracing#990. Example output:

$ RUSTDOC_LOG=debug rustdoc +rustc2
warning: some trace filter directives would enable traces that are disabled statically
 | `debug` would enable the DEBUG level for all targets
 = note: the static max level is `info`
 = help: to enable DEBUG logging, remove the `max_level_info` feature

r? @Mark-Simulacrum
cc @hawkw ❤️

@jyn514 jyn514 added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Oct 7, 2020
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 7, 2020
@jyn514 jyn514 changed the title Upgrade to tracing 0.2.13 Upgrade to tracing-subscriber 0.2.13 Oct 7, 2020
@jyn514
Copy link
Member Author

jyn514 commented Oct 7, 2020

This found a bug in the test suite!


---- [ui] ui/issues/issue-18075.rs stdout ----
normalized stderr:
warning: some trace filter directives would enable traces that are disabled statically
 | `rustc::middle=debug` would enable the DEBUG level for the `rustc::middle` target
 = note: the static max level is `info`
 = help: to enable DEBUG logging, remove the `max_level_info` feature

@Mark-Simulacrum
Copy link
Member

Looking at that test, the proposed change makes it useless, and it could just be deleted. I think we should do that.

(#18075 is the issue it was tracking, and that issue looks unlikely to reoccur anyway).

@jyn514
Copy link
Member Author

jyn514 commented Oct 7, 2020

I could change it to rustc_middle=info instead if that helps - that would have effectively the previous behavior but without the warning from tracing.

@Mark-Simulacrum
Copy link
Member

I don't think that helps personally, the test is just likely no longer useful. We can prune it.

The primary motivation is to get the changes from
tokio-rs/tracing#990. Example output:

```
$ RUSTDOC_LOG=debug rustdoc +rustc2
warning: some trace filter directives would enable traces that are disabled statically
 | `debug` would enable the DEBUG level for all targets
 = note: the static max level is `info`
 = help: to enable DEBUG logging, remove the `max_level_info` feature
```

- Remove useless test

  This was testing for an ICE when passing `RUST_LOG=rustc_middle`.  I
  noticed it because it started giving the tracing warning (because tests
  are not run with debug-logging enabled). Since this bug seems unlikely
  to re-occur, I just removed it altogether.
@jyn514
Copy link
Member Author

jyn514 commented Oct 8, 2020

Ok, I removed the unhelpful test.

@Mark-Simulacrum
Copy link
Member

I think this is good -- @bors r+

That said, it would be great if searching for max_level_info or "some trace filter directives would enable traces that are disabled statically" in CONTRIBUTING.md or rustc-dev-guide would immediately yield results on what to do -- right now that message is likely helpful in the sense that you know you have a problem, but you still don't know what to do to fix it :)

Maybe we could include those two things in the config.toml.example by the debug-logging flag, too, which would make ripgrep in the repository for them yield something useful too.

@bors
Copy link
Contributor

bors commented Oct 8, 2020

📌 Commit 8b22d07 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 Oct 8, 2020
@bors
Copy link
Contributor

bors commented Oct 8, 2020

⌛ Testing commit 8b22d07 with merge ccea570...

@bors
Copy link
Contributor

bors commented Oct 8, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: Mark-Simulacrum
Pushing ccea570 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 8, 2020
@bors bors merged commit ccea570 into rust-lang:master Oct 8, 2020
@rustbot rustbot added this to the 1.49.0 milestone Oct 8, 2020
@jyn514 jyn514 deleted the tracing branch October 8, 2020 15:54
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 8, 2020
…crum

Make `max_log_info` easily greppable (for figuring out why debug logging is disabled)

Follow-up to rust-lang#77678 (comment). I'll make a PR to the dev-guide shortly changing `debug = true` to `debug-logging = true` and using this text.

Ideally wouldn't be merged before rust-lang#77678, but it practice it won't hurt anything.

r? @Mark-Simulacrum
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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.

5 participants