search

SeaQL / sea-query

🔱 A dynamic SQL query builder for MySQL, Postgres and SQLite
https://www.sea-ql.org
Other
1.18k stars 195 forks source link

Allow range of versions for dependencies #664

Closed Sytten closed 1 year ago

Sytten commented 1 year ago

PR Info

In my diesel integration, the version of those two dependencies is not able to be the same as the one diesel uses because diesel uses a range and we still don't have public dependencies implemented (https://github.com/rust-lang/rust/issues/44663) so it fails to build because some trait are not implemented on the same version of the children dependencies.

I think this should not impact people using sqlx, but we have to test it to be sure.

Changes

Sytten commented 1 year ago

Hugh sadly it seems like it changes the location of the problem, now sqlx complains because the version is wrong. I am not sure what we should do in this, create features with different version ipnetwork19, ipnetwork20? Since sqlx is the primary target for sea-query I guess we can also accept that the diesel integration will have a bad experience. @billy1624 @tyt2y3 if you have other ideas...

Sytten commented 1 year ago

The cargo WG seems to go in the direction of recommending libraries to commit a lock file for the CI to work (https://github.com/rust-lang/cargo/issues/8728), this is likely the way to go for the diesel integration (along with some documentation on why the user build might fail and the relevant cargo update --precise command) if we want to keep a fixed version of the dependencies. If this is acceptable I will close the PR and commit the lock file in #658.

epage commented 1 year ago

We've not decided yet on Cargo.lock

However, we have added recommendations for version requirements

Sytten commented 1 year ago

@epage Yeah I know, but until cargo introduces a way to mark public dependencies I don't see any other way to make my changes work :/ When you have 

Lib -> Dep A -> Subdep
    -> Dep B -> Subdep

There is simply no way to ensure both Subdep will the same version for CI to work.

epage commented 1 year ago

I understand, I was just wanting to clarify where we are currently at. I'm hopeful we'll be changing the guidelines soon. I know for my own projects, I have switched to committing my Cargo.lock files and I've talked to other maintainers who have done the same.

Also, we have been discussing scaling back the public-dependencies RFC to not affect package resolution, see https://github.com/rust-lang/rust/issues/44663#issuecomment-1572992175