Commit 639808ac authored by Peter Hutterer's avatar Peter Hutterer
Browse files

tools: add commit message checks to ci-fairy



Useful for checking things like signed-off-by and some more cosmetic things
that we expect in general from the commit message.

Current checks:
- s-o-b present or not (depending on commandline toggle)
- textwidth < 80 chars
- second line of commit message is an empty line
- no 'fixup!' or 'squash!' messages in the history
- git author information is set
Signed-off-by: Peter Hutterer's avatarPeter Hutterer <peter.hutterer@who-t.net>
parent 91efd551
......@@ -116,13 +116,8 @@ sanity check:
check commits:
extends: .pip_install
script:
- pip3 install GitPython
- pip3 install pytest
- |
pytest --junitxml=results.xml \
--tb=line \
--assert=plain \
./.gitlab-ci/check-commit.py
- pip3 install .
- ci-fairy check-commits --signed-off-by --junit-xml=results.xml
except:
- master@freedesktop/ci-templates
variables:
......
......@@ -193,3 +193,40 @@ allows for the selection of the root node. For example: ::
qemu needed? true
$ ci-fairy generate-template --root=/packages/ubuntu --config distributions.yml template.tpl
qemu needed? false
Checking commits
----------------
Git commit messages have a few standard requirements that apply across
projects. ``ci-fairy`` can verify the commits to be merged for those
standards. ::
$ ci-fairy check-commits master..HEAD
By default, this checks for any leftover ``fixup!``/``squash!`` commits and
some cosmetic requirements. Other options to check for is whether a
``Signed-off-by:`` line is present (``--signed-off-by``) or ensure that such
a line is **not** included (``--no-signed-off-by``). See the ``--help``
output for a list fo checks available.
``ci-fairy`` uses the CI environment where available and will do the right
thing without requiring options. It does need the ``$FDO_UPSTREAM_REPO``
variable to be set to the upstream project's full name. An example
``.gitlab-ci.yml`` section:
.. code-block:: yaml
variables:
FDO_UPSTREAM_REPO: libinput/libinput
commit message check:
image: something_that_has_pip
before_script:
- pip3 install git+http://gitlab.freedesktop.org/freedesktop/ci-templates
script:
- ci-fairy check-commits --signed-off-by
``ci-fairy`` uses ``$CI_MERGE_REQUEST_TARGET_BRANCH_NAME`` if available,
otherwise it defaults to the ``master`` branch of the ``$FDO_UPSTREAM_REPO``.
For the above snippet, the commit range checked will thus be
``upstream/master..HEAD``.
......@@ -60,13 +60,8 @@ sanity check:
check commits:
extends: .pip_install
script:
- pip3 install GitPython
- pip3 install pytest
- |
pytest --junitxml=results.xml \
--tb=line \
--assert=plain \
./.gitlab-ci/check-commit.py
- pip3 install .
- ci-fairy check-commits --signed-off-by --junit-xml=results.xml
except:
- master@freedesktop/ci-templates
variables:
......
......@@ -37,6 +37,30 @@ logger.addHandler(logger_handler)
logger.setLevel(logging.ERROR)
def make_junit_xml(filename, test_suite_id, test_suite_name, failures):
'''
Write a JUNIT XML compatible file to filename based on the failures
dictionary where each failure is a tuple of (id, short-message,
full-message).
'''
import xml.etree.cElementTree as ET
root = ET.Element('testsuites')
suite = ET.SubElement(root, 'testsuite',
id=test_suite_id,
name=test_suite_name,
tests=str(len(failures)),
errors=str(len(failures)))
for f in failures:
tcaseid, name, message = f
tcaseid = f'{test_suite_id}.{tcaseid}'
tcase = ET.SubElement(suite, 'testcase', id=tcaseid, name=name)
failure = ET.SubElement(tcase, 'failure', message=name, type='ERROR')
failure.text = message
ET.ElementTree(root).write(filename, encoding='UTF-8', xml_declaration=True)
class LazyGitlab(object):
def __init__(self):
self.initialized = False
......@@ -68,6 +92,97 @@ class LazyGitlab(object):
return object.__getattribute__(self, name)
class GitCommitValidator(object):
def __init__(self, commit):
self.commit = commit
self.errors = []
def _check_author_email(self):
# The full address is @users.noreply.gitlab.freedesktop.org but
# let's make this more generic
if '@users.noreply' in self.commit.author.email:
self.__error('git author email invalid\n'
'Please set your name and email with the commands\n'
' git config --global user.name Your Name\n'
' git config --global user.email your.email@provider.com\n')
def _check_fixup_squash(self):
if (self.commit.message.startswith('fixup!') or
self.commit.message.startswith('squash!')):
self.__error('Leftover "fixup!" or "squash!" commit found, please squash')
def _check_second_line(self):
try:
second_line = self.commit.message.split('\n')[1]
if second_line != '':
self.__error('Second line in commit message must be empty')
except IndexError:
pass
def check(self):
self._check_author_email()
self._check_fixup_squash()
self._check_second_line()
def check_sob(self, must_have_sob):
lines = self.commit.message.split('\n')
sob = [l for l in lines if l.startswith('Signed-off-by:')]
if not must_have_sob:
if sob:
self.__error('Do not use Signed-off-by in commits')
else:
if not sob:
self.__error('Missing "Signed-off-by: author information"')
def check_text_width(self, tw):
lines = self.commit.message.split('\n')
for l in lines:
if len(l) >= tw:
self.__error(f'Commit messages must not exceed {tw} characters')
break
@property
def failed(self):
return bool(self.errors)
def __error(self, errormsg):
self.errors.append(errormsg)
@property
def error(self):
if not self.failed:
return None
msg = (f'Commit message check failed\n\n'
f' commit: {str(self.commit)}\n'
f' author: {self.commit.author.name} <{self.commit.author.email}>\n'
f'\n'
f' {self.commit.summary}\n'
f'\n'
f'\n'
f'After correcting the issues below, force-push to the same branch.\n'
f'This will re-trigger the CI.\n\n'
f'---\n')
for idx, err in enumerate(self.errors):
msg += f'{idx+1}. {err}\n---\n'
return msg
@property
def junit_tuple(self):
'''
A tuple of (id, short message, full description)
'''
if not self.failed:
return None
msg = '\n\n'.join([f'{idx + 1}. {err}' for idx, err in enumerate(self.errors)])
return (str(self.commit),
f'Failed {len(self.errors)} commit message checks',
msg)
@click.group()
@click.option('-v', '--verbose', count=True, help='increase verbosity')
@click.option('--gitlab-url', help='GitLab URL with transport protocol, e.g. http://gitlab.freedesktop.org')
......@@ -309,6 +424,86 @@ def generate_template(ctx, config, root, template, output_file):
template.stream(data).dump(output_file)
@ci_fairy.command()
@click.argument('commit-range', type=str, required=False, default='HEAD')
@click.option('--branch', type=str,
help='Check commits compared to this branch '
'(default: $CI_MERGE_REQUEST_TARGET_BRANCH_NAME if present or master')
@click.option('--signed-off-by/--no-signed-off-by', default=None,
help='Require a Signed-off-by tag to be (or not be) present')
@click.option('--textwidth', type=int, default=80,
help='Require the commit message to be less than N characters wide.')
@click.option('--junit-xml', help='junit output file to write to')
@click.pass_context
def check_commits(ctx, commit_range, branch, signed_off_by, textwidth, junit_xml):
'''
Check a commit range for some properties. Some checks are always
performed, others can be changed with commandline arguments.
Where a commit range is given, that range is validated. Otherwise, the
commit range is "branch..HEAD", where branch is either the given one, or
$CI_MERGE_REQUEST_TARGET_BRANCH_NAME, or master. When run in the GitLab
CI environment, the FDO_UPSTREAM_REPO is added and the commit range
defaults to "upstream-repo/branchname..HEAD".
'''
try:
repo = git.Repo('.', search_parent_directories=True)
except git.exc.InvalidGitRepositoryError:
logger.error('This must be run from within the git repository')
sys.exit(1)
# if we're running in the CI we add the upstream project
# as remote 'cifairy' so we can build the commit range appropriately.
# We don't do this when running locally because we don't want to mess
# with a user's git repo.
if os.getenv('CI'):
upstream = os.getenv('FDO_UPSTREAM_REPO')
if not upstream:
logger.warning('$FDO_UPSTREAM_REPO not set, using local branches to compare')
upstream = os.getenv('CI_PROJECT_PATH')
host = os.getenv('CI_SERVER_HOST')
url = f'https://{host}/{upstream}'
remote = 'cifairy'
if remote not in repo.remotes:
upstream = repo.create_remote(remote, url)
else:
upstream = repo.remotes[remote]
upstream.fetch()
remote_prefix = f'{remote}/'
else:
remote_prefix = ''
# a user-specified range takes precedence. if it's just a single commit
# sha fall back to the target_branch..$sha range
if '..' not in commit_range:
sha = commit_range
target_branch = branch or os.getenv('CI_MERGE_REQUEST_TARGET_BRANCH_NAME') or 'master'
commit_range = f'{remote_prefix}{target_branch}..{sha}'
logger.debug(f'Using commit range {commit_range}')
failures = []
for commit in repo.iter_commits(commit_range):
validator = GitCommitValidator(commit)
validator.check()
if signed_off_by is not None:
validator.check_sob(signed_off_by)
if textwidth > 0:
validator.check_text_width(textwidth)
if validator.failed:
failures.append(validator)
logger.error(validator.error)
if junit_xml and failures:
sha = str(next(repo.iter_commits('HEAD')))[:8]
make_junit_xml(junit_xml, f'commitmsg.{sha}', f'Commit message check for {sha}',
[f.junit_tuple for f in failures])
sys.exit(1 if failures else 0)
def main():
ci_fairy()
......
......@@ -30,6 +30,7 @@ def gitlab_project_path():
@pytest.fixture
def gitlab_default_env():
return {
'CI': '1',
'CI_SERVER_URL': GITLAB_TEST_URL,
'CI_PROJECT_PATH': GITLAB_TEST_PROJECT_PATH,
'CI_PROJECT_ID': GITLAB_TEST_PROJECT_ID,
......@@ -392,3 +393,230 @@ def test_template_root_path():
result = runner.invoke(ci_fairy.ci_fairy, args)
assert result.exit_code == 0
assert result.stdout == 'health: 65'
def test_commits_needs_git_repo(caplog):
runner = CliRunner()
with runner.isolated_filesystem():
args = ['check-commits']
result = runner.invoke(ci_fairy.ci_fairy, args)
assert result.exit_code == 1
assert 'This must be run from within the git repository' in ''.join(caplog.messages)
def test_commits_noop(gitlab_default_env):
env = gitlab_default_env
env['CI'] = ''
runner = CliRunner(env=env)
with runner.isolated_filesystem():
args = ['check-commits']
repo = git.Repo.init()
config = repo.config_writer()
config.set_value('user', 'email', 'test@example.com')
config.set_value('user', 'name', 'test user')
config.release()
with open('test', 'w') as fd:
fd.write('foo')
repo.index.add(['test'])
repo.index.commit('initial commit')
# nothing to compare here, we're on master
result = runner.invoke(ci_fairy.ci_fairy, args)
assert result.exit_code == 0
def test_commits_fixup_squash(caplog, gitlab_default_env):
env = gitlab_default_env
env['CI'] = ''
runner = CliRunner(env=env)
with runner.isolated_filesystem():
args = ['check-commits']
repo = git.Repo.init()
config = repo.config_writer()
config.set_value('user', 'email', 'test@example.com')
config.set_value('user', 'name', 'test user')
config.release()
with open('test', 'w') as fd:
fd.write('foo')
repo.index.add(['test'])
repo.index.commit('initial commit')
b = repo.create_head('mybranch')
repo.head.reference = b
with open('test', 'w') as fd:
fd.write('bar')
repo.index.add(['test'])
repo.index.commit('fixup! blah')
# whack on another commit to verify we're testing the range, not
# just HEAD
fd.write('baz')
repo.index.add(['test'])
repo.index.commit('normal commit')
result = runner.invoke(ci_fairy.ci_fairy, args)
assert result.exit_code == 1
assert 'Leftover "fixup!"' in ''.join(caplog.messages)
b = repo.create_head('mybranch2', 'master')
repo.head.reference = b
repo.head.reset(index=True, working_tree=True)
result = runner.invoke(ci_fairy.ci_fairy, args)
assert result.exit_code == 0
with open('test', 'w') as fd:
fd.write('bar')
repo.index.add(['test'])
repo.index.commit('squash! blah')
result = runner.invoke(ci_fairy.ci_fairy, args)
assert result.exit_code == 1
assert 'Leftover "fixup!"' in ''.join(caplog.messages)
def test_commits_sob(caplog, gitlab_default_env):
env = gitlab_default_env
env['CI'] = ''
runner = CliRunner(env=env)
with runner.isolated_filesystem():
repo = git.Repo.init()
config = repo.config_writer()
config.set_value('user', 'email', 'test@example.com')
config.set_value('user', 'name', 'test user')
config.release()
with open('test', 'w') as fd:
fd.write('foo')
repo.index.add(['test'])
repo.index.commit('initial commit')
b = repo.create_head('mybranch')
repo.head.reference = b
with open('test', 'w') as fd:
fd.write('bar')
repo.index.add(['test'])
repo.index.commit('Lacking a s-o-b')
args = ['check-commits']
result = runner.invoke(ci_fairy.ci_fairy, args + ['--no-signed-off-by'])
assert result.exit_code == 0
result = runner.invoke(ci_fairy.ci_fairy, args + ['--signed-off-by'])
assert result.exit_code == 1
assert 'Missing "Signed-off-by: author information"' in ''.join(caplog.messages)
b = repo.create_head('mybranch2', 'master')
repo.head.reference = b
repo.head.reset(index=True, working_tree=True)
with open('test', 'w') as fd:
fd.write('bar')
repo.index.add(['test'])
repo.index.commit('Commit\n\nSigned-off-by: first last <email>')
result = runner.invoke(ci_fairy.ci_fairy, args + ['--no-signed-off-by'])
assert result.exit_code == 1
assert 'Do not use Signed-off-by in commits' in ''.join(caplog.messages)
result = runner.invoke(ci_fairy.ci_fairy, args + ['--signed-off-by'])
assert result.exit_code == 0
# Signed-off-by: must be at the beginning of the line
b = repo.create_head('mybranch3', 'master')
repo.head.reference = b
repo.head.reset(index=True, working_tree=True)
with open('test', 'w') as fd:
fd.write('bar')
repo.index.add(['test'])
repo.index.commit('Commit\n\n___Signed-off-by: first last <email>')
result = runner.invoke(ci_fairy.ci_fairy, args + ['--no-signed-off-by'])
assert result.exit_code == 0
result = runner.invoke(ci_fairy.ci_fairy, args + ['--signed-off-by'])
assert result.exit_code == 1
assert 'Missing "Signed-off-by: author information"' in ''.join(caplog.messages)
def test_commits_msgformat(caplog, gitlab_default_env):
env = gitlab_default_env
env['CI'] = ''
runner = CliRunner(env=env)
with runner.isolated_filesystem():
args = ['check-commits']
repo = git.Repo.init()
config = repo.config_writer()
config.set_value('user', 'email', 'test@example.com')
config.set_value('user', 'name', 'test user')
config.release()
with open('test', 'w') as fd:
fd.write('foo')
repo.index.add(['test'])
repo.index.commit('initial commit')
b = repo.create_head('mybranch')
repo.head.reference = b
with open('test', 'w') as fd:
fd.write('bar')
repo.index.add(['test'])
# second line must be empty
repo.index.commit('three\nline\ncommit')
result = runner.invoke(ci_fairy.ci_fairy, args)
assert result.exit_code == 1
assert 'Second line in commit message must be empty' in ''.join(caplog.messages)
b = repo.create_head('mybranch2', 'master')
repo.head.reference = b
repo.head.reset(index=True, working_tree=True)
result = runner.invoke(ci_fairy.ci_fairy, args)
assert result.exit_code == 0
with open('test', 'w') as fd:
fd.write('bar')
repo.index.add(['test'])
repo.index.commit('too long a line' * 10)
result = runner.invoke(ci_fairy.ci_fairy, args)
assert result.exit_code == 1
assert 'Commit messages must not exceed' in ''.join(caplog.messages)
result = runner.invoke(ci_fairy.ci_fairy, args + ['--textwidth=1000'])
assert result.exit_code == 0
def test_commits_gitlabemail(caplog, gitlab_default_env):
env = gitlab_default_env
env['CI'] = ''
runner = CliRunner(env=env)
with runner.isolated_filesystem():
args = ['check-commits']
repo = git.Repo.init()
config = repo.config_writer()
config.set_value('user', 'email', 'testuser@users.noreply.gitlab.freedesktop.org')
config.set_value('user', 'name', 'test user')
config.release()
with open('test', 'w') as fd:
fd.write('foo')
repo.index.add(['test'])
repo.index.commit('initial commit')
b = repo.create_head('mybranch')
repo.head.reference = b
with open('test', 'w') as fd:
fd.write('bar')
repo.index.add(['test'])
repo.index.commit('second commit')
result = runner.invoke(ci_fairy.ci_fairy, args)
assert result.exit_code == 1
assert 'git author email invalid' in ''.join(caplog.messages)
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment