From 8ee05166434b0c8340c34d7a73e39aab489ac587 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sun, 19 Sep 2021 11:17:38 -0700 Subject: [PATCH 1/3] chore: remove `--bins` from CI `cargo check` runs So...it turns out that passing `--bins` actually makes `cargo check` do *nothing* on library-only projects without binaries. This was...surprising. It turns out that because of this, our MSRV check CI run is basically a nop, and we accidentally merged code that doesn't compile on 1.42.0. I'm very dumb lol. --- .github/workflows/CI.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 484f7c0705..eb5e60d683 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -21,7 +21,7 @@ jobs: uses: actions-rs/cargo@v1 with: command: check - args: --all --bins + args: --all check: # Run `cargo check` first to ensure that the pushed code at least compiles. @@ -37,7 +37,7 @@ jobs: uses: actions-rs/cargo@v1 with: command: check - args: --all --bins --tests --benches + args: --all --tests --benches cargo-hack: runs-on: ubuntu-latest @@ -279,4 +279,4 @@ jobs: uses: actions-rs/clippy-check@v1 with: token: ${{ secrets.GITHUB_TOKEN }} - args: --all --bins --examples --tests --benches -- -D warnings + args: --all --examples --tests --benches -- -D warnings From 1a95f7bc05ae584ed50f6c8c786010996fdfef4f Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sun, 19 Sep 2021 11:19:48 -0700 Subject: [PATCH 2/3] subscriber: use `f64` consts as module imports This fixes the build on Rust 1.42.0, where primitive types don't have associated consts. My bad! --- tracing-subscriber/src/filter/env/field.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tracing-subscriber/src/filter/env/field.rs b/tracing-subscriber/src/filter/env/field.rs index e9a2fdd152..970850f92e 100644 --- a/tracing-subscriber/src/filter/env/field.rs +++ b/tracing-subscriber/src/filter/env/field.rs @@ -219,7 +219,7 @@ impl fmt::Display for ValueMatch { match self { ValueMatch::Bool(ref inner) => fmt::Display::fmt(inner, f), ValueMatch::F64(ref inner) => fmt::Display::fmt(inner, f), - ValueMatch::NaN => fmt::Display::fmt(&f64::NAN, f), + ValueMatch::NaN => fmt::Display::fmt(&std::f64::NAN, f), ValueMatch::I64(ref inner) => fmt::Display::fmt(inner, f), ValueMatch::U64(ref inner) => fmt::Display::fmt(inner, f), ValueMatch::Pat(ref inner) => fmt::Display::fmt(inner, f), @@ -355,7 +355,9 @@ impl<'a> Visit for MatchVisitor<'a> { Some((ValueMatch::NaN, ref matched)) if value.is_nan() => { matched.store(true, Release); } - Some((ValueMatch::F64(ref e), ref matched)) if (value - *e).abs() < f64::EPSILON => { + Some((ValueMatch::F64(ref e), ref matched)) + if (value - *e).abs() < std::f64::EPSILON => + { matched.store(true, Release); } _ => {} From bd9501cad01a436818615cdea43319d366cd9037 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sun, 19 Sep 2021 11:45:50 -0700 Subject: [PATCH 3/3] subscriber: fix the build on Rust 1.42.0 Due to the CI issue fixed by PR #1580, we accidentally merged `tracing-subscriber` changes that don't compile on our MSRV, Rust 1.42.0. This includes: - use of associated consts on primitive types (`u64::MAX` rather than `std::u64::MAX`) - use of `#[cfg(...)]` on `if` expressions - use of the `str::strip_suffix` method, which wasn't stable on 1.42.0 This commit fixes these issues. These changes are mostly very simple and mechanical. The only significant change is changing the `StaticDirective` parsing code to not use `strip_suffix`, which changes the implementation around a bit...but it's still pretty straightforward. This should fix 1.42.0 compatibility. --- tracing-subscriber/src/filter/directive.rs | 23 ++++++++------ .../src/filter/layer_filters.rs | 12 ++++---- tracing-subscriber/src/layer/context.rs | 30 ++++++++++--------- tracing-subscriber/src/layer/layered.rs | 10 ++++--- tracing-subscriber/src/registry/mod.rs | 10 ++++--- 5 files changed, 49 insertions(+), 36 deletions(-) diff --git a/tracing-subscriber/src/filter/directive.rs b/tracing-subscriber/src/filter/directive.rs index 5c6b38eb4b..02954be0ae 100644 --- a/tracing-subscriber/src/filter/directive.rs +++ b/tracing-subscriber/src/filter/directive.rs @@ -312,16 +312,21 @@ impl FromStr for StaticDirective { )); } + if !maybe_fields.ends_with("}]") { + return Err(ParseError::msg("expected fields list to end with '}]'")); + } + let fields = maybe_fields - .strip_suffix("}]") - .ok_or_else(|| ParseError::msg("expected fields list to end with '}]'"))?; - field_names.extend(fields.split(',').filter_map(|s| { - if s.is_empty() { - None - } else { - Some(String::from(s)) - } - })); + .trim_end_matches("}]") + .split(',') + .filter_map(|s| { + if s.is_empty() { + None + } else { + Some(String::from(s)) + } + }); + field_names.extend(fields); }; let level = part1.parse()?; return Ok(Self { diff --git a/tracing-subscriber/src/filter/layer_filters.rs b/tracing-subscriber/src/filter/layer_filters.rs index e3492ffefe..69549062f1 100644 --- a/tracing-subscriber/src/filter/layer_filters.rs +++ b/tracing-subscriber/src/filter/layer_filters.rs @@ -409,7 +409,7 @@ where impl FilterId { const fn disabled() -> Self { - Self(u64::MAX) + Self(std::u64::MAX) } /// Returns a `FilterId` that will consider _all_ spans enabled. @@ -548,7 +548,7 @@ impl fmt::Binary for FilterId { impl FilterMap { pub(crate) fn set(self, FilterId(mask): FilterId, enabled: bool) -> Self { - if mask == u64::MAX { + if mask == std::u64::MAX { return self; } @@ -570,7 +570,7 @@ impl FilterMap { #[inline] pub(crate) fn any_enabled(self) -> bool { - self.bits != u64::MAX + self.bits != std::u64::MAX } } @@ -727,8 +727,10 @@ impl FilterState { pub(crate) fn filter_map(&self) -> FilterMap { let map = self.enabled.get(); #[cfg(debug_assertions)] - if self.counters.in_filter_pass.get() == 0 { - debug_assert_eq!(map, FilterMap::default()); + { + if self.counters.in_filter_pass.get() == 0 { + debug_assert_eq!(map, FilterMap::default()); + } } map diff --git a/tracing-subscriber/src/layer/context.rs b/tracing-subscriber/src/layer/context.rs index a4dc508206..7c205a7965 100644 --- a/tracing-subscriber/src/layer/context.rs +++ b/tracing-subscriber/src/layer/context.rs @@ -295,20 +295,22 @@ where // If we found a span, and our per-layer filter enables it, return that // span! #[cfg(feature = "registry")] - if let Some(span) = span?.try_with_filter(self.filter) { - Some(span) - } else { - // Otherwise, the span at the *top* of the stack is disabled by - // per-layer filtering, but there may be additional spans in the stack. - // - // Currently, `LookupSpan` doesn't have a nice way of exposing access to - // the whole span stack. However, if we can downcast the innermost - // subscriber to a a `Registry`, we can iterate over its current span - // stack. - // - // TODO(eliza): when https://github.com/tokio-rs/tracing/issues/1459 is - // implemented, change this to use that instead... - self.lookup_current_filtered(subscriber) + { + if let Some(span) = span?.try_with_filter(self.filter) { + Some(span) + } else { + // Otherwise, the span at the *top* of the stack is disabled by + // per-layer filtering, but there may be additional spans in the stack. + // + // Currently, `LookupSpan` doesn't have a nice way of exposing access to + // the whole span stack. However, if we can downcast the innermost + // subscriber to a a `Registry`, we can iterate over its current span + // stack. + // + // TODO(eliza): when https://github.com/tokio-rs/tracing/issues/1459 is + // implemented, change this to use that instead... + self.lookup_current_filtered(subscriber) + } } #[cfg(not(feature = "registry"))] diff --git a/tracing-subscriber/src/layer/layered.rs b/tracing-subscriber/src/layer/layered.rs index dae885269d..8d6c6747da 100644 --- a/tracing-subscriber/src/layer/layered.rs +++ b/tracing-subscriber/src/layer/layered.rs @@ -449,10 +449,12 @@ where // for internal debugging purposes, so only print them if alternate mode // is enabled. #[cfg(feature = "registry")] - if alt { - s.field("inner_is_registry", &self.inner_is_registry) - .field("has_layer_filter", &self.has_layer_filter) - .field("inner_has_layer_filter", &self.inner_has_layer_filter); + { + if alt { + s.field("inner_is_registry", &self.inner_is_registry) + .field("has_layer_filter", &self.has_layer_filter) + .field("inner_has_layer_filter", &self.inner_has_layer_filter); + } } s.field("layer", &self.layer) diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index ad0d8e0964..04ba54a37f 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -273,10 +273,12 @@ where // by the selected filter ID. #[cfg(feature = "registry")] - if !curr.is_enabled_for(self.filter) { - // The current span in the chain is disabled for this - // filter. Try its parent. - continue; + { + if !curr.is_enabled_for(self.filter) { + // The current span in the chain is disabled for this + // filter. Try its parent. + continue; + } } return Some(curr);