1446538 - mozilla-central fails to build with latest rust nightly

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

A recent rust nightly update seems to have introduced changes that breaks building mozilla-central. Searchfox builds m-c with the latest rust nightly every day, so this started breaking with rust nightly that was pushed on or around 2018-03-15. I can reproduce in a local build if I use the latest rust 2018-03-15 nightly.

The error is this:

 1:30.77 error[E0658]: use of unstable library feature 'try_reserve': new API (see issue #48043)
 1:30.77   --> media/mp4parse-rust/mp4parse/src/lib.rs:59:20
 1:30.77    |
 1:30.77 59 |         return vec.try_reserve(size);
 1:30.77    |                    ^^^^^^^^^^^
 1:30.77    |
 1:30.77    = help: add #![feature(try_reserve)] to the crate attributes to enable
 1:30.77 
 1:30.81 error[E0308]: mismatched types
 1:30.81   --> media/mp4parse-rust/mp4parse/src/lib.rs:59:16
 1:30.81    |
 1:30.81 59 |         return vec.try_reserve(size);
 1:30.81    |                ^^^^^^^^^^^^^^^^^^^^^ expected (), found enum `std::collections::CollectionAllocErr`
 1:30.81    |
 1:30.81    = note: expected type `std::result::Result<_, ()>`
 1:30.81               found type `std::result::Result<_, std::collections::CollectionAllocErr>`
 1:30.81 
 1:30.81 error[E0658]: use of unstable library feature 'try_reserve': new API (see issue #48043)
 1:30.81   --> media/mp4parse-rust/mp4parse/src/lib.rs:71:13
 1:30.81    |
 1:30.81 71 |         buf.try_reserve(size)?;
 1:30.81    |             ^^^^^^^^^^^
 1:30.81    |
 1:30.81    = help: add #![feature(try_reserve)] to the crate attributes to enable
 1:30.81 
 1:30.82 error[E0277]: the trait bound `(): std::convert::From<std::collections::CollectionAllocErr>` is not satisfied
 1:30.82   --> media/mp4parse-rust/mp4parse/src/lib.rs:71:9
 1:30.82    |
 1:30.82 71 |         buf.try_reserve(size)?;
 1:30.82    |         ^^^^^^^^^^^^^^^^^^^^^^ the trait `std::convert::From<std::collections::CollectionAllocErr>` is not implemented for `()`
 1:30.82    |
 1:30.82    = note: required by `std::convert::From::from`
 1:30.82 
 1:31.30    Compiling clap v2.29.0
 1:31.63 error: aborting due to 4 previous errors
 1:31.63 
 1:31.63 Some errors occurred: E0277, E0308, E0658.
 1:31.63 For more information about an error, try `rustc --explain E0277`.
 1:31.64 error: Could not compile `mp4parse`.
 1:31.64 warning: build failed, waiting for other jobs to finish...
 2:22.82 error: build failed

Comment 1

[Mike Hommey [:glandium]]

Oh waw, this could be viewed as a pretty bad bug in the rust compiler feature detection: try_reserve in this case is provided by the mp4parse_fallible crate via a trait, but it was recently added to nightly in libstd, as an unstable API.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Pretty sure https://github.com/rust-lang/rust/pull/48648 is the regressing change on the rust side.

Comment 3

[Mike Hommey [:glandium]]

This should be fixed by https://github.com/rust-lang/rust/pull/48552

Comment 4

[Mike Hommey [:glandium]]

Note that the code will break when try_reserve stabilizes (because of the incompatible Result type). The mp4parse crate should change the try_reserve calls to try_reserve(...).map_err(|_| ()) which would be a noop with the mp4parse_fallible implementation and would fix the type with the libstd implementation.

Comment 5

[Simon Sapin (:SimonSapin)]

We can also rename the methods in our FallibleVec trait, or qualify calls to them like this: FallibleVec::try_reserve(&mut vec, n)

Comment 6

[Matthew Gregan [:kinetik]]

(In reply to Simon Sapin (:SimonSapin) from comment #5)
> qualify calls to them like this: FallibleVec::try_reserve(&mut vec, n)

I've taken this approach for now.

Comment 7

[Matthew Gregan [:kinetik]]

Attachment: Bug 1446538 - Update mp4parse-rust to ded91a16.

Qualify calls to our FallibleVec trait to avoid clashes with new additions
to nightly libstd.

Review commit: https://reviewboard.mozilla.org/r/229988/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/229988/

Comment 8

[Simon Sapin (:SimonSapin)]

Comment on attachment 8961212 [details]
Bug 1446538 - Update mp4parse-rust to ded91a16.

https://reviewboard.mozilla.org/r/229988/#review235726

::: media/mp4parse-rust/mp4parse.h:108
(Diff revision 1)
>    Mp4parseByteData extra_data;
>    Mp4parseSinfInfo protected_data;
>  } Mp4parseTrackVideoInfo;
>  
>  typedef struct {
> -  intptr_t (*read)(uint8_t*, size_t, void*);
> +  intptr_t (*read)(uint8_t*, uintptr_t, void*);

I don’t know if this change makes a meaningful difference, but I assume it comes from updating cbindgen?

Comment 9

[Matthew Gregan [:kinetik]]

(In reply to Simon Sapin (:SimonSapin) from comment #8)
> >  typedef struct {
> > -  intptr_t (*read)(uint8_t*, size_t, void*);
> > +  intptr_t (*read)(uint8_t*, uintptr_t, void*);
> 
> I don’t know if this change makes a meaningful difference, but I assume it
> comes from updating cbindgen?

That's right.  The callbacks we were passing from Gecko were already declared using uintptr_t (since this was the correct mapping for usize, and rusty-cheddar used that mapping before we switched to cbindgen), so it's a slight improvement.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/959eb9adc89a
Update mp4parse-rust to ded91a16.  r=SimonSapin

Comment 11

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/959eb9adc89a