commit-msg hook in python

This moves the business logic of the commit-msg hook from the hook
script to python. This makes the commit-msg hook much more portable
across shells and OSes, as well as easier to unit test.

This should fix #127

.github/workflows/checks.yml

@@ -92,8 +92,8 @@ jobs:
       - name: Tests (sanity)
         run: tools\windows\run_tests.bat "gitlint\tests\cli\test_cli.py::CLITests::test_lint"
 
-      - name: Tests (ignore test_cli.py)
-        run: pytest --ignore gitlint\tests\cli\test_cli.py -rw -s gitlint
+      - name: Tests (ignore cli\*)
+        run: pytest --ignore gitlint\tests\cli -rw -s gitlint
 
       - name: Tests (test_cli.py only - continue-on-error:true)
         run: tools\windows\run_tests.bat "gitlint\tests\cli\test_cli.py"

gitlint/cli.py

@@ -19,9 +19,11 @@
 from gitlint.config import LintConfigBuilder, LintConfigError, LintConfigGenerator
 from gitlint.git import GitContext, GitContextError, git_version
 from gitlint import hooks
+from gitlint.shell import shell
 from gitlint.utils import ustr, LOG_FORMAT
 
 DEFAULT_CONFIG_FILE = ".gitlint"
+DEFAULT_COMMIT_MSG_EDITOR = "vim"
 
 # Since we use the return code to denote the amount of errors, we need to change the default click usage error code
 click.UsageError.exit_code = USAGE_ERROR_CODE
@@ -212,7 +214,7 @@ def cli(  # pylint: disable=too-many-arguments
                                               ignore_stdin, staged, verbose, silent, debug)
         LOG.debug(u"Configuration\n%s", ustr(config))
 
-        ctx.obj = (config, config_builder, commits, msg_filename)
+        ctx.obj = [config, config_builder, commits, msg_filename, None]
 
         # If no subcommand is specified, then just lint
         if ctx.invoked_subcommand is None:
@@ -238,6 +240,9 @@ def lint(ctx):
     msg_filename = ctx.obj[3]
 
     gitcontext = build_git_context(lint_config, msg_filename, 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[4] = gitcontext
 
     number_of_commits = len(gitcontext.commits)
     # Exit if we don't have commits in the specified range. Use a 0 exit code, since a popular use-case is one
@@ -315,6 +320,72 @@ def uninstall_hook(ctx):
         ctx.exit(GIT_CONTEXT_ERROR_CODE)
 
 
+@cli.command("run-hook")
+@click.pass_context
+def run_hook(ctx):
+    """ Runs the gitlint commit-msg hook. """
+
+    exit_code = 1
+    while exit_code > 0:
+        try:
+            click.echo(u"gitlint: checking commit message...")
+            ctx.invoke(lint)
+            click.echo(u"gitlint: " + click.style("OK", fg='green') + u" (no violations in commit message)")
+        except click.exceptions.Exit as e:
+            click.echo(u"-----------------------------------------------")
+            click.echo(u"gitlint: " + click.style("Your commit message contains the above violations.", fg='red'))
+
+            value = None
+            while value not in ["y", "n", "e"]:
+                click.echo("Continue with commit anyways (this keeps the current commit message)? "
+                           "[y(es)/n(no)/e(dit)] ", nl=False)
+
+                # Ideally, we'd want to use click.getchar() or click.prompt() to get user's input here instead of
+                # input(). However, those functions currently don't support getting answers from stdin.
+                # This wouldn't be a huge issue since this is unlikely to occur in the real world,
+                # were it not that we use a stdin to pipe answers into gitlint in our integration tests.
+                # If that ever changes, we can revisit this.
+                # Related click pointers:
+                # - https://github.com/pallets/click/issues/1370
+                # - https://github.com/pallets/click/pull/1372
+                # - From https://click.palletsprojects.com/en/7.x/utils/#getting-characters-from-terminal
+                #   Note that this function will always read from the terminal, even if stdin is instead a pipe.
+                #
+                # We also need a to use raw_input() in Python2 as input() is unsafe (and raw_input() doesn't exist in
+                # Python3). See https://stackoverflow.com/a/4960216/381010
+                input_func = input
+                if sys.version_info[0] == 2:
+                    input_func = raw_input  # noqa pylint: disable=undefined-variable
+
+                value = input_func()
+
+            if value == "y":
+                LOG.debug("run-hook: commit message accepted")
+                ctx.exit(0)
+            elif value == "e":
+                LOG.debug("run-hook: editing commit message")
+                msg_filename = ctx.obj[3]
+                if msg_filename:
+                    msg_filename.seek(0)
+                    editor = os.environ.get("EDITOR", DEFAULT_COMMIT_MSG_EDITOR)
+                    msg_filename_path = os.path.realpath(msg_filename.name)
+                    LOG.debug("run-hook: %s %s", editor, msg_filename_path)
+                    shell([editor, msg_filename_path])
+                else:
+                    click.echo(u"Editing only possible when --msg-filename is specified.")
+                    ctx.exit(e.exit_code)
+            elif value == "n":
+                LOG.debug("run-hook: commit message declined")
+                click.echo(u"Commit aborted.")
+                click.echo(u"Your commit message: ")
+                click.echo(u"-----------------------------------------------")
+                click.echo(ctx.obj[4].commits[0].message.full)
+                click.echo(u"-----------------------------------------------")
+                ctx.exit(e.exit_code)
+
+            exit_code = e.exit_code
+
+
 @cli.command("generate-config")
 @click.pass_context
 def generate_config(ctx):

gitlint/files/commit-msg

@@ -8,74 +8,21 @@ stdin_available=1
 (exec < /dev/tty) 2> /dev/null || stdin_available=0
 
 if [ $stdin_available -eq 1 ]; then
-    # Set bash color codes in case we have a tty
-    RED="\033[31m"
-    YELLOW="\033[33m"
-    GREEN="\033[32m"
-    END_COLOR="\033[0m"
-
     # Now that we know we have a functional tty, set stdin to it so we can ask the user questions :-)
     exec < /dev/tty
-else
-    # Unset bash colors if we don't have a tty
-    RED=""
-    YELLOW=""
-    GREEN=""
-    END_COLOR=""
 fi
 
-run_gitlint(){
-   echo "gitlint: checking commit message..."
-   python -m gitlint.cli --staged --msg-filename "$1"
-   gitlint_exit_code=$?
-}
-
-# Prompts a given yes/no question.
-# Returns 0 if user answers yes, 1 if no
-# Reprompts if different answer
-ask_yes_no_edit(){
-    ask_yes_no_edit_result="no"
-    # If we don't have a stdin available, then just return "No".
-    if [ $stdin_available -eq 0 ]; then
-        ask_yes_no_edit_result="no"
-        return;
-    fi
-    # Otherwise, ask the question until the user answers yes or no
-    question="$1"
-    while true; do
-        read -p "$question" yn
-        case $yn in
-             [Yy]* ) ask_yes_no_edit_result="yes"; return;;
-             [Nn]* ) ask_yes_no_edit_result="no"; return;;
-             [Ee]* ) ask_yes_no_edit_result="edit"; return;;
-        esac
-    done
-}
-
-run_gitlint "$1"
+gitlint --staged --msg-filename "$1" run-hook
+exit_code=$?
 
-while [ $gitlint_exit_code -gt 0 ]; do
-    echo "-----------------------------------------------"
-    echo "gitlint: ${RED}Your commit message contains the above violations.${END_COLOR}"
-    ask_yes_no_edit "Continue with commit anyways (this keeps the current commit message)? [y(es)/n(no)/e(dit)] "
-    if [ $ask_yes_no_edit_result = "yes" ]; then
-        exit 0
-    elif [ $ask_yes_no_edit_result = "edit" ]; then
-        EDITOR=${EDITOR:-vim}
-        $EDITOR "$1"
-        run_gitlint "$1"
-    else
-        echo "Commit aborted."
-        echo "Your commit message: "
-        echo "-----------------------------------------------"
-        cat "$1"
-        echo "-----------------------------------------------"
-
-        exit $gitlint_exit_code
-    fi
-done
+# If we fail to find the gitlint binary (command not found), let's retry by executing as a python module.
+# This is the case for Atlassian SourceTree, where $PATH deviates from the user's shell $PATH.
+if [ $exit_code -eq 127 ]; then
+    echo "Fallback to python module execution"
+    python -m gitlint.cli --staged --msg-filename "$1" run-hook
+    exit_code=$?
+fi
 
-echo "gitlint: ${GREEN}OK${END_COLOR} (no violations in commit message)"
-exit 0
+exit $exit_code
 
 ### gitlint commit-msg hook end ###

gitlint/shell.py

@@ -2,13 +2,20 @@
 """
 This module implements a shim for the 'sh' library, mainly for use on Windows (sh is not supported on Windows).
 We might consider removing the 'sh' dependency alltogether in the future, but 'sh' does provide a few
-capabilities wrt dealing with more edge-case environments on *nix systems that might be useful.
+capabilities wrt dealing with more edge-case environments on *nix systems that are useful.
 """
 
 import subprocess
 import sys
 from gitlint.utils import ustr, USE_SH_LIB
 
+
+def shell(cmd):
+    """ Convenience function that opens a given command in a shell. Does not use 'sh' library. """
+    p = subprocess.Popen(cmd, shell=True)
+    p.communicate()
+
+
 if USE_SH_LIB:
     from sh import git  # pylint: disable=unused-import,import-error
     # import exceptions separately, this makes it a little easier to mock them out in the unit tests
@@ -21,7 +28,7 @@ class CommandNotFound(Exception):
 
     class ShResult(object):
         """ Result wrapper class. We use this to more easily migrate from using https://amoffat.github.io/sh/ to using
-        the builtin subprocess. module """
+        the builtin subprocess module """
 
         def __init__(self, full_cmd, stdout, stderr='', exitcode=0):
             self.full_cmd = full_cmd
@@ -51,7 +58,7 @@ def _exec(*args, **kwargs):
             no_command_error = FileNotFoundError  # noqa pylint: disable=undefined-variable
 
         pipe = subprocess.PIPE
-        popen_kwargs = {'stdout': pipe, 'stderr': pipe, 'shell': kwargs['_tty_out']}
+        popen_kwargs = {'stdout': pipe, 'stderr': pipe, 'shell': kwargs.get('_tty_out', False)}
         if '_cwd' in kwargs:
             popen_kwargs['cwd'] = kwargs['_cwd']
 

gitlint/tests/base.py

@@ -1,10 +1,14 @@
 # -*- coding: utf-8 -*-
 
+import contextlib
 import copy
 import io
 import logging
 import os
 import re
+import shutil
+import sys
+import tempfile
 
 try:
     # python 2.x
@@ -56,6 +60,15 @@ def setUp(self):
         # in gitlint.cli that normally takes care of this
         logging.getLogger('gitlint').propagate = False
 
+    @staticmethod
+    @contextlib.contextmanager
+    def tempdir():
+        tmpdir = tempfile.mkdtemp()
+        try:
+            yield tmpdir
+        finally:
+            shutil.rmtree(tmpdir)
+
     @staticmethod
     def get_sample_path(filename=""):
         # Don't join up empty files names because this will add a trailing slash
@@ -72,6 +85,15 @@ def get_sample(filename=""):
             sample = ustr(content.read())
         return sample
 
+    @staticmethod
+    def patch_input(side_effect):
+        """ Patches the built-in input() with a provided side-effect """
+        module_path = "builtins.input"
+        if sys.version_info[0] == 2:
+            module_path = "__builtin__.raw_input"
+        patched_module = patch(module_path, side_effect=side_effect)
+        return patched_module
+
     @staticmethod
     def get_expected(filename="", variable_dict=None):
         """ Utility method to read an expected file from gitlint/tests/expected and return it as a string.

gitlint/tests/cli/test_cli.py

@@ -1,12 +1,10 @@
 # -*- coding: utf-8 -*-
 
-import contextlib
+
 import io
 import os
 import sys
 import platform
-import shutil
-import tempfile
 
 import arrow
 
@@ -34,15 +32,6 @@
 from gitlint.utils import DEFAULT_ENCODING
 
 
-@contextlib.contextmanager
-def tempdir():
-    tmpdir = tempfile.mkdtemp()
-    try:
-        yield tmpdir
-    finally:
-        shutil.rmtree(tmpdir)
-
-
 class CLITests(BaseTestCase):
     USAGE_ERROR_CODE = 253
     GIT_CONTEXT_ERROR_CODE = 254
@@ -281,7 +270,7 @@ def test_lint_staged_msg_filename(self, sh, _):
             u"commit-1/file-1\ncommit-1/file-2\n",        # git diff-tree
         ]
 
-        with tempdir() as tmpdir:
+        with self.tempdir() as tmpdir:
             msg_filename = os.path.join(tmpdir, "msg")
             with io.open(msg_filename, 'w', encoding=DEFAULT_ENCODING) as f:
                 f.write(u"WIP: msg-filename tïtle\n")
@@ -307,7 +296,7 @@ def test_lint_staged_negative(self, _):
     def test_msg_filename(self, _):
         expected_output = u"3: B6 Body message is missing\n"
 
-        with tempdir() as tmpdir:
+        with self.tempdir() as tmpdir:
             msg_filename = os.path.join(tmpdir, "msg")
             with io.open(msg_filename, 'w', encoding=DEFAULT_ENCODING) as f:
                 f.write(u"Commït title\n")