Skip to content

Commit

Permalink
Lint single commit using --commit
Browse files Browse the repository at this point in the history
Support for new convenience CLI flag `--commit` which allows for each
linting of a single commit.

Implements #141
  • Loading branch information
jorisroovers committed Sep 27, 2021
1 parent ff601f6 commit 3d48ea0
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 27 deletions.
19 changes: 10 additions & 9 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ Options:
-c TEXT Config flags in format <rule>.<option>=<value>
(e.g.: -c T1.line-length=80). Flag can be
used multiple times to set multiple config values.
--commit TEXT Hash (SHA) of specific commit to lint.
--commits TEXT The range of commits to lint. [default: HEAD]
-e, --extra-path PATH Path to a directory or python module with extra
user-defined rules
Expand Down Expand Up @@ -249,19 +249,21 @@ git log -1 --pretty=%B 62c0519 | gitlint
Note that gitlint requires that you specify `--pretty=%B` (=only print the log message, not the metadata),
future versions of gitlint might fix this and not require the `--pretty` argument.

## Linting a range of commits
## Linting specific commits

_Introduced in gitlint v0.9.0 (experimental in v0.8.0)_
Gitlint allows users to lint a specific commit:
```sh
gitlint --commit 019cf40580a471a3958d3c346aa8bfd265fe5e16
gitlint --commit 019cf40 # short SHAs work too
```

Gitlint allows users to lint a number of commits at once like so:
You can also lint multiple commits at once like so:

```sh
# Lint a specific commit range:
gitlint --commits "019cf40...d6bc75a"
# You can also use git's special references:
gitlint --commits "origin..HEAD"
# Or specify a single specific commit in refspec format, like so:
gitlint --commits "019cf40^...019cf40"
```

The `--commits` flag takes a **single** refspec argument or commit range. Basically, any range that is understood
Expand All @@ -274,9 +276,8 @@ script to lint an arbitrary set of commits, like shown in the example below.
#!/bin/sh
for commit in $(git rev-list master); do
commit_msg=$(git log -1 --pretty=%B $commit)
echo "$commit"
echo "$commit_msg" | gitlint
echo "Commit $commit"
gitlint --commit $commit
echo "--------"
done
```
Expand Down
19 changes: 13 additions & 6 deletions gitlint/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ def get_stdin_data():
return False


def build_git_context(lint_config, msg_filename, refspec):
def build_git_context(lint_config, msg_filename, commit_hash, refspec):
""" Builds a git context based on passed parameters and order of precedence """

# Determine which GitContext method to use if a custom message is passed
Expand Down Expand Up @@ -173,7 +173,11 @@ def build_git_context(lint_config, msg_filename, refspec):

# 3. Fallback to reading from local repository
LOG.debug("No --msg-filename flag, no or empty data passed to stdin. Using the local repo.")
return GitContext.from_local_repository(lint_config.target, refspec)

if commit_hash and refspec:
raise GitLintUsageError("--commit and --commits are mutually exclusive, use one or the other.")

return GitContext.from_local_repository(lint_config.target, refspec=refspec, commit_hash=commit_hash)


def handle_gitlint_error(ctx, exc):
Expand All @@ -192,9 +196,10 @@ def handle_gitlint_error(ctx, exc):
class ContextObj:
""" Simple class to hold data that is passed between Click commands via the Click context. """

def __init__(self, config, config_builder, refspec, msg_filename, gitcontext=None):
def __init__(self, config, config_builder, commit_hash, refspec, msg_filename, gitcontext=None):
self.config = config
self.config_builder = config_builder
self.commit_hash = commit_hash
self.refspec = refspec
self.msg_filename = msg_filename
self.gitcontext = gitcontext
Expand All @@ -210,6 +215,7 @@ def __init__(self, config, config_builder, refspec, msg_filename, gitcontext=Non
@click.option('-c', multiple=True,
help="Config flags in format <rule>.<option>=<value> (e.g.: -c T1.line-length=80). " +
"Flag can be used multiple times to set multiple config values.") # pylint: disable=bad-continuation
@click.option('--commit', envvar='GITLINT_COMMIT', default=None, help="Hash (SHA) of specific commit to lint.")
@click.option('--commits', envvar='GITLINT_COMMITS', default=None, help="The range of commits to lint. [default: HEAD]")
@click.option('-e', '--extra-path', envvar='GITLINT_EXTRA_PATH',
help="Path to a directory or python module with extra user-defined rules",
Expand All @@ -232,7 +238,7 @@ def __init__(self, config, config_builder, refspec, msg_filename, gitcontext=Non
@click.version_option(version=gitlint.__version__)
@click.pass_context
def cli( # pylint: disable=too-many-arguments
ctx, target, config, c, commits, extra_path, ignore, contrib,
ctx, target, config, c, commit, commits, extra_path, ignore, contrib,
msg_filename, ignore_stdin, staged, fail_without_commits, verbose,
silent, debug,
):
Expand All @@ -254,7 +260,7 @@ def cli( # pylint: disable=too-many-arguments
fail_without_commits, verbose, silent, debug)
LOG.debug("Configuration\n%s", config)

ctx.obj = ContextObj(config, config_builder, commits, msg_filename)
ctx.obj = ContextObj(config, config_builder, commit, commits, msg_filename)

# If no subcommand is specified, then just lint
if ctx.invoked_subcommand is None:
Expand All @@ -270,9 +276,10 @@ def lint(ctx):
""" Lints a git repository [default command] """
lint_config = ctx.obj.config
refspec = ctx.obj.refspec
commit_hash = ctx.obj.commit_hash
msg_filename = ctx.obj.msg_filename

gitcontext = build_git_context(lint_config, msg_filename, refspec)
gitcontext = build_git_context(lint_config, msg_filename, commit_hash, refspec)
# Set gitcontext in the click context, so we can use it in command that are ran after this
# in particular, this is used by run-hook
ctx.obj.gitcontext = gitcontext
Expand Down
17 changes: 11 additions & 6 deletions gitlint/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,22 +364,27 @@ def from_staged_commit(commit_msg_str, repository_path):
return context

@staticmethod
def from_local_repository(repository_path, refspec=None):
def from_local_repository(repository_path, refspec=None, commit_hash=None):
""" Retrieves the git context from a local git repository.
:param repository_path: Path to the git repository to retrieve the context from
:param refspec: The commit(s) to retrieve
:param refspec: The commit(s) to retrieve (mutually exclusive with `commit_sha`)
:param commit_hash: Hash of the commit to retrieve (mutually exclusive with `refspec`)
"""

context = GitContext(repository_path=repository_path)

# If no refspec is defined, fallback to the last commit on the current branch
if refspec is None:
if refspec:
sha_list = _git("rev-list", refspec, _cwd=repository_path).split()
elif commit_hash: # Single commit, just pass it to `git log -1`
# Even though we have already been passed the commit hash, we ask git to retrieve this hash and
# return it to us. This way we verify that the passed hash is a valid hash for the target repo and we
# also convert it to the full hash format (we might have been passed a short hash).
sha_list = [_git("log", "-1", commit_hash, "--pretty=%H", _cwd=repository_path).replace("\n", "")]
else: # If no refspec is defined, fallback to the last commit on the current branch
# We tried many things here e.g.: defaulting to e.g. HEAD or HEAD^... (incl. dealing with
# repos that only have a single commit - HEAD^... doesn't work there), but then we still get into
# problems with e.g. merge commits. Easiest solution is just taking the SHA from `git log -1`.
sha_list = [_git("log", "-1", "--pretty=%H", _cwd=repository_path).replace("\n", "")]
else:
sha_list = _git("rev-list", refspec, _cwd=repository_path).split()

for sha in sha_list:
commit = LocalGitCommit(context, sha)
Expand Down
33 changes: 33 additions & 0 deletions gitlint/tests/cli/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,39 @@ def test_lint_multiple_commits_configuration_rules(self, sh, _):
self.assertEqual(stderr.getvalue(), expected)
self.assertEqual(result.exit_code, 2)

@patch('gitlint.cli.get_stdin_data', return_value=False)
@patch('gitlint.git.sh')
def test_lint_commit(self, sh, _):
""" Test for --commit option """

sh.git.side_effect = [
"6f29bf81a8322a04071bb794666e48c443a90360\n", # git log -1 <SHA> --pretty=%H
# git log --pretty <FORMAT> <SHA>
"test åuthor1\x00test-email1@föo.com\x002016-12-03 15:28:15 +0100\x00åbc\n"
"WIP: commït-title1\n\ncommït-body1",
"#", # git config --get core.commentchar
"commit-1-branch-1\ncommit-1-branch-2\n", # git branch --contains <sha>
"commit-1/file-1\ncommit-1/file-2\n", # git diff-tree
]

with patch('gitlint.display.stderr', new=StringIO()) as stderr:
result = self.cli.invoke(cli.cli, ["--commit", "foo"])
self.assertEqual(result.output, "")

self.assertEqual(stderr.getvalue(), self.get_expected("cli/test_cli/test_lint_commit_1"))
self.assertEqual(result.exit_code, 2)

@patch('gitlint.cli.get_stdin_data', return_value=False)
@patch('gitlint.git.sh')
def test_lint_commit_negative(self, sh, _):
""" Negative test for --commit option """

# Try using --commit and --commits at the same time (not allowed)
result = self.cli.invoke(cli.cli, ["--commit", "foo", "--commits", "foo...bar"])
expected_output = "Error: --commit and --commits are mutually exclusive, use one or the other.\n"
self.assertEqual(result.output, expected_output)
self.assertEqual(result.exit_code, self.USAGE_ERROR_CODE)

@patch('gitlint.cli.get_stdin_data', return_value=u'WIP: tïtle \n')
def test_input_stream(self, _):
""" Test for linting when a message is passed via stdin """
Expand Down
2 changes: 2 additions & 0 deletions gitlint/tests/expected/cli/test_cli/test_lint_commit_1
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
1: T5 Title contains the word 'WIP' (case-insensitive): "WIP: commït-title1"
3: B5 Body message is too short (12<20): "commït-body1"
64 changes: 59 additions & 5 deletions gitlint/tests/git/test_git_commit.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,22 +75,23 @@ def test_get_latest_commit(self, sh):
self.assertListEqual(sh.git.mock_calls, expected_calls)

@patch('gitlint.git.sh')
def test_from_local_repository_specific_ref(self, sh):
sample_sha = "myspecialref"
def test_from_local_repository_specific_refspec(self, sh):
sample_refspec = "åbc123..def456"
sample_sha = "åbc123"

sh.git.side_effect = [
sample_sha,
sample_sha, # git rev-list <sample_refspec>
"test åuthor\x00test-emåil@foo.com\x002016-12-03 15:28:15 +0100\x00åbc\n"
"cömmit-title\n\ncömmit-body",
"#", # git config --get core.commentchar
"file1.txt\npåth/to/file2.txt\n",
"foöbar\n* hürdur\n"
]

context = GitContext.from_local_repository("fåke/path", sample_sha)
context = GitContext.from_local_repository("fåke/path", refspec=sample_refspec)
# assert that commit info was read using git command
expected_calls = [
call("rev-list", sample_sha, **self.expected_sh_special_args),
call("rev-list", sample_refspec, **self.expected_sh_special_args),
call("log", sample_sha, "-1", "--pretty=%aN%x00%aE%x00%ai%x00%P%n%B", **self.expected_sh_special_args),
call('config', '--get', 'core.commentchar', _ok_code=[0, 1], **self.expected_sh_special_args),
call('diff-tree', '--no-commit-id', '--name-only', '-r', '--root', sample_sha,
Expand Down Expand Up @@ -127,6 +128,59 @@ def test_from_local_repository_specific_ref(self, sh):
# All expected calls should've happened at this point
self.assertListEqual(sh.git.mock_calls, expected_calls)

@patch('gitlint.git.sh')
def test_from_local_repository_specific_commit_hash(self, sh):
sample_hash = "åbc123"

sh.git.side_effect = [
sample_hash, # git log -1 <sample_hash>
"test åuthor\x00test-emåil@foo.com\x002016-12-03 15:28:15 +0100\x00åbc\n"
"cömmit-title\n\ncömmit-body",
"#", # git config --get core.commentchar
"file1.txt\npåth/to/file2.txt\n",
"foöbar\n* hürdur\n"
]

context = GitContext.from_local_repository("fåke/path", commit_hash=sample_hash)
# assert that commit info was read using git command
expected_calls = [
call("log", "-1", sample_hash, "--pretty=%H", **self.expected_sh_special_args),
call("log", sample_hash, "-1", "--pretty=%aN%x00%aE%x00%ai%x00%P%n%B", **self.expected_sh_special_args),
call('config', '--get', 'core.commentchar', _ok_code=[0, 1], **self.expected_sh_special_args),
call('diff-tree', '--no-commit-id', '--name-only', '-r', '--root', sample_hash,
**self.expected_sh_special_args),
call('branch', '--contains', sample_hash, **self.expected_sh_special_args)
]

# Only first 'git log' call should've happened at this point
self.assertEqual(sh.git.mock_calls, expected_calls[:1])

last_commit = context.commits[-1]
self.assertIsInstance(last_commit, LocalGitCommit)
self.assertEqual(last_commit.sha, sample_hash)
self.assertEqual(last_commit.message.title, "cömmit-title")
self.assertEqual(last_commit.message.body, ["", "cömmit-body"])
self.assertEqual(last_commit.author_name, "test åuthor")
self.assertEqual(last_commit.author_email, "test-emåil@foo.com")
self.assertEqual(last_commit.date, datetime.datetime(2016, 12, 3, 15, 28, 15,
tzinfo=dateutil.tz.tzoffset("+0100", 3600)))
self.assertListEqual(last_commit.parents, ["åbc"])
self.assertFalse(last_commit.is_merge_commit)
self.assertFalse(last_commit.is_fixup_commit)
self.assertFalse(last_commit.is_squash_commit)
self.assertFalse(last_commit.is_revert_commit)

# First 2 'git log' calls should've happened at this point
self.assertListEqual(sh.git.mock_calls, expected_calls[:3])

self.assertListEqual(last_commit.changed_files, ["file1.txt", "påth/to/file2.txt"])
# 'git diff-tree' should have happened at this point
self.assertListEqual(sh.git.mock_calls, expected_calls[:4])

self.assertListEqual(last_commit.branches, ["foöbar", "hürdur"])
# All expected calls should've happened at this point
self.assertListEqual(sh.git.mock_calls, expected_calls)

@patch('gitlint.git.sh')
def test_get_latest_commit_merge_commit(self, sh):
sample_sha = "d8ac47e9f2923c7f22d8668e3a1ed04eb4cdbca9"
Expand Down
24 changes: 23 additions & 1 deletion qa/test_commits.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,38 @@ def test_lint_empty_commit_range(self):
def test_lint_single_commit(self):
""" Tests `gitlint --commits <sha>^...<same sha>` """
self.create_simple_commit("Sïmple title.\n")
first_commit_sha = self.get_last_commit_hash()
self.create_simple_commit("Sïmple title2.\n")
commit_sha = self.get_last_commit_hash()
refspec = f"{commit_sha}^...{commit_sha}"
self.create_simple_commit("Sïmple title3.\n")
output = gitlint("--commits", refspec, _cwd=self.tmp_git_repo, _tty_in=True, _ok_code=[2])

expected = ("1: T3 Title has trailing punctuation (.): \"Sïmple title2.\"\n" +
"3: B6 Body message is missing\n")

# Lint using --commit <commit sha>
output = gitlint("--commit", commit_sha, _cwd=self.tmp_git_repo, _tty_in=True, _ok_code=[2])
self.assertEqual(output.exit_code, 2)
self.assertEqualStdout(output, expected)

# Lint a single commit using --commits <refspec> pointing to the single commit
output = gitlint("--commits", refspec, _cwd=self.tmp_git_repo, _tty_in=True, _ok_code=[2])
self.assertEqual(output.exit_code, 2)
self.assertEqualStdout(output, expected)

# Lint the first commit in the repository. This is a use-case that is not supported by --commits
# As <sha>^...<sha> is not correct refspec in case <sha> points to the initial commit (which has no parents)
expected = ("1: T3 Title has trailing punctuation (.): \"Sïmple title.\"\n" +
"3: B6 Body message is missing\n")
output = gitlint("--commit", first_commit_sha, _cwd=self.tmp_git_repo, _tty_in=True, _ok_code=[2])
self.assertEqual(output.exit_code, 2)
self.assertEqualStdout(output, expected)

# Assert that indeed --commits <refspec> is not supported when <refspec> points the the first commit
refspec = f"{first_commit_sha}^...{first_commit_sha}"
output = gitlint("--commits", refspec, _cwd=self.tmp_git_repo, _tty_in=True, _ok_code=[254])
self.assertEqual(output.exit_code, 254)

def test_lint_staged_stdin(self):
""" Tests linting a staged commit. Gitint should lint the passed commit message andfetch additional meta-data
from the underlying repository. The easiest way to test this is by inspecting `--debug` output.
Expand Down

0 comments on commit 3d48ea0

Please sign in to comment.