Make `mach bootstrap` install rust (via rustup) in MozillaBuild shell

RESOLVED FIXED

Status

mozilla.org
MozillaBuild
RESOLVED FIXED
2 years ago
6 months ago

People

(Reporter: rillian, Assigned: Nat)

Tracking

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

We're planning to start shipping rust code in gecko later this year. At some point we'll start requiring it for default builds, at which point we'll want some solution for everyone to get the toolchain installed.
(Reporter)

Comment 1

2 years ago
I filed this under MozillaBuild, but there are several ideas:

- Add rust as a manual prerequisite, but that's not very friendly.

- Add it to MozillaBuild, but the rust toolchain is ~100 MB and releases every six weeks.

- We might be able to do something with mach's bootstrap commands to do a lazy install/update?
(Reporter)

Updated

2 years ago
Blocks: 1135640
Alternatively, if we are going to switch to msys2, we could submit rust to their repo and install that with our other dependencies via its package manager.

Comment 3

2 years ago
Since we plan to use msys2's package manager and more "native" system bootstrapping (outside the context of MozillaBuild), I think installing Rust should be the first task performed by `mach bootstrap` on Windows. Currently, `mach bootstrap` reports Windows is unsupported. Let's change that.
Well, but before we switch to msys2, I guess it still makes sense to integrate rust inside MozillaBuild so that Windows developers can build with rust now. RyanVM, probably try this in the next version of MozillaBuild?
Flags: needinfo?(ryanvm)

Comment 5

2 years ago
Per discussion in Mozlando, there was talk about installing Rust some other way, since apparently we'll want to track the upstream stable release quite aggressively. Can't remember if we actually decided anything though...
Looks like I'd need a v1.7 nightly build to get a 32-bit MSVC ABI installer? How far out is v1.7 from release?
Flags: needinfo?(ryanvm)
Should be ~3 months after 1.5, which was released on Dec 10.
Ah to clarify there are actually 32-bit MSVC ABI installers right now (on stable), we just do not recommend them for use because there are some unimplemented portions of that platform. Namely if a Rust program panics it will abort the entire program rather than being catchable (like with other platforms).

A 32-bit MSVC platform which supports catching panics is in the works, but it will land in 1.7 at the earliest and is more likely to go to 1.8 (we're largely just waiting for LLVM support)
At this point, we only ship a 32-bit version of MozillaBuild, so presumably we'd want to wait for that?
If you don't rely on the ability to catch panics in Rust code then it should be fine to use Rust on 32-bit MSVC. I believe, however, that currently Gecko is catching panics (via spawning threads), so this may be important.
I guess we probably could ship multirust or multirust-rs with MozillaBuild, and make the build system detect and fetch the toolchain via that tool accordingly.

It seems multirust depends on MSYS2, while multirust-rs provides Windows installer directly, which we can probably use.

Does it sound feasible?
The multirust and multirust-rs projects are essentially the same (we're in the process of transitioning to multirust-rs from multirust), and they largely just manage Rust installations for you. I don't think that Gecko would necessarily benefit much from them as they'd still have to download all of the compiler. For developers switching between versions they can be quite useful, but perhaps Gecko won't be doing it so much? An example of this is that Servo doesn't use multirust to manage the version of Rust used to compile it.
I'm fairly sure someone's screamed bloody murder increasing the size of stuff needed in order to build before.  With the caveat that this is very possibly stop-energy that should be ignored in a first cut, will updates of an existing rust-toolchain be compressed-diff-based, or will they be download-it-all-at-full-size-based?  Just trying to empathize with people for whom regularly downloading ~100MB is chore, and/or at least get us thinking about ways that hit might be mitigated.
Hi guys. Was a resolution reached about whether the rust compiler will be included in MozillaBuild, or if mach will take care of downloading it in `mach bootstrap` or if it should be downloaded with the msys2 package manager?
Note, rustc appears in an in-tree tooltool manifest which is used for downloading rustc in build/release automation.

For example:
https://hg.mozilla.org/mozilla-central/file/6adc822f5e27/testing/mozharness/configs/builds/releng_base_windows_64_builds.py#l97
=>
https://hg.mozilla.org/mozilla-central/file/6adc822f5e27/browser/config/tooltool-manifests/win64/releng.manifest#l9

This same manifest also includes e.g. mozmake and Visual Studio.

tooltool is a tool for downloading large binary files (usually toolchains) written in mozilla.

It would make sense to me for this manifest to instead include mozilla build, and mozilla build to include everything like rustc, visual studio redistributables, etc, otherwise the split between what goes in the tooltool manifest, and what goes in mozilla-build might not be clear.

My preferred developer workflow would be:

1) install python and hg
2) check out mozilla central
3) `mach bootstrap`
4) choose / create a mozconfig
5) `mach build`

If we got this working, maybe release automation could perform the same steps, and might not need the extra mozharness layer that it currently uses to e.g. invoke tooltool.
I think what we should do here in to include the rustup-init bootstrap tool (https://www.rustup.rs/) in mozilla-build, and then have `mach bootstrap` invoke it to install a toolchain if none is available. This is a lot smaller (4MB) than packaging the rust toolchain itself. Alternatively, 'mach bootstrap' could download rustup-init itself. I'm not sure which is a better experience.
Blocks: 1283898
Unfortunately Windows users don't actually run `mach bootstrap` right now since we require them to install MozillaBuild. There's work afoot to make msys2 a supported build platform on Windows, and `mach bootstrap` works there now, so we could solve things that way, but we might also need to do a new MozillaBuild install to smooth over the transition. In that case we should probably just ship a copy of Rust in MozillaBuild. It'd obviously be nice to ship rustup, but I don't know if we can do that usefully in MozillaBuild as it stands. Maybe we just install latest stable in MozillaBuild and hope that msys2 becomes viable before we have to rev that? :)
Yeah, I think we'll need to bite the download size bullet and just ship the latest stable version for now.
Nat added preliminary support for Windows to `mach bootstrap`. The whole thing is behind an environment variable. But we could probably make the Rust bits enabled by default and keep the msys2 stuff behind an environment variable until it is ready. (I'd prefer to not ship another MozillaBuild if possible.)
Also, at least glandium has postulated the idea of downloading toolchains during configure. I'm not opposed to the idea and think Rust would be a good candidate to test that approach.
Okay, I think making `mach bootstrap` work in the existing MozillaBuild environment purely to install rust via `rustup` would be a fine step forward. (I've already manually installed rust using rustup locally and am using it with MozillaBuild.)
Summary: Add rust to MozillaBuild → Make `mach bootstrap` install rust (via rustup) in MozillaBuild shell
(Assignee)

Comment 22

a year ago
Created attachment 8768987 [details]
Bug 1177128 - Added bootstrapper for MozillaBuild that installs rustup.

Review commit: https://reviewboard.mozilla.org/r/62964/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/62964/
Attachment #8768987 - Flags: review?(gps)
Attachment #8768988 - Flags: review?(gps)
(Assignee)

Comment 23

a year ago
Created attachment 8768988 [details]
Bug 1177128 - Modified bootstrap.py to dispatch between MozillaBuild and MSYS2 bootstrappers.

Review commit: https://reviewboard.mozilla.org/r/62966/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/62966/
Comment on attachment 8768988 [details]
Bug 1177128 - Modified bootstrap.py to dispatch between MozillaBuild and MSYS2 bootstrappers.

https://reviewboard.mozilla.org/r/62966/#review59932

Need 2 minor changes. On next submission, you can roll this into the previous commit, as the previous commit is worthless without this one.

::: python/mozboot/mozboot/bootstrap.py:10
(Diff revision 1)
> +import os
>  import os.path

Combine these 2 lines to `import os` (importing os will make `os.path` available).

::: python/mozboot/mozboot/bootstrap.py:195
(Diff revision 1)
>              cls = FreeBSDBootstrapper
>              args['version'] = platform.release()
>              args['flavor'] = platform.system()
>  
>          elif sys.platform.startswith('win32') or sys.platform.startswith('msys'):
> +            if os.environ.get('MSYSTEM') == 'MINGW64':

What are your system settings? I get MSYSTEM=msys on both 32 and 64 bit msys2 installs on my machine. MozillaBuild is reported as `MINGW32` however.

I think a better way to detect MozillaBuild is for the presence of the `MOZILLABUILD` environment variable. This is set by `start-shell.bat`.
Attachment #8768988 - Flags: review?(gps) → review-
Comment on attachment 8768987 [details]
Bug 1177128 - Added bootstrapper for MozillaBuild that installs rustup.

https://reviewboard.mozilla.org/r/62964/#review59934

I like where this is going. But there are a number of needed changes to make things happy.

::: python/mozboot/mozboot/mozillabuild.py:6
(Diff revision 1)
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +import os
> +import os.path

This line isn't needed.

::: python/mozboot/mozboot/mozillabuild.py:15
(Diff revision 1)
> +    def __init__(self, **kwargs):
> +       BaseBootstrapper.__init__(self, **kwargs)

This shouldn't be needed since this is exactly what will happen if __init__ is not defined on this class.

::: python/mozboot/mozboot/mozillabuild.py:25
(Diff revision 1)
> +        self.http_download_and_save(
> +                'https://static.rust-lang.org/rustup/dist/i686-pc-windows-msvc/rustup-init.exe',
> +                'rustup-init.exe')

This will save things to the current directory. We want to use a well-defined directory. Or a temporary directory (and delete the file when done).

::: python/mozboot/mozboot/mozillabuild.py:26
(Diff revision 1)
> +    def install_system_packages(self):
> +        self.install_rustup()
> +
> +    def install_rustup(self):
> +        self.http_download_and_save(
> +                'https://static.rust-lang.org/rustup/dist/i686-pc-windows-msvc/rustup-init.exe',

Should we pin the version here? So running `mach bootstrap` on an old checkout will fetch an old Rust toolchain (that that commit is known to build with). Or is that not how `rustup` works?

::: python/mozboot/mozboot/mozillabuild.py:30
(Diff revision 1)
> +        self.http_download_and_save(
> +                'https://static.rust-lang.org/rustup/dist/i686-pc-windows-msvc/rustup-init.exe',
> +                'rustup-init.exe')
> +        self.run(['rustup-init.exe', '--no-modify-path', '--default-host',
> +            'x86_64-pc-windows-msvc', '--default-toolchain', 'stable', '-y'])
> +        mozillabuild_dir = os.environ.get('MOZILLABUILD')

Since this variable is required, use `os.environ['MOZILLABUILD']`.

::: python/mozboot/mozboot/mozillabuild.py:31
(Diff revision 1)
> +        os.rename(os.path.expanduser('~') + '/.cargo',
> +                mozillabuild_dir + '.cargo')

This feels a bit odd. Can the rustup installer not specify a location for the `.cargo` directory?

If the destination directory exists, this will also fail due to the file existing.

Do we have to do a move here? Does the installer put files in the `.cargo` directory? Could we change the environment variables of the installer to specify the home directory in terms of `MOZILLABUILD`?

Also, is it important for MozillaBuild to have its own Rust install? Can we just use whatever is in ~/.cargo?

::: python/mozboot/mozboot/mozillabuild.py:33
(Diff revision 1)
> +        self.run(['rustup-init.exe', '--no-modify-path', '--default-host',
> +            'x86_64-pc-windows-msvc', '--default-toolchain', 'stable', '-y'])
> +        mozillabuild_dir = os.environ.get('MOZILLABUILD')
> +        os.rename(os.path.expanduser('~') + '/.cargo',
> +                mozillabuild_dir + '.cargo')
> +        os.remove('rustup-init.exe')

There needs to be a `finally` block in here to ensure the installer is always removed.

::: python/mozboot/mozboot/mozillabuild.py:61
(Diff revision 1)
> +    def http_download_and_save(self, url, dest):
> +        f = urllib2.urlopen(url)
> +        with open(dest, 'wb') as out:
> +            while True:
> +                data = f.read(4096)
> +                if data:
> +                    out.write(data)
> +                else:
> +                    break

Let's put this on the base class. Also, let's integrate hash verification as part of the downloading using SHA-256. The Python standard library is notoriously bad at properly verifying TLS security and I don't want to take any chances.
Attachment #8768987 - Flags: review?(gps)
(Assignee)

Comment 26

a year ago
https://reviewboard.mozilla.org/r/62964/#review59934

> This feels a bit odd. Can the rustup installer not specify a location for the `.cargo` directory?
> 
> If the destination directory exists, this will also fail due to the file existing.
> 
> Do we have to do a move here? Does the installer put files in the `.cargo` directory? Could we change the environment variables of the installer to specify the home directory in terms of `MOZILLABUILD`?
> 
> Also, is it important for MozillaBuild to have its own Rust install? Can we just use whatever is in ~/.cargo?

You cannot specify a location for the `.cargo` directory (for now).

The installer does put files in the `.cargo` directory. I suppose we could change the environment variables of the installer to specify the home directory in terms of `MOZILLABUILD` (or at least I can't think of a reason why we couldn't).

We could also put (or just use) whatever is in `~/.cargo`, but on some level I think its unclean to put MozillaBuild stuff outside of the MozillaBuild folder, also what if the user already has a rustup install that they're using? We don't want to mess with that (for example, change the toolchain and not change it back).
(Assignee)

Comment 27

a year ago
https://reviewboard.mozilla.org/r/62964/#review59934

> Should we pin the version here? So running `mach bootstrap` on an old checkout will fetch an old Rust toolchain (that that commit is known to build with). Or is that not how `rustup` works?

This is just `rustup-init.exe` which just installs `rustup`. The actual of version of `rustc` that gets installed with `rustup` is determined when we run `rustup-init` with the `--default-host` and `--default-toolchain` flags. We *could* pin the version of `rustup-init`, but I'm not sure what the benefits would be.

Comment 28

a year ago
The rustup installer can customize the installation directory. `rustup-init` and `rustup` will obey the `CARGO_HOME` environment variable, so you can set it to control where rustup is installed. You'll also want to set `RUSTUP_HOME`, since that is where toolchains are stored. rustup won't write anywhere else but these directories and the temp directory.

I'm happy to help with any rustup matters.
https://reviewboard.mozilla.org/r/62964/#review59934

> This is just `rustup-init.exe` which just installs `rustup`. The actual of version of `rustc` that gets installed with `rustup` is determined when we run `rustup-init` with the `--default-host` and `--default-toolchain` flags. We *could* pin the version of `rustup-init`, but I'm not sure what the benefits would be.

We strive to make our builds reproducible. So if Firefox 49 builds with Rust 1.9 and Firefox 50 requires Rust 1.10, trying to build Firefox 49 will always build with Rust 1.9.

Truthfully, we don't do a good job at this. So just running `rustup` is probably acceptable. We can add determinism in later.

> You cannot specify a location for the `.cargo` directory (for now).
> 
> The installer does put files in the `.cargo` directory. I suppose we could change the environment variables of the installer to specify the home directory in terms of `MOZILLABUILD` (or at least I can't think of a reason why we couldn't).
> 
> We could also put (or just use) whatever is in `~/.cargo`, but on some level I think its unclean to put MozillaBuild stuff outside of the MozillaBuild folder, also what if the user already has a rustup install that they're using? We don't want to mess with that (for example, change the toolchain and not change it back).

This isn't really MozillaBuild installing Rust: it is `mach bootstrap` installing Rust. I think putting things in the default location of `~/.cargo` is acceptable. The only other alternative would be `~/.mozbuild/` (technically the value of `state_dir` within the bootstrapper since users can specify default locations).

Let's go for `~/.cargo` for now.
(Assignee)

Comment 30

a year ago
Comment on attachment 8768987 [details]
Bug 1177128 - Added bootstrapper for MozillaBuild that installs rustup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62964/diff/1-2/
Attachment #8768987 - Flags: review?(gps)
(Assignee)

Updated

a year ago
Attachment #8768988 - Attachment is obsolete: true
(Assignee)

Comment 31

a year ago
Comment on attachment 8768987 [details]
Bug 1177128 - Added bootstrapper for MozillaBuild that installs rustup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62964/diff/2-3/
(Assignee)

Comment 32

a year ago
Comment on attachment 8768987 [details]
Bug 1177128 - Added bootstrapper for MozillaBuild that installs rustup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62964/diff/3-4/
Comment on attachment 8768987 [details]
Bug 1177128 - Added bootstrapper for MozillaBuild that installs rustup.

https://reviewboard.mozilla.org/r/62964/#review60368

This is looking much better!

Biggest issue is the need for an immutable URL for rustup so hashes don't change unexpectedly. Hopefully someone from Rust land can provide one.

::: python/mozboot/mozboot/base.py:423
(Diff revision 4)
> +            raise RuntimeError('Hash of downloaded file does not match expected
> +                hash')

IIRC RuntimeError is reserved for low-level Python errors. Just make this ValueError or plain old Exception.

::: python/mozboot/mozboot/mozillabuild.py:24
(Diff revision 4)
> +
> +    def install_rustup(self):
> +        try:
> +            self.http_download_and_save(
> +                    'https://static.rust-lang.org/rustup/dist/i686-pc-windows-msvc/rustup-init.exe',
> +                    os.environ['TMP'] + '/rustup-init.exe',

Use `tempfile.gettempdir()` to find the temporary directory.

::: python/mozboot/mozboot/mozillabuild.py:26
(Diff revision 4)
> +        try:
> +            self.http_download_and_save(
> +                    'https://static.rust-lang.org/rustup/dist/i686-pc-windows-msvc/rustup-init.exe',
> +                    os.environ['TMP'] + '/rustup-init.exe',
> +                    'a45ab7462b567dacddaf6e9e48bb43a1b9c1db4404ba77868f7d6fc685282a46')
> +            self.run(['rustup-init.exe', '--no-modify-path', '--default-host',

The full path to rustup-init.exe should be specified. Otherwise default path search rules apply and there is no guarantee you'll run what was downloaded (or even find it!).

::: python/mozboot/mozboot/mozillabuild.py:29
(Diff revision 4)
> +                    os.environ['TMP'] + '/rustup-init.exe',
> +                    'a45ab7462b567dacddaf6e9e48bb43a1b9c1db4404ba77868f7d6fc685282a46')
> +            self.run(['rustup-init.exe', '--no-modify-path', '--default-host',
> +                'x86_64-pc-windows-msvc', '--default-toolchain', 'stable', '-y'])
> +            mozillabuild_dir = os.environ['MOZILLABUILD']
> +

Since this URL doesn't have a version number in it, I'm guessing the URL isn't immutable and the file (and the hash) can change over time.

We should use a URL that is immutable.

::: python/mozboot/mozboot/mozillabuild.py:40
(Diff revision 4)
> +    def upgrade_mercurial(self, current):
> +        pass
> +
> +    def upgrade_python(self, current):
> +        pass
> +
> +    def install_browser_packages(self):
> +        pass
> +
> +    def install_mobile_android_packages(self):
> +        pass
> +
> +    def install_mobile_android_artifact_mode_packages(self):
> +        pass
> +
> +    def _update_package_manager(self):
> +        pass

We should add some kind of "bootstrap isn't fully implemented in MozillaBuild" warning message somewhere.

::: python/mozboot/mozboot/mozillabuild.py:40
(Diff revision 4)
> +    def upgrade_mercurial(self, current):
> +        pass

You can `pip install --upgrade Mercurial` from MozillaBuild to upgrade Mercurial. It's probably worth implementing this.

You should use `MOZILLABUILD\python\Scripts\pip.exe`.
Attachment #8768987 - Flags: review?(gps)
(Assignee)

Comment 34

a year ago
Comment on attachment 8768987 [details]
Bug 1177128 - Added bootstrapper for MozillaBuild that installs rustup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62964/diff/4-5/
Attachment #8768987 - Flags: review?(gps)
Comment on attachment 8768987 [details]
Bug 1177128 - Added bootstrapper for MozillaBuild that installs rustup.

https://reviewboard.mozilla.org/r/62964/#review60436

Looking better.

I tried running this and encountered several errors. Did you not test this?

::: python/mozboot/mozboot/base.py:411
(Diff revision 5)
>          Child classes should reimplement this.
>          """
>          print(PYTHON_UNABLE_UPGRADE % (current, MODERN_PYTHON_VERSION))
> +
> +    def http_download_and_save(self, url, dest, sha256hexhash):
> +        f = urllib2.urlopen(url)

You forgot to import `urllib2`.

::: python/mozboot/mozboot/base.py:423
(Diff revision 5)
> +            raise ValueError('Hash of downloaded file does not match expected
> +                hash')

There is a syntax error here because strings can't wrap multiple lines like this.

I'm not a stickler for the 80 character limit. Just put "hash" on the same line.

::: python/mozboot/mozboot/mozillabuild.py:9
(Diff revision 5)
> +
> +import os
> +import sys
> +import subprocess
> +import tempfile
> +import urllib2

Unused import.

::: python/mozboot/mozboot/mozillabuild.py:44
(Diff revision 5)
> +                f.write('    WIN_HOME=$(cd "$HOME" && pwd)\n')
> +                f.write('    PATH="$WIN_HOME/.cargo/bin:$PATH"\n')
> +                f.write('    export PATH\n')
> +                f.write('fi')
> +        finally:
> +            os.remove(rustup_init)

This needs to handle the file not existing or it may raise.
Attachment #8768987 - Flags: review?(gps)
(Assignee)

Comment 36

a year ago
Comment on attachment 8768987 [details]
Bug 1177128 - Added bootstrapper for MozillaBuild that installs rustup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62964/diff/5-6/
Attachment #8768987 - Flags: review?(gps)
(Assignee)

Comment 37

a year ago
https://reviewboard.mozilla.org/r/62964/#review60436

My pull didn't work and I tested the old version by accident (which does work).
Comment on attachment 8768987 [details]
Bug 1177128 - Added bootstrapper for MozillaBuild that installs rustup.

https://reviewboard.mozilla.org/r/62964/#review60458

There are still some rough edges here (e.g. messaging around Windows support and providing more context when performing actions). But this is good enough to land.
Attachment #8768987 - Flags: review?(gps) → review+

Comment 39

a year ago
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3c8609dde0ff
Added bootstrapper for MozillaBuild that installs rustup. r=gps

Comment 40

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3c8609dde0ff
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Assignee: nobody → nhakkakzadeh

Updated

10 months ago
Duplicate of this bug: 1294083

Updated

6 months ago
Depends on: 1325787
You need to log in before you can comment on or make changes to this bug.