fix: repair for rustc rustc 1.71.0-nightly (ce5919fce 2023-05-15)

[johnrichardrinehart]

cargo build --release --features avif-decoder

with

$ cargo --version && rustc --version           
cargo 1.71.0-nightly (13413c64f 2023-05-10)
rustc 1.71.0-nightly (ce5919fce 2023-05-15)

yields

$ cargo build --release --features avif-decoder
    Blocking waiting for file lock on package cache
    Updating crates.io index
    Blocking waiting for file lock on package cache
   Compiling mp4parse v0.12.0
error[E0277]: cannot multiply `u64` by `NonZeroU8`
    --> /home/john/.cargo/registry/src/index.crates.io-6f17d22bba15001f/mp4parse-0.12.0/src/lib.rs:2547:62
     |
2547 |                 static_assertions::const_assert!(<$lhs>::MAX * <$rhs>::MAX <= <$output>::MAX);
     |                                                              ^ no implementation for `u64 * NonZeroU8`
...
2557 | impl_mul!((U8, std::num::NonZeroU8) => (U16, u16));
     | -------------------------------------------------- in this macro invocation
     |
     = help: the trait `Mul<NonZeroU8>` is not implemented for `u64`
     = help: the following other types implement trait `Mul<Rhs>`:
               <&'a u64 as Mul<u64>>
               <&u64 as Mul<&u64>>
               <u64 as Mul<&u64>>
               <u64 as Mul>
     = note: this error originates in the macro `impl_mul` (in Nightly builds, run with -Z macro-backtrace for more info)

note: erroneous constant used
    --> /home/john/.cargo/registry/src/index.crates.io-6f17d22bba15001f/mp4parse-0.12.0/src/lib.rs:2557:1
     |
2557 | impl_mul!((U8, std::num::NonZeroU8) => (U16, u16));
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: this note originates in the macro `static_assertions::const_assert` which comes from the expansion of the macro `impl_mul` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: cannot multiply `u64` by `NonZeroU8`
    --> /home/john/.cargo/registry/src/index.crates.io-6f17d22bba15001f/mp4parse-0.12.0/src/lib.rs:2547:62
     |
2547 |                 static_assertions::const_assert!(<$lhs>::MAX * <$rhs>::MAX <= <$output>::MAX);
     |                                                              ^ no implementation for `u64 * NonZeroU8`
...
2558 | impl_mul!((U32, std::num::NonZeroU8) => (U32MulU8, u64));
     | -------------------------------------------------------- in this macro invocation
     |
     = help: the trait `Mul<NonZeroU8>` is not implemented for `u64`
     = help: the following other types implement trait `Mul<Rhs>`:
               <&'a u64 as Mul<u64>>
               <&u64 as Mul<&u64>>
               <u64 as Mul<&u64>>
               <u64 as Mul>
     = note: this error originates in the macro `impl_mul` (in Nightly builds, run with -Z macro-backtrace for more info)

note: erroneous constant used
    --> /home/john/.cargo/registry/src/index.crates.io-6f17d22bba15001f/mp4parse-0.12.0/src/lib.rs:2558:1
     |
2558 | impl_mul!((U32, std::num::NonZeroU8) => (U32MulU8, u64));
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: this note originates in the macro `static_assertions::const_assert` which comes from the expansion of the macro `impl_mul` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0277`.
error: could not compile `mp4parse` (lib) due to 2 previous errors

This is an mp4parse which is addressed in mozilla/mp4parse-rust#405 . Applying the fixes described, there, yields a image-rs/image issue:

$ cargo build --release --features avif-decoder
   Compiling image v0.24.6 (/home/john/code/repos/forks/image)
warning: unused imports: `ImageFormatHint`, `UnsupportedErrorKind`, `UnsupportedError`
  --> src/dynimage.rs:20:20
   |
20 | use crate::error::{ImageFormatHint, UnsupportedError, UnsupportedErrorKind};
   |                    ^^^^^^^^^^^^^^^  ^^^^^^^^^^^^^^^^  ^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

error[E0277]: the trait bound `Option<&[u8]>: AsRef<[u8]>` is not satisfied
  --> src/codecs/avif/decoder.rs:40:24
   |
40 |             .send_data(coded, None, None, None)
   |              --------- ^^^^^ the trait `AsRef<[u8]>` is not implemented for `Option<&[u8]>`
   |              |
   |              required by a bound introduced by this call
   |
note: required by a bound in `dav1d::Decoder::send_data`
  --> /home/john/.cargo/registry/src/index.crates.io-6f17d22bba15001f/dav1d-0.6.1/src/lib.rs:76:25
   |
76 |     pub fn send_data<T: AsRef<[u8]>>(
   |                         ^^^^^^^^^^^ required by this bound in `Decoder::send_data`

error[E0599]: no method named `is_empty` found for enum `Option` in the current scope
  --> src/codecs/avif/decoder.rs:44:44
   |
44 |         let alpha_picture = if !alpha_item.is_empty() {
   |                                            ^^^^^^^^ method not found in `Option<&[u8]>`
   |
note: the method `is_empty` exists on the type `&[u8]`
  --> /rustc/ce5919fcef67103098219e1868f741e56fc90963/library/core/src/slice/mod.rs:156:5
help: consider using `Option::expect` to unwrap the `&[u8]` value, panicking if the value is an `Option::None`
   |
44 |         let alpha_picture = if !alpha_item.expect("REASON").is_empty() {
   |                                           +++++++++++++++++

error[E0277]: the trait bound `Option<&[u8]>: AsRef<[u8]>` is not satisfied
  --> src/codecs/avif/decoder.rs:47:28
   |
47 |                 .send_data(alpha_item, None, None, None)
   |                  --------- ^^^^^^^^^^ the trait `AsRef<[u8]>` is not implemented for `Option<&[u8]>`
   |                  |
   |                  required by a bound introduced by this call
   |
note: required by a bound in `dav1d::Decoder::send_data`
  --> /home/john/.cargo/registry/src/index.crates.io-6f17d22bba15001f/dav1d-0.6.1/src/lib.rs:76:25
   |
76 |     pub fn send_data<T: AsRef<[u8]>>(
   |                         ^^^^^^^^^^^ required by this bound in `Decoder::send_data`

error[E0599]: no method named `ok` found for enum `Option` in the current scope
  --> src/codecs/avif/decoder.rs:53:56
   |
53 |         let icc_profile = ctx.icc_colour_information().ok().map(|x| x.to_vec());
   |                                                        ^^
   |
note: the method `ok` exists on the type `std::result::Result<&[u8], mp4parse::Error>`
  --> /rustc/ce5919fcef67103098219e1868f741e56fc90963/library/core/src/result.rs:631:5
help: consider using `Option::expect` to unwrap the `std::result::Result<&[u8], mp4parse::Error>` value, panicking if the value is an `Option::None`
   |
53 |         let icc_profile = ctx.icc_colour_information().expect("REASON").ok().map(|x| x.to_vec());
   |                                                       +++++++++++++++++
help: there is a method with a similar name
   |
53 |         let icc_profile = ctx.icc_colour_information().or().map(|x| x.to_vec());
   |                                                        ~~

Some errors have detailed explanations: E0277, E0599.
For more information about an error, try `rustc --explain E0277`.
warning: `image` (lib) generated 1 warning
error: could not compile `image` (lib) due to 4 previous errors; 1 warning emitted

It seems that master (and 0.24.6) are broken on nightly.

[fintelia]

Thanks for reporting this. We can't actually publish a release with a git dependency (crates.io will block the upload) so we'll have to wait for an update from mp4parse.

This bug is somewhat weird though because theoretically nightly shouldn't be breaking code that works on stable.

[johnrichardrinehart]

Thanks for reporting this. We can't actually publish a release with a git dependency (crates.io will block the upload) so we'll have to wait for an update from mp4parse.

Quick question: Does merging code to main/master automatically trigger a release in image-rs/image? If not, then I think it could make sense and be a good idea to merge something like this to the trunk so that other users aren't hitting the same problem. We can always patch, later, when mp4parse cuts a new release. I'm prodding only because mp4parse doesn't appear to be so active and I'm worried they may not respond for some time (months). My thinking is if a major downstream user like image was having problems on their trunk then it might accelerate a response from them. Obviously, I don't want to block a crates.io release, though.

Thanks for your timely response! Please note that there are 2 issues building image with nightly. One issue is the responsibility of mp4parse-rust (looks like they simply need to cut a new release). The other issue is with image. I could have made this a bit clearer by breaking the PR into two commits, but the solution is actually split cleanly across 2 files within the 1 commit:

1. The cargo.toml changes "fix" the mp4parse issue on nightly (happy to merge an appropriate fix for this only after they cut a new release or, generally, address Doesn't build with nightly mozilla/mp4parse-rust#405 ).
2. The avif changes address the other nightly compilation errors. These errors may be the responsibility of rustc authors, considering the compatibility guarantee that you mentioned. But, I'm actually not sure because I don't know if methods on Vec<T> are supposed to be callable on Option<Vec<T>>, which seems to be the root of the problem in the avif decoder logic.

This bug is somewhat weird though because theoretically nightly shouldn't be breaking code that works on stable.

For sure - I was surprised to encounter it. Were you able to reproduce (no stress if you didn't try). Would you prefer that I open an issue at https://github.com/rust-lang/rust ?

[fintelia]

We don't automatically release on every merge to main, but we try to keep the branch in a state where we could release whenever necessary.

I should hopefully have time this weekend to investigate what's going on here and see what we can do to fix it

[johnrichardrinehart]

We don't automatically release on every merge to main, but we try to keep the branch in a state where we could release whenever necessary.

I should hopefully have time this weekend to investigate what's going on here and see what we can do to fix it

No problem. I'll try to circle back in a week to see if there's any progress. Thanks @fintelia .

[fintelia]

I posted more details on the linked issue, but the conclusion is that this will also impact stable rustc starting with 1.70 but is fixed on the mp4parse main branch. Hopefully we'll get a new crates.io release soon (requested in mozilla/mp4parse-rust#387)

[johnrichardrinehart]

I posted more details on the linked issue, but the conclusion is that this will also impact stable rustc starting with 1.70 but is fixed on the mp4parse main branch. Hopefully we'll get a new crates.io release soon (requested in mozilla/mp4parse-rust#387)

I think this addresses the mp4parse problem. Does it also address the second problem addressed in this PR (described above)?

[fintelia]

I believe that the second problem may just be that mp4parse made some breaking changes going from 0.12 to the (unreleased) 0.16

[fintelia]

The mp4parse maintainers published a 0.12.1 release, so the build failures are addressed. However, there's now a 0.17.0 release. I adjusted this PR to upgrade to it, which seems to be working other than a few small rustfmt issues

[johnrichardrinehart]

Thanks @fintelia ! What's the process for merge/release?

[fintelia]

For merging, it is just a matter of getting CI passing. Releases don't happen on any particular schedule. It is just whenever we've accumulated enough unreleased changes or (more commonly) when someone creates an issue asking us to do a release so they can get some specific feature/fix

[johnrichardrinehart]

Totally makes sense - thanks for explaining. I'd like to merge so I fixed the cargo fmt CI issue. Please let me know if there's anything else that needs done.