Add style check to CI

.clang-format

@@ -0,0 +1,12 @@
+# See https://wiki.apertis.org/Guidelines/Coding_conventions#Code_formatting
+BasedOnStyle: GNU
+AlwaysBreakAfterDefinitionReturnType: All
+BreakBeforeBinaryOperators: None
+BinPackParameters: false
+SpaceAfterCStyleCast: true
+PointerAlignment: Right
+# Our column limit is actually 80, but setting that results in clang-format
+# making a lot of dubious hanging-indent choices; disable it and assume the
+# developer will line wrap appropriately. clang-format will still check
+# existing hanging indents.
+ColumnLimit: 0

.gitlab-ci.yml

@@ -227,8 +227,20 @@ build:
 ##
 # Stage: Test
 #
+# Runs static checks.
 # Runs the unit tests.
 ##
+style-check-diff:
+  extends:
+  - '.fdo.distribution-image@fedora'
+  - '.fedora.container.common'
+  needs:
+  - build.container.fedora@x86_64
+  stage: test
+  when: always
+  script:
+    - .gitlab-ci/run-style-check-diff.sh
+
 test:
   extends:
     - '.fdo.distribution-image@fedora'

.gitlab-ci/clang-format-diff.py

@@ -0,0 +1,133 @@
+#!/usr/bin/env python3
+#
+# === clang-format-diff.py - ClangFormat Diff Reformatter ---*- python -*-=== #
+#
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+# ===---------------------------------------------------------------------=== #
+
+"""
+This script reads input from a unified diff and reformats all the changed
+lines. This is useful to reformat all the lines touched by a specific patch.
+Example usage for git/svn users:
+
+  git diff -U0 --no-color HEAD^ | clang-format-diff.py -p1 -i
+  svn diff --diff-cmd=diff -x-U0 | clang-format-diff.py -i
+
+"""
+from __future__ import absolute_import, division, print_function
+
+import argparse
+import difflib
+import re
+import subprocess
+import sys
+
+if sys.version_info.major >= 3:
+    from io import StringIO
+else:
+    from io import BytesIO as StringIO
+
+
+def main():
+    parser = argparse.ArgumentParser(
+        description=__doc__,
+        formatter_class=argparse.RawDescriptionHelpFormatter)
+    parser.add_argument('-i', action='store_true', default=False,
+                        help='apply edits to files instead of displaying a '
+                             'diff')
+    parser.add_argument('-p', metavar='NUM', default=0,
+                        help='strip the smallest prefix containing P slashes')
+    parser.add_argument('-regex', metavar='PATTERN', default=None,
+                        help='custom pattern selecting file paths to reformat '
+                        '(case sensitive, overrides -iregex)')
+    parser.add_argument('-iregex', metavar='PATTERN',
+                        default=r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hh|hpp|m|mm|inc'
+                                r'|js|ts|proto|protodevel|java|cs)',
+                        help='custom pattern selecting file paths to reformat '
+                             '(case insensitive, overridden by -regex)')
+    parser.add_argument('-sort-includes', action='store_true', default=False,
+                        help='let clang-format sort include blocks')
+    parser.add_argument('-v', '--verbose', action='store_true',
+                        help='be more verbose, ineffective without -i')
+    parser.add_argument('-style',
+                        help='formatting style to apply (LLVM, Google, '
+                        'Chromium, Mozilla, WebKit)')
+    parser.add_argument('-binary', default='clang-format',
+                        help='location of binary to use for clang-format')
+    args = parser.parse_args()
+
+    # Extract changed lines for each file.
+    filename = None
+    lines_by_file = {}
+    for line in sys.stdin:
+        match = re.search(r'^\+\+\+\ (.*?/){%s}(\S*)' % args.p, line)
+        if match:
+            filename = match.group(2)
+        if filename is None:
+            continue
+
+        if args.regex is not None:
+            if not re.match('^%s$' % args.regex, filename):
+                continue
+        else:
+            if not re.match('^%s$' % args.iregex, filename, re.IGNORECASE):
+                continue
+
+        match = re.search(r'^@@.*\+(\d+)(,(\d+))?', line)
+        if match:
+            start_line = int(match.group(1))
+            line_count = 1
+            if match.group(3):
+                line_count = int(match.group(3))
+            if line_count == 0:
+                continue
+            end_line = start_line + line_count - 1
+            lines_by_file.setdefault(filename, []).extend(
+                ['-lines', str(start_line) + ':' + str(end_line)])
+
+    # Reformat files containing changes in place.
+    # We need to count amount of bytes generated in the output of
+    # clang-format-diff. If clang-format-diff doesn't generate any bytes it
+    # means there is nothing to format.
+    format_line_counter = 0
+    for filename, lines in lines_by_file.items():
+        if args.i and args.verbose:
+            print('Formatting {}'.format(filename))
+        command = [args.binary, filename]
+        if args.i:
+            command.append('-i')
+        if args.sort_includes:
+            command.append('-sort-includes')
+        command.extend(lines)
+        if args.style:
+            command.extend(['-style', args.style])
+        p = subprocess.Popen(command,
+                             stdout=subprocess.PIPE,
+                             stderr=None,
+                             stdin=subprocess.PIPE,
+                             universal_newlines=True)
+        stdout, _ = p.communicate()
+        if p.returncode != 0:
+            sys.exit(p.returncode)
+
+        if not args.i:
+            with open(filename) as f:
+                code = f.readlines()
+            formatted_code = StringIO(stdout).readlines()
+            diff = difflib.unified_diff(code, formatted_code,
+                                        filename, filename,
+                                        '(before formatting)',
+                                        '(after formatting)')
+            diff_string = ''.join(diff)
+            if diff_string:
+                format_line_counter += sys.stdout.write(diff_string)
+
+    if format_line_counter > 0:
+        sys.exit(1)
+
+
+if __name__ == '__main__':
+    main()

.gitlab-ci/run-style-check-diff.sh

@@ -0,0 +1,62 @@
+#!/bin/bash
+
+set -e
+
+ancestor_horizon=31  # days (one month)
+
+# Recently, git is picky about directory ownership. Tell it not to worry.
+git config --global --add safe.directory "$PWD"
+
+# We need to add a new remote for the upstream target branch, since this script
+# could be running in a personal fork of the repository which has out of date
+# branches.
+#
+# Limit the fetch to a certain date horizon to limit the amount of data we get.
+# If the branch was forked from origin/main before this horizon, it should
+# probably be rebased.
+if ! git ls-remote --exit-code upstream >/dev/null 2>&1 ; then
+    git remote add upstream https://gitlab.gnome.org/GNOME/gnome-control-center.git
+fi
+
+# Work out the newest common ancestor between the detached HEAD that this CI job
+# has checked out, and the upstream target branch (which will typically be
+# `upstream/main` or `upstream/glib-2-62`).
+# `${CI_MERGE_REQUEST_TARGET_BRANCH_NAME}` or `${CI_MERGE_REQUEST_SOURCE_BRANCH_NAME}`
+# are only defined if we’re running in a merge request pipeline,
+# fall back to `${CI_DEFAULT_BRANCH}` or `${CI_COMMIT_BRANCH}` respectively
+# otherwise.
+
+source_branch="${CI_MERGE_REQUEST_SOURCE_BRANCH_NAME:-${CI_COMMIT_BRANCH}}"
+target_branch="${CI_MERGE_REQUEST_TARGET_BRANCH_NAME:-${CI_DEFAULT_BRANCH}}"
+git fetch --shallow-since="$(date --date="${ancestor_horizon} days ago" +%Y-%m-%d)" origin "${source_branch}"
+git fetch --shallow-since="$(date --date="${ancestor_horizon} days ago" +%Y-%m-%d)" upstream "${target_branch}"
+
+newest_common_ancestor_sha=$(git merge-base upstream/${target_branch} origin/${source_branch})
+if [ -z "${newest_common_ancestor_sha}" ]; then
+    echo "Couldn’t find common ancestor with upstream main branch. This typically"
+    echo "happens if you branched from main a long time ago. Please update"
+    echo "your clone, rebase, and re-push your branch."
+    exit 1
+fi
+
+git diff -U0 --no-color "${newest_common_ancestor_sha}" | .gitlab-ci/clang-format-diff.py -binary "clang-format" -p1
+exit_status=$?
+
+# The style check is not infallible. The clang-format configuration cannot
+# perfectly describe GGNOME Settings’ coding style: in particular, it cannot align
+# function arguments. The documented coding style for GNOME Settings takes priority over
+# clang-format suggestions. Hopefully we can eventually improve clang-format to
+# be configurable enough for our coding style. That’s why this CI check is OK
+# to fail: the idea is that people can look through the output and ignore it if
+# it’s wrong. (That situation can also happen if someone touches pre-existing
+# badly formatted code and it doesn’t make sense to tidy up the wider coding
+# style with the changes they’re making.)
+echo ""
+echo "Note that clang-format output is advisory and cannot always match the"
+echo "GNOME Settings coding style, documented at:"
+echo "   https://gitlab.gnome.org/GNOME/gnome-control-center/blob/main/docs/CODING_STYLE.md"
+echo "Warnings from this tool can be ignored in favour of the documented "
+echo "coding style, or in favour of matching the style of existing"
+echo "surrounding code."
+
+exit ${exit_status}