search

TimelyDataflow / abomonation

A mortifying serialization library for Rust
MIT License
317 stars 30 forks source link

NonZeroI16 is nightly only #15

Closed ryzhyk closed 5 years ago

ryzhyk commented 5 years ago

This breaks abomonation, and, because differential depends on abomonation 0.7.*, by transitivity breaks differential as well:

https://github.com/TimelyDataflow/abomonation/blob/fe437396503117213261f8ee75dd1834ffc74a11/src/lib.rs#L292

The problem is that NonZeroI16 and related types are deprecated after rustc 1.26 and also marked nightly only for some reason.

frankmcsherry commented 5 years ago

This one: https://doc.rust-lang.org/std/num/struct.NonZeroI16.html ?

It should be stable as of 1.34.

ryzhyk commented 5 years ago

Oh, I see. I think this is still a problem, though, as I would prefer not to depend on the latest compiler. E.g., folks running the build farm here insist on using Rust from the Linux distro, which is apparently 1.28 on rhel.

frankmcsherry commented 5 years ago

Ok. There is a bit of a conflict in that other people would like to be using NonZeroI* types. :D

Are you able to patch in version 0.7.0 for abomonation by using:

https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#overriding-dependencies

ryzhyk commented 5 years ago

I tried 

[patch.crates-io]
abomonation = "0.7.0"

but

error: failed to resolve patches for `https://github.com/rust-lang/crates.io-index`

Caused by:
  patch for `abomonation` in `https://github.com/rust-lang/crates.io-index` points to the same source, but patches must point to different sources
ryzhyk commented 5 years ago

Isn't there a way to put these behind a feature gate, so that people with recent compilers can enjoy the functionality without breaking stuff for less up-to-date environments?

frankmcsherry commented 5 years ago

I've just pushed a 0.7.2 that has them behind the #[cfg(feature = "nonzero")] Rust feature flag (thanks @benesch!). But, I think you'll need to get the version up at some point. I can't build timely or differential with 1.28.0 (via rustup) due to edition issues. The tests for abomonation fail because Ipv6Addr::LOCALHOST only got added in 1.30, etc.

If only there were a company that could virtualize environments to allow you to use more recent version... ;)

frankmcsherry commented 5 years ago

Oops. We'll have to return to this. That feature flag apparently sucks. Or at least, it is not actually visible (this effectively turns off support for NonZeroI*).

frankmcsherry commented 5 years ago

One thing you can try, though, is to pin Cargo.lock to a specific abomonation version. As long as you don't update it, you should sit at 0.7.0. It's not great, but .. none of this is. :)

ryzhyk commented 5 years ago

Sorry, I'm confused. 0.7.2 seems to work. Are you saying that it is going to break again soon or will @benesch's PR solve the issue when merged?

Thanks, Leonid

benesch commented 5 years ago

Assuming I did things right, the next version will restore support for abomonating NonZeroI types in Rust >= 1.34 without breaking compilation for Rust < 1.34. 0.7.2 solved your compilation issue at the expense of breaking abomonation of NonZeroI for everyone, whoops!

ryzhyk commented 5 years ago

Got it, thanks!

frankmcsherry commented 5 years ago

The reached conclusion here (with @ryzhyk's blessing) is to move forward and not sweat the recentness of nonzero signed integers. Feel free to reopen if that seems in error.