whisperfish / libsignal-service-rs

A Rust version of the libsignal-service-java library for communicating with Signal servers.
https://whisperfish.github.io/libsignal-service-rs/libsignal_service
GNU Affero General Public License v3.0
68 stars 31 forks source link

RFC: minimum supported Rust version #62

Open rubdos opened 3 years ago

rubdos commented 3 years ago

I'd like to introduce an MSRV for libsignal-service-rs, because we (Whisperfish) are constrained to 1.44 by Jolla. I think we can persuade Jolla relatively easily to bump the version, but this will have a significant delay (2 months every time).

@gferon, @Michael-F-Bryan, would you be okay with making everything compatible down to 1.44? If so, I'd kindly ask @gferon to alter the Github Actions to have a fixed version on 1.44. I'll gladly write the patches for compat, both here and in -protocol!

kunerd commented 3 years ago

Hopefully this will help. Currently the following things won't work with rust 1.44:

error[E0658]: `match` is not allowed in a `const fn`
   --> /home/mersdk/.cargo/git/checkouts/libsignal-protocol-rs-bd2698d95a103c0d/2d00a19/libsignal-protocol/src/errors.rs:104:9
    |
104 | /         match code {
105 | |             sys::SG_ERR_NOMEM => Some(InternalError::NoMemory),
106 | |             sys::SG_ERR_INVAL => Some(InternalError::InvalidArgument),
107 | |             sys::SG_ERR_UNKNOWN => Some(InternalError::Unknown),
...   |
136 | |             _ => None,
137 | |         }
    | |_________^
    |
    = note: see issue #49146 <https://github.com/rust-lang/rust/issues/49146> for more information
    = help: add `#![feature(const_if_match)]` to the crate attributes to enable

error[E0658]: `match` is not allowed in a `const fn`
   --> /home/mersdk/.cargo/git/checkouts/libsignal-protocol-rs-bd2698d95a103c0d/2d00a19/libsignal-protocol/src/errors.rs:142:9
    |
142 | /         match self {
143 | |             InternalError::NoMemory => sys::SG_ERR_NOMEM,
144 | |             InternalError::InvalidArgument => sys::SG_ERR_INVAL,
145 | |             InternalError::Unknown => sys::SG_ERR_UNKNOWN,
...   |
166 | |             InternalError::SerializationError => sys::SG_ERR_INVALID_PROTO_BUF,
167 | |         }
    | |_________^
    |
    = note: see issue #49146 <https://github.com/rust-lang/rust/issues/49146> for more information
    = help: add `#![feature(const_if_match)]` to the crate attributes to enable
error[E0658]: procedural macros cannot be expanded to expressions
  --> /home/mersdk/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-util-0.6.2/src/sync/poll_semaphore.rs:21:29
   |
21 |               inner: Box::pin(async_stream::stream! {
   |  _____________________________^
22 | |                 loop {
23 | |                     match semaphore.clone().acquire_owned().await {
24 | |                         Ok(permit) => yield permit,
...  |
27 | |                 }
28 | |             }),
   | |_____________^
   |
   = note: see issue #54727 <https://github.com/rust-lang/rust/issues/54727> for more information
   = help: add `#![feature(proc_macro_hygiene)]` to the crate attributes to enable
gferon commented 3 years ago

Sounds good to me! Feel free to prepare a PR to try it out 😄

rubdos commented 3 years ago

error[E0658]: procedural macros cannot be expanded to expressions

That may be a deal-breaker. If 1.44-dev means it's kinda-nightly, the feature flag should fix that. That would mean MSRV 1.44-nightly OR 1.46-stable.

kunerd commented 3 years ago

Maybe I'm wrong, but the macro feature is missing in tokio_util, so we need to add #![feature(proc_macro_hygiene)] there or not?

rubdos commented 3 years ago

Maybe I'm wrong, but the macro feature is missing in tokio_util, so we need to add #![feature(proc_macro_hygiene)] there or not?

Indeed, because they removed it themselves and upped their minimum supported version. I hadn't even thought about that yet.

proc_macro_hygiene is used a lot nowadays, I don't think we're getting around that. I think we better push Jolla to up their compiler. Maybe there's a tokio-util that we can manually pin/downgrade to in Cargo.lock that has the feature flag, but I don't think so, especially since Tokio 1.0 has been released a few weeks ago. I'm currently doing the migration to Tokio 1.0 in WF, fwiw.

kunerd commented 3 years ago

From my quick cargo tree search it seems that there are 3-4 transitive dependencies pointing to that exact same version, so I don't think we can get around this easily. I think we should get Jolla to update to the latest stable and/or give us some hints to get the sailfishos/rust project building.