search

jonasbb / serde_with

This crate provides custom de/serialization helpers to use in combination with serde's `with`-annotation and with the improved `serde_as`-annotation.
https://docs.rs/serde_with
Apache License 2.0
636 stars 67 forks source link

Fix building on nightly. #749

Closed iddm closed 3 months ago

iddm commented 3 months ago

It fixes an issue when the crate couldn't be built using nightly due to time dependency.

See https://github.com/rust-lang/rust/issues/125319.

iddm commented 3 months ago

The problem is reproducible using currently nightly rust:

Without the fix, cargo +nightly test

error[E0282]: type annotations needed for `Box<_>`
  --> /home/<user>/.cargo/registry/src/index.crates.io-6f17d22bba15001f/time-0.3.20/src/format_description/parse/mod.rs:83:9
   |
83 |     let items = format_items
   |         ^^^^^
...
86 |     Ok(items.into())
   |              ---- type must be known at this point
   |
help: consider giving `items` an explicit type, where the placeholders `_` are specified
   |
83 |     let items: Box<_> = format_items
   |              ++++++++

   Compiling url v2.5.0
   Compiling num-bigint v0.4.5
   Compiling num-iter v0.1.45
   Compiling regex-automata v0.4.5
For more information about this error, try `rustc --explain E0282`.
error: could not compile `time` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...

With the update dependency, it builds fine. In the future, I suggest running the tests using +nightly to prevent this from happening again.

P.S. Running the tests using nightly might only seem unnecessary, but it is crucial: when there is another crate depending one way or another - on serde_with, and this crate is built with nightly for testing, for example, to use the sanitiser - it should be possible to build it this way.

YaacovHazan commented 3 months ago

@iddm why the Rust CI cancelled?

iddm commented 3 months ago

@iddm why the Rust CI cancelled?

The MSRV of this crate has to be increased as well. Now it is too old and the new time crate requires it to be higher.

iddm commented 3 months ago

Thanks for rasing that issue. I think this only needs a bump in the Cargo.lock. An update of the Cargo.toml possible but the ~ version requirement is there on purpose. You can simply bump the MSRV everywhere to 1.67 to get the CI going.

The CI is checking on nightly, not sure why you imply otherwise.

Thanks for the response! So I just add the ~ back and the old version 0.3.11 then and push an updated Cargo.lock?

jonasbb commented 3 months ago

I created a smaller PR with #750 that for now doesn't touch chrono, thus avoiding those errors.