Diesel: uuid 0.7 not being used because of features in manifest

[ascclemens]

Setup

Versions

• Rust: rust version 1.30.0 (da5f414c2 2018-10-24)
• Diesel: 1.3.3
• Database: Any
• Operating System: Any

Feature Flags

• diesel: uuid

Problem Description

As far as I can tell, uuid 0.7 will never be used because it doesn't match the requirements. While 0.7 is indeed >= 0.2 and < 0.8.0, it does not have a use_std feature, which locks uuid to 0.6. The feature was renamed to std in 0.7 and is a default feature.

I forked diesel and changed Cargo.toml to use std, then used a patch section in one of my projects. Updating to my fork removed uuid 0.6 and added uuid 0.7. Reverting back to the official git adds uuid 0.6 back.

Checklist

• [x] I have already looked over the issue tracker for similar issues.
• [x] This issue can be reproduced on Rust's stable channel. (Your issue will be
  closed if this is not the case)

[weiznich]

Uuid 0.7 get's never used because the last release of diesel has no support for uuid 0.7, only master supports the new version.

[ascclemens]

No, master does not. That's why I made this issue.

master explicitly will not use uuid 0.7, since uuid 0.7 misses the feature that master requests.

[weiznich]

If uuid changed the name of that feature there is no way for diesel to support both versions. That means we cannot update uuid to a newer version without releasing diesel 2.0 because this would remove support for old versions of uuid.

[ascclemens]

Perhaps you should revert #1861 to explicitly show that you won't support uuid 0.7, then.

For what it's worth, use_std was deprecated in uuid 0.6.

[weiznich]

For what it's worth, use_std was deprecated in uuid 0.6.

This doesn't help us because we also support older versions of uuid.

[terry90]

If I understand it correctly, uuid won't be updated unless a major version is coming out ? :cry:
Is it better to support older versions instead of newer ? It prevents people to update lot of things

Edit: if you want, you can temporarily use:

[patch.crates-io]
diesel = { git = "https://github.com/terry90/diesel.git" }

[weiznich]

@terry90 That's unfortunate but there is really no way to solve this without a major version bump (or uuid reverts that change…). So nothing we could do about this, sorry.

[weiznich]

Looked into this a bit further. It seems like we could workaround that issue with the std / no-std feature by just not depending on that feature. But this won't allow us to support uuid 0.7 because they changed some function that we use. So no way to do this update without a breaking change.

[DoumanAsh]

If you have unstable crate as part of public API, it is already wrong to consider diesel stable. Being part of feature though, I don't see big problem to update in next patch version with mention of need to upgrade uuid, not a big deal

[fairingrey]

@terry90 I tried using your patch, but I'm getting a ton of errors now and I'm not sure what happened. I've attached the error output here: error.txt

Perhaps this might be related to https://github.com/rust-lang/rust/issues/50504, but I'm not sure why your patch in particular breaks it when it builds just fine without it included.

In regards to the issue, maybe this could be patched in diesel 1.4.x if it's a breaking change? Kind of agree with @DoumanAsh.

[terry90]

@fairingrey Sorry I didn't mentioned, you should cargo update or remove your Cargo.lock

@DoumanAsh I agree too, I think I'm not the only one to use Rocket and Diesel, which seems close to a full web framework like Rails with Activerecord, and this issue unfortunately makes the latest diesel incompatible with rocket. The moving parts in the Rust ecosystem - I think - should not be ignored.

If you don't want to make this a breaking change until a minor or major version, another solution would be to maintain a parallel branch, and adds this to the README.md. While I don't think this is the best option, this is something.

Edit: Great, I just saw you started a PR

[weiznich]

So I've opened a PR that could fix that. Unfortunately that requires an unstable cargo feature, so that's nothing we could merge directly (The required feature seems to be planned for rust 1.31).

If you have unstable crate as part of public API, it is already wrong to consider diesel stable. Being part of feature though, I don't see big problem to update in next patch version with mention of need to upgrade uuid, not a big deal

I agree too, I think I'm not the only one to use Rocket and Diesel, which seems close to a full web framework like Rails with Activerecord, and this issue unfortunately makes the latest diesel incompatible with rocket. The moving parts in the Rust ecosystem - I think - should not be ignored.

That's unfortunate but but there is really nothing we could do about that (Removing uuid from diesel 1.0 is not possible any more …). The only way would be to release diesel 2.0 as soon as possible, but I do not see that happening (because we would like to include some more things in such an release…). Another "solution" would be to break our stability guarantee to make no breaking change, but I think we all agree that we generally do not want to do this (If we do that now, who draws the line when it's ok to do a breaking change and when it's not ok?).

That said both, rocket and uuid have not reached 1.0 yet. That means if you decided to use them you should except breaking things here. (To be clear: It is absolutely ok for them to do this changes, but don't expect a library that has declared some form of stable feature set to follow them.)

For using them as public dependency of a crate that already has reached 1.0 means that you must accept that there may be breaking changes that you cannot support any more. Also that's ok in my point of view (at least as long as the old version does not contain any security issues, which is not the case here).

There are several workarounds for this issue that neither depends on rocket, nor on diesel or uuid.

• It is possible to add support for own types to diesel. (See this test for an example). This could be used in combination with the new type pattern to add support for whatever uuid version you want outside of diesel.
• Basically the same is possible on the rocket side

Therefore I do not see this as critical issue that needs to be solved now.

[DoumanAsh]

That means if you decided to use them you should except breaking things here

That's actually also argument on upgrading your public dependencies until their reach 1.0 (though only as minor version, not patch as user might need to perform actions to ensure compatibility, i.e. upgrading uuid)

Though it is not critical as you say.

[terry90]

@weiznich I see your point and I agree. Thanks for digging through

Edit: As @weiznich said, I guess it should look like that https://github.com/terry90/uuid_diesel_0_7_pg so not a big deal. Feel free to use it in your project if you don't want to write it.
This is not usable with the master branch of diesel until #1916 is merged though, I'm using 1.3.3

[SergioBenitez]

That said both, rocket and uuid have not reached 1.0 yet. That means if you decided to use them you should except breaking things here. (To be clear: It is absolutely ok for them to do this changes, but don't expect a library that has declared some form of stable feature set to follow them.)

To be clear, strictly speaking, it is _not_ okay, in Rust, for pre 1.0 libraries to have breaking changes between 0.v.a and 0.v.b releases, where b > a. In other words, all 0.v.x releases must be backwards compatible. Rocket takes this very seriously and, to my knowledge, has never violated this guarantee.

I don't have any particularly good ideas about how to resolve the issue at hand, unfortunately. Perhaps Rocket can widen the accepted range of uuid. Unfortunately this isn't a possibility with Rocket as Rocket's Uuid type derefs into the uuid crate's Uuid, so any breaking changes are inherited.

[terry90]

@SergioBenitez I don't see the problem, what is the use of Rocket's Uuid aside FromParam ?
Once you have uuid::Uuid, what forbid you to use it like you want ? Here a simple newtype wrapper fixes this issue.
I don't think it's Rocket's responsibility to fix this btw.

[SergioBenitez]

@terry90 The problem, as I stated, is that the wrapper derefs to the internal type, revealing all of its methods. So, if the internal type changes due to a version bump, Rocket user's code breaks.

[terry90]

Oh yes, sorry I misunderstood. Then, would it be acceptable to create an additional wrapper type ?

[julien1619]

Hi! Is there any plan to upgrade uuid despite the strong arguments given here? I would really benefit from this update.

Currently uuid is activated using a feature flag, so, is it possible to add a new feature flag, like uuid-next that will expose an uuid implemented in Diesel code and internally depending on the new version of the uuid crate? It could avoid breaking compatibility, or I misunderstood something?

[weiznich]

@julien1619 See #1915 :wink:

[Taha-Firoz]

@weiznich I can't seem to get Diesel to work with the Rocket Contrib Uuid on version 0.7.4. I can get my project to compile with uuid 0.7.4 however it doesn't work when I run a query with it, the same thing happens when I use a rocket contrib uuid in a query.
However, when I move back my Uuid crate to 0.6 everything works, I can run queries with either the rocket contrib Uuid or the uuid crate's Uuid.

I don't have a problem with using 0.6 but it's confusing me as to why it isn't working since you've added support for 0.7. I also tried moving my version down to 0.7.0 but I can't run any queries until I'm back on 0.6.

Any help would be appreciated, Thank you.