Closed Bug 1213627 Opened 5 years ago Closed 5 years ago

Add ninja to the desktop-build image to support building clang 3.7

Categories

(Release Engineering :: General, defect)

defect
Not set
normal

Tracking

(firefox44 fixed)

RESOLVED FIXED
Tracking Status
firefox44 --- fixed

People

(Reporter: ehsan, Assigned: ehsan)

Details

Attachments

(2 files, 1 obsolete file)

No description provided.
Attachment #8672342 - Flags: review?(dustin)
Comment on attachment 8672342 [details] [diff] [review]
Add ninja to the desktop-build image to support building clang 3.7

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

::: testing/docker/centos6-build/system-setup.sh
@@ +424,5 @@
>  
>  # TC-VCS
>  npm install -g taskcluster-vcs@2.3.12
>  
> +# Ninja

As with the other download-and-build stanzas, this should cd to $BUILD.  Then there's no need to explicitly delete the git repo.

@@ +425,5 @@
>  # TC-VCS
>  npm install -g taskcluster-vcs@2.3.12
>  
> +# Ninja
> +git clone git://github.com/martine/ninja.git

Like the other builds in system-setup.sh, we need to make sure that we're building a predictable version of ninja, and that the input bits are verified with a hash of some sort.  Ideally the bits would come from Mozilla-hosted infra so that we're not at the whim of someone else (like martine) to delete them.

Probably the easiest way to accomplish this is to download the appropriate release (https://github.com/martine/ninja/releases/tag/v1.6.0) and push it to tooltool, with a comment about its source.  But downloading a tarball or zip from GitHub would be OK too, as long as the result was verified with a hash.

@@ +431,5 @@
> +git checkout release
> +./configure.py --bootstrap
> +cp ninja /usr/local/bin/ninja
> +# Old versions of Cmake can only find ninja in this locatipn!
> +cp ninja /usr/local/bin/ninja-build

Might as well make one of these a symlink

::: testing/docker/desktop-build/VERSION
@@ +1,1 @@
> +0.1.11

You'll also need to bump centos6-build-upd
Attachment #8672342 - Flags: review?(dustin) → review-
Thanks for the review!
Attachment #8673094 - Flags: review?(dustin)
Attachment #8672342 - Attachment is obsolete: true
Comment on attachment 8673094 [details] [diff] [review]
Add ninja to the desktop-build image to support building clang 3.7

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

Let me know if you need a hand pushing docker images before this lands.
Attachment #8673094 - Flags: review?(dustin) → review+
Thanks, I'm not in a rush.  Planning to land this when the trees reopen.  Do I need to do something specific to regenerate the docker image after it lands?
You need to regenerate *before* it lands, otherwise all of the tasks will look for an image that doesn't exist.

If you've already built them and can push them somewhere that I can pull and re-push to taskcluster, that might be easiest.  Otherwise, I can apply your patch and rebuild.
I'd appreciate if you can please rebuild the image.  I have no idea how to do that.  :-)

Please let me know when I'm clear to land this.  Thanks!!
Flags: needinfo?(dustin)
OK!  I ran testing/docker/build.sh <image> for each image, then

docker push taskcluster/centos6-build:0.1.2
docker push taskcluster/centos6-build-upd:20151013094100
docker push taskcluster/desktop-build:0.1.11

so you should be good to land!
Flags: needinfo?(dustin)
Hmm, there seems to be a problem with the new docker image.  It seems to have no files in /usr/local/bin:

$ docker run -ti taskcluster/desktop-build:0.1.11
[root@taskcluster-worker ~]# ls /usr/local/bin
[root@taskcluster-worker ~]#
$ docker run -ti taskcluster/centos6-build:0.1.2
[root@taskcluster-worker ~]# ls /usr/local/bin
ninja  ninja-build
[root@taskcluster-worker ~]#
Flags: needinfo?(dustin)
https://hg.mozilla.org/mozilla-central/rev/7b9a08825f7d
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Oh, the Dockerfiles' FROM lines need to be updated to point to the correct image name:

diff --git a/testing/docker/centos6-build-upd/Dockerfile b/testing/docker/centos6-build-upd/Dockerfile
--- a/testing/docker/centos6-build-upd/Dockerfile
+++ b/testing/docker/centos6-build-upd/Dockerfile
@@ -1,9 +1,9 @@
-FROM          taskcluster/centos6-build:0.1.1
+FROM          taskcluster/centos6-build:0.1.2
 MAINTAINER    Dustin J. Mitchell <dustin@mozilla.com>
 
 ### update to latest from upstream repositories
 # if this becomes a long list of packages, consider bumping the
 # centos6-build version
 RUN yum update -y
 
 # Set a default command useful for debugging
diff --git a/testing/docker/desktop-build/Dockerfile b/testing/docker/desktop-build/Dockerfile
--- a/testing/docker/desktop-build/Dockerfile
+++ b/testing/docker/desktop-build/Dockerfile
@@ -1,9 +1,9 @@
-FROM          taskcluster/centos6-build-upd:20150930093701
+FROM          taskcluster/centos6-build-upd:20151013094100
 MAINTAINER    Dustin J. Mitchell <dustin@mozilla.com>
 
 # Add build scripts; these are the entry points from the taskcluster worker, and
 # operate on environment variables
 ADD             bin /home/worker/bin
 RUN             chmod +x /home/worker/bin/*
 
 # Generate machine uuid file


Sorry that I completely missed that in the review!  I'll re-generate the images with that change and land.
Flags: needinfo?(dustin)
Comment on attachment 8673661 [details]
MozReview Request: Bug 1213627: update Dockerfile FROM lines to match; r?ehsan

https://reviewboard.mozilla.org/r/21951/#review19717

Thanks, sorry I missed this!
Attachment #8673661 - Flags: review?(ehsan) → review+
OK, I re-pushed the latter two images after confirming they contain ninja.
https://hg.mozilla.org/mozilla-central/rev/2e7dc4e0222e
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.