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

Updated mysql and disabled default features #202

Merged
merged 6 commits into from
Jan 4, 2022
Merged

Updated mysql and disabled default features #202

merged 6 commits into from
Jan 4, 2022

Conversation

TobiasDeBruijn
Copy link
Contributor

Changes

  • Bumped mysql to v22.0.0
  • Disabled default features on mysql
  • Added flate2 crate with zlib feature, This previously came in via the mysql crate but now it no longer is, so it must be specified by us

Reasoning

MySQL default ships with native-tls, i.e. OpenSSL enabled by default. My PR to the mysql crate makes this optional but keeps OpenSSL enabled by default. However, when using refinery it is still pulled in, even if it is not enabled in my binary crate due to the way Cargo resolves features.

This PR makes mysql features optional, allowing the end user to choose if they want native-tls, or rustls e.g. By default mysql also came with the feature flate2/zlib, with disabled default features flate2/zlib is no longer included. When using refinery with only the mysql feature enabled it compiles fine. However, with the default feature set it no longer does (cargo b fails with too many errors to count).

Backwards compatibility

Backwards compatibility is maintained as the OpenSSL feature is still enabled by default via the mysql crate.

Thanks

- Bumped mysql to v22.0.0
- Disabled default features on mysql
- Added flate2 crate with zlib feature, This previously came in via the mysql crate but now it no longer is, so it must be specified by us
@jxs
Copy link
Member

jxs commented Dec 31, 2021

Hi, and thanks for your interest! This seems legit, but can we make it so that flate2 is only imported when the mysql feature is enabled (like we do with mysql_async, tiberius, tokio-postgres) and leave a comment above stating the purpose?
thanks

refinery_core/Cargo.toml Outdated Show resolved Hide resolved
refinery_core/Cargo.toml Outdated Show resolved Hide resolved
refinery_core/Cargo.toml Outdated Show resolved Hide resolved
refinery_core/Cargo.toml Outdated Show resolved Hide resolved
@TobiasDeBruijn
Copy link
Contributor Author

Good point to also allow 21.0.0, that does not break backwards compatibility either (with regards to OpenSSL) as the mysql crate shipped it irregardless of if you use default features or not.

@jxs jxs merged commit 4564ec2 into rust-db:main Jan 4, 2022
@jxs
Copy link
Member

jxs commented Jan 4, 2022

yeah, sorry for the spam fixing the typos also.
Thanks!

@jxs jxs mentioned this pull request Jan 4, 2022
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