search

Smithay / calloop

A callback-based Event Loop
MIT License
177 stars 34 forks source link

chore: use tracing instead of log crate #195

Closed joshka closed 3 weeks ago

joshka commented 4 weeks ago

Fixes: https://github.com/Smithay/calloop/issues/194

joshka commented 4 weeks ago

Tracing logs for a sample app that hits these -

image

vs without tracing:

image
joshka commented 4 weeks ago

Fixed a log::warn that I'd missed. The CI still breaks due to the log downgrade. The MSRV of the current version of log (1.60.0) seems to be lower than calloop's MSRV (1.63.0). Does this mean that this is not necessary any more?

My only hesitancy is for the long-term maintenance of the tracing crate. I know that there was a period where tracing was effectively unmaintained. Looking at the repo it looks like it's maintained better now, but I'm just worried about needing to rollback to log if trading ever becomes unmaintained again.

The other thing I noted (in #194 in case you missed it) is that this brings back in the proc-macro2 and syn crates to the dependency list (which I saw you were trying to remove by avoiding thiserror).

notgull commented 4 weeks ago

The CI still breaks due to the log downgrade. The MSRV of the current version of log (1.60.0) seems to be lower than calloop's MSRV (1.63.0). Does this mean that this is not necessary any more?

Yes, it's probably not necessary.

The other thing I noted (in #194 in case you missed it) is that this brings back in the proc-macro2 and syn crates to the dependency list (which I saw you were trying to remove by avoiding thiserror).

If you import tracing without default features it doesn't have these dependencies though, right?

joshka commented 4 weeks ago

If you import tracing without default features it doesn't have these dependencies though, right?

Actually yep, you're right - I must have checked before I added that

calloop v0.14.0 (/Users/joshka/local/calloop)
├── bitflags v2.5.0
├── polling v3.7.2
│   ├── cfg-if v1.0.0
│   ├── rustix v0.38.34
│   │   ├── bitflags v2.5.0
│   │   ├── errno v0.3.9
│   │   │   └── libc v0.2.155
│   │   └── libc v0.2.155
│   └── tracing v0.1.40
│       ├── log v0.4.21
│       ├── pin-project-lite v0.2.14
│       └── tracing-core v0.1.32
├── rustix v0.38.34 (*)
├── slab v0.4.9
│   [build-dependencies]
│   └── autocfg v1.3.0
└── tracing v0.1.40 (*)
notgull commented 4 weeks ago

Please rebase on master to fix the CI issues. Then we should be good to go.

joshka commented 4 weeks ago

Please rebase on master to fix the CI issues. Then we should be good to go.

Done

codecov[bot] commented 4 weeks ago

Codecov Report

Attention: Patch coverage is 23.07692% with 20 lines in your changes missing coverage. Please review.

Project coverage is 86.39%. Comparing base (5433747) to head (3ce54aa).

Files Patch % Lines
src/loop_logic.rs 25.00% 15 Missing :warning:
src/sources/ping/eventfd.rs 0.00% 2 Missing :warning:
src/sources/signals.rs 0.00% 2 Missing :warning:
src/sources/mod.rs 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #195 +/- ## ========================================== + Coverage 85.55% 86.39% +0.84% ========================================== Files 13 15 +2 Lines 1862 2051 +189 ========================================== + Hits 1593 1772 +179 - Misses 269 279 +10 ``` | [Flag](https://app.codecov.io/gh/Smithay/calloop/pull/195/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay) | Coverage Δ | | |---|---|---| | [macos-latest](https://app.codecov.io/gh/Smithay/calloop/pull/195/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay) | `85.71% <27.27%> (?)` | | | [ubuntu-latest](https://app.codecov.io/gh/Smithay/calloop/pull/195/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay) | `85.99% <20.00%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.