From 20d86e5460cd91ab806b1249e42c65a98c678f93 Mon Sep 17 00:00:00 2001 From: rohilla21 Date: Thu, 3 May 2018 17:23:02 +0530 Subject: [PATCH] cEP-0016.md: Git Commit Content Inspection This cEP the coala process rules which will be enforced by GitCommitBear to handle special types of commit messages like git revert and git merge. Closes https://github.com/coala/cEPs/issues/112 --- cEP-0016.md | 178 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 cEP-0016.md diff --git a/cEP-0016.md b/cEP-0016.md new file mode 100644 index 000000000..9794ebcae --- /dev/null +++ b/cEP-0016.md @@ -0,0 +1,178 @@ +Git Commit Content Inspection +============================= + +|Metadata | | +|--------------|--------------------------------------------| +| cEP | 16 | +| Version | 1.0 | +| Title | Git commit inspection | +| Authors | Kriti Rohillla | +| Status | Implementation due | +| Type | Process | + +## Abstract + +This document describes coala process rules which will be enforced by the +GitCommitBear. + +This cEP describes some new changes in the implementation of GitCommitBear +based on which the commits made to coala will be detected and verified. + + +## Introduction + +The GitCommitBear at present performs a check on the content of regular commits +made to coala. However, there are many special types of commit messages that +should be only used in conjunction with patches containing a special type of +content. + +For example, git revert commit which is used to revert any previous commit and +git merge commit which is generated whenever someone performs a git rebase and +tries to merge master onto the pull request. Such commits generate a default +commit message that is very concise and not according to coala’s standards. +The coala project wants to allow `git revert` commits, but does not want to +accept `git merge` commits as the repositories are strictly fast-forward merges +that do not have an extra commit. + +Also in cases where some tests are not required by any pull request made to +coala, some special sequences can be added to git commit messages to instruct +tools to operate in a certain way. + +This project is about detecting such commits and verifying that they are correct +according to the configuration settings in .coafile + + +## Proposed Change + +Here is the detailed implementation stepwise: + +1. We start by identifying different types of special commits namely revert, +merge and travis commits. +2. Each type of commit is then handled as a separate case. +3. For travis commits: + * Merged commits are identified first. + * It's checked whether the merge commit contains `[ci skip]`. + * It is checked that the commit body explains a valid reason why this change + should not include travis builds. +4. For merge commits: + * Merge commits are identified by checking the parents of the commit. + * In case the bear finds that the commit tries to merge master onto the pull + request, message is displayed to perform a rebase instead. +5. For revert commits: + * Revert commits are identified based on the format of the commit. + * For revert commits, the patches of both commits are compared to ensure that + they are in fact reverse of each other. Minor differences are allowed. + * In case of total mismatch, a note in the commit message highlighting that + its not a clean revert is expected. + * A message requesting improvement in the commit message is displayed + otherwise. + +## Code Prototypes + +A prototype for identifying merge commits: + +```python + def is_merge_commit(self, shortlog, body): + commit_hash = run_shell_command('git rev-parse HEAD') + + command = 'git show --no-patch --format="%P" ' + command += str(commit_hash) + + commit_parents = run_shell_command(command) + + if commit_parents > 1: + yield Result(self, 'This is a merge commit.') +``` + +A prototype for identifying travis commits: + +```python + def is_travis_commit(self, shortlog, body): + ci_regex = re.compile(r'ci skip') + + if is_merge_commit and ci_regex.match(shortlog): + yield Result(self, 'This is travis commit.') +``` + +A prototype for identifying revert commits: + +```python + def is_revert_commit(self, shortlog, body): + revert_shortlog_regex = re.compile(r'^Revert\s"[\S+\s*]*"') + headline_regex = re.compile( + r'^This reverts commit [0-9a-f]{40}') + sha_regex = re.compile(r'\b[0-9a-f]{40}\b') + + if not allow_revert_commits and revert_shortlog_regex.match(shortlog): + reverted_shortlog = shortlog.lstrip('Revert ') + + if len(body) != 0: + body = body.strip('\n') + blankline = body.find('\n\n') + headline = body[:blankline] if blankline != -1 else body + + if headline_regex.match(headline): + commit_sha = re.search(sha_regex, headline) + commit_sha = commit_sha.group(0) + command = 'git log -n 1 ' + str(commit_sha) + rstdout, rstderr = run_shell_command(command) + + if rstderr: + self.err('git:', repr(stderr)) + yield Result(self, 'Invalid revert commit.') + + rstdout = rstdout.rstrip('\n') + rpos = rstdout.find('\n') + old_shortlog = rstdout[:rpos] if rpos != -1 else rstdout + + if old_shortlog == reverted_shortlog: + yield Result(self, 'This is a valid revert commit.') + + else: + yield Result(self, 'Invalid revert commit.') +``` + +## Handling the special commits once identified + +The GitCommitBear checks the format of all the commits made to coala. With +above changes, the special commits will be identified and treated separately +in the following manner: + +Merge commits + +```python + if merge_commit: + yield Result(self, 'Merge commits are not allowed. Please change your commit.') +``` + +Travis commits + +```python + if travis_commit: + # skip travis build on this PR +``` + +Revert commits + +```python + import urllib3 + from unidiff import PatchSet + + if revert_commit: + http1 = remote_url.rstrp('.git') + '/pull/' + pull_index + '.diff' + diff1 = urllib3.PoolManager() + + http2 = remote_url.rstrp('.git') + '/pull/' + pull_index + '.diff' + diff2 = urllib3.PoolManager() + + encoding = diff.headers.getparam('charset') + + patch1 = PatchSet(diff1, encoding=encoding) + patch2 = PatchSet(diff2, encoding=encoding) + + # compare patch1 and patch2 using unidiff + if not patch1 == patch2: + return + else: + yield Result(self, 'Invalid revert commit.') +```