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

switch log crate to tracing #9906

Merged
merged 1 commit into from
Aug 30, 2021
Merged

Conversation

dzvon
Copy link
Contributor

@dzvon dzvon commented Aug 15, 2021

Fixes #9274

  1. Replace log crate with tracing of every crate that depend on log.
  2. Change all log record macro with tracing.
  3. Add LoggerFormatter make output format be the same as original.

@lnicola
Copy link
Member

lnicola commented Aug 15, 2021

It looks like some test code is still using env-logger.

@dzvon
Copy link
Contributor Author

dzvon commented Aug 15, 2021

---- tidy::check_licenses stdout ----
thread 'tidy::check_licenses' panicked at 'different set of licenses!
New Licenses:
  Apache-2.0 WITH LLVM-exception OR Apache-2.0 OR MIT

Missing Licenses:
', crates/rust-analyzer/tests/slow-tests/tidy.rs:264:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    tidy::check_licenses

test result: FAILED. 18 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 8.48s

I believe this problem is caused by chrono crate, its license is MIT OR Apache-2.0. tracing-appender introduce it, I'd better remove tracing-appender dependence.

@lnicola
Copy link
Member

lnicola commented Aug 15, 2021

The license test is in crates/rust-analyzer/tests/slow-tests/tidy.rs.

@lnicola
Copy link
Member

lnicola commented Aug 15, 2021

You don't have to remove that crate, you can update the allowed license list.

@dzvon
Copy link
Contributor Author

dzvon commented Aug 16, 2021

Only one last issue left in CI checks.

error[E0432]: unresolved import `std::sync::atomic::AtomicU64`
  --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tracing-appender-0.1.2/src/non_blocking.rs:54:5

Maybe we should submit a PR to tracing-appender or just remove it? What do you think? @lnicola

@dzvon dzvon requested a review from lnicola August 16, 2021 01:03
@dzvon
Copy link
Contributor Author

dzvon commented Aug 16, 2021

This tokio-rs/tracing#1508 PR will fix the compilation error, but it's a breaking change, and will take a little time to be released.

@matklad
Copy link
Member

matklad commented Aug 16, 2021

Is there a lighter weight alternative to tracing appender? I don't think it makes sense to rust-analyzer to depend on chrono, it shouldn't care which year it is outside.

@dzvon
Copy link
Contributor Author

dzvon commented Aug 16, 2021

@matklad I totally agree with you, I'll find a way to replace tracing appender.

@matklad
Copy link
Member

matklad commented Aug 16, 2021

@dzvon
Copy link
Contributor Author

dzvon commented Aug 16, 2021

This time I think it's okay.

@dzvon dzvon force-pushed the switch-log-to-tracing branch 3 times, most recently from fff8d0f to 0c5e071 Compare August 20, 2021 07:06
@dzvon dzvon force-pushed the switch-log-to-tracing branch from 0c5e071 to ba0947d Compare August 30, 2021 07:14
@lnicola
Copy link
Member

lnicola commented Aug 30, 2021

Thanks!

bors r+

changelog internal (first contribution) switch from the log to tracing

@bors
Copy link
Contributor

bors bot commented Aug 30, 2021

@bors bors bot merged commit 43ed536 into rust-lang:master Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch from log to tracing
4 participants