1319230 - Add a Taskcluster job to build with the tup backend

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Michael Shal [:mshal]]

There should be a separate Taskcluster job that builds the tree with the tup backend. We'll need to have tup in tooltool for this.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

I got pretty far with this, but the builds hit an issue to do with Docker: "fuse: failed to open /dev/fuse: Operation not permitted".

From searching around about this, the workaround I can find suggests adding "--cap-add SYS_ADMIN --device /dev/fuse" to the `docker run` command line, which sounds like it will significantly compromise the container's isolation. I'm not sure if this is an option for TC or if there is a better workaround available.

Comment 2

[Chris Manchester (limited bugmail, email directly)]

Ok, I found the section in the docker docs: https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities, which says we need "--cap-add SYS_ADMIN" for this.

To add this would require patching docker-worker I expect, as in bug 1221661, but more to the point we were ok with adding that feature because it was only for testers, this could be a significant hurdle when we get to the point of evaluating a Tup build's suitability for shipping.

Comment 3

[Gregory Szorc [:gps]]

CAP_SYS_ADMIN is basically root and would almost completely undermine the process sandbox used by docker-worker. I reckon you'll have a hard time convincing people to give Docker containers that capability. See http://man7.org/linux/man-pages/man7/capabilities.7.html for more.

I suspect there's a way to make fuse work without CAP_SYS_ADMIN. CAP_SYS_ADMIN is probably the most convenient. Although it likely requires changes to docker-worker to expose fuse to containers.

Comment 4

[Gregory Szorc [:gps]]

I bet the first problem here is that cgroups isn't allowing access to the /dev/fuse device. You can fix that as a one-off. Why it needs CAP_SYS_ADMIN, I dunno. Extended attributes?

Comment 5

[Mike Hommey [:glandium]]

mount()

Comment 6

[Gregory Szorc [:gps]]

mount() itself /might/ be safe to expose to the container because presumably the container was started with its own mount namespace. But, yeah, if you can't call mount() without CAP_SYS_ADMIN, then you are in a predicament. We may have to solve this by having the container started with a fuse filesystem pre-mounted. That requires docker-worker cooperation. That also means moving the tup fuse bits into docker-worker. That'll be a rabbit hole.

Comment 7

[Michael Shal [:mshal]]

I'll also be looking at using an LD_PRELOAD shim again in tup instead of fuse, assuming that will work inside docker. If it does, that would get around us having to fiddle with capabilities I believe. (tup used to use LD_PRELOAD, but I switched it to fuse a while back for various reasons, like staticically-linked binary support).

Comment 8

[Gregory Szorc [:gps]]

LD_PRELOAD will work inside Docker as long as the library being loaded doesn't require system calls that the process doesn't have privileges to call :)

Comment 9

[Michael Shal [:mshal]]

Tup now has an LD_PRELOAD shim available again, and it appears to work fine in Docker. This is also nice because when we get to linking libxul, we won't have the same performance issues that we had with FUSE. I'll handle that in bug 1387098.

Comment 10

[Michael Shal [:mshal]]

Attachment: Bug 1319230 - Add a Taskcluster job to build with the tup backend;

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

Comment 11

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8963393 [details]
Bug 1319230 - Add a Taskcluster job to build with the tup backend;

https://reviewboard.mozilla.org/r/232300/#review237742

Looks sensible to me, but see my one small comment.  Exciting to see this moving forward!

::: testing/mozharness/configs/builds/releng_sub_linux_configs/64_tup.py:3
(Diff revision 1)
> +import os
> +
> +config = {

I expect you want something like https://searchfox.org/mozilla-central/source/testing/mozharness/configs/builds/releng_sub_linux_configs/64_rusttests.py#29 here, since artifact builds probably don't make sense for Tup-backend builds.  (Yet?)

Comment 12

[Michael Shal [:mshal]]

(In reply to Nick Alexander :nalexander from comment #11)
> I expect you want something like
> https://searchfox.org/mozilla-central/source/testing/mozharness/configs/
> builds/releng_sub_linux_configs/64_rusttests.py#29 here, since artifact
> builds probably don't make sense for Tup-backend builds.  (Yet?)

Ahh, good catch! I think chmanchester had artifact+tup working at one point, but it appears they don't work together now.

Comment 13

[Michael Shal [:mshal]]

Comment on attachment 8963393 [details]
Bug 1319230 - Add a Taskcluster job to build with the tup backend;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/232300/diff/1-2/

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/18a14e2a17c6
Add a Taskcluster job to build with the tup backend; r=nalexander

Comment 15

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/18a14e2a17c6