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

Broken compile with tracing-core 0.1.22 #270

Closed
arlyon opened this issue Feb 4, 2022 · 6 comments · Fixed by #271
Closed

Broken compile with tracing-core 0.1.22 #270

arlyon opened this issue Feb 4, 2022 · 6 comments · Fixed by #271

Comments

@arlyon
Copy link

arlyon commented Feb 4, 2022

A recent change to tracing-core in tokio-rs/tracing#1891 which is used by recently released tracing 0.1.29 causes broken compiles due to Kind being changed to a u8 rather than an enum. Manually pinning tracing-core to 0.1.21 fixes the issue.

error[E0004]: non-exhaustive patterns: `Kind(0_u8)` and `Kind(3_u8..=u8::MAX)` not covered
  --> /home/arlyon/.cargo/registry/src/github.com-1ecc6299db9ec823/console-api-0.1.1/src/common.rs:20:15
   |
20 |         match kind {
   |               ^^^^ patterns `Kind(0_u8)` and `Kind(3_u8..=u8::MAX)` not covered
   |
   = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
   = note: the matched value is of type `tracing_core::Kind`

Happy to open a PR; how should we handle cases where the tracing_core::metadata::Kind is not covered? Maybe we should also use a u8 to match?

@arlyon
Copy link
Author

arlyon commented Feb 4, 2022

It should be noted that this also breaks cargo install tokio-console

@hawkw
Copy link
Member

hawkw commented Feb 4, 2022

Hmm, that's...very surprising. The previous design of the Kind enum was intended specifically to prevent exhaustive matching; if it was ever being matched exhaustively, that seems very wrong.

@hawkw
Copy link
Member

hawkw commented Feb 4, 2022

Happy to open a PR; how should we handle cases where the tracing_core::metadata::Kind is not covered? Maybe we should also use a u8 to match?

We should probably skip those cases and do nothing. However, since this appears to be an accidental breaking change, the best solution is probably to revert it upstream.

@seanmonstar
Copy link
Member

The problem was that since it was an enum internally, the exhaustiveness bleeds through to the constants of the wrapper struct. We had the same problem adding Version::HTTP_3, and could only do so in a breaking change, because people had exhaustively matched the version. It now includes "dont match me" variant.

@hawkw
Copy link
Member

hawkw commented Feb 4, 2022

In this case, I am fairly sure that the console is approximately the only code that matched on this type, since there should be no publicly accessible way for user code to actually get a Kind from a Metadata, without accessing explicitly unstable, #[doc(hidden)] code. The only reason that console-api had a function that matched on a Kind was because I knew about the type and I wrote it. :) That From impl really has no reason to exist, because the only use of the Kind type in public APIs of tracing-core is to pass it to the Metadata constructor. However, rather than removing it, I'm going to fix it by making it not try to match exhaustively, so that we don't have to break console-api.

Based on that, I've determined that in this case, the accidental breaking change probably has a very limited blast radius, because there's no actual reason for user code to actually match this type. So I'm going to fix this issue in the console rather than reverting the change upstream.

hawkw added a commit that referenced this issue Feb 4, 2022
The `tracing_core::metadata::Kind` type was not intended to be matched
exhaustively, so that its internal representation could be changed
without causing a breaking change. However, some fairly recent compiler
changes broke this, and made the type exhaustively matched. This
resulted in `tracing-core` 0.1.22 breaking `console-api`.

This PR fixes the breakage by changing `console-api` to avoid
exhaustively matching on `Kind`s.

Fixes #270
@hawkw hawkw closed this as completed in #271 Feb 4, 2022
hawkw added a commit that referenced this issue Feb 4, 2022
The `tracing_core::metadata::Kind` type was not intended to be matched
exhaustively, so that its internal representation could be changed
without causing a breaking change. However, some fairly recent compiler
changes broke this, and made the type exhaustively matched. This
resulted in `tracing-core` 0.1.22 breaking `console-api`.

This PR fixes the breakage by changing `console-api` to avoid
exhaustively matching on `Kind`s.

Fixes #270
hawkw added a commit that referenced this issue Feb 4, 2022
<a name="0.1.2"></a>
## 0.1.2 (2022-02-04)

#### Bug Fixes

* fix accidental exhaustive matching on `metadata::Kind` (#271)
  ([d9aafaa](d9aafaa), closes [#270](270))
hawkw added a commit that referenced this issue Feb 4, 2022
<a name="0.1.2"></a>
## 0.1.2 (2022-02-04)

#### Bug Fixes

* fix accidental exhaustive matching on `metadata::Kind` (#271)
  ([d9aafaa](d9aafaa), closes [#270](270))
@arlyon
Copy link
Author

arlyon commented Feb 11, 2022

Thanks for the quick fix here :)

hawkw added a commit that referenced this issue Feb 18, 2022
<a name="0.1.2"></a>
## 0.1.2 (2022-02-18)

#### Bug Fixes

*  console-api dependencies to require 0.1.2 (#274) ([b95f683](b95f683), closes [#270](270))
*  missing histogram in task details (#296) ([884f4ec](884f4ec), closes [#296](296))
hawkw added a commit that referenced this issue Feb 18, 2022
<a name="0.1.2"></a>
## 0.1.2 (2022-02-18)

#### Bug Fixes

*  console-api dependencies to require 0.1.2 (#274) ([b95f683](b95f683), closes [#270](270))
*  missing histogram in task details (#296) ([884f4ec](884f4ec), closes [#296](296))
hawkw added a commit that referenced this issue Feb 18, 2022
<a name="0.1.2"></a>
## 0.1.2 (2022-02-18)

#### Bug Fixes

*  console-api dependencies to require 0.1.2 (#274) ([b95f683](b95f683), closes [#270](270))
*  missing histogram in task details (#296) ([884f4ec](884f4ec), closes [#296](296))
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 a pull request may close this issue.

3 participants