Skip to content

avoid duplicating the RUSTC_LOG env var name #107839

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

Merged
merged 2 commits into from
Feb 10, 2023
Merged

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Feb 9, 2023

We also have the env var name here:

/// This allows tools to enable rust logging without having to magically match rustc's
/// tracing crate version.
pub fn init_rustc_env_logger() {
init_env_logger("RUSTC_LOG");
}

Redundantly having this name twice doesn't seem great. Looks like rustc_log::init_rustc_env_logger is dead code anyway.

r? @oli-obk

@rustbot rustbot added 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 Feb 9, 2023
@RalfJung
Copy link
Member Author

RalfJung commented Feb 9, 2023

Hm, this breaks a doctest

//! ```toml
//! [dependencies]
//! rustc_ast = { path = "../rust/compiler/rustc_ast" }
//! rustc_log = { path = "../rust/compiler/rustc_log" }
//! rustc_span = { path = "../rust/compiler/rustc_span" }
//! ```
//!
//! ```
//! fn main() {
//! rustc_log::init_rustc_env_logger().unwrap();
//!
//! let edition = rustc_span::edition::Edition::Edition2021;
//! rustc_span::create_session_globals_then(edition, || {
//! /* ... */
//! });
//! }
//! ```

But real tools call rustc_driver::init_rustc_env_logger() anyway, don't they?

@RalfJung
Copy link
Member Author

RalfJung commented Feb 9, 2023

Cc @dtolnay who added this doctest about a year ago.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 9, 2023

yea, we should just change that doctest.

@RalfJung
Copy link
Member Author

RalfJung commented Feb 9, 2023

yea, we should just change that doctest.

To do what? Not importing rustc_driver is the entire point of that test.

We could also just have a constant in rustc_log with the env var name, and avoid the duplication that way.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 9, 2023

To do what?

just calling init_env_logger("YOUR_TOOL_LOG"). This is just a test, it doesn't have to use or duplicate RUSTC_LOG

@rust-log-analyzer

This comment has been minimized.

@RalfJung
Copy link
Member Author

All right, done.

(The use of set_var is concerning given the plans to make that function unsafe. IMO we should have a function that you give the value of the env var, i.e. the log filter description. Then we could also remove some set_var from Miri. But that's a separate discussion.)

@oli-obk
Copy link
Contributor

oli-obk commented Feb 10, 2023

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Feb 10, 2023

📌 Commit 623ed8e 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 Feb 10, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 10, 2023
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#107789 (Avoid exposing type parameters and implementation details sourced from macro expansions)
 - rust-lang#107836 (Handle properly when there is no crate attrs)
 - rust-lang#107839 (avoid duplicating the RUSTC_LOG env var name)
 - rust-lang#107866 (Allow wasi-libc to initialize its environment variables lazily.)
 - rust-lang#107876 (create symlink only for non-windows operating systems)
 - rust-lang#107882 (Cleanup typos in en_US/borrowck.ftl)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d8e4d99 into rust-lang:master Feb 10, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 10, 2023
@RalfJung RalfJung deleted the rustc-log branch February 11, 2023 21:31
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-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.

5 participants