1360521 - invoking rustup from ./mach bootstrap should download specific rust versions, rather than "stable"

Status: RESOLVED WONTFIX

(Firefox Build System :: General, defect)

Description

[Krzysztof Jan Modras [:chrmod]]

Attachment: cargo.patch

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20170418122848



Actual results:

Rust 1.17 was released to stable channel on 2017-04-27 -  that introduced a breaking change in Cargo.toml behavior which now assumes that `build.rs` file should be used if existing (https://github.com/rust-lang/cargo/pull/3664).

With Rust 1.17 build fails with following error: 
> 0:02.77 error[E0463]: can't find crate for `cheddar`
> 0:02.77  --> /home/chrmod/Projects/cliqz-oss/browser-f/mozilla-release/media/libstagefright/binding/mp4parse_capi/build.rs:1:1
> 0:02.77   |
> 0:02.77 1 | extern crate cheddar;
> 0:02.77   | ^^^^^^^^^^^^^^^^^^^^^ can't find crate
> 0:02.77 
> 0:02.77 error: aborting due to previous error
> 0:02.77 
> 0:02.78 error: Could not compile `mp4parse_capi`.
> 0:02.78 
> 0:02.78 Caused by:
> 0:02.78   process didn't exit successfully: `/home/chrmod/.cargo/bin/rustc --crate-name build_script_build /home/chrmod/Projects/cliqz-oss/browser-f/mozilla-release/media/libstagefright/binding/mp4parse_capi/build.rs --color always --crate-type bin --emit=dep-info,link -C opt-level=2 -C codegen-units=1 -C debuginfo=2 -C metadata=ed7f13d3448c2b9a -C extra-filename=-ed7f13d3448c2b9a --out-dir /home/chrmod/Projects/cliqz-oss/browser-f/obj/toolkit/library/gtest/rust/./release/build/mp4parse_capi-ed7f13d3448c2b9a -L dependency=/home/chrmod/Projects/cliqz-oss/browser-f/obj/toolkit/library/gtest/rust/./release/deps` (exit code: 101)



Expected results:

To fix the problem `build = false` has to be specified in `media/libstagefright/binding/mp4parse_capi/Cargo.toml`

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

Maire this came from our partner and looks media-related. Do you know who can look into it?

Comment 2

[Nathan Froyd [:froydnj]]

We haven't seen this problem when upgrading our builds to 1.17 (see bug 1360364).

Ralph, do you know what's going on here?

Comment 3

[alexander]

Important - this happened when trying to build current released version 53.

Comment 4

[Nathan Froyd [:froydnj]]

(In reply to alexander from comment #3)
> Important - this happened when trying to build current released version 53.

Ah.  I think the position would be that we don't guarantee forward compatibility on release versions, then.  Just like we wouldn't guarantee that some new version of GCC or MSVC would work to compile a specific release.

Comment 5

[alexander]

Nathan, but this mean, that no one can not build current release version using usual way to do so. That's wierd, isn't it? I can partially understand this position for the old releases, but current one?

Rust is the one of build tools. May be it's better to point somehow which version of it must be used to make a build? For example, I know that version 53 can be build with Visual Studio 2015 (or GCC 4.9, etc.). With Rust it's not clear. Build instruction have nothing about installing Rust, it's ask for running ./mach bootstrap

Comment 6

[Nathan Froyd [:froydnj]]

(In reply to alexander from comment #5)
> Nathan, but this mean, that no one can not build current release version
> using usual way to do so. That's wierd, isn't it? I can partially understand
> this position for the old releases, but current one?

If by "usual way to do so", you mean "running mach bootstrap and then attempting to build with the tools that it downloads", then yes, this is a little weird.  Partially this is because mach bootstrap relies on rustup'ing the stable version of Rust, which as you have noted, does not work in this case.  This behavior is a bug: we should be rustup'ing to a known-good version of Rust, rather than grabbing the stable version.

But I don't know that we've really advertised mach bootstrap as the way to get build tools for stable releases; it's mostly been intended to download whatever build tools you need for day-to-day development.

We built 53 with Rust 1.14, fwiw:

https://hg.mozilla.org/releases/mozilla-release/file/tip/browser/config/tooltool-manifests/linux64/releng.manifest#l19

Does that address your questions?

Comment 7

[alexander]

(In reply to Nathan Froyd [:froydnj] from comment #6)
> (In reply to alexander from comment #5)
> > Nathan, but this mean, that no one can not build current release version
> > using usual way to do so. That's wierd, isn't it? I can partially understand
> > this position for the old releases, but current one?
> 
> If by "usual way to do so", you mean "running mach bootstrap and then
> attempting to build with the tools that it downloads", then yes, this is a
> little weird.  Partially this is because mach bootstrap relies on rustup'ing
> the stable version of Rust, which as you have noted, does not work in this
> case.  This behavior is a bug: we should be rustup'ing to a known-good
> version of Rust, rather than grabbing the stable version.
> 
> But I don't know that we've really advertised mach bootstrap as the way to
> get build tools for stable releases; it's mostly been intended to download
> whatever build tools you need for day-to-day development.
> 
> We built 53 with Rust 1.14, fwiw:
> 
> https://hg.mozilla.org/releases/mozilla-release/file/tip/browser/config/
> tooltool-manifests/linux64/releng.manifest#l19
> 
> Does that address your questions?

Thanks for all this information, very useful. We need to change our build system to reflect new knowledge in it. Information about ./mach bootstrap do not shown directly in build instruction (https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Windows_Prerequisites). When developer will try to build a browser without Rust (./mach build), he will see a suggestion to run "./mach bootstrap" first. Don't know, is it ok or not.

For now we know how to resolve a problem on our side, thanks for you help. It is up to Mozillians to decide what to do with this ticket.

Comment 8

[Nathan Froyd [:froydnj]]

(In reply to alexander from comment #7)
> For now we know how to resolve a problem on our side, thanks for you help.
> It is up to Mozillians to decide what to do with this ticket.

You're welcome!  I'm going to re-purpose this ticket for changing mach bootstrap to download specific Rust versions, rather than indiscriminately downloading the stable version.

Comment 9

[Ralph Giles (:rillian)]

I think the actual problem here is that our rustup policy was relying on stable rustc being backward-compatible. Which it is. But we're also relying on cargo, which is not. The issue is caused by a behaviour change in cargo. :(

I'm uncomfortable with just freezing the rust version `mach boostrap` installs. I think that damages its usefulness in setting up a dev environment for Rust language work, and doing so means running bootstrap from different branches will clobber each other. Like Nathan in #6 I think of the `mach bootstrap` command as supporting development of mozilla-central more than for building releases.

Of course anyone trying to build 53 after setting up such a dev environment would encounter this bug, and the correct fix is 'rustup default 1.16.0'. The question is, how to communicate this? I don't think it would occur to me to re-run `mach bootstap` in the hope it would fix a build issue by downgrading some build dependency. Maybe filing a bug for support is the best we can do here?

Comment 10

[Ralph Giles (:rillian)]

FWIW, I looked at backporting the patch from bug 1338655 (which addresses the issue in Firefox 54 and 55) but we hit a compatibility issue going the other direction. Our official builds use rust 1.14.0 (1.15.1 on Android) and the version of cargo in that release rejects then new `build = false` syntax. We can only fix this for Firefox 53 by removing the build.rs file from the source tree completely.

Comment 11

[Krzysztof Jan Modras [:chrmod]]

another alternative would be to satisfy all dependencies (cheddar in this case) and this could be done by either adding it to third_party/rust or maybe by allowing cargo to download the missing ones?

Another topic is ./mach bootstrap which is currently the only automated way to satisfy build dependencies. At CLIQZ we run bootstrap before every build (that is how we learned about failing rust dependency that fast). 
It is good to know that Mozilla CI uses fixed configuration like Rust 14, but I find that setup to be kinda obscure and prone to change. To help others to be able to reproduce release builds, a `./mach bootstrap-release` could be introduced to prepare build environment. In such case we could guarantee build reproducibility overtime. What do you think?

Comment 12

[Gregory Szorc [:gps]]

We're thinking about integrating toolchain installation into `mach build` (likely at the configure phase). This would include Rust installation.

Comment 13

[Landry Breuil (:gaston)]

(In reply to Gregory Szorc [:gps] from comment #12)
> We're thinking about integrating toolchain installation into `mach build`
> (likely at the configure phase). This would include Rust installation.

Please, dont make that mandatory. Think about build environments without network access, or where rustup doesnt exist....

Comment 14

[Gregory Szorc [:gps]]

(In reply to Landry Breuil (:gaston) from comment #13)
> (In reply to Gregory Szorc [:gps] from comment #12)
> > We're thinking about integrating toolchain installation into `mach build`
> > (likely at the configure phase). This would include Rust installation.
> 
> Please, dont make that mandatory. Think about build environments without
> network access, or where rustup doesnt exist....

It can't be mandatory because it wouldn't be compatible with downstream builders (like you). However, it may become the default because it should result in a better experience ("it just works") for the average developer.

Comment 15

[Ralph Giles (:rillian)]

(In reply to Krzysztof Jan Modras from comment #11)
> another alternative would be to satisfy all dependencies (cheddar in this
> case) and this could be done by either adding it to third_party/rust or
> maybe by allowing cargo to download the missing ones?
> 
> Another topic is ./mach bootstrap which is currently the only automated way
> to satisfy build dependencies. At CLIQZ we run bootstrap before every build
> (that is how we learned about failing rust dependency that fast).

Thanks for doing integration testing we obviously don't! :-)

> a `./mach bootstrap-release` could be introduced to prepare build environment.

I don't think this idea is workable. Mozilla CI has it's own, very complicated build harness which already embeds all this information. See the docker images under the taskcluster directory for example. A `./mach bootstrap-release` command would be a duplication of that and hard to keep up-to-date. The only solution here is to converge developer and integration build automation so it's easier to build in the expected and tested environments when that's what a developer wants. Unfortunately, that's not a near-term solution.

In the meantime, asking for support seems to have worked in this case. We have too different workarounds for this specific issue, and it looks like it's specific to Firefox 53. Let us know if you find more!