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

Backports/2.1.4 #3959

Merged
merged 14 commits into from
Mar 15, 2024
Merged

Backports/2.1.4 #3959

merged 14 commits into from
Mar 15, 2024

Conversation

1Dragoon
Copy link

@1Dragoon 1Dragoon commented Mar 9, 2024

Hopefully I'm doing this correctly:

  • Fix impl SqlOrd postgres > postgres_backend feature flag.
  • Allow Queryable to be used with multiple table names
  • Fix chrono deprecation warnings
  • Adjusted some unit tests to reflect chrono changes

Note: One unit test is failing, but I'm uncertain whether it's caused by my local build environment. It was already failing without any of these changes.

failures:

---- pg::expression::extensions::interval_dsl::tests::intervals_match_pg_values_f64 stdout ----
thread 'pg::expression::extensions::interval_dsl::tests::intervals_match_pg_values_f64' panicked at /home/1Dragoon/.cargo/registry/src/index.crates.io-6f17d22bba15001f/quickcheck-1.0.3/src/tester.rs:165:28:
[quickcheck] TEST FAILED. Arguments: (-2578.740551172832)

snf and others added 3 commits March 9, 2024 12:28
fixing test for Queryable

fixing tests for Queryable

rustfmt tests

Update diesel_derives/tests/queryable.rs

Co-authored-by: Georg Semmler <[email protected]>

fixing tests for Queryable

rustfmt tests
The PostgreSQL types are included when using the postgres_backend
feature flag, the same feature flag should be used for the SqlOrd
implementation of these types.
Cargo.toml Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
@1Dragoon
Copy link
Author

1Dragoon commented Mar 10, 2024

So this is odd, clippy is tossing lints about chrono deprecated methods, yet chrono 0.4.20 doesn't have the replacement methods. Bumping chrono to anything higher (even from 0.4.20 to 0.4.21) results in a libc conflict in the compile tests, which can't be resolved because r2d2 has a parking_lot dependency that requires a lower version than what chrono and time require.

It does look like r2d2 has an interest in removing parking_lot according to this: sfackler/r2d2#140

@1Dragoon 1Dragoon force-pushed the backports/2.1.4 branch 4 times, most recently from 03810ce to 6e0aef1 Compare March 10, 2024 05:48
@weiznich
Copy link
Member

Thanks for opening this PR.
For the chrono issues I would prefer to pull in the relevant changes from the master branch:

#3953
and
4c7b6ea

That should hopefully resolve those issues without changing the minimal supported chrono version and prevent that breaking change.

For the tests we switch to the suggested alternatives, for the real
implementation we continue to use the deprecated functions as the
suggested alternatives do not exist in our minimal supported chrono version.
@1Dragoon
Copy link
Author

1Dragoon commented Mar 10, 2024

Sure, merged those in. The CI is failing here still:

image

I've seen this before. In my local setup I had to fix this by modifying ./bin/test to specify the db type as a feature directly, maybe the CI needs to be adjusted somehow?

Here are the specific changes: https://github.com/diesel-rs/diesel/pull/3959/files#diff-4e821476d5e4f5626c6f9fca337a47afe96a5e3daf23bccb02ad0bb109a9a407

Also note that in my local setup, mysql is failing numerous tests, but I'm not sure the cause as I just used the docker-compose method mentioned in the contributor document. Everything else works fine.

@1Dragoon 1Dragoon force-pushed the backports/2.1.4 branch 4 times, most recently from 02a03fe to 012ae9c Compare March 14, 2024 03:35
@1Dragoon
Copy link
Author

No matter what I do I can't seem to get the MSRV to pass

We also need to fix the sqlite-bundled job to the yesterday nightly due
to rust-lang/rust#120830
@weiznich
Copy link
Member

I pushed another commit that hopefully fixes the MSRV build. Will try to do a final review and a release tomorrow.

@weiznich weiznich merged commit c96c870 into diesel-rs:2.1.x Mar 15, 2024
31 checks passed
@weiznich
Copy link
Member

Published as https://github.com/diesel-rs/diesel/releases/tag/v2.1.5

Thanks again for your work on the backports.

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.

4 participants