| Summary: | Run continuous integration on OSs newer than Ubuntu 14.04 | ||
|---|---|---|---|
| Product: | dbus | Reporter: | Simon McVittie <smcv> |
| Component: | core | Assignee: | Simon McVittie <smcv> |
| Status: | RESOLVED FIXED | QA Contact: | D-Bus Maintainers <dbus> |
| Severity: | enhancement | ||
| Priority: | low | Keywords: | patch |
| Version: | 1.10 | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | review? | ||
| i915 platform: | i915 features: | ||
| Attachments: |
travis-ci: add an explicit copyright/license statement
ci-build: retab with 4-space indentation travis-ci: consistently use ci_* for parameter variables travis-ci: consistently use yes/no instead of yes/empty travis-ci: run in bash, with the "unofficial strict mode" travis-ci: introduce maybe_fail_tests() to make test failure more obvious travis-ci: add an install script instead of open-coding it in .travis.yml travis-ci: Add and use infrastructure to build and test in Docker |
||
|
Description
Simon McVittie
2016-11-28 15:59:08 UTC
Patches are not ready yet, removing patch tag. 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). 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. 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. Created attachment 128247 [details] [review] travis-ci: consistently use yes/no instead of yes/empty 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. Created attachment 128249 [details] [review] travis-ci: introduce maybe_fail_tests() to make test failure more obvious 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. 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. 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 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 on attachment 128245 [details] [review] ci-build: retab with 4-space indentation Review of attachment 128245 [details] [review]: ----------------------------------------------------------------- Looks legit. r+ Comment on attachment 128246 [details] [review] travis-ci: consistently use ci_* for parameter variables Review of attachment 128246 [details] [review]: ----------------------------------------------------------------- r+ Comment on attachment 128247 [details] [review] travis-ci: consistently use yes/no instead of yes/empty Review of attachment 128247 [details] [review]: ----------------------------------------------------------------- r+ Comment on attachment 128248 [details] [review] travis-ci: run in bash, with the "unofficial strict mode" Review of attachment 128248 [details] [review]: ----------------------------------------------------------------- r+ 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 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 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`. (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. 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.