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

chore: change tracing-subscriber dependencies to enable std #1659

Closed
wants to merge 3 commits into from

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Oct 20, 2021

Motivation

Currently, some tracing-subscriber dependencies in other crates use
default-features = false, to ensure that unneeded default features of
tracing-subscriber are not inadvertently enabled. However, some of the
features those crates enable also require the std feature flag. The
default-features = false config disables the std feature, so the
required features are not available.

Solution

This branch changes those crates' dependencies on tracing-subscriber
to also enable the std feature.

Alternatives

As an alternative solution, we could change tracing-subscriber's
feature flags so that features that require the standard library also
enable the std feature, rather than having those features require
it. I think this might actually provide a better experience for most
users.

## Motivation

Currently, some `tracing-subscriber` dependencies in other crates use
`default-features = false`, to ensure that unneeded default features of
`tracing-subscriber` are not inadvertently enabled. However, some of the
features those crates enable also require the `std` feature flag. The
`default-features = false` config _disables_ the `std` feature, so the
required features are not available.

## Solution

This branch changes those crates' dependencies on `tracing-subscriber`
to also enable the `std` feature.

## Alternatives

As an alternative solution, we could change `tracing-subscriber`'s
feature flags so that features that require the standard library _also_
enable the `std` feature, rather than having those features _require_
it. I think this _might_ actually provide a better experience for most
users.
@hawkw hawkw requested review from yaahc and a team as code owners October 20, 2021 18:38
hawkw added a commit that referenced this pull request Oct 20, 2021
## Motivation

Currently, some `tracing-subscriber` dependencies in other crates use
`default-features = false`, to ensure that unneeded default features of
`tracing-subscriber` are not inadvertently enabled. However, some of the
features those crates enable also require the `std` feature flag. The
`default-features = false` config _disables_ the `std` feature, so the
required features are not available.

## Solution

This branch changes `tracing-subscriber`'s `fmt`, `registry`, and
`env-filter` feature flags to also enable the `std` feature flag
automatically.

## Alternatives

As an alternative solution, we could change crates that depend on
`tracing-subscriber` with `default-features = false` to also explicitly
ensure that the `std` feature is enabled (which might be worth doing
regardless). This is what PR #1659 does. However, I think the approach
in this branch is more correct; the feature flags that require standard
library support should automatically enable it. This provides a better
experience for most users, who are simply adding `default-features = false`
in order to avoid enabling un-needed features, not to support
`no_std`, and would like enabling a feature flag to *actually* enable
that feature.

`no_std` users will know not to enable `registry`, `fmt`, and
`env-filter` because the documentation notes that those features require
the standard library.
@hawkw
Copy link
Member Author

hawkw commented Oct 20, 2021

@yaahc fwiw I think #1660 is probably the better approach, WDYT?

hawkw added a commit that referenced this pull request Oct 20, 2021
## Motivation

Currently, some `tracing-subscriber` dependencies in other crates use
`default-features = false`, to ensure that unneeded default features of
`tracing-subscriber` are not inadvertently enabled. However, some of the
features those crates enable also require the `std` feature flag. The
`default-features = false` config _disables_ the `std` feature, so the
required features are not available.

## Solution

This branch changes `tracing-subscriber`'s `fmt`, `registry`, and
`env-filter` feature flags to also enable the `std` feature flag
automatically.

## Alternatives

As an alternative solution, we could change crates that depend on
`tracing-subscriber` with `default-features = false` to also explicitly
ensure that the `std` feature is enabled (which might be worth doing
regardless). This is what PR #1659 does. However, I think the approach
in this branch is more correct; the feature flags that require standard
library support should automatically enable it. This provides a better
experience for most users, who are simply adding `default-features = false`
in order to avoid enabling un-needed features, not to support
`no_std`, and would like enabling a feature flag to *actually* enable
that feature.

`no_std` users will know not to enable `registry`, `fmt`, and
`env-filter` because the documentation notes that those features require
the standard library.
@hawkw
Copy link
Member Author

hawkw commented Oct 20, 2021

#1660 also fixes the actual issue, but we should maybe merge this anyway for explicitness sake?

@yaahc
Copy link
Collaborator

yaahc commented Oct 20, 2021

@yaahc fwiw I think #1660 is probably the better approach, WDYT?

That makes sense to me. 👍

#1660 also fixes the actual issue, but we should maybe merge this anyway for explicitness sake?

I don't expect that the explicitness is benefitial, since presumably the other features which now transitively enable std are the features that tracing-error/tracing-flame primarily depend upon. I'm imagining a scenario where in the future tracing-subscriber somehow gets updated such that those features can be enabled without the std feature, where these crates would have no need for the std feature anymore.

@hawkw
Copy link
Member Author

hawkw commented Oct 20, 2021

I'm imagining a scenario where in the future tracing-subscriber somehow gets updated such that those features can be enabled without the std feature, where these crates would have no need for the std feature anymore.

Hmm, yeah, that's a good point. Going to just close this one, then!

@hawkw hawkw closed this Oct 20, 2021
hawkw added a commit that referenced this pull request Oct 21, 2021
## Motivation

Currently, some `tracing-subscriber` dependencies in other crates use
`default-features = false`, to ensure that unneeded default features of
`tracing-subscriber` are not inadvertently enabled. However, some of the
features those crates enable also require the `std` feature flag. The
`default-features = false` config _disables_ the `std` feature, so the
required features are not available.

## Solution

This branch changes `tracing-subscriber`'s `fmt`, `registry`, and
`env-filter` feature flags to also enable the `std` feature flag
automatically.

## Alternatives

As an alternative solution, we could change crates that depend on
`tracing-subscriber` with `default-features = false` to also explicitly
ensure that the `std` feature is enabled (which might be worth doing
regardless). This is what PR #1659 does. However, I think the approach
in this branch is more correct; the feature flags that require standard
library support should automatically enable it. This provides a better
experience for most users, who are simply adding `default-features = false`
in order to avoid enabling un-needed features, not to support
`no_std`, and would like enabling a feature flag to *actually* enable
that feature.

`no_std` users will know not to enable `registry`, `fmt`, and
`env-filter` because the documentation notes that those features require
the standard library.
hawkw added a commit that referenced this pull request Oct 21, 2021
## Motivation

Currently, some `tracing-subscriber` dependencies in other crates use
`default-features = false`, to ensure that unneeded default features of
`tracing-subscriber` are not inadvertently enabled. However, some of the
features those crates enable also require the `std` feature flag. The
`default-features = false` config _disables_ the `std` feature, so the
required features are not available.

## Solution

This branch changes `tracing-subscriber`'s `fmt`, `registry`, and
`env-filter` feature flags to also enable the `std` feature flag
automatically.

## Alternatives

As an alternative solution, we could change crates that depend on
`tracing-subscriber` with `default-features = false` to also explicitly
ensure that the `std` feature is enabled (which might be worth doing
regardless). This is what PR #1659 does. However, I think the approach
in this branch is more correct; the feature flags that require standard
library support should automatically enable it. This provides a better
experience for most users, who are simply adding `default-features = false`
in order to avoid enabling un-needed features, not to support
`no_std`, and would like enabling a feature flag to *actually* enable
that feature.

`no_std` users will know not to enable `registry`, `fmt`, and
`env-filter` because the documentation notes that those features require
the standard library.
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