From 3f51963b2d6f97aced791854be91a206d04294dd Mon Sep 17 00:00:00 2001
From: Philip Withnall <withnall@endlessm.com>
Date: Thu, 25 Apr 2019 11:23:26 +0100
Subject: [PATCH] ci: Add checks for banned keywords in merge requests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Using the same approach as we have for code style checks (the
`style-check-diff` CI job), check the diff for any banned keywords like
‘TODO’, and also check the commit messages.

The keyword ‘TODO’ is often used by developers to indicate a part of a
commit which needs further work, and hence which shouldn’t yet be merged.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Fixes: #1551
---
 .gitlab-ci.yml                |  8 ++++
 .gitlab-ci/check-todos.py     | 89 +++++++++++++++++++++++++++++++++++
 .gitlab-ci/run-check-todos.sh | 16 +++++++
 3 files changed, 113 insertions(+)
 create mode 100755 .gitlab-ci/check-todos.py
 create mode 100755 .gitlab-ci/run-check-todos.sh

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 50803ee09..685e84aec 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -37,6 +37,14 @@ style-check-diff:
   script:
     - .gitlab-ci/run-style-check-diff.sh
 
+check-todos:
+  extends: .only-default
+  image: $DEBIAN_IMAGE
+  stage: style-check
+  allow_failure: true
+  script:
+    - .gitlab-ci/run-check-todos.sh
+
 fedora-x86_64:
   extends: .build
   image: $FEDORA_IMAGE
diff --git a/.gitlab-ci/check-todos.py b/.gitlab-ci/check-todos.py
new file mode 100755
index 000000000..83b3eee7a
--- /dev/null
+++ b/.gitlab-ci/check-todos.py
@@ -0,0 +1,89 @@
+#!/usr/bin/env python3
+#
+# Copyright © 2019 Endless Mobile, Inc.
+#
+# SPDX-License-Identifier: LGPL-2.1-or-later
+#
+# Original author: Philip Withnall
+
+"""
+Checks that a merge request doesn’t add any instances of the string ‘todo’
+(in uppercase), or similar keywords. It may remove instances of that keyword,
+or move them around, according to the logic of `git log -S`.
+"""
+
+import argparse
+import re
+import subprocess
+import sys
+
+
+# We have to specify these keywords obscurely to avoid the script matching
+# itself. The keyword ‘fixme’ (in upper case) is explicitly allowed because
+# that’s conventionally used as a way of marking a workaround which needs to
+# be merged for now, but is to be grepped for and reverted or reworked later.
+BANNED_KEYWORDS = [
+    'TO' + 'DO',
+    'X' + 'XX',
+    'W' + 'IP',
+]
+
+
+def main():
+    parser = argparse.ArgumentParser(
+        description='Check a range of commits to ensure they don’t contain '
+                    'banned keywords.')
+    parser.add_argument('commits',
+                        help='SHA to diff from, or range of commits to diff')
+    args = parser.parse_args()
+
+    banned_words_seen = set()
+    seen_in_log = False
+    seen_in_diff = False
+
+    # Check the log messages for banned words.
+    log_process = subprocess.run(
+        ['git', 'log', '--no-color', args.commits + '..HEAD'],
+        stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding='utf-8',
+        check=True)
+    log_lines = log_process.stdout.strip().split('\n')
+
+    for line in log_lines:
+        for keyword in BANNED_KEYWORDS:
+            if re.search('(^|\W+){}(\W+|$)'.format(keyword), line):
+                banned_words_seen.add(keyword)
+                seen_in_log = True
+
+    # Check the diff for banned words.
+    diff_process = subprocess.run(
+        ['git', 'diff', '-U0', '--no-color', args.commits],
+        stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding='utf-8',
+        check=True)
+    diff_lines = diff_process.stdout.strip().split('\n')
+
+    for line in diff_lines:
+        if not line.startswith('+ '):
+            continue
+
+        for keyword in BANNED_KEYWORDS:
+            if re.search('(^|\W+){}(\W+|$)'.format(keyword), line):
+                banned_words_seen.add(keyword)
+                seen_in_diff = True
+
+    if banned_words_seen:
+        if seen_in_log and seen_in_diff:
+            where = 'commit message and diff'
+        elif seen_in_log:
+            where = 'commit message'
+        elif seen_in_diff:
+            where = 'commit diff'
+
+        print('Saw banned keywords in a {}: {}. '
+              'This indicates the branch is a work in progress and should not '
+              'be merged in its current '
+              'form.'.format(where, ', '.join(banned_words_seen)))
+        sys.exit(1)
+
+
+if __name__ == '__main__':
+    main()
diff --git a/.gitlab-ci/run-check-todos.sh b/.gitlab-ci/run-check-todos.sh
new file mode 100755
index 000000000..786fd71d6
--- /dev/null
+++ b/.gitlab-ci/run-check-todos.sh
@@ -0,0 +1,16 @@
+#!/bin/bash
+
+set +e
+
+# We need to add a new remote for the upstream master, since this script could
+# be running in a personal fork of the repository which has out of date branches.
+git remote add upstream https://gitlab.gnome.org/GNOME/glib.git
+git fetch upstream
+
+# 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/master` or `upstream/glib-2-62`).
+# `${CI_MERGE_REQUEST_TARGET_BRANCH_NAME}` is only defined if we’re running in
+# a merge request pipeline; fall back to `${CI_DEFAULT_BRANCH}` otherwise.
+newest_common_ancestor_sha=$(diff --old-line-format='' --new-line-format='' <(git rev-list --first-parent upstream/${CI_MERGE_REQUEST_TARGET_BRANCH_NAME:-${CI_DEFAULT_BRANCH}}) <(git rev-list --first-parent HEAD) | head -1)
+./.gitlab-ci/check-todos.py "${newest_common_ancestor_sha}"