Skip to content

Commit

Permalink
internal: remove spurious regex dependency
Browse files Browse the repository at this point in the history
- replace tokio's env-filter with a smaller&simpler targets filter
- reshuffle logging infra a bit to make sure there's only a single place
  where we read environmental variables
- use anyhow::Result in rust-analyzer binary
  • Loading branch information
matklad committed Jun 18, 2023
1 parent fcfc6af commit 424ef77
Show file tree
Hide file tree
Showing 14 changed files with 197 additions and 242 deletions.
37 changes: 0 additions & 37 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion crates/hir-ty/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ limit.workspace = true
expect-test = "1.4.0"
tracing = "0.1.35"
tracing-subscriber = { version = "0.3.16", default-features = false, features = [
"env-filter",
"registry",
] }
tracing-tree = "0.2.1"
Expand Down
5 changes: 3 additions & 2 deletions crates/hir-ty/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use syntax::{
ast::{self, AstNode, HasName},
SyntaxNode,
};
use tracing_subscriber::{layer::SubscriberExt, EnvFilter, Registry};
use tracing_subscriber::{layer::SubscriberExt, Registry};
use tracing_tree::HierarchicalLayer;
use triomphe::Arc;

Expand All @@ -52,7 +52,8 @@ fn setup_tracing() -> Option<tracing::subscriber::DefaultGuard> {
return None;
}

let filter = EnvFilter::from_env("CHALK_DEBUG");
let filter: tracing_subscriber::filter::Targets =
env::var("CHALK_DEBUG").ok().and_then(|it| it.parse().ok()).unwrap_or_default();
let layer = HierarchicalLayer::default()
.with_indent_lines(true)
.with_ansi(false)
Expand Down
1 change: 0 additions & 1 deletion crates/rust-analyzer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ mimalloc = { version = "0.1.30", default-features = false, optional = true }
lsp-server = { version = "0.7.0", path = "../../lib/lsp-server" }
tracing = "0.1.35"
tracing-subscriber = { version = "0.3.16", default-features = false, features = [
"env-filter",
"registry",
"fmt",
"tracing-log",
Expand Down
90 changes: 36 additions & 54 deletions crates/rust-analyzer/src/bin/logger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,26 @@ use std::{
sync::Arc,
};

use rust_analyzer::Result;
use anyhow::Context;
use tracing::{level_filters::LevelFilter, Event, Subscriber};
use tracing_log::NormalizeEvent;
use tracing_subscriber::{
filter::Targets,
fmt::{
format::Writer, writer::BoxMakeWriter, FmtContext, FormatEvent, FormatFields,
FormattedFields, MakeWriter,
},
layer::SubscriberExt,
registry::LookupSpan,
util::SubscriberInitExt,
EnvFilter, Registry,
Registry,
};
use tracing_tree::HierarchicalLayer;

pub(crate) struct Logger {
filter: EnvFilter,
file: Option<File>,
pub(crate) struct LoggerConfig {
pub(crate) log_file: Option<File>,
pub(crate) filter: String,
pub(crate) chalk_filter: Option<String>,
}

struct MakeWriterStderr;
Expand All @@ -38,62 +40,42 @@ impl<'a> MakeWriter<'a> for MakeWriterStderr {
}
}

impl Logger {
pub(crate) fn new(file: Option<File>, filter: Option<&str>) -> Logger {
let filter = filter.map_or(EnvFilter::default(), EnvFilter::new);

Logger { filter, file }
}
impl LoggerConfig {
pub(crate) fn init(self) -> anyhow::Result<()> {
let mut filter: Targets = self
.filter
.parse()
.with_context(|| format!("invalid log filter: `{}`", self.filter))?;

let mut chalk_layer = None;
if let Some(chalk_filter) = self.chalk_filter {
let level: LevelFilter =
chalk_filter.parse().with_context(|| "invalid chalk log filter")?;
chalk_layer = Some(
HierarchicalLayer::default()
.with_indent_lines(true)
.with_ansi(false)
.with_indent_amount(2)
.with_writer(io::stderr),
);
filter = filter
.with_target("chalk_solve", level)
.with_target("chalk_ir", level)
.with_target("chalk_recursive", level);
};

pub(crate) fn install(self) -> Result<()> {
// The meaning of CHALK_DEBUG I suspected is to tell chalk crates
// (i.e. chalk-solve, chalk-ir, chalk-recursive) how to filter tracing
// logs. But now we can only have just one filter, which means we have to
// merge chalk filter to our main filter (from RA_LOG env).
//
// The acceptable syntax of CHALK_DEBUG is `target[span{field=value}]=level`.
// As the value should only affect chalk crates, we'd better manually
// specify the target. And for simplicity, CHALK_DEBUG only accept the value
// that specify level.
let chalk_level_dir = std::env::var("CHALK_DEBUG")
.map(|val| {
val.parse::<LevelFilter>().expect(
"invalid CHALK_DEBUG value, expect right log level (like debug or trace)",
)
})
.ok();

let chalk_layer = HierarchicalLayer::default()
.with_indent_lines(true)
.with_ansi(false)
.with_indent_amount(2)
.with_writer(io::stderr);

let writer = match self.file {
let writer = match self.log_file {
Some(file) => BoxMakeWriter::new(Arc::new(file)),
None => BoxMakeWriter::new(io::stderr),
};
let ra_fmt_layer =
tracing_subscriber::fmt::layer().event_format(LoggerFormatter).with_writer(writer);

match chalk_level_dir {
Some(val) => {
Registry::default()
.with(
self.filter
.add_directive(format!("chalk_solve={val}").parse()?)
.add_directive(format!("chalk_ir={val}").parse()?)
.add_directive(format!("chalk_recursive={val}").parse()?),
)
.with(ra_fmt_layer)
.with(chalk_layer)
.init();
}
None => {
Registry::default().with(self.filter).with(ra_fmt_layer).init();
}
};

let registry = Registry::default().with(filter).with(ra_fmt_layer);
match chalk_layer {
Some(chalk_layer) => registry.with(chalk_layer).init(),
None => registry.init(),
}
Ok(())
}
}
Expand Down
63 changes: 33 additions & 30 deletions crates/rust-analyzer/src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,11 @@
mod logger;
mod rustc_wrapper;

use std::{
env, fs,
path::{Path, PathBuf},
process,
};
use std::{env, fs, path::PathBuf, process};

use anyhow::Context;
use lsp_server::Connection;
use rust_analyzer::{cli::flags, config::Config, from_json, Result};
use rust_analyzer::{cli::flags, config::Config, from_json};
use vfs::AbsPathBuf;

#[cfg(all(feature = "mimalloc"))]
Expand All @@ -25,7 +22,7 @@ static ALLOC: mimalloc::MiMalloc = mimalloc::MiMalloc;
#[global_allocator]
static ALLOC: jemallocator::Jemalloc = jemallocator::Jemalloc;

fn main() {
fn main() -> anyhow::Result<()> {
if std::env::var("RA_RUSTC_WRAPPER").is_ok() {
let mut args = std::env::args_os();
let _me = args.next().unwrap();
Expand All @@ -41,14 +38,7 @@ fn main() {
}

let flags = flags::RustAnalyzer::from_env_or_exit();
if let Err(err) = try_main(flags) {
tracing::error!("Unexpected error: {}", err);
eprintln!("{err}");
process::exit(101);
}
}

fn try_main(flags: flags::RustAnalyzer) -> Result<()> {
#[cfg(debug_assertions)]
if flags.wait_dbg || env::var("RA_WAIT_DBG").is_ok() {
#[allow(unused_mut)]
Expand All @@ -58,14 +48,8 @@ fn try_main(flags: flags::RustAnalyzer) -> Result<()> {
}
}

let mut log_file = flags.log_file.as_deref();

let env_log_file = env::var("RA_LOG_FILE").ok();
if let Some(env_log_file) = env_log_file.as_deref() {
log_file = Some(Path::new(env_log_file));
}
setup_logging(flags.log_file.clone())?;

setup_logging(log_file)?;
let verbosity = flags.verbosity();

match flags.subcommand {
Expand Down Expand Up @@ -102,7 +86,7 @@ fn try_main(flags: flags::RustAnalyzer) -> Result<()> {
Ok(())
}

fn setup_logging(log_file: Option<&Path>) -> Result<()> {
fn setup_logging(log_file_flag: Option<PathBuf>) -> anyhow::Result<()> {
if cfg!(windows) {
// This is required so that windows finds our pdb that is placed right beside the exe.
// By default it doesn't look at the folder the exe resides in, only in the current working
Expand All @@ -115,23 +99,42 @@ fn setup_logging(log_file: Option<&Path>) -> Result<()> {
}
}
}

if env::var("RUST_BACKTRACE").is_err() {
env::set_var("RUST_BACKTRACE", "short");
}

let log_file = env::var("RA_LOG_FILE").ok().map(PathBuf::from).or(log_file_flag);
let log_file = match log_file {
Some(path) => {
if let Some(parent) = path.parent() {
let _ = fs::create_dir_all(parent);
}
Some(fs::File::create(path)?)
Some(
fs::File::create(&path)
.with_context(|| format!("can't create log file at {}", path.display()))?,
)
}
None => None,
};
let filter = env::var("RA_LOG").ok();
// deliberately enable all `error` logs if the user has not set RA_LOG, as there is usually useful
// information in there for debugging
logger::Logger::new(log_file, filter.as_deref().or(Some("error"))).install()?;

logger::LoggerConfig {
log_file,
// Deliberately enable all `error` logs if the user has not set RA_LOG, as there is usually
// useful information in there for debugging.
filter: env::var("RA_LOG").ok().unwrap_or_else(|| "error".to_string()),
// The meaning of CHALK_DEBUG I suspected is to tell chalk crates
// (i.e. chalk-solve, chalk-ir, chalk-recursive) how to filter tracing
// logs. But now we can only have just one filter, which means we have to
// merge chalk filter to our main filter (from RA_LOG env).
//
// The acceptable syntax of CHALK_DEBUG is `target[span{field=value}]=level`.
// As the value should only affect chalk crates, we'd better manually
// specify the target. And for simplicity, CHALK_DEBUG only accept the value
// that specify level.
chalk_filter: env::var("CHALK_DEBUG").ok(),
}
.init()?;

profile::init();

Expand All @@ -146,8 +149,8 @@ const STACK_SIZE: usize = 1024 * 1024 * 8;
fn with_extra_thread(
thread_name: impl Into<String>,
thread_intent: stdx::thread::ThreadIntent,
f: impl FnOnce() -> Result<()> + Send + 'static,
) -> Result<()> {
f: impl FnOnce() -> anyhow::Result<()> + Send + 'static,
) -> anyhow::Result<()> {
let handle = stdx::thread::Builder::new(thread_intent)
.name(thread_name.into())
.stack_size(STACK_SIZE)
Expand All @@ -158,7 +161,7 @@ fn with_extra_thread(
Ok(())
}

fn run_server() -> Result<()> {
fn run_server() -> anyhow::Result<()> {
tracing::info!("server version {} will start", rust_analyzer::version());

let (connection, io_threads) = Connection::stdio();
Expand Down

0 comments on commit 424ef77

Please sign in to comment.