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

Adjust Once to not violate feature additivity principle #760

Merged
merged 1 commit into from
Jun 22, 2020

Conversation

nagisa
Copy link
Contributor

@nagisa nagisa commented Jun 21, 2020

When the following dependencies and their features were specified:

tracing-core = { version = "0.1", default-features = false, features = ["std"] }
tracing = { version = "0.1", default-features = false }

The build would fail with the following error:

error[E0412]: cannot find type `Once` in crate `tracing_core`
   --> tracing/src/lib.rs:840:35
    |
840 |     pub type Once = tracing_core::Once<()>;
    |                                   ^^^^ not found in `tracing_core`
    |

This happened because tracing-core exports Once only if its std
feature is disabled. And the depending tracing crate assumed that if
its std feature was disabled, so would be the std feature in
tracing-core.

This is a violation of the undocumented "features must be additive"
guideline
. In this commit tracing-core is adjusted to export Once
regardless of whether std is disabled or not.

@nagisa nagisa requested review from hawkw and a team as code owners June 21, 2020 17:09
When the following dependencies and their features were specified:

```toml
tracing-core = { version = "0.1", default-features = false, features = ["std"] }
tracing = { version = "0.1", default-features = false }
```

The build would fail with the following error:

```
error[E0412]: cannot find type `Once` in crate `tracing_core`
   --> tracing/src/lib.rs:840:35
    |
840 |     pub type Once = tracing_core::Once<()>;
    |                                   ^^^^ not found in `tracing_core`
    |
```

This happened because `tracing-core` exports `Once` only if its `std`
feature is disabled. And the depending `tracing` crate assumed that if
its `std` feature was disabled, so would be the `std` feature in
`tracing-core`.

This is a violation of the [undocumented "features must be additive"
guideline][ag]. In this commit tracing-core is adjusted to export `Once`
regardless of whether `std` is disabled or not.

[ag]: rust-lang/cargo#4328
@nagisa nagisa force-pushed the nagisa/fix-feature-additivity branch from 36175a5 to 6c69806 Compare June 21, 2020 17:09
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix, this is definitely more correct. I guess we never tested the case where the tracing-core/std feature was enabled, but not by tracing (which is kind of a weird edge case).

@hawkw hawkw merged commit b36d59d into tokio-rs:master Jun 22, 2020
hawkw added a commit that referenced this pull request Jul 8, 2020
Changed

- Replaced use of `inner_local_macros` with `$crate::` (#729)

Added

- `must_use` warning to guards returned by `dispatcher::set_default`
  (#686)
- `fmt::Debug` impl to `dyn Value`s (#696)
- Functions to convert between `span::Id` and `NonZeroU64` (#770)
- More obvious warnings in documentation (#769)

Fixed

- Compiler error when `tracing-core/std` feature is enabled but
  `tracing/std` is not (#760)
- Clippy warning on vtable address comparison in `callsite::Identifier`
  (#749)
- Documentation formatting issues (#715, #771)

Thanks to @bkchr, @majecty, @taiki-e, @nagisa, and @nqvz for
contributing to this release!
hawkw added a commit that referenced this pull request Jul 8, 2020
### Changed

- Replaced use of `inner_local_macros` with `$crate::` (#729)

### Added

- `must_use` warning to guards returned by `dispatcher::set_default`
  (#686)
- `fmt::Debug` impl to `dyn Value`s (#696)
- Functions to convert between `span::Id` and `NonZeroU64` (#770)
- More obvious warnings in documentation (#769)

### Fixed

- Compiler error when `tracing-core/std` feature is enabled but
  `tracing/std` is not (#760)
- Clippy warning on vtable address comparison in `callsite::Identifier`
  (#749)
- Documentation formatting issues (#715, #771)

Thanks to @bkchr, @majecty, @taiki-e, @nagisa, and @nvzqz for
contributing to this release!
hawkw added a commit that referenced this pull request Jul 8, 2020
Added

- **attributes**: Support for arbitrary expressions as fields in
  `#[instrument]` (#672)
- **attributes**: `#[instrument]` now emits a compiler warning when
  ignoring unrecognized input (#672, #786)
- Improved documentation on using `tracing` in async code (#769)

Changed

- Updated `tracing-core` dependency to 0.1.11

Fixed

- **macros**: Excessive monomorphization in macros, which could lead to
  longer compilation times (#787)
- **log**: Compiler warnings in macros when `log` or `log-always` features
  are enabled (#753)
- Compiler error when `tracing-core/std` feature is enabled but
  `tracing/std` is not (#760)

Thanks to @nagisa for contributing to this release!
hawkw added a commit that referenced this pull request Jul 8, 2020
### Added

- **attributes**: Support for arbitrary expressions as fields in
  `#[instrument]` (#672)
- **attributes**: `#[instrument]` now emits a compiler warning when
  ignoring unrecognized input (#672, #786)
- Improved documentation on using `tracing` in async code (#769)

### Changed

- Updated `tracing-core` dependency to 0.1.11

### Fixed

- **macros**: Excessive monomorphization in macros, which could lead to
  longer compilation times (#787)
- **log**: Compiler warnings in macros when `log` or `log-always` features
  are enabled (#753)
- Compiler error when `tracing-core/std` feature is enabled but
  `tracing/std` is not (#760)

Thanks to @nagisa, and everyone who contributed to the new `tracing-core`
and `tracing-attributes` versions, for contributing to this release!
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.

2 participants