Closed Bug 1175359 Opened 5 years ago Closed 4 years ago

Enable rust for linux64 treeherder builds

Categories

(Infrastructure & Operations Graveyard :: CIDuty, task)

x86_64
Linux
task
Not set

Tracking

(firefox45 fixed)

RESOLVED FIXED
Tracking Status
firefox45 --- fixed

People

(Reporter: rillian, Assigned: rillian)

References

Details

Attachments

(2 files, 10 obsolete files)

3.56 KB, patch
mshal
: review+
Details | Diff | Splinter Review
1.12 KB, patch
mshal
: review+
Details | Diff | Splinter Review
Now that we have rust code in the tree, we need to start testing it. This bug is about enabling rust in continuous integration builds so this code gets tested.
This is a generalization of the current setup.sh which should work on all platforms. The patch only references it on linux64 releng builds to make rustc available along with other imported toolchains.
Assignee: nobody → giles
Attached patch Enable rust in automation builds (obsolete) — Splinter Review
Pass --enable-rust. This needs to be made conditional to the linux64 platform.
I've added been porting our Linux64 jobs to TaskCluster. This may be easier to develop in since it's docker based. If you're interested: https://bugzilla.mozilla.org/show_bug.cgi?id=1155749 I think it would be great to get the rust tests working this way, especially since BuildBot will be going the way of the dodo.
Fix typo in the setup script. With this change we build and pass tests with rust enabled on linux64. As expected, other targets fail because the mozconfig patch isn't properly conditional.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=56db250ea91a
Attachment #8623395 - Attachment is obsolete: true
Define a fragment we can include which makes the tooltool rust toolchain accessible, and include it in the linux64 nightly mozconfig.

Things I'm worried about:

- Is this mozconfig used outside of automation builds? I don't want to require developers to install rust yet.

- This defines both LD_LIBRARY_PATH and DYLD_LIBRARY_PATH which is ugly.

Better suggestions welcome!

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2387359ed106
Attachment #8623398 - Attachment is obsolete: true
(In reply to Morgan Phillips [:mrrrgn] from comment #4)
> I've added been porting our Linux64 jobs to TaskCluster.

Being about to iterate on a local docker container instead of try would be awesome! Can you link me to some instructions to get started?
Depends on: 1175746
Enable rust for linux64 debug as well. What do you think, Ted?

This builds, but I can't verify the unit tests because of bug 1175746.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a259db4b886
Attachment #8623876 - Attachment is obsolete: true
Attachment #8623961 - Flags: review?(ted)
Attachment #8623873 - Flags: review?(ted)
(In reply to Ralph Giles (:rillian) from comment #6)
> - Is this mozconfig used outside of automation builds? I don't want to
> require developers to install rust yet.

No, the in-tree mozconfigs are automation only.

> - This defines both LD_LIBRARY_PATH and DYLD_LIBRARY_PATH which is ugly.

That's definitely not great. What needs this set?
Comment on attachment 8623873 [details] [diff] [review]
Part 1 - Update releng tooltool manifest to import rust v2

Review of attachment 8623873 [details] [diff] [review]:
-----------------------------------------------------------------

This probably wants a releng review, I'm not super up to speed on tooltool stuff.
Attachment #8623873 - Flags: review?(ted)
Comment on attachment 8623961 [details] [diff] [review]
Part 2 - Enable rust for linux64 nightly builds v3

Review of attachment 8623961 [details] [diff] [review]:
-----------------------------------------------------------------

::: build/mozconfig.rust
@@ +1,3 @@
> +# Options to enable rust in automation builds.
> +
> +mk_add_options "export PATH=$PATH:$PWD/rustc/rustc/bin"

You don't want to use $PWD here, you want $topsrcdir, like:
https://dxr.mozilla.org/mozilla-central/source/build/unix/mozconfig.linux#16

I think this should be called "mozconfig.rust.linux" and live in build/unix. Other platforms are likely to wind up with different options.
Attachment #8623961 - Flags: review?(ted)
(In reply to Ralph Giles (:rillian) from comment #7)
> (In reply to Morgan Phillips [:mrrrgn] from comment #4)
> > I've added been porting our Linux64 jobs to TaskCluster.
> 
> Being about to iterate on a local docker container instead of try would be
> awesome! Can you link me to some instructions to get started?

I need to write something up in the wiki. In the meantime, you can find the image in tree at:
testing/docker/desktop-build
and you can pull the latest one with `docker pull quay.io/mrrrgn/desktop-build:0.0.19

Lastly, you can see how to run a job locally in my last blog post: http://linuxpoetry.com/blog/22/

Let me know if I can help you. I'll try to get a wiki article up with some details about the containers (we have a sprint planned at Whistler so that may be a good time).
Comment on attachment 8623873 [details] [diff] [review]
Part 1 - Update releng tooltool manifest to import rust v2

Passing tooltool manifest update to mshal for releng review.

The new setup.sh is an attempt to be more generic so we don't need so many copies. Original is at https://github.com/mozilla/rust-tooltool/blob/master/setup.sh
Attachment #8623873 - Flags: review?(mshal)
Thanks for the review. Updated patch in response to feedback.

> No, the in-tree mozconfigs are automation only.

Great.

>> - This defines both LD_LIBRARY_PATH and DYLD_LIBRARY_PATH which is ugly.
> 
> That's definitely not great. What needs this set?

We need to add the tooltool package to the PATH so the configure check for rustc can find it. Further, rustc relies on the loader for its shared libraries, so we also need to add the package to LD_LIBRARY_PATH on linux and DYLD_LIBRARY_PATH on mac. I haven't looked at Windows yet.

> I think this should be called "mozconfig.rust.linux" and live in build/unix.

I've renamed it, and dropped the DYLD_LIBRARY_PATH define to make it linux-specific. Paths are now relative to $topsrcdir.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b142b586aaf
Attachment #8623961 - Attachment is obsolete: true
Attachment #8624394 - Flags: review?(ted)
(In reply to Ralph Giles (:rillian) from comment #14)
> We need to add the tooltool package to the PATH so the configure check for
> rustc can find it.

If it doesn't already, we should make --enable-rust check $RUSTC in the environment first, and then we can just set RUSTC=/path/to/rustc in the mozconfig.

> Further, rustc relies on the loader for its shared
> libraries, so we also need to add the package to LD_LIBRARY_PATH on linux
> and DYLD_LIBRARY_PATH on mac. I haven't looked at Windows yet.

OK, that's a pain, but I guess we can live with it.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #15)

> If it doesn't already, we should make --enable-rust check $RUSTC in the
> environment first, and then we can just set RUSTC=/path/to/rustc in the
> mozconfig.

Patch in bug 1176294 for that.
Update patch to use RUSTC instead of messing with the PATH per bug 1176294.

There's a consistency check for options in the various mozconfigs. There's a whitelist (https://dxr.mozilla.org/mozilla-central/source/browser/config/mozconfigs/whitelist) but we want this code to ride the trains, so I enabled it for beta and release as well as nightly and debug.
Attachment #8624394 - Attachment is obsolete: true
Attachment #8624394 - Flags: review?(ted)
Attachment #8624912 - Flags: review?(ted)
Per glandium's comments, the patch from bug 1176294 isn't actually necessary for RUSTC to to work. Just these to patches are also green on try.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6adabdeb17ed
Comment on attachment 8623873 [details] [diff] [review]
Part 1 - Update releng tooltool manifest to import rust v2

Can you try to use the "unpack": "True" attribute in the releng.manifest for the rust and sccache packages instead of using the setup.sh script? The directory it gets unpacked to will be the basename of the file, so you can use "filename": "rust.tar.gz" or some such.
Attachment #8623873 - Flags: review?(mshal) → feedback+
Repackage rustc in a generic tarball so we can use the unpack attribute. Also use the unpack attribute for other tools so we can dispense with setup.sh.
Attachment #8623873 - Attachment is obsolete: true
Attachment #8625571 - Flags: review?(mshal)
Update mozconfig.rust.linux to use a shorter path since we're repackaging the rust toolchain from the official tarball.
Attachment #8624912 - Attachment is obsolete: true
Attachment #8624912 - Flags: review?(ted)
Attachment #8625572 - Flags: review?(ted)
Attachment #8625571 - Flags: review?(mshal) → review+
Blocks: 1177791
Comment on attachment 8625572 [details] [diff] [review]
Part 2 - Enable rust for linux64 automation builds v6

Review of attachment 8625572 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/config/mozconfigs/linux64/nightly
@@ +16,4 @@
>  # Use ccache
>  . "$topsrcdir/build/mozconfig.cache"
>  
> +# Enable rust

Either put this everywhere or don't put it anywhere. (I suspect the filename is self-documenting.)
Attachment #8625572 - Flags: review?(ted) → review+
Thanks! I've removed the comment.
Add rustc to the linux64-mulet manifest as well.

This build uses the nightly mozconfig, but a different tooltool manifest.
Attachment #8625571 - Attachment is obsolete: true
Attachment #8626717 - Flags: review?(mshal)
Comment on attachment 8626717 [details] [diff] [review]
Part 1 - Update releng tooltool manifest to import rust v7

LGTM - I guess a '-p all' try should help find any other random platform bustage.
Attachment #8626717 - Flags: review?(mshal) → review+
Depends on: 1177951
Comment on attachment 8625572 [details] [diff] [review]
Part 2 - Enable rust for linux64 automation builds v6

Review of attachment 8625572 [details] [diff] [review]:
-----------------------------------------------------------------

I just thought about this while discussing with Lars and Alex: this shouldn't ride the trains until it's really ready to (which at the very least would require rust to not include its own jemalloc), and this patch makes it so that it would.

At most, it should go in the nightly mozconfig, but even there, that rides to aurora, which is probably not desirable. I wonder if we have a variable available in mozconfigs to distinguish between nightly and aurora. I guess we don't, otherwise we wouldn't have different "nightly" mozconfigs on aurora (which, to me sounds silly)
Attachment #8625572 - Flags: review+ → review-
(In reply to Mike Hommey [:glandium] from comment #30)

> I just thought about this while discussing with Lars and Alex: this
> shouldn't ride the trains until it's really ready to

I disagree, but we should talk about that on the relevant bug. I thought the separate allocators were only a problem when passing ownership of memory between languages, which my code avoids.
Here's a try push with rust in _all_ the linux64 releng.manifests. It fails on valgrind (build doesn't use tooltool, bug 965151) mulet (may not invoke tooltool? I filed bug 1177951) various b2b emulators, and taskcluster.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d3281f00f46b

:mrrrgn I was confused when we talked Friday night. The taskcluster build does call tooltool as expected, configure finds the rust toolchain, it builds and runs tests successfully. I'm not sure why the job's failing. Maybe "mv: cannot stat '*.x-test.linux-x86_64.tar.bz2': No such file or directory" at the very end?
Flags: needinfo?(winter2718)
(In reply to Ralph Giles (:rillian) from comment #31)
> (In reply to Mike Hommey [:glandium] from comment #30)
> 
> > I just thought about this while discussing with Lars and Alex: this
> > shouldn't ride the trains until it's really ready to
> 
> I disagree, but we should talk about that on the relevant bug. I thought the
> separate allocators were only a problem when passing ownership of memory
> between languages, which my code avoids.

It also adds a significant memory overhead, especially if rust's jemalloc is configured with the defaults. It also blinds about:memory.
(In reply to Ralph Giles (:rillian) from comment #32)
> Here's a try push with rust in _all_ the linux64 releng.manifests. It fails
> on valgrind (build doesn't use tooltool, bug 965151) mulet (may not invoke
> tooltool? I filed bug 1177951) various b2b emulators, and taskcluster.
> 
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=d3281f00f46b
> 
> :mrrrgn I was confused when we talked Friday night. The taskcluster build
> does call tooltool as expected, configure finds the rust toolchain, it
> builds and runs tests successfully. I'm not sure why the job's failing.
> Maybe "mv: cannot stat '*.x-test.linux-x86_64.tar.bz2': No such file or
> directory" at the very end?

Ah, glad it's working. :) The failed move is indeed the problem. That can be changed via the DIST_UPLOAD environment variable (inside the task config). Sorry about that.
Flags: needinfo?(winter2718)
Ok. To sumarize, the linux64 desktop taskcluster build orange isn't from this patch set. The various b2g emulator build orange is from a timeout doing 'git update-ref' to match the manifest, which I don't think is this patch either.

So that leaves the two blocking issues already filed: valgrind.sh doesn't call tooltool, and neither does the linux64-mulet build.
Depends on: 965151
No longer depends on: 965151
Depends on: 965151
We can simply disable the rust code for valgrind until someone has time to fix valgrind. There is no value in blocking the non-valgrind builds on Linux.
Depends on: 1221317
I've been trying to get this to work over the last couple of weeks. The valgrind job uses tooltool now (yay!) so we're no longer blocked on that. However, the new gtk3 support is also patched in through tooltool, and our need to set LD_LIBRARY_PATH conflicts.

Glandium suggested moving the actual export line to a separate finalization step, which I did with a build/unix/mozconfig.ldpath fragment. That's (a) noisy and brittle, and (b) I can't actually get it to work; it somehow breaks the fc-cache invocation in the gtk3 tooltool package's setup.sh.

After further discussion, I filed https://github.com/rust-lang/rust/issues/29941 to request rustc official builds set an executable-relative rpath ($ORIGIN/../lib on linux, @executable_path/../lib on macosx). Turns out there's already an `--enable-rpath` switch for this in rust's configure script, so using this with a custom build is the immediate way forward.
Or I could add a wrapper script to the rust-tooltool packager.
I've spent the last several days trying to do a custom build of rust with --enable-rpath, but haven't been able to build a version which works on the integration images. I've tried various combinations of centos6, debian7, debian6, ubuntu lts images, and custom-build gcc (4.8.5 still builds on centos 6) and rust's --enable-llvm-static-stdcpp to try to construct a toolchain which can build rustc, running on a docker container old enough I don't get missing glibc version layers.

So far unsuccessful. Trying https://github.com/rust-lang/rust-buildbot next, which builds a lot of toolchain from source on top of centos5.
One thing that is not entirely clear to me is whether --enable-rpath is supposed to enable rpath for the rust compiler itself, of to enable the rust compiler setting it in things it produces, by default. Arguably, the latter is a superset of the former, but is not really desirable.

You're also not saying how your various attempts have failed.
(In reply to Mike Hommey [:glandium] from comment #40)

> You're also not saying how your various attempts have failed.

Generally variations on:

  /lib64/libc.so.6: version `GLIBC_2.15' not found (required by /builds/slave/try-l64-d-00000000000000000000/build/src/rustc/bin/../lib/librustc_llvm-a5fc0d6c.so)
  /usr/lib64/libstdc++.so.6: version `GLIBCXX_3.4.20' not found (required by /builds/slave/try-l64-d-00000000000000000000/build/src/rustc/bin/../lib/librustc_llvm-a5fc0d6c.so)

See for example https://treeherder.mozilla.org/#/jobs?repo=try&revision=d3990191ba10
(In reply to Mike Hommey [:glandium] from comment #40)

> One thing that is not entirely clear to me is whether --enable-rpath is
> supposed to enable rpath for the rust compiler itself, of to enable the rust
> compiler setting it in things it produces, by default. Arguably, the latter
> is a superset of the former, but is not really desirable.

I believe it enables it only when building the rust compiler itself.

`configure --enable-rpath` sets CFG_ENABLE_RPATH in config.mk. That appends `-C rpath` to CFG_RUSTC_FLAGS in mk/main.mk, which is passed to rustc during the rust build. The default value is still hard-coded false in the compiler source, so I don't see a way for this setting to affect the output of the built compiler.

https://github.com/rust-lang/rust/blob/master/src/librustc/session/config.rs#L479
My builds script efforts are available at https://github.com/rillian/rust-build and https://github.com/rillian/rust-buildbot.
I still get libstdc++ errors with rustc build with --enable-rpath --enable-llvm-static-stdcpp build with the rust-buildbot container.

 10:25:34     INFO -  /builds/slave/try-l64-d-00000000000000000000/build/src/rustc/bin/rustc: /usr/lib64/libstdc++.so.6: version `GLIBCXX_3.4.14' not found (required by /builds/slave/try-l64-d-00000000000000000000/build/src/rustc/bin/../lib/librustc_llvm-a5fc0d6c.so)
 10:25:34     INFO -  /builds/slave/try-l64-d-00000000000000000000/build/src/rustc/bin/rustc: /usr/lib64/libstdc++.so.6: version `GLIBCXX_3.4.15' not found (required by /builds/slave/try-l64-d-00000000000000000000/build/src/rustc/bin/../lib/librustc_llvm-a5fc0d6c.so)

I'm starting to wonder if the official rust binaries can work at all on our build machines. Now trying with the rust-buildbot linux container.
To clarify, the --enable-rpath option to the ./configure script of Rust itself will only build the compiler with rpath business. The compiler itself will never emit rpath information by default, and the `-C rpath` flag can be used to turn that on.

Those build failures seem somewhat suspicious, are you using the nightly builds or a compiler you're building yourself? If you build with --enable-llvm-static-stdcpp there shouldn't be any errors about a GLIBCXX version because that shouldn't even be a dependency. Additionally, we build our nightlies on really old CentOS 5 machines to have maximal glibc compatibility as well, so unless you're running even older builders than that it should in theory be working!

Note that if you're building your own compiler, however, the issues may come up again :(
Thanks, Alex. Building on the rust-buildbot slaves/linux docker container (the one mentioned it the readme, and based on ubuntu:14.04) with --enable-rpath ----enable-llvm-static-stdcpp I don't get the GLIBCXX errors, but I do some from GLIBC. https://treeherder.mozilla.org/logviewer.html#?job_id=14168670&repo=try#L11546

From what you say, official rust builds use the centos5-based container from the rust-buildbot slaves/dist Dockerfile, and I don't get the GLIBC version errors with rustc builds from that container. So either something about the centos5 toolchain is breaking --enable-llvm-static-stdcpp, or I misidentified the rustc packages I sent to try. I'll give that another go tomorrow.

To be clear, I'm trying to build rust 1.4 with --enable-rpath because that seemed easier than figuring out why adding rustc/lib to LD_LIBRARY_PATH broke the gtk3 build we use. If I can reproduce the GLIBCXX version errors, I'll try the wrapper script from comment #38 to set LD_LIBRARY_PATH just when invoking an official rustc build, which should work around the gtk3 issue until we have rust code that needs to talk to it.

> Note that if you're building your own compiler, however, the issues may come up again :(

I have to build my own C/C++ compiler on centos5 or centos6 (or debian6) to be able to build the rust compiler there; the included gcc doesn't support the C++11 features llvm uses.
I must have been mistaken before. Building rustc 1.4 on Centos 5 with --enable-rpath and --enable-llvm-static-stdcpp results in a build which works on try. Order is restored to the universe.

Asking for review again because it's been a while, and we need to whitelist the fragment to allow it in nightly (but not beta/release) builds.
Attachment #8625572 - Attachment is obsolete: true
Attachment #8626717 - Attachment is obsolete: true
Attachment #8693137 - Flags: review?(mshal)
The mulet linux64 build uses the same mozconfig as the desktop build, but a different tooltool manifest. This patch adds the same rustc 1.4 build to that manifest.

pushed to try. https://treeherder.mozilla.org/#/jobs?repo=try&revision=32276613f6c0
Attachment #8693175 - Flags: review?(mshal)
Glad to hear that it worked out! And yeah we had to end up compiling a custom gcc for that exact same reason as well (unfortunately). I wonder if that exacerbated --enable-llvm-static-stdcpp not working previously as well?
Comment on attachment 8693137 [details] [diff] [review]
Enable rust in linux64 automation builds v8

>diff --git a/browser/config/mozconfigs/linux64/debug b/browser/config/mozconfigs/linux64/debug
>index 60beae8..f03930e 100644
>--- a/browser/config/mozconfigs/linux64/debug
>+++ b/browser/config/mozconfigs/linux64/debug
>@@ -18,5 +18,6 @@ export MOZ_PACKAGE_JSSHELL=1
> 
> ac_add_options --with-branding=browser/branding/nightly
> 
>+. "$topsrcdir/build/unix/mozconfig.rust"

So we don't want rust for linux32? If we do, this line might be better placed in build/unix/mozconfig.linux
Attachment #8693137 - Flags: review?(mshal) → review+
Attachment #8693175 - Flags: review?(mshal) → review+
Thanks for the review. I just want linux64 until we get the general cross-compiling story sorted out.
(In reply to Ralph Giles (:rillian) from comment #51)
> Thanks for the review. I just want linux64 until we get the general
> cross-compiling story sorted out.

Ok, something to keep in mind then if we end up adding linux32 as well.
Component: Platform Support → Buildduty
Product: Release Engineering → Infrastructure & Operations
Product: Infrastructure & Operations → Infrastructure & Operations Graveyard
You need to log in before you can comment on or make changes to this bug.