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

I think that for an option library like uuid, its feature can be reexported at the same time. #1669

Closed
driftluo opened this Issue Apr 29, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@driftluo

driftluo commented Apr 29, 2018

Issue:

Diesel 1.1.* only support uuid 0.5.*, and I use uuid need its v4 feature, so on my Cargo.toml will be:

[dependencies.uuid]
features = ["serde", "v4"]
version = "^0.5.0"

[dependencies.diesel]
features = ["postgres", "chrono", "uuid", "r2d2"]
version = "^1.1.1"

But diesel 1.2.* support to uuid 0.6.*, when I cargo update, uuid doesn't update to 0.6, then will Rustc will complain like follow:

error[E0277]: the trait bound `uuid::Uuid: diesel::Expression` is not satisfied
 --> src\models\article_tag_relation.rs:9:10
  |
9 | #[derive(Insertable, Debug, Clone, Deserialize, Serialize)]
  |          ^^^^^^^^^^ the trait `diesel::Expression` is not implemented for `uuid::Uuid`
  |
  = note: required because of the requirements on the impl of `diesel::Expression` for `&'insert uuid::Uuid`
  = note: required because of the requirements on the impl of `diesel::expression::AsExpression<diesel::sql_types::Uuid>` for `&'insert uuid::Uuid`

So, I think that for an option library like uuid, its feature can be reexported at the same time. In this way, you do not need to pay attention to the version of the corresponding library, let it upgrade at the same time with the diesel.

If you accept this view, I will submit a pull request to complete this function.

@sgrif

This comment has been minimized.

Member

sgrif commented Apr 29, 2018

This wouldn't solve your original problem, as the v4 feature would not be enabled since the versions don't match up.

@driftluo

This comment has been minimized.

driftluo commented Apr 29, 2018

However, when I write it like this, my project is working properly:

[dependencies.uuid]
features = ["serde", "v4"]
version = "^0.6.0"

[dependencies.diesel]
features = ["postgres", "chrono", "uuid", "r2d2"]
version = "^1.1.1"

On Cargo.lock , only a 0.6.3 version of the uuid library.

I think diesel features add the following:

[features]
v4 = ["uuid/v4"]

I will be able to use this instead of reimport the uuid library just to turn on an features:

[dependencies.diesel]
features = ["postgres", "chrono", "uuid", "r2d2", "v4"]
version = "^1.1.1"
@driftluo

This comment has been minimized.

driftluo commented Apr 29, 2018

Ok, I looked at the source code, uuid is converted from trait FromSql IntoSql, not the corresponding type in the uuid crate, there seems to be no way to inherit the feature, this is my idealization, I underestimated the complexity of the problem, but this is not something that cannot be achieved

@sgrif

This comment has been minimized.

Member

sgrif commented Apr 29, 2018

I do not want to re-export every feature of every optional dependency we support

@sgrif

This comment has been minimized.

Member

sgrif commented May 1, 2018

While I can appreciate the sentiment here, I don't think this approach makes sense in Diesel. For most applications, they will likely want to enable at least one feature on the other crates (usually serde). This requires syncing versions. There's really no way around it.

@sgrif sgrif closed this May 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment