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

1.4.x: Update 'libsqlite3-sys' to 0.22 #2797

Merged
merged 2 commits into from
Jun 4, 2021

Conversation

SergioBenitez
Copy link
Contributor

Update libsqlite3-sys. Without this update, Rocket's rocket_contrib, which depends on diesel, cannot update other dependencies, resulting in quite a few out of date dependencies.

Unless the CI does something special, this will almost certainly fail the CI. The 1.4.x branch seems to be in a rather broken state. rust-toolchain points to an MSRV that is incorrect, and the source doesn't build due to a deny. In any case, here's the PR.

@SergioBenitez SergioBenitez changed the title Update 'libsqlite3-sys' to 0.22 1.4.x: Update 'libsqlite3-sys' to 0.22 May 29, 2021
@Rustin170506 Rustin170506 requested a review from a team May 30, 2021 02:55
@weiznich
Copy link
Member

Looks good from my side, but this needs a Changelog entry before merging + a rebase/merge of the commit that fixes the mysql CI

Unless the CI does something special, this will almost certainly fail the CI. The 1.4.x branch seems to be in a rather broken state. rust-toolchain points to an MSRV that is incorrect, and the source doesn't build due to a deny. In any case, here's the PR.

To clarify that neither is the branch in a "broken" state nor is the MSRV set incorrectly. (We don't define MSRV as diesel and any version of it's dependencies will built at that rust version, but only: There is a set of dependency versions where diesel and all dependencies build at that version. We check this by using -Z minimal-versions + some manual updates to fix broken dependencies). CI is expected to succeed. It set's RUSTFLAGS=--cap-lints=warn in a similar way than cargo handles denys in dependencies.

@SergioBenitez
Copy link
Contributor Author

SergioBenitez commented Jun 4, 2021

Unless the CI does something special, this will almost certainly fail the CI. The 1.4.x branch seems to be in a rather broken state. rust-toolchain points to an MSRV that is incorrect, and the source doesn't build due to a deny. In any case, here's the PR.

To clarify that neither is the branch in a "broken" state nor is the MSRV set incorrectly. (We don't define MSRV as diesel and any version of it's dependencies will built at that rust version, but only: There is a set of dependency versions where diesel and all dependencies build at that version. We check this by using -Z minimal-versions + some manual updates to fix broken dependencies). CI is expected to succeed. It set's RUSTFLAGS=--cap-lints=warn in a similar way than cargo handles denys in dependencies.

Got it. So the CI does do something special. This deviates significantly from the norm in Rust ecosystem, where, aside from system-level dependencies, crates are generally expected to compile with cargo check. I also see no mention of this at all anywhere in the repository. Specifically, doing as CONTRIBUTING suggests results in the myriad errors I allude to.

Looks good from my side, but this needs a Changelog entry before merging + a rebase/merge of the commit that fixes the mysql CI

Done, though I wasn't quite sure which date to use in the CHANGELOG, so I've just used today's.

@SergioBenitez
Copy link
Contributor Author

The CI is failing on nightly because derive-error-chain uses dependency versions like 0.11.x which appear to no longer be accepted by Cargo. This seems to be caused by rust-lang/cargo#9508. Polling there.

@weiznich
Copy link
Member

weiznich commented Jun 4, 2021

Thanks for the update. I've field https://github.com/rust-lang/rust/issues/85986 for the regression, let's see what's the response there. I will wait till monday or so with the actual release to see if we need to take actions on that cargo regression or not.

Got it. So the CI does do something special. This deviates significantly from the norm in Rust ecosystem, where, aside from system-level dependencies, crates are generally expected to compile with cargo check. I also see no mention of this at all anywhere in the repository. Specifically, doing as CONTRIBUTING suggests results in the myriad errors I allude to.

Right it does something special. I do not want to include that into CONTRIBUTING as we normally do not accept PR's to any other branch than master, beside special cases like this one. The next major release will likely fix that issue, by just removing #[deny(warnings)] before release.

@weiznich weiznich merged commit c6e00e3 into diesel-rs:1.4.x Jun 4, 2021
@weiznich
Copy link
Member

weiznich commented Jun 7, 2021

Just want to leave a comment here that I haven't found the time to do a release today. I will try finding some time tomorrow.

@weiznich
Copy link
Member

weiznich commented Jun 8, 2021

Published as 1.4.7 🎉

@SergioBenitez
Copy link
Contributor Author

Awesome. Thank you!

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