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

Prepare a 2.0.4 Release #3597

Closed
wants to merge 7 commits into from

Conversation

weiznich
Copy link
Member

As suggested in #3594 it might be reasonable to cut a 2.0.4 release to bring the r-a fix faster to our users.

Thanks for @adamchalmers for preparing the branch.

@diesel-rs/core I would like to cut a release on monday if noone as objections.

@weiznich weiznich requested a review from a team April 14, 2023 18:06
This commit fixes an issue where out of bound text/binary binds on `sql_query` using
the sqlite backend causes a panic in an internal drop impl. It was
caused that we pushed to the list of released binds before we actually
checked whether that bind was possible or not. This caused us trying to
bind to the same invalid bind again in the drop impl, while being on
the normal error path. It's fixed by just moving the relevant push after
the actual bind. In addition a regression test for this problem is added.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamchalmers Can you remove this file. It shouldn't be here, as the 2.0.x branch uses the macro_rules based table! macro.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

weiznich and others added 2 commits April 14, 2023 13:13
It turns out that we relay on a type resolution hack for our derives
that only work with rust 2015. That's not a problem with rustc, as it
uses whatever edition a crate is using. So it will use rust-2015 for
name resolution in the expansion of our proc macros, due to the fact
that `diesel_derives` is a edition = 2015 crate. The problem is that
rust-analyzer does not understand this, as they use basically the name
resolution rules from the 2018 edition. (To be clear that's something
that's not "standard" conform there, but it does not seem to be a
priority to fix that).
As this affects our core type system (== all the SQL types, …), that has
pretty serve effects on what users see in their IDE.

Fortunately the workaround is simple as we just need to use an
alternative way to provide an diesel item in diesel itself to make our
macros work there. It seems like both rustc and rust-analyzer agree on
the `extern crate self as diesel;` solution.

After this basically all known cases of rust-analyzer issues are gone
away (at least as far as I tested).
@adamchalmers
Copy link

Hmm, the "Check Minimal support rust version (1.56.0)" action is failing because libsqlite3-sys 0.26 requires 1.58 (because it uses format!("{x}") syntax).

But I don't know why it's even trying version 0.26, because it should use the minimal version.

@nickelc
Copy link

nickelc commented Apr 15, 2023

Is it possible to pick up #3435 as well?

@Ten0
Copy link
Member

Ten0 commented Apr 15, 2023

Is it possible to pick up xxx as well?

Should we release a 2.1 and get everything in? Or are there breaking changes other than bugfixes that I missed?

@weiznich
Copy link
Member Author

@Ten0 Its my plan to do a 2.1 release sometime soon, but its not ready to release it now due to #3566. I have at least a partial fix for that but I had now time to open a PR yet.

This commit fixes an issue where empty owned strings (and `Vec<u8>`)
were serialised to null values instead of to just empty values. This
happened due to a passing null ptrs to the sqlite API at the wrong
location.

Also two regression tests are added for this issue.
@adamchalmers
Copy link

Is it possible to pick up #3435 as well?

I've added this commit to the release. @weiznich let me know if you want me to remove it.

diesel_tests/Cargo.toml Outdated Show resolved Hide resolved
adamchalmers and others added 2 commits April 17, 2023 08:18
Co-authored-by: Georg Semmler <github@weiznich.de>
@adamchalmers
Copy link

OK, all tests pass now!

@weiznich
Copy link
Member Author

Thanks for updating the PR. I did not find the time to day to to the release, will try that again tomorrow.

@weiznich
Copy link
Member Author

I've manually merged this to the relevant branch + done the release.

@weiznich weiznich closed this Apr 18, 2023
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.

5 participants