sfackler / rust-postgres

Native PostgreSQL driver for the Rust programming language
Apache License 2.0
3.42k stars 436 forks source link

(dev-)dependency eui48 is abandoned and will stop compiling #1073

Closed ds-cbo closed 10 months ago

ds-cbo commented 10 months ago

While compiling tokio-postgres with cargo 1.75.0-nightly (794d0a825 2023-10-03), I got the following error:

The following warnings were discovered during the build. These warnings are an
indication that the packages contain code that will become an error in a
future release of Rust. These warnings typically cover changes to close
soundness problems, unintended or undocumented behavior, or critical problems
that cannot be fixed in a backwards-compatible fashion, and are not expected
to be in wide use.

Each warning should contain a link for more information on what the warning
means and how to resolve it.

To solve this problem, you can try the following approaches:

- If the issue is not solved by updating the dependencies, a fix has to be
implemented by those dependencies. You can help with that by notifying the
maintainers of this problem (e.g. by creating a bug report) or by proposing a
fix to the maintainers (e.g. by creating a pull request):

  - rustc-serialize@0.3.24
  - Repository: https://github.com/rust-lang/rustc-serialize
  - Detailed warning command: `cargo report future-incompatibilities --id 2 --package rustc-serialize@0.3.24`

- If waiting for an upstream fix is not an option, you can use the `[patch]`
section in `Cargo.toml` to use your own version of the dependency. For more
information, see:
https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html#the-patch-section

The package `rustc-serialize v0.3.24` currently triggers the following future incompatibility lints:
> warning: impl method assumes more implied bounds than the corresponding trait method
>     --> /Users/me/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-serialize-0.3.24/src/serialize.rs:1155:41
>      |
> 1155 |     fn decode<D: Decoder>(d: &mut D) -> Result<Cow<'static, T>, D::Error> {
>      |                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace this type to make the impl signature compatible: `Result<Cow<'a, T>, <D as serialize::Decoder>::Error>`
>      |
>      = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
>      = note: for more information, see issue #105572 <https://github.com/rust-lang/rust/issues/105572>
>      = note: `-W implied-bounds-entailment` implied by `-W future-incompatible`
>      = help: to override `-W future-incompatible` add `#[allow(implied_bounds_entailment)]`
>

However, it is no longer possible to file a report with rustc-serialize since they got archived in August 2021.

The dependency within rust-postgres which pulls in rustc-serialize is eui48. They have been made aware of this issue in January 2022, but there is no response from the author and seemingly only maintainer of the repo @abaumhauer who hasn't committed to the repo since August 2020.

To make sure that rust-postgres will still compile with future versions of the Rust compiler, I would suggest replacing the eui48 dependency. Others have seemed to move to one of:

Out of these, I would suggest using the first one since it has the least amount of dependencies and is probably least likely to break as a result.

sfackler commented 10 months ago

rustc-serialize appears to be an optional dependency of an optional dependency. It's not clear to me why cargo would be complaining about it unless you have actually activated it?

ds-cbo commented 10 months ago

eui48 is a non-optional dev-dependency of tokio-postgres, so is already included with a simple cargo test. rustc-serialize is an optional but enabled-by-default feature of eui48 versions 1.0 and up, but required dependency on the 0.4 branches.

We might also be able to get away with this issue partially by setting default-features = false on our eui48-1 dependencies, but that won't fix eui48-04 being a hard dev-dependency. Perhaps make that one disabled by default? Then at least cargo test will keep working in the future.

sfackler commented 10 months ago

You can't define optional dev-dependencies unfortunately. https://github.com/rust-lang/cargo/issues/1596

ds-cbo commented 10 months ago

That's a bummer. Would you agree to disable default features (that's only backwards compatability support for rustc-serialize) for eui48-1 now and eventually drop support for eui48-04 when https://github.com/rust-lang/rust/issues/105572 is turned into a hard compilation error?

sfackler commented 10 months ago

Yeah that seems like the best route forward.

sfackler commented 10 months ago

Actually, I think we'd be better off just removing the 0.4 tests rather than dropping support entirely for now. Once the MSRV of tokio-postgres is on a version that doesn't build rustc-serialize we can remove it entirely.

ds-cbo commented 10 months ago

I already had that approach in mind, PR is incoming