Bug 98889 - Run continuous integration on OSs newer than Ubuntu 14.04
Summary: Run continuous integration on OSs newer than Ubuntu 14.04
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.10
Hardware: Other All
: low enhancement
Assignee: Simon McVittie
QA Contact: D-Bus Maintainers
URL:
Whiteboard: review?
Keywords: patch
Depends on:
Blocks:
 
Reported: 2016-11-28 15:59 UTC by Simon McVittie
Modified: 2017-02-24 19:48 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
travis-ci: add an explicit copyright/license statement (2.97 KB, patch)
2016-11-28 17:55 UTC, Simon McVittie
Details | Splinter Review
ci-build: retab with 4-space indentation (12.76 KB, patch)
2016-11-28 17:58 UTC, Simon McVittie
Details | Splinter Review
travis-ci: consistently use ci_* for parameter variables (6.55 KB, patch)
2016-11-28 17:59 UTC, Simon McVittie
Details | Splinter Review
travis-ci: consistently use yes/no instead of yes/empty (3.06 KB, patch)
2016-11-28 17:59 UTC, Simon McVittie
Details | Splinter Review
travis-ci: run in bash, with the "unofficial strict mode" (1.55 KB, patch)
2016-11-28 18:00 UTC, Simon McVittie
Details | Splinter Review
travis-ci: introduce maybe_fail_tests() to make test failure more obvious (2.63 KB, patch)
2016-11-28 18:00 UTC, Simon McVittie
Details | Splinter Review
travis-ci: add an install script instead of open-coding it in .travis.yml (5.06 KB, patch)
2016-11-28 18:01 UTC, Simon McVittie
Details | Splinter Review
travis-ci: Add and use infrastructure to build and test in Docker (5.63 KB, patch)
2016-11-28 18:03 UTC, Simon McVittie
Details | Splinter Review

Description Simon McVittie 2016-11-28 15:59:08 UTC
Travis-CI currently builds and tests dbus on Ubuntu 14.04. However, Bug #98666 introduces an optional dependency that is not satisfiable in 14.04.

The maintainers of Travis-CI show no signs of offering a less obsolete build environment, and recommend using Docker as a workaround. I'm trying that out, with extra builds in Debian 8, Debian 9 (currently a development snapshot, to be released next year) and Ubuntu 16.04 LTS.

This is based on the CI build scripts that I contributed to OSTree, which in turn were based on the ones currently in dbus. I've kept them sufficiently generic that there's no reason they couldn't be used with a non-Travis CI framework (Jenkins?), or even with a non-Debian-derived build OS (Fedora?) at some point in the future.
Comment 1 Simon McVittie 2016-11-28 16:06:45 UTC
Patches are not ready yet, removing patch tag.
Comment 2 Simon McVittie 2016-11-28 17:55:33 UTC
Created attachment 128244 [details] [review]
travis-ci: add an explicit copyright/license statement

---

I'm being permissive (Expat's MIT/X11 license) because I want to reuse these scripts elsewhere, such as for OSTree.

This patch series is for master, or for dbus-1.10 with commits 095ca78, 823801b, b5c229d, 6292e7c cherry-picked (which I intend to do - they don't touch files other than tools/ci-build.sh and .travis.yml, and it's simplest if we have the same CI setup on both master and the latest stable-branch).
Comment 3 Simon McVittie 2016-11-28 17:58:57 UTC
Created attachment 128245 [details] [review]
ci-build: retab with 4-space indentation

This realigns it with the script loosely based on this one that I
sent to OSTree.

---

Here is the diff with --ignore-space-change, which is what reviewers should actually look at:

diff --git a/tools/ci-build.sh b/tools/ci-build.sh
index a50ec16..9ee097e 100755
--- a/tools/ci-build.sh
+++ b/tools/ci-build.sh
@@ -224,3 +224,5 @@ case "$dbus_ci_buildsys" in
         ( cd DESTDIR && find . )
         ;;
 esac
+
+# vim:set sw=4 sts=4 et:

It also replaces each U+0009 CHARACTER TABULATION with 4x U+0020 SPACE.
Comment 4 Simon McVittie 2016-11-28 17:59:22 UTC
Created attachment 128246 [details] [review]
travis-ci: consistently use ci_* for parameter  variables

This aligns it with the more generic script based on this one that
I sent to OSTree.
Comment 5 Simon McVittie 2016-11-28 17:59:39 UTC
Created attachment 128247 [details] [review]
travis-ci: consistently use yes/no instead of yes/empty
Comment 6 Simon McVittie 2016-11-28 18:00:03 UTC
Created attachment 128248 [details] [review]
travis-ci: run in bash, with the "unofficial strict  mode"

set -u forces us to set all variables that we use (for example with the
${foo:=bar} syntax to take an existing value or set a default), or use the
${foo:-bar} syntax to make it explicit that the variable might be unset.

set -o pipefail (which is a bash feature) detects failure in non-last
elements of a pipeline.
Comment 7 Simon McVittie 2016-11-28 18:00:26 UTC
Created attachment 128249 [details] [review]
travis-ci: introduce maybe_fail_tests() to make test  failure more obvious
Comment 8 Simon McVittie 2016-11-28 18:01:11 UTC
Created attachment 128250 [details] [review]
travis-ci: add an install script instead of open-coding  it in .travis.yml

---

This also adds the groundwork for running CI tests on a non-Debian-derived distribution, if someone wants to.
Comment 9 Simon McVittie 2016-11-28 18:03:23 UTC
Created attachment 128251 [details] [review]
travis-ci: Add and use infrastructure to build and test  in Docker

Debian stable, Debian testing and Ubuntu LTS provide a reasonable
spectrum of old and new distributions. I'm only doing one build on
each to avoid a combinatorial explosion of options.

The Docker images don't have any deb-src apt sources set up, so don't
use `apt-get build-dep`; just include dependencies manually.

---

Known issue: I also tried switching from a "production" to "debug" build as the default, but test-dbus.sh fails on at least Ubuntu Xenial, with a leaked fd. I think that might actually be a fd leaking in from outside the tests, for example from Docker itself - I haven't chased that up yet.
Comment 10 Simon McVittie 2016-11-28 18:04:17 UTC
I'm probably not going to block on review for the CI stuff, because it's only the CI stuff, and it isn't run at all in production; so only review it if you particularly feel like doing so.
Comment 11 Philip Withnall 2016-11-29 11:05:01 UTC
Comment on attachment 128244 [details] [review]
travis-ci: add an explicit copyright/license statement

Review of attachment 128244 [details] [review]:
-----------------------------------------------------------------

r+

::: .travis.yml
@@ +1,1 @@
> +# Copyright © 2015-2016 Collabora Ltd.

Seems to be Unicode corruption here, but I’m assuming that’s due to an unhealthy combination of git-bz, Splinter, Bugzilla and duct tape.
Comment 12 Philip Withnall 2016-11-29 11:05:36 UTC
Comment on attachment 128245 [details] [review]
ci-build: retab with 4-space indentation

Review of attachment 128245 [details] [review]:
-----------------------------------------------------------------

Looks legit. r+
Comment 13 Philip Withnall 2016-11-29 11:07:05 UTC
Comment on attachment 128246 [details] [review]
travis-ci: consistently use ci_* for parameter  variables

Review of attachment 128246 [details] [review]:
-----------------------------------------------------------------

r+
Comment 14 Philip Withnall 2016-11-29 11:08:25 UTC
Comment on attachment 128247 [details] [review]
travis-ci: consistently use yes/no instead of yes/empty

Review of attachment 128247 [details] [review]:
-----------------------------------------------------------------

r+
Comment 15 Philip Withnall 2016-11-29 11:10:21 UTC
Comment on attachment 128248 [details] [review]
travis-ci: run in bash, with the "unofficial strict  mode"

Review of attachment 128248 [details] [review]:
-----------------------------------------------------------------

r+
Comment 16 Philip Withnall 2016-11-29 11:11:08 UTC
Comment on attachment 128249 [details] [review]
travis-ci: introduce maybe_fail_tests() to make test  failure more obvious

Review of attachment 128249 [details] [review]:
-----------------------------------------------------------------

r+
Comment 17 Philip Withnall 2016-11-29 11:13:19 UTC
Comment on attachment 128250 [details] [review]
travis-ci: add an install script instead of open-coding  it in .travis.yml

Review of attachment 128250 [details] [review]:
-----------------------------------------------------------------

r+ with or without the nitpick.

::: .travis.yml
@@ -55,5 @@
> -    xauth
> -    xmlto
> -    xsltproc
> -    xvfb
> -  # Ubuntu 14.04's autoconf-archive is too old

Nitpick: This comment didn’t get forwarded to the bash script, and it seems vaguely useful.
Comment 18 Philip Withnall 2016-11-29 11:17:34 UTC
Comment on attachment 128251 [details] [review]
travis-ci: Add and use infrastructure to build and test  in Docker

Review of attachment 128251 [details] [review]:
-----------------------------------------------------------------

r+ with or without the additional documentation (though preferably with).

::: tools/ci-build.sh
@@ +35,4 @@
>  : "${ci_test_fatal:=yes}"
>  : "${ci_variant:=production}"
>  
> +if [ -n "$ci_docker" ]; then

Ideally there would be a bit of documentation explaining about the Docker workflow you’re using, and that `ci_docker` takes `yes` or `no`.
Comment 19 Simon McVittie 2016-11-29 13:16:30 UTC
(In reply to Philip Withnall from comment #17)
> Nitpick: This comment didn’t get forwarded to the bash script, and it seems
> vaguely useful.

Added it.

(In reply to Philip Withnall from comment #18)
> Ideally there would be a bit of documentation explaining about the Docker
> workflow you’re using, and that `ci_docker` takes `yes` or `no`.

ci_docker actually takes an image name suitable for "docker pull", like fedora:25 or debian:jessie-slim. I've added some brief documentation in the version I'm currently trying out.
Comment 20 Simon McVittie 2017-02-24 19:48:36 UTC
I landed these a while ago; they were in 1.11.10.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.