Skip to content

Add feature to inhibit log capture #219

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

Merged
merged 1 commit into from
Nov 14, 2021
Merged

Add feature to inhibit log capture #219

merged 1 commit into from
Nov 14, 2021

Conversation

jsha
Copy link
Collaborator

@jsha jsha commented Nov 13, 2021

This is necessary to work properly when built as a Rust dependency (a
rare use case).

Fixes #212.

/cc @sagebind for review. I think this is the last thing we need before uploading rustls-ffi and making it usable as a backend for curl-rust.

This is necessary to work properly when built as a Rust dependency (a
rare use case).
Copy link
Contributor

@sagebind sagebind left a comment

Choose a reason for hiding this comment

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

Looks OK from my perspective.

Due to the additive nature of Cargo features, it might make more sense from a Rust perspective to invert this feature (log_capture) which is disabled by default, but enabled by the Makefile, but I don't see any immediate issues with doing it this way.

@jsha
Copy link
Collaborator Author

jsha commented Nov 14, 2021

It's funny, I chose the negative flag exactly because of the additive nature of Cargo features. If crate A is expecting logs to not be captured, it shouldn't be possible for crate B somewhere else in the dependency tree to add a feature that causes logs to be captured after all.

What's the way in which additivity of features advocates for using a positive feature?

@sagebind
Copy link
Contributor

If crate A is expecting logs to not be captured

I guess I was thinking of it in an inverse way -- crate A doesn't really have any expectations at all about logs, but when using it from C you want the logs to be captured. I was also thinking that if other crates were to decide to use rustls-ffi, almost universally they would want to enable no_log_capture which would become just extra ceremony, but when being used from C you use the Makefile, which could trivially request a non-default feature be enabled automatically.

But I'm fine with either way, there's an understandable logic behind the inverse feature as well.

@jsha jsha merged commit 244ea16 into main Nov 14, 2021
@jsha jsha deleted the inhibit-log-capture branch November 14, 2021 17:38
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.

Add no_log_capture feature
2 participants