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

Allow range of versions for dependencies #664

Closed
wants to merge 1 commit into from

Conversation

Sytten
Copy link
Contributor

@Sytten Sytten commented Jul 14, 2023

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 (rust-lang/rust#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

  • Allowing a range of versions of dependencies pre 1

@Sytten
Copy link
Contributor Author

Sytten commented Jul 14, 2023

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
Copy link
Contributor Author

Sytten commented Jul 14, 2023

The cargo WG seems to go in the direction of recommending libraries to commit a lock file for the CI to work (rust-lang/cargo#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
Copy link

epage commented Jul 17, 2023

We've not decided yet on Cargo.lock

However, we have added recommendations for version requirements

@Sytten
Copy link
Contributor Author

Sytten commented Jul 17, 2023

@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
Copy link

epage commented Jul 17, 2023

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 rust-lang/rust#44663 (comment)

@Sytten Sytten closed this Jul 21, 2023
@Sytten Sytten mentioned this pull request Jul 21, 2023
1 task
@Sytten Sytten deleted the fix/range-versions branch July 21, 2023 16:06
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