Bug 1427312 - Add mechanism to create tasks to compare Firefox builds. r=dustin draft
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 28 Dec 2017 12:14:34 +0900
changeset 716711 12389af6e40563e9c94990caf7028d90064d44a8
parent 716710 200ba1aec4af2f08e7a61172e0e860c8db21911c
child 745083 3f373f47b79af24ac676756ac0e0d9ee41ffb485
push id94489
push userbmo:mh+mozilla@glandium.org
push dateSat, 06 Jan 2018 05:26:55 +0000
reviewersdustin
bugs1427312
milestone59.0a1
Bug 1427312 - Add mechanism to create tasks to compare Firefox builds. r=dustin There are e.g. some build infrastructure changes that we want to have a controlled impact on the Firefox builds we produce. We have, in multiple occasions, gone through manual work to compare Firefox builds, most of the time using the diffoscope tool (https://diffoscope.org/). This change introduces a new task kind that takes two Firefox builds as input, either by name (reference to a build from the current task graph) or by index (reference to a build from a previous push), and compares them. In order to get a Firefox build by index, we rely on dummy tasks with an optimization we expect to always hit, so we add the necessary bits to ensure those dummy tasks can go through up to the optimization phase and be optimized out there.
taskcluster/ci/diffoscope/kind.yml
taskcluster/ci/docker-image/kind.yml
taskcluster/docker/diffoscope/Dockerfile
taskcluster/docker/diffoscope/get_and_diffoscope
taskcluster/docs/kinds.rst
taskcluster/docs/transforms.rst
taskcluster/taskgraph/transforms/diffoscope.py
taskcluster/taskgraph/transforms/job/__init__.py
taskcluster/taskgraph/transforms/task.py
taskcluster/taskgraph/util/verify.py
taskcluster/taskgraph/util/workertypes.py
new file mode 100644
--- /dev/null
+++ b/taskcluster/ci/diffoscope/kind.yml
@@ -0,0 +1,59 @@
+# 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/.
+
+loader: taskgraph.loader.transform:loader
+
+kind-dependencies:
+  - build
+  - toolchain
+
+transforms:
+  - taskgraph.transforms.diffoscope:transforms
+  - taskgraph.transforms.use_toolchains:transforms
+  - taskgraph.transforms.job:transforms
+  - taskgraph.transforms.task:transforms
+
+# Note: --exclude-command .--line-numbers is because of
+# https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=879003
+# That skips `objdump --disassemble --line-numbers` and falls back to
+# `objdump --disassemble`
+# Note: the .chk excludes are for files that are known to differ between
+# builds because they are signed with an ephemeral private key that is
+# generated for each build.
+job-defaults:
+  args: >-
+    --no-default-limits
+    --max-page-size 100000000
+    --max-page-diff-block-lines 10000
+    --exclude-directory-metadata
+    --exclude-command .--line-numbers
+    --exclude **/*freeblpriv3.chk
+    --exclude **/*nssdbm3.chk
+    --exclude **/*softokn3.chk
+
+# Make a task for each diff we might want. The following are just examples,
+# Both original and new can point to builds from the full set of tasks or
+# from other sets through an index-search. Other kinds than `build` can be
+# compared (for example, static-analysis), provided you adjust the
+# kind-dependencies above.
+
+# jobs:
+#   android-build-vs-previous-try:
+#     symbol: A
+#     new: build-android-api-16/opt
+#     original: {index-search: gecko.v2.try.revision.aabd5deb0156f9b55ab60ad6a01ebfc4580bf2e1.mobile.android-api-16-opt}
+#   linux64-build-vs-previous-try:
+#     symbol: L
+#     new: build-linux64/opt
+#     original: {index-search: gecko.v2.try.revision.aabd5deb0156f9b55ab60ad6a01ebfc4580bf2e1.firefox.linux64-opt}
+#     extra-args: >-
+#       --exclude-command .--hex-dump=.gnu_debuglink
+#   macosx-build-vs-previous-try:
+#     symbol: M
+#     new: build-macosx64/opt
+#     original: {index-search: gecko.v2.try.revision.aabd5deb0156f9b55ab60ad6a01ebfc4580bf2e1.firefox.macosx64-opt}
+#   win32-build-vs-previous-try:
+#     symbol: W
+#     new: build-win32/opt
+#     original: {index-search: gecko.v2.try.revision.aabd5deb0156f9b55ab60ad6a01ebfc4580bf2e1.firefox.win32-opt}
--- a/taskcluster/ci/docker-image/kind.yml
+++ b/taskcluster/ci/docker-image/kind.yml
@@ -38,8 +38,10 @@ jobs:
   google-play-strings:
     symbol: I(gps)
   funsize-balrog-submitter:
     symbol: I(fbs)
   beet-mover:
     symbol: I(bm)
   update-verify:
     symbol: I(uv)
+  diffoscope:
+    symbol: I(diff)
new file mode 100644
--- /dev/null
+++ b/taskcluster/docker/diffoscope/Dockerfile
@@ -0,0 +1,43 @@
+FROM debian:stretch-20171210
+MAINTAINER Mike Hommey <mhommey@mozilla.com>
+
+RUN mkdir /builds
+RUN useradd -d /builds/worker -s /bin/bash -m worker
+WORKDIR /builds/worker
+
+# Set variable normally configured at login, by the shells parent process, these
+# are taken from GNU su manual
+ENV HOME=/builds/worker \
+    SHELL=/bin/bash \
+    USER=worker \
+    LOGNAME=worker \
+    HOSTNAME=taskcluster-worker \
+    LANG=en_US.UTF-8 \
+    LC_ALL=en_US.UTF-8 \
+    DEBIAN_FRONTEND=noninteractive
+
+# Set a default command useful for debugging
+CMD ["/bin/bash", "--login"]
+
+# Set apt sources list to a snapshot.
+RUN for s in debian_stretch debian_stretch-updates debian-security_stretch/updates; do \
+      echo "deb [check-valid-until=no] http://snapshot.debian.org/archive/${s%_*}/20171222T153610Z/ ${s#*_} main"; \
+    done > /etc/apt/sources.list
+
+RUN apt-get update -q && \
+    apt-get install -yyq diffoscope libc++abi1 locales python3-setuptools python2.7 python-pip git && \
+    sed -i '/en_US.UTF-8/s/^# *//' /etc/locale.gen && \
+    locale-gen && \
+    git clone https://anonscm.debian.org/git/reproducible/diffoscope.git /tmp/diffoscope && \
+    git -C /tmp/diffoscope checkout 202caf9d5d134e95f870d5f19f89511d635c27e4 && \
+    (cd /tmp/diffoscope && python3 setup.py install ) && \
+    rm -rf /tmp/diffoscope && \
+    apt-get clean
+
+# %include taskcluster/docker/recipes/run-task
+COPY topsrcdir/taskcluster/docker/recipes/run-task /builds/worker/bin/run-task
+
+COPY get_and_diffoscope /builds/worker/bin/get_and_diffoscope
+
+RUN chown -R worker:worker /builds/worker/bin && chmod 755 /builds/worker/bin/*
+
new file mode 100644
--- /dev/null
+++ b/taskcluster/docker/diffoscope/get_and_diffoscope
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+set -e
+set -x
+
+cd /builds/worker
+
+mkdir a b
+
+# Until https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=879010 is
+# implemented, it's better to first manually extract the data.
+# Plus dmg files are not supported yet.
+
+case "$ORIG_URL" in
+*/target.zip|*/target.apk)
+	curl -sL "$ORIG_URL" > a.zip
+	curl -sL "$NEW_URL" > b.zip
+	unzip -d a a.zip
+	unzip -d b b.zip
+	;;
+*/target.tar.bz2)
+	curl -sL "$ORIG_URL" | tar -C a -jxf -
+	curl -sL "$NEW_URL" | tar -C b -jxf -
+	;;
+*/target.dmg)
+	# We don't have mach available to call mach artifact toolchain.
+	# This is the trivial equivalent for those toolchains we use here.
+	for t in $MOZ_TOOLCHAINS; do
+		curl -sL https://queue.taskcluster.net/v1/task/${t#*@}/artifacts/${t%@*} | tar -Jxf -
+	done
+	for tool in lipo otool; do
+		ln -s /builds/worker/cctools/bin/x86_64-apple-darwin*-$tool bin/$tool
+	done
+	export PATH=$PATH:/builds/worker/bin
+	curl -sL "$ORIG_URL" > a.dmg
+	curl -sL "$NEW_URL" > b.dmg
+	for i in a b; do
+		dmg/dmg extract $i.dmg $i.hfs
+		dmg/hfsplus $i.hfs extractall / $i
+	done
+	;;
+esac
+
+# Builds are 99% of the time differing in some small ways, so it's not
+# really useful to report a failure (at least not until we actually
+# care about the builds being 100% identical).
+diffoscope \
+	--html diff.html \
+	--text diff.txt \
+	--progress \
+	$DIFFOSCOPE_ARGS \
+	a b || true
--- a/taskcluster/docs/kinds.rst
+++ b/taskcluster/docs/kinds.rst
@@ -352,8 +352,14 @@ Dummy tasks to consolidate beetmover dep
 
 post-beetmover-checksums-dummy
 ------------------------------
 Dummy tasks to consolidate beetmover-checksums dependencies to avoid taskcluster limits on number of dependencies per task.
 
 packages
 --------
 Tasks used to build packages for use in docker images.
+
+diffoscope
+----------
+Tasks used to compare pairs of Firefox builds using https://diffoscope.org/.
+As of writing, this is mainly meant to be used in try builds, by editing
+taskcluster/ci/diffoscope/kind.yml for your needs.
--- a/taskcluster/docs/transforms.rst
+++ b/taskcluster/docs/transforms.rst
@@ -141,16 +141,17 @@ following ``run-using`` are available
   * ``hazard``
   * ``mach``
   * ``mozharness``
   * ``mozharness-test``
   * ``run-task``
   * ``spidermonkey`` or ``spidermonkey-package`` or ``spidermonkey-mozjs-crate`` or ``spidermonkey-rust-bindings``
   * ``debian-package``
   * ``toolchain-script``
+  * ``always-optimized``
 
 
 Task Descriptions
 -----------------
 
 Every kind needs to create tasks, and all of those tasks have some things in
 common.  They all run on one of a small set of worker implementations, each
 with their own idiosyncracies.  And they all report to TreeHerder in a similar
new file mode 100644
--- /dev/null
+++ b/taskcluster/taskgraph/transforms/diffoscope.py
@@ -0,0 +1,158 @@
+# 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/.
+"""
+This transform construct tasks to perform diffs between builds, as
+defined in kind.yml
+"""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+from taskgraph.transforms.base import TransformSequence
+from taskgraph.util.schema import (
+    Schema,
+    validate_schema,
+)
+from taskgraph.util.taskcluster import get_artifact_url
+from voluptuous import (
+    Any,
+    Optional,
+    Required,
+)
+
+transforms = TransformSequence()
+
+index_or_string = Any(
+    basestring,
+    {Required('index-search'): basestring},
+)
+
+diff_description_schema = Schema({
+    # Name of the diff task.
+    Required('name'): basestring,
+
+    # Treeherder symbol.
+    Required('symbol'): basestring,
+
+    # relative path (from config.path) to the file the task was defined in.
+    Optional('job-from'): basestring,
+
+    # Original and new builds to compare.
+    Required('original'): index_or_string,
+    Required('new'): index_or_string,
+
+    # Arguments to pass to diffoscope, used for job-defaults in
+    # taskcluster/ci/diffoscope/kind.yml
+    Optional('args'): basestring,
+
+    # Extra arguments to pass to diffoscope, that can be set per job.
+    Optional('extra-args'): basestring,
+})
+
+
+@transforms.add
+def validate(config, tasks):
+    for task in tasks:
+        yield validate_schema(
+            diff_description_schema, task,
+            "In diff task {!r}:".format(task.get('name', 'unknown')))
+
+
+@transforms.add
+def fill_template(config, tasks):
+    dummy_tasks = {}
+
+    for task in tasks:
+        name = task['name']
+
+        deps = {}
+        urls = {}
+        previous_artifact = None
+        for k in ('original', 'new'):
+            value = task[k]
+            if isinstance(value, basestring):
+                deps[k] = value
+                task_id = '<{}>'.format(k)
+                os_hint = value
+            else:
+                index = value['index-search']
+                if index not in dummy_tasks:
+                    dummy_tasks[index] = {
+                        'label': 'index-search-' + index,
+                        'description': index,
+                        'worker-type': 'invalid/always-optimized',
+                        'run': {
+                            'using': 'always-optimized',
+                        },
+                        'optimization': {
+                            'index-search': [index],
+                        }
+                    }
+                    yield dummy_tasks[index]
+                deps[index] = 'index-search-' + index
+                task_id = '<{}>'.format(index)
+                os_hint = index.split('.')[-1]
+            if 'linux' in os_hint:
+                artifact = 'target.tar.bz2'
+            elif 'macosx' in os_hint:
+                artifact = 'target.dmg'
+            elif 'android' in os_hint:
+                artifact = 'target.apk'
+            elif 'win' in os_hint:
+                artifact = 'target.zip'
+            else:
+                raise Exception(
+                    'Cannot figure out the OS for {!r}'.format(value))
+            if previous_artifact is not None and previous_artifact != artifact:
+                raise Exception(
+                    'Cannot compare builds from different OSes')
+            url = get_artifact_url(task_id, 'public/build/{}'.format(artifact))
+            urls[k] = {'task-reference': url}
+            previous_artifact = artifact
+
+        taskdesc = {
+            'label': 'diff-' + name,
+            'description': name,
+            'treeherder': {
+                'symbol': task['symbol'],
+                'platform': 'diff/opt',
+                'kind': 'other',
+                'tier': 2,
+            },
+            'worker-type': 'aws-provisioner-v1/gecko-{}-b-linux'.format(
+               config.params['level']),
+            'worker': {
+                'docker-image': {'in-tree': 'diffoscope'},
+                'artifacts': [{
+                    'type': 'file',
+                    'path': '/builds/worker/diff.html',
+                    'name': 'public/diff.html',
+                }, {
+                    'type': 'file',
+                    'path': '/builds/worker/diff.txt',
+                    'name': 'public/diff.txt',
+                }],
+                'env': {
+                    'ORIG_URL': urls['original'],
+                    'NEW_URL': urls['new'],
+                    'DIFFOSCOPE_ARGS': ' '.join(
+                        task[k] for k in ('args', 'extra-args') if k in task)
+                },
+                'max-run-time': 1800,
+            },
+            'run': {
+                'using': 'run-task',
+                'checkout': False,
+                'command': '/builds/worker/bin/get_and_diffoscope '
+                           '"$ORIG_URL" "$NEW_URL"',
+            },
+            'dependencies': deps,
+        }
+
+        if artifact.endswith('.dmg'):
+            taskdesc['toolchains'] = [
+                'linux64-cctools-port',
+                'linux64-libdmg',
+            ]
+
+        yield taskdesc
--- a/taskcluster/taskgraph/transforms/job/__init__.py
+++ b/taskcluster/taskgraph/transforms/job/__init__.py
@@ -189,16 +189,22 @@ def run_job_using(worker_implementation,
         if worker_implementation in for_run_using:
             raise Exception("run_job_using({!r}, {!r}) already exists: {!r}".format(
                 run_using, worker_implementation, for_run_using[run_using]))
         for_run_using[worker_implementation] = (func, schema, defaults)
         return func
     return wrap
 
 
+@run_job_using('always-optimized', 'always-optimized',
+               Schema({'using': 'always-optimized'}))
+def always_optimized(config, job, taskdesc):
+    pass
+
+
 def configure_taskdesc_for_run(config, job, taskdesc, worker_implementation):
     """
     Run the appropriate function for this job against the given task
     description.
 
     This will raise an appropriate error if no function exists, or if the job's
     run is not valid according to the schema.
     """
--- a/taskcluster/taskgraph/transforms/task.py
+++ b/taskcluster/taskgraph/transforms/task.py
@@ -552,16 +552,20 @@ task_description_schema = Schema({
 
     }, {
         Required('implementation'): 'invalid',
         # an invalid task is one which should never actually be created; this is used in
         # release automation on branches where the task just doesn't make sense
         Extra: object,
 
     }, {
+        Required('implementation'): 'always-optimized',
+        Extra: object,
+
+    }, {
         Required('implementation'): 'push-apk',
 
         # list of artifact URLs for the artifacts that should be beetmoved
         Required('upstream-artifacts'): [{
             # taskId of the task with the artifact
             Required('taskId'): taskref_or_string,
 
             # type of signing task (for CoT)
@@ -1066,16 +1070,21 @@ def build_push_apk_breakpoint_payload(co
     task_def['payload'] = task['worker']['payload']
 
 
 @payload_builder('invalid')
 def build_invalid_payload(config, task, task_def):
     task_def['payload'] = 'invalid task - should never be created'
 
 
+@payload_builder('always-optimized')
+def build_always_optimized_payload(config, task, task_def):
+    task_def['payload'] = {}
+
+
 @payload_builder('native-engine')
 def build_macosx_engine_payload(config, task, task_def):
     worker = task['worker']
     artifacts = map(lambda artifact: {
         'name': artifact['name'],
         'path': artifact['path'],
         'type': artifact['type'],
         'expires': task_def['expires'],
--- a/taskcluster/taskgraph/util/verify.py
+++ b/taskcluster/taskgraph/util/verify.py
@@ -144,17 +144,18 @@ def verify_dependency_tiers(task, taskgr
 
         for task in taskgraph.tasks.itervalues():
             # Buildbot bridge tasks cannot have tiers, so we cannot enforce
             # this check for them
             if task.task.get("workerType") == "buildbot-bridge":
                 continue
             tier = tiers[task.label]
             for d in task.dependencies.itervalues():
-                if taskgraph[d].task.get("workerType") == "buildbot-bridge":
+                if taskgraph[d].task.get("workerType") in ("buildbot-bridge",
+                                                           "always-optimized"):
                     continue
                 if "dummy" in taskgraph[d].kind:
                     continue
                 if tier < tiers[d]:
                     raise Exception(
                         '{} (tier {}) cannot depend on {} (tier {})'
                         .format(task.label, printable_tier(tier),
                                 d, printable_tier(tiers[d])))
@@ -176,8 +177,19 @@ def verify_bbb_builders_valid(task, task
         return
     if task.task.get('workerType') == 'buildbot-bridge':
         buildername = task.task['payload']['buildername']
         if buildername not in valid_builders:
             logger.warning(
                 '{} uses an invalid buildbot buildername ("{}") '
                 ' - contact #releng for help'
                 .format(task.label, buildername))
+
+
+@verifications.add('optimized_task_graph')
+def verify_always_optimized(task, taskgraph, scratch_pad):
+    """
+        This function ensures that always-optimized tasks have been optimized.
+    """
+    if task is None:
+        return
+    if task.task.get('workerType') == 'always-optimized':
+        raise Exception('Could not optimize the task {!r}'.format(task.label))
--- a/taskcluster/taskgraph/util/workertypes.py
+++ b/taskcluster/taskgraph/util/workertypes.py
@@ -28,16 +28,17 @@ WORKER_TYPES = {
     'aws-provisioner-v1/gecko-t-win10-64-gpu': ('generic-worker', 'windows'),
     'releng-hardware/gecko-t-win10-64-hw': ('generic-worker', 'windows'),
     'aws-provisioner-v1/gecko-t-win7-32': ('generic-worker', 'windows'),
     'aws-provisioner-v1/gecko-t-win7-32-gpu': ('generic-worker', 'windows'),
     'releng-hardware/gecko-t-win7-32-hw': ('generic-worker', 'windows'),
     'aws-provisioner-v1/taskcluster-generic': ('docker-worker', 'linux'),
     'buildbot-bridge/buildbot-bridge': ('buildbot-bridge', None),
     'invalid/invalid': ('invalid', None),
+    'invalid/always-optimized': ('always-optimized', None),
     'null-provisioner/human-breakpoint': ('push-apk-breakpoint', None),
     'null-provisioner/human-breakpoint': ('push-apk-breakpoint', None),
     'releng-hardware/gecko-t-linux-talos': ('native-engine', 'linux'),
     'scriptworker-prov-v1/balrogworker-v1': ('balrog', None),
     'scriptworker-prov-v1/beetmoverworker-v1': ('beetmover', None),
     'scriptworker-prov-v1/pushapk-v1': ('push-apk', None),
     "scriptworker-prov-v1/signing-linux-v1": ('scriptworker-signing', None),
     'releng-hardware/gecko-t-osx-1010': ('generic-worker', 'macosx'),