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.

Sign up for GitHub

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

Jump to bottom

tracing-tree gets activated by chalk-recursive #686

Closed
matklad opened this issue Feb 22, 2021 · 2 comments · Fixed by #687
Closed

tracing-tree gets activated by chalk-recursive #686

matklad opened this issue Feb 22, 2021 · 2 comments · Fixed by #687
Labels
C-chalk-recursive Issues related to the chalk-recursive crate

Comments

@matklad
Copy link
Member

matklad commented Feb 22, 2021

chalk-ir crate has an optional dep on tracing-tree, but this dep is in the default feature. chalk-recursive depends on chalk-ir without default-features = false, and so it unconditionally pulls tracing tree.

Either tracing-full should not be a default feature, or chalk crates should use default-features=false for path dependencies.
@detrumi
Copy link
Member

detrumi commented Feb 22, 2021 •
edited
Loading

I'm assuming you meant chalk-solve instead of chalk-ir, because that's the crate with the optional dependency on tracing-tree.

Using default-features=false inside chalk crates would mean that they wouldn't have tracing enabled, so I don't think that's an option.
Likewise, making it non-default would have the same result. Edit: Making it non-default would work if we specified the feature in Chalk's crates

We should instead propagate the feature to all crates that depend on chalk-solve, so in this case just chalk-recursive (chalk-engine and chalk-integration shouldn't really be used from outside). Then consumers of chalk-recursive can can opt out of tracing support.

@matklad
Copy link
Member Author

matklad commented Feb 22, 2021

A related issue is that chalk doesn't set default-featuers = false on tracing-subscriber. It probably should?

@matklad matklad mentioned this issue Feb 22, 2021
Closed
@detrumi detrumi mentioned this issue Feb 27, 2021
Merged
@detrumi detrumi added the C-chalk-recursive Issues related to the chalk-recursive crate label Feb 27, 2021
@bors bors closed this as completed in 4e3d174 Feb 27, 2021
@memoryruins memoryruins mentioned this issue May 8, 2021
Merged
bors added a commit that referenced this issue May 10, 2021
@bors
Auto merge of #703 - memoryruins:conditional, r=jackh726
3d0dacc 
fix(chalk-recursive): allow chalk-solve's default-features to be disabled

Without this change, one of the issues mentioned in #686 continue to persist after #687 because `chalk-solve`'s features were enabled unconditionally when depending on `chalk-recursive`.

Now `chalk-solve`'s `tracing-full` feature is enabled when `chalk-recursive`'s `tracing-full` feature is enabled (which it continues to be by default; it may be worth considering disabling these by default in the future, as #686 suggested, to minimize this sort of issue).
@memoryruins memoryruins mentioned this issue May 24, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-chalk-recursive Issues related to the chalk-recursive crate
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Bump dependencies and propagate tracing features detrumi/chalk
2 participants
@matklad @detrumi