Tag Archives: CI

Coding style checks in CI

For a few weeks now, we’ve had coding style and thinko checks running in CI for GLib, long enough to see that it’s working OK (check out this pipeline!) and is perhaps time to share with others.

There are two jobs in the checks, both of which run in a new style-check stage of our CI pipeline, which runs before anything else. One job checks the coding style of a merge request, using clang-format. The other job checks for any lines which introduce TODO comments (or similar). These jobs are intended to be fast, but also to not fail the pipeline if they produce warnings. Coding style is subjective, and nobody has yet come up with a mechanical style description which doesn’t have ugly corner cases. So the intention of these jobs is to help remind people of the coding style, to avoid reviewers having to check coding style manually, and hence to only end up thinking about it or discussing it when the CI says the style is wrong.

The first job, style-check-diff, uses a script to work out the diff from the merge request’s branch point, and then passes that to clang-format-diff.py, a script from LLVM which uses clang-format to reformat only the changed lines in a diff. If any reformatting occurs, that means the merge request differs from the GLib coding style (as described by .clang-format) and should be warned about.

There are some limitations to clang-format: it can’t completely describe how the GLib coding style aligns function arguments. That’s unfortunate, because GNOME Builder aligns function arguments by default (and it’s nice behaviour). Hopefully somebody will implement support in clang-format for this sometime, so that it can accurately describe the coding style which is used by a large number of GNOME projects.

These coding style checks are work by Pavlo Solntsev and Emmanuel Fleury. Thanks!

The second job, check-todos, also uses a script to work out the diff from the merge request’s branch point. It passes that to a little Python program which checks for banned words in the commit message and added lines in the diff. The aim is to prevent any comments containing TODO or WIP from accidentally getting merged, as these strings are often used by developers to indicate something they need to come back to before they’re finished — and it’s easy to miss them!

I’ve been using the convention that comments containing FIXME are OK to be merged, as they indicate something that will need updating in future, but can be merged as-is for now (such as a workaround).

Feedback, improvements, and copying welcome. These scripts should run on any CI image which has git, clang and Python.

Running GitLab CI on autotools projects

Inspired by the talk at FOSDEM, I’ve just enabled GitLab’s continuous integration (CI) for building make distcheck for Walbottle, and it was delightfully easy. The results are on Walbottle’s GitLab page.

Steps

  1. Create a ci branch to contain the mess you’ll make while iterating over the correct compile steps.
  2. Create and push a .gitlab-ci.yml file containing build rules similar to the following: 
    image: debian:unstable

before_script:
  - apt update -qq
  - apt install -y -qq build-essential autoconf automake pkg-config libtool m4 autoconf-archive gtk-doc-tools libxml2-utils gobject-introspection libgirepository1.0-dev libglib2.0-dev libjson-glib-dev

stages:
  - build

# FIXME: Re-enable valgrind once running the tests under it doesn’t take forever (it causes timeouts).
# Re-add valgrind to apt-install line above
build-distcheck:
  stage: build
  script:
    - mkdir build
    - cd build
    - ../autogen.sh --disable-valgrind
    - make V=1 VERBOSE=1
    - DISTCHECK_CONFIGURE_FLAGS=--disable-valgrind make distcheck V=1 VERBOSE=1

  # The files which are to be made available in GitLab
  artifacts:
    paths:
      - build/*
  3. Iterate a few times until you get all the dependencies right.
  4. Fix any problems you find (because this might well find problems with your dependency declaration in configure.ac, or other distcheck problems in your project).
  5. Merge ci to master and profit from CI results on every branch and master commit.

Looking at the .gitlab-ci.yml file

For information on the overall layout of the YAML file, and the phases available, you’re best off looking at the comprehensive GitLab documentation. Here are some notes about the autotools-and-C–specific bits of it:

  • The image is a Docker image; I picked a Debian one from the Docker hub.
  • Package installation seems to need to be done in the before_script phase, or the packages can’t be found (which is presumably a protection against rogue build systems).
  • I chose to build distcheck in my build rule because that runs the build, runs the tests, and tries various srcdir ? builddir configurations. You can add other build targets (like build-distcheck to try other build setups).
  • Pass V=1 VERBOSE=1 to get verbose build and test log output in your CI build logs, otherwise you will struggle to work out what is causing any failures.
  • Note that configure flags passed to ./configure are not automatically passed in again when ./configure is run as part of distcheck — so use DISTCHECK_CONFIGURE_FLAGS for that. Ideally, your project will be less fragile than mine, and hence not need any of this.
  • Export the whole build directory as an artifact on success, so you can look at any of the build objects, or the generated tarball, or documentation. You could limit this (for example, to just the tarball) if you’re sure you’ll never need the rest of it.