Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for linting a range of commits natively. #21

Merged
merged 1 commit into from
Mar 12, 2017
+105 −99
Merged

Add support for linting a range of commits natively. #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions gitlint/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ def build_config(ctx, target, config_path, c, extra_path, ignore, verbose, silen
@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('--commits', default="HEAD", help="The range of commits to lint. [default: HEAD]")
@click.option('-e', '--extra-path', help="Path to a directory with extra user-defined rules",
type=click.Path(exists=True, resolve_path=True, file_okay=False, readable=True))
@click.option('--ignore', default="", help="Ignore rules (comma-separated by id or name).")
Expand All @@ -79,14 +80,14 @@ def build_config(ctx, target, config_path, c, extra_path, ignore, verbose, silen
@click.option('-d', '--debug', help="Enable debugging output.", is_flag=True)
@click.version_option(version=gitlint.__version__)
@click.pass_context
def cli(ctx, target, config, c, extra_path, ignore, verbose, silent, debug):
def cli(ctx, target, config, c, commits, extra_path, ignore, verbose, silent, debug):
""" Git lint tool, checks your git commit messages for styling issues """

# Get the lint config from the commandline parameters and
# store it in the context (click allows storing an arbitrary object in ctx.obj).
config, config_builder = build_config(ctx, target, config, c, extra_path, ignore, verbose, silent, debug)

ctx.obj = (config, config_builder)
ctx.obj = (config, config_builder, commits)

# If no subcommand is specified, then just lint
if ctx.invoked_subcommand is None:
Expand All @@ -101,7 +102,7 @@ def lint(ctx):
try:
if sys.stdin.isatty():
# If target has not been set explicitly before, fallback to the current directory
gitcontext = GitContext.from_local_repository(lint_config.target)
gitcontext = GitContext.from_local_repository(lint_config.target, ctx.obj[2])
else:
stdin_str = ustr(sys.stdin.read())
gitcontext = GitContext.from_commit_msg(stdin_str)
Expand All @@ -117,8 +118,21 @@ def lint(ctx):

# Let's get linting!
linter = GitLinter(lint_config)
violations = linter.lint(last_commit)
linter.print_violations(violations)
number_of_commits = len(gitcontext.commits)
first_violation = True

for commit in gitcontext.commits:
violations = linter.lint(commit)
if violations:
# Display the commit hash & new lines intelligently
if number_of_commits > 1 and commit.sha:
click.echo(u"{0}Commit {1}:".format(
"\n" if not first_violation or commit is last_commit else "",
commit.sha[:10]
))
linter.print_violations(violations)
first_violation = False

exit_code = min(MAX_VIOLATION_ERROR_CODE, len(violations))
ctx.exit(exit_code)

Expand Down
92 changes: 47 additions & 45 deletions gitlint/git.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,26 +54,18 @@ class GitCommit(object):
In the context of gitlint, only the git context and commit message are required.
"""

def __init__(self, context, message, date=None, author_name=None, author_email=None, parents=None,
def __init__(self, context, message, sha=None, date=None, author_name=None, author_email=None, parents=None,
is_merge_commit=False, changed_files=None):
self.context = context
self.message = message
self.sha = sha
self.date = date
self.author_name = author_name
self.author_email = author_email
self.date = date

# parent commit hashes
if not parents:
self.parents = []
else:
self.parents = parents

self.parents = parents or []
self.is_merge_commit = is_merge_commit

if not changed_files:
self.changed_files = []
else:
self.changed_files = changed_files
self.changed_files = changed_files or []

def __unicode__(self):
format_str = u"Author: %s <%s>\nDate: %s\n%s" # pragma: no cover
Expand All @@ -88,7 +80,8 @@ def __repr__(self):
def __eq__(self, other):
# skip checking the context as context refers back to this obj, this will trigger a cyclic dependency
return isinstance(other, GitCommit) and self.message == other.message and \
self.author_name == other.author_name and self.author_email == other.author_email and \
self.sha == other.sha and self.author_name == other.author_name and \
self.author_email == other.author_email and \
self.date == other.date and self.parents == other.parents and \
self.is_merge_commit == other.is_merge_commit and self.changed_files == other.changed_files # noqa

Expand Down Expand Up @@ -117,10 +110,12 @@ def from_commit_msg(commit_msg_str):
return context

@staticmethod
def from_local_repository(repository_path):
def from_local_repository(repository_path, refspec="HEAD"):
""" 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
"""

context = GitContext()
try:
# Special arguments passed to sh: http://amoffat.github.io/sh/special_arguments.html
Expand All @@ -129,22 +124,44 @@ def from_local_repository(repository_path):
'_cwd': repository_path
}

# Get info from the local git repository
# https://git-scm.com/docs/pretty-formats
commit_msg = sh.git.log("-1", "--pretty=%B", **sh_special_args)
commit_author_name = sh.git.log("-1", "--pretty=%aN", **sh_special_args)
commit_author_email = sh.git.log("-1", "--pretty=%aE", **sh_special_args)
# %aI -> ISO 8601-like format, while %aI is strict ISO 8601, it seems to be less widely supporte
commit_date_str = sh.git.log("-1", "--pretty=%ai", **sh_special_args)
commit_parents = sh.git.log("-1", "--pretty=%P", **sh_special_args).split(" ")
commit_is_merge_commit = len(commit_parents) > 1

# changed files in last commit
changed_files_str = sh.git("diff-tree", "--no-commit-id", "--name-only", "-r", "HEAD", **sh_special_args)
sha_list = sh.git("rev-list",
# If refspec contains a dot it is a range
# eg HEAD^.., upstream/master...HEAD
'--max-count={0}'.format(-1 if "." in refspec else 1),
refspec, **sh_special_args).split()

for sha in sha_list:
# Get info from the local git repository: https://git-scm.com/docs/pretty-formats
raw_commit = sh.git.log(sha, "-1", "--pretty=%aN,%aE,%ai,%P%n%B",
**sh_special_args).split("\n")

(name, email, date, parents), commit_msg = raw_commit[0].split(","), "\n".join(raw_commit[1:])

commit_parents = parents.split(" ")
commit_is_merge_commit = len(commit_parents) > 1

# changed files in last commit
changed_files = sh.git("diff-tree", "--no-commit-id", "--name-only",
"-r", sha, **sh_special_args).split()

# "YYYY-MM-DD HH:mm:ss Z" -> ISO 8601-like format
# Use arrow for datetime parsing, because apparently python is quirky around ISO-8601 dates:
# http://stackoverflow.com/a/30696682/381010
commit_date = arrow.get(ustr(date), "YYYY-MM-DD HH:mm:ss Z").datetime

# Create Git commit object with the retrieved info
commit_msg_obj = GitCommitMessage.from_full_message(commit_msg)
commit = GitCommit(context, commit_msg_obj, sha=sha, author_name=name,
author_email=email, date=commit_date, changed_files=changed_files,
parents=commit_parents, is_merge_commit=commit_is_merge_commit)

context.commits.append(commit)

except CommandNotFound:
error_msg = u"'git' command not found. You need to install git to use gitlint on a local repository. " + \
u"See https://git-scm.com/book/en/v2/Getting-Started-Installing-Git on how to install git."
raise GitContextError(error_msg)
raise GitContextError(
u"'git' command not found. You need to install git to use gitlint on a local repository. "
u"See https://git-scm.com/book/en/v2/Getting-Started-Installing-Git on how to install git."
)
except ErrorReturnCode as e: # Something went wrong while executing the git command
error_msg = e.stderr.strip()
if b"Not a git repository" in error_msg:
Expand All @@ -153,21 +170,6 @@ def from_local_repository(repository_path):
error_msg = u"An error occurred while executing '{0}': {1}".format(e.full_cmd, error_msg)
raise GitContextError(error_msg)

# "YYYY-MM-DD HH:mm:ss Z" -> ISO 8601-like format
# Use arrow for datetime parsing, because apparently python is quirky around ISO-8601 dates:
# http://stackoverflow.com/a/30696682/381010
commit_date = arrow.get(ustr(commit_date_str), "YYYY-MM-DD HH:mm:ss Z").datetime

# Create Git commit object with the retrieved info
changed_files = [changed_file for changed_file in changed_files_str.strip().split("\n")]
commit_msg_obj = GitCommitMessage.from_full_message(commit_msg)
commit = GitCommit(context, commit_msg_obj, author_name=commit_author_name, author_email=commit_author_email,
date=commit_date, changed_files=changed_files, parents=commit_parents,
is_merge_commit=commit_is_merge_commit)

# Create GitContext info with the commit object and return

context.commits.append(commit)
return context

def __eq__(self, other):
Expand Down
12 changes: 5 additions & 7 deletions gitlint/tests/test_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,12 @@ def test_version(self):
def test_lint(self, sys, sh):
sys.stdin.isatty.return_value = True

def git_log_side_effect(*args, **_kwargs):
return_values = {'--pretty=%B': u"commït-title\n\ncommït-body", '--pretty=%aN': u"test åuthor",
'--pretty=%aE': u"test-email@föo.com", '--pretty=%ai': "2016-12-03 15:28:15 01:00",
'--pretty=%P': u"åbc"}
return return_values[args[1]]
def git_log_side_effect(*_args, **_kwargs):
return (u"test åuthor,test-email@föo.com,2016-12-03 15:28:15 01:00,åbc\n"
u"commït-title\n\ncommït-body")

sh.git.log.side_effect = git_log_side_effect
sh.git.return_value = u"file1.txt\npåth/to/file2.txt\n"
sh.git.side_effect = ["6f29bf81a8322a04071bb794666e48c443a90360", u"file1.txt\npåth/to/file2.txt\n"]

with patch('gitlint.display.stderr', new=StringIO()) as stderr:
result = self.cli.invoke(cli.cli)
Expand Down Expand Up @@ -200,7 +198,7 @@ def test_generate_config_negative(self):
@patch('gitlint.cli.sys')
def test_git_error(self, sys, sh):
sys.stdin.isatty.return_value = True
sh.git.log.side_effect = CommandNotFound("git")
sh.git.side_effect = CommandNotFound("git")
result = self.cli.invoke(cli.cli)
self.assertEqual(result.exit_code, self.GIT_CONTEXT_ERROR_CODE)

Expand Down
76 changes: 34 additions & 42 deletions gitlint/tests/test_git.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,27 @@
class GitTests(BaseTestCase):
@patch('gitlint.git.sh')
def test_get_latest_commit(self, sh):
def git_log_side_effect(*args, **_kwargs):
return_values = {'--pretty=%B': u"cömmit-title\n\ncömmit-body", '--pretty=%aN': u"test åuthor",
'--pretty=%aE': u"test-emåil@foo.com", '--pretty=%ai': u"2016-12-03 15:28:15 01:00",
'--pretty=%P': u"åbc"}
return return_values[args[1]]
sample_sha = "d8ac47e9f2923c7f22d8668e3a1ed04eb4cdbca9"

def git_log_side_effect(*_args, **_kwargs):
return (u"test åuthor,test-emåil@foo.com,2016-12-03 15:28:15 01:00,åbc\n"
u"cömmit-title\n\ncömmit-body")

sh.git.side_effect = [sample_sha, u"file1.txt\npåth/to/file2.txt\n"]
sh.git.log.side_effect = git_log_side_effect
sh.git.return_value = u"file1.txt\npåth/to/file2.txt\n"

context = GitContext.from_local_repository(u"fake/påth")
expected_sh_special_args = {
'_tty_out': False,
'_cwd': u"fake/påth"
}
# assert that commit info was read using git command
expected_calls = [call('-1', '--pretty=%B', _cwd=u"fake/påth", _tty_out=False),
call('-1', '--pretty=%aN', _cwd=u"fake/påth", _tty_out=False),
call('-1', '--pretty=%aE', _cwd=u"fake/påth", _tty_out=False),
call('-1', '--pretty=%ai', _cwd=u"fake/påth", _tty_out=False),
call('-1', '--pretty=%P', _cwd=u"fake/påth", _tty_out=False)]

self.assertListEqual(sh.git.log.mock_calls, expected_calls)
expected_calls = [
call("rev-list", "--max-count=1", "HEAD", **expected_sh_special_args),
call.log(sample_sha, "-1", "--pretty=%aN,%aE,%ai,%P%n%B", _cwd=u"fake/påth", _tty_out=False),
call('diff-tree', '--no-commit-id', '--name-only', '-r', sample_sha, **expected_sh_special_args)
]
self.assertListEqual(sh.git.mock_calls, expected_calls)

last_commit = context.commits[-1]
self.assertEqual(last_commit.message.title, u"cömmit-title")
Expand All @@ -44,36 +43,32 @@ def git_log_side_effect(*args, **_kwargs):
tzinfo=dateutil.tz.tzoffset("+0100", 3600)))
self.assertListEqual(last_commit.parents, [u"åbc"])
self.assertFalse(last_commit.is_merge_commit)

# assert that changed files are read using git command
sh.git.assert_called_once_with('diff-tree', '--no-commit-id', '--name-only', '-r', 'HEAD',
**expected_sh_special_args)
self.assertListEqual(last_commit.changed_files, ["file1.txt", u"påth/to/file2.txt"])

@patch('gitlint.git.sh')
def test_get_latest_commit_merge_commit(self, sh):
def git_log_side_effect(*args, **_kwargs):
return_values = {'--pretty=%B': u"Merge \"foo bår commit\"", '--pretty=%aN': u"test åuthor",
'--pretty=%aE': u"test-emåil@foo.com", '--pretty=%ai': u"2016-12-03 15:28:15 01:00",
'--pretty=%P': u"åbc def"}
return return_values[args[1]]
sample_sha = "d8ac47e9f2923c7f22d8668e3a1ed04eb4cdbca9"

def git_log_side_effect(*_args, **_kwargs):
return (u"test åuthor,test-emåil@foo.com,2016-12-03 15:28:15 01:00,åbc def\n"
u"Merge \"foo bår commit\"")

sh.git.side_effect = [sample_sha, u"file1.txt\npåth/to/file2.txt\n"]
sh.git.log.side_effect = git_log_side_effect
sh.git.return_value = u"file1.txt\npåth/to/file2.txt\n"

context = GitContext.from_local_repository(u"fåke/path")
expected_sh_special_args = {
'_tty_out': False,
'_cwd': u"fåke/path"
}
# assert that commit info was read using git command
expected_calls = [call('-1', '--pretty=%B', _cwd=u"fåke/path", _tty_out=False),
call('-1', '--pretty=%aN', _cwd=u"fåke/path", _tty_out=False),
call('-1', '--pretty=%aE', _cwd=u"fåke/path", _tty_out=False),
call('-1', '--pretty=%ai', _cwd=u"fåke/path", _tty_out=False),
call('-1', '--pretty=%P', _cwd=u"fåke/path", _tty_out=False)]
expected_calls = [
call("rev-list", "--max-count=1", "HEAD", **expected_sh_special_args),
call.log(sample_sha, "-1", "--pretty=%aN,%aE,%ai,%P%n%B", _cwd=u"fåke/path", _tty_out=False),
call('diff-tree', '--no-commit-id', '--name-only', '-r', sample_sha, **expected_sh_special_args)
]

self.assertListEqual(sh.git.log.mock_calls, expected_calls)
self.assertEqual(sh.git.mock_calls, expected_calls)

last_commit = context.commits[-1]
self.assertEqual(last_commit.message.title, u"Merge \"foo bår commit\"")
Expand All @@ -84,45 +79,42 @@ def git_log_side_effect(*args, **_kwargs):
tzinfo=dateutil.tz.tzoffset("+0100", 3600)))
self.assertListEqual(last_commit.parents, [u"åbc", "def"])
self.assertTrue(last_commit.is_merge_commit)

# assert that changed files are read using git command
sh.git.assert_called_once_with('diff-tree', '--no-commit-id', '--name-only', '-r', 'HEAD',
**expected_sh_special_args)
self.assertListEqual(last_commit.changed_files, ["file1.txt", u"påth/to/file2.txt"])

@patch('gitlint.git.sh')
def test_get_latest_commit_command_not_found(self, sh):
sh.git.log.side_effect = CommandNotFound("git")
sh.git.side_effect = CommandNotFound("git")
expected_msg = "'git' command not found. You need to install git to use gitlint on a local repository. " + \
"See https://git-scm.com/book/en/v2/Getting-Started-Installing-Git on how to install git."
with self.assertRaisesRegex(GitContextError, expected_msg):
GitContext.from_local_repository(u"fåke/path")

# assert that commit message was read using git command
sh.git.log.assert_called_once_with('-1', '--pretty=%B', _tty_out=False, _cwd=u"fåke/path")
sh.git.assert_called_once_with("rev-list", "--max-count=1", "HEAD", _tty_out=False, _cwd=u"fåke/path")

@patch('gitlint.git.sh')
def test_get_latest_commit_git_error(self, sh):
# Current directory not a git repo
err = b"fatal: Not a git repository (or any of the parent directories): .git"
sh.git.log.side_effect = ErrorReturnCode("git log -1 --pretty=%B", b"", err)
sh.git.side_effect = ErrorReturnCode("git rev-list --max-count=1 HEAD", b"", err)

with self.assertRaisesRegex(GitContextError, u"fåke/path is not a git repository."):
GitContext.from_local_repository(u"fåke/path")

# assert that commit message was read using git command
sh.git.log.assert_called_once_with('-1', '--pretty=%B', _tty_out=False, _cwd=u"fåke/path")

sh.git.log.reset_mock()
sh.git.assert_called_once_with("rev-list", "--max-count=1", "HEAD",
_tty_out=False, _cwd=u"fåke/path")
sh.git.reset_mock()
err = b"fatal: Random git error"
sh.git.log.side_effect = ErrorReturnCode("git log -1 --pretty=%B", b"", err)
sh.git.side_effect = ErrorReturnCode("git rev-list --max-count=1 HEAD", b"", err)

expected_msg = u"An error occurred while executing 'git log -1 --pretty=%B': {0}".format(err)
expected_msg = u"An error occurred while executing 'git rev-list --max-count=1 HEAD': {0}".format(err)
with self.assertRaisesRegex(GitContextError, expected_msg):
GitContext.from_local_repository(u"fåke/path")

# assert that commit message was read using git command
sh.git.log.assert_called_once_with('-1', '--pretty=%B', _tty_out=False, _cwd=u"fåke/path")
sh.git.assert_called_once_with("rev-list", "--max-count=1", "HEAD",
_tty_out=False, _cwd=u"fåke/path")

def test_from_commit_msg_full(self):
gitcontext = GitContext.from_commit_msg(self.get_sample("commit_message/sample1"))
Expand Down