Closed Bug 1324435 Opened 5 years ago Closed 5 years ago

mach / rustup / whatever should make 32-bit Firefox build "just work" on 64-bit machine

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox53 affected, firefox54 fixed)

RESOLVED FIXED
mozilla54
Tracking Status
firefox53 --- affected
firefox54 --- fixed

People

(Reporter: Gijs, Assigned: rillian)

References

Details

Attachments

(2 files)

My rust apparently was happily doing 64-bit stuff only, and then after trying to build (failed, please run rustup) and running rustup, and fixing things around for bug 1323901 because today is Monday and software is awful, I got this:

 0:03.99 checking for rustc... 'c:/Users/GIJSKR~1/CARGO~1/bin/rustc.exe'
 0:03.99 checking for cargo... 'c:/Users/GIJSKR~1/CARGO~1/bin/cargo.exe'
 0:04.02 checking rustc version... 1.13.0
 0:04.07 checking cargo support for --frozen... yes
 0:04.14 DEBUG: Executing: `'c:/Users/GIJSKR~1/CARGO~1/bin/rustc.exe' --crate-type staticlib --target=i686-pc-windows-msvc -o 'c:\users\gijskr~1\appdata\local\temp\conftestktukng.rlib' 'c:\users\gijskr~1\appdata\local\temp\conftestc5qfdy.rs'`
 0:04.14 DEBUG: The command returned non-zero exit status 101.
 0:04.14 DEBUG: Its error output was:
 0:04.14 DEBUG: | error[E0463]: can't find crate for `std`
 0:04.14 DEBUG: |
 0:04.14 DEBUG: | error: aborting due to previous error
 0:04.14 DEBUG: |
 0:04.14 ERROR: Cannot compile for i686-pc-mingw32 with c:/Users/GIJSKR~1/CARGO~1/bin/rustc.exe
 0:04.14 The target may be unsupported, or you may not have
 0:04.14 a rust std library for that target installed. Try:
 0:04.14
 0:04.14   rustup target add i686-pc-windows-msvc
 0:04.14
 0:04.15 *** Fix above errors and then restart with\
 0:04.15                "d:/mozilla-build/mozmake/mozmake -f client.mk build"
 0:04.17 client.mk:375: recipe for target 'configure' failed
 0:04.17 mozmake: *** [configure] Error 1
2


3 hoops was enough for a Monday. Let's get rid of this #4.
Over to Ralph for triage. Smells like a missing feature in `mach bootstrap`.
Flags: needinfo?(giles)
I'd hoped the `rustup target add` suggestion would be sufficient here, but I can have the installer add the 32-bit windows target, and the armv7-linux-androideabi target if building for android is selected.
Assignee: nobody → giles
Flags: needinfo?(giles)
Duplicate of this bug: 1331665
To fix this we just need to run `rustup target add i686-pc-windows-msvc` in the bootstrap code, probably somewhere here:
https://dxr.mozilla.org/mozilla-central/rev/3e275d37a06236981bff399b7d7aa0646be3fee7/python/mozboot/mozboot/base.py#566

We'll probably want to add another method like `ensure_rust_targets`, and make it do something like:
```
rustup = self.which('rustup')
if rustup:
  if rust.platform() == 'x86_64-pc-windows-msvc':
    if not filter(lambda l: l.startswith('i686-pc-windows-msvc') and ('installed' in l or 'default' in l), subprocess.check_output([rustup, 'target', 'list']).splitlines()):
      subprocess.check_call([rustup, 'target', 'add', 'i686-pc-windows-msvc']).splitlines()):
```

We'll need to call it in the `if modern` block as well as after the block that tries to install Rust, so that we run the check regardless of whether someone already has a new-enough version of Rust installed.
This is a reasonable plan. It's not really an 'ensure' method since it does nothing if rustup is installed, at least until there are platform-specific alternatives for installing cross-libstd.

I wrote a thing to parse the installed targets out of `rustup target list`, but I don't know of a way to check without rustup, aside from slow, configure-style try-to-build-something tests. Calling `rustup target add i686-pc-windows-msvc` unconditionally doesn't work because someone may have manually installed i686 rust instead, and rustup errors if we try to add the host-required toolchain target.

> if rust.platform() == 'x86_64-pc-windows-msvc':

I'd like to do a similar check for whether we're bootstrapping for fennec builds and install armv7-linux-androideabi. Any suggestions what to check for that? I was thinking of stashing the 'application' string in the Bootstrapper instance and checking that, but there's also calling `getattr(self.instance, 'add_%s_rust_targets' % application)()` from Bootstrap::bootstrap() if we want to be fancy.
Comment on attachment 8828126 [details]
Bug 1324435 - mozboot: ensure win32 rust support.

https://reviewboard.mozilla.org/r/105634/#review106448

::: python/mozboot/mozboot/base.py:570
(Diff revision 1)
>      def ensure_rust_modern(self):
>          modern, version = self.is_rust_modern()
>  
>          if modern:
>              print('Your version of Rust (%s) is new enough.' % version)
> +            self.ensure_rust_targets(self.which('rustup'))

This will break if we have modern rust but no rustup.
Comment on attachment 8828125 [details]
Bug 1324435 - mozboot: Install rust i686-msvc target.

https://reviewboard.mozilla.org/r/105632/#review107892
Attachment #8828125 - Flags: review?(ted) → review+
Comment on attachment 8828126 [details]
Bug 1324435 - mozboot: ensure win32 rust support.

https://reviewboard.mozilla.org/r/105634/#review107894
Attachment #8828126 - Flags: review?(ted) → review+
(In reply to Ralph Giles (:rillian) | needinfo me from comment #5)
> I wrote a thing to parse the installed targets out of `rustup target list`,
> but I don't know of a way to check without rustup, aside from slow,
> configure-style try-to-build-something tests. Calling `rustup target add
> i686-pc-windows-msvc` unconditionally doesn't work because someone may have
> manually installed i686 rust instead, and rustup errors if we try to add the
> host-required toolchain target.

I think this is fine. We're doing best-effort here in bootstrap--things should Just Work if you let bootstrap do all the work. If someone is installing their own Rust toolchain then they're reponsible for ensuring that they installed the right one for what they're trying to build. The checks in configure will ensure that we fail with a useful error message if they get it wrong.

> I'd like to do a similar check for whether we're bootstrapping for fennec
> builds and install armv7-linux-androideabi. Any suggestions what to check
> for that? I was thinking of stashing the 'application' string in the
> Bootstrapper instance and checking that, but there's also calling
> `getattr(self.instance, 'add_%s_rust_targets' % application)()` from
> Bootstrap::bootstrap() if we want to be fancy.

I was thinking of the same thing, but I don't know enough about how the android version of the bootstrapper works to reliably answer this. I guess you could add a method on BaseBootstrapper like `get_rust_targets`, make it default to [rust.platform()], and override that in the android bootstrapper?
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #12)

> I was thinking of the same thing, but I don't know enough about how the
> android version of the bootstrapper works to reliably answer this. I guess
> you could add a method on BaseBootstrapper like `get_rust_targets`, make it
> default to [rust.platform()], and override that in the android bootstrapper?

There's no separate android boostrapper, just the 'application' switch in bootstrap.py. Put another way, the bootstrapper is subclassed by host, rather than target. I'll hack something up.
Pushed by rgiles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/71a74dd0762a
mozboot: Install rust i686-msvc target. r=ted
https://hg.mozilla.org/integration/autoland/rev/27c882a157c4
mozboot: ensure win32 rust support. r=ted
https://hg.mozilla.org/mozilla-central/rev/71a74dd0762a
https://hg.mozilla.org/mozilla-central/rev/27c882a157c4
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Product: Core → Firefox Build System
Hi everyone,

just tried building Firefox today and checked out mozilla-release.
I use Windows 7 x64 and get the following error:


 0:15.47 checking for rustc... c:/Users/User/.cargo/bin/rustc.exe
 0:15.48 checking for cargo... c:/Users/User/.cargo/bin/cargo.exe
 0:15.60 checking rustc version... 1.30.1
 0:15.71 checking cargo version... 1.30.0
 0:16.11 DEBUG: Executing: `c:/Users/User/.cargo/bin/rustc.exe --crate-type stat
iclib --target=i686-pc-windows-msvc -o 'c:\users\user\appdata\local\temp\conftes
tkr8ouz.rlib' 'c:\users\user\appdata\local\temp\conftestaimqal.rs'`
 0:16.11 DEBUG: The command returned non-zero exit status 1.
 0:16.11 DEBUG: Its error output was:
 0:16.11 DEBUG: | error[E0463]: can't find crate for `std`
 0:16.12 DEBUG: |   |
 0:16.12 DEBUG: |   = note: the `i686-pc-windows-msvc` target may not be install
ed
 0:16.12 DEBUG: |
 0:16.12 DEBUG: | error: aborting due to previous error
 0:16.12 DEBUG: |
 0:16.13 DEBUG: | For more information about this error, try `rustc --explain E0
463`.
 0:16.13 ERROR: Cannot compile for i686-pc-mingw32 with c:/Users/User/.cargo/bin
/rustc.exe
 0:16.13 The target may be unsupported, or you may not have
 0:16.14 a rust std library for that target installed. Try:
 0:16.14   rustup target add i686-pc-windows-msvc
 0:16.23 *** Fix above errors and then restart with\
 0:16.23                "c:/mozilla-build/bin/mozmake.EXE -f client.mk build"
 0:16.26 mozmake.EXE: *** [client.mk:127: configure] Error 1

Any idea how to fix this?
(In reply to Ralph Giles (:rillian) | needinfo me from comment #5)

Hi Ralph, not sure how this is working here. So I thought of contacting you directly through replying to your comment.
Would appreciate any help on the build of FF on Windows 7 x64. Please see above. Thanks a ton!
Flags: needinfo?(giles)
You can run `./mach bootstrap`, or you can run `rustup target add i686-pc-windows-msvc`, as the error message indicates.  If neither of those work, please file a separate bug.
Flags: needinfo?(giles)
(In reply to Nathan Froyd [:froydnj] from comment #18)
> You can run `./mach bootstrap`, or you can run `rustup target add
> i686-pc-windows-msvc`, as the error message indicates.  If neither of those
> work, please file a separate bug.

Thanks for the fast reply. Appreciate it!

Well, I got it to work when running './mach bootstrap' a second time. Seems to work now.
Would have been nice of course if it 'just worked' :)

Do you want me to file a separate bug for it?
If you can reproduce the conditions that lead to `./mach bootstrap` not installing the standard library for i686-pc-windows-msvc, then yes.  If you can't reproduce it, filing a bug isn't going to be super-helpful.
You need to log in before you can comment on or make changes to this bug.