Don't require a TTY connected to STDIN to read from local git repo (#44)

Before this commit, gitlint defaulted to reading the latest commit message
from the local git repository when it found a TTY connected to STDIN,
falling back to reading from STDIN in all other cases.

With this commit, gitlint will do the opposite which is more sane: it will
only read from STDIN in case there's input on STDIN, falling back to reading
from the local git repo in all other cases.

This is especially useful for environments where there is no TTY attached to
STDIN like in many CI environments.

This fixes #40, #42.

.pylintrc

@@ -28,7 +28,7 @@ argument-rgx=[a-z_][a-z0-9_]{1,30}$
 method-rgx=([a-z_][a-z0-9_]{2,50}|setUp|tearDown)$
 
 # Allow 'id' as variable name everywhere
-good-names=id,c
+good-names=id,c,_
 
 bad-names=__author__
 

gitlint/cli.py

@@ -3,6 +3,7 @@
 import logging
 import os
 import platform
+import select
 import sys
 
 import click
@@ -92,6 +93,19 @@ def build_config(ctx, target, config_path, c, extra_path, ignore, verbose, silen
     ctx.exit(CONFIG_ERROR_CODE)  # return CONFIG_ERROR_CODE on config error
 
 
+def stdin_has_data():
+    """ Helper function that indicates whether the stdin has data incoming or not """
+    # This code was taken from:
+    # https://stackoverflow.com/questions/3762881/how-do-i-check-if-stdin-has-some-data
+
+    # Caveat, this probably doesn't work on Windows because the code is dependent on the unix SELECT syscall.
+    # Details: https://docs.python.org/2/library/select.html#select.select
+    # This isn't a real problem now, because gitlint as a whole doesn't support Windows (see #20).
+    # If we ever do, we probably want to fall back to the old detection mechanism of reading from the local git repo
+    # in case there's no TTY connected to STDIN.
+    return select.select([sys.stdin, ], [], [], 0.0)[0]
+
+
 @click.group(invoke_without_command=True, epilog="When no COMMAND is specified, gitlint defaults to 'gitlint lint'.")
 @click.option('--target', type=click.Path(exists=True, resolve_path=True, file_okay=False, readable=True),
               help="Path of the target git repository. [default: current working directory]")
@@ -142,12 +156,12 @@ def lint(ctx):
     """ Lints a git repository [default command] """
     lint_config = ctx.obj[0]
 
-    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, ctx.obj[2])
-    else:
+    # If we get data via stdin, then let's consider that our commit message, otherwise parse it from the local git repo.
+    if stdin_has_data():
         stdin_str = ustr(sys.stdin.read())
         gitcontext = GitContext.from_commit_msg(stdin_str)
+    else:
+        gitcontext = GitContext.from_local_repository(lint_config.target, ctx.obj[2])
 
     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

gitlint/tests/test_cli.py

@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 
 import os
+import sys
 import platform
 
 try:
@@ -43,12 +44,10 @@ def test_version(self):
         result = self.cli.invoke(cli.cli, ["--version"])
         self.assertEqual(result.output.split("\n")[0], "cli, version {0}".format(__version__))
 
+    @patch('gitlint.cli.stdin_has_data', return_value=False)
     @patch('gitlint.git.sh')
-    @patch('gitlint.cli.sys')
-    def test_lint(self, sys, sh):
+    def test_lint(self, sh, _):
         """ Test for basic simple linting functionality """
-        sys.stdin.isatty.return_value = True
-
         sh.git.side_effect = ["6f29bf81a8322a04071bb794666e48c443a90360",
                               u"test åuthor\x00test-email@föo.com\x002016-12-03 15:28:15 01:00\x00åbc\n"
                               u"commït-title\n\ncommït-body",
@@ -59,11 +58,10 @@ def test_lint(self, sys, sh):
             self.assertEqual(stderr.getvalue(), u'3: B5 Body message is too short (11<20): "commït-body"\n')
             self.assertEqual(result.exit_code, 1)
 
+    @patch('gitlint.cli.stdin_has_data', return_value=False)
     @patch('gitlint.git.sh')
-    @patch('gitlint.cli.sys')
-    def test_lint_multiple_commits(self, sys, sh):
+    def test_lint_multiple_commits(self, sh, _):
         """ Test for --commits option """
-        sys.stdin.isatty.return_value = True
 
         sh.git.side_effect = ["6f29bf81a8322a04071bb794666e48c443a90360\n" +  # git rev-list <SHA>
                               "25053ccec5e28e1bb8f7551fdbb5ab213ada2401\n" +
@@ -90,11 +88,10 @@ def test_lint_multiple_commits(self, sys, sh):
             self.assertEqual(stderr.getvalue(), expected)
             self.assertEqual(result.exit_code, 3)
 
+    @patch('gitlint.cli.stdin_has_data', return_value=False)
     @patch('gitlint.git.sh')
-    @patch('gitlint.cli.sys')
-    def test_lint_multiple_commits_config(self, sys, sh):
+    def test_lint_multiple_commits_config(self, sh, _):
         """ Test for --commits option where some of the commits have gitlint config in the commit message """
-        sys.stdin.isatty.return_value = True
 
         # Note that the second commit title has a trailing period that is being ignored by gitlint-ignore: T3
         sh.git.side_effect = ["6f29bf81a8322a04071bb794666e48c443a90360\n" +  # git rev-list <SHA>
@@ -121,7 +118,8 @@ def test_lint_multiple_commits_config(self, sys, sh):
             self.assertEqual(stderr.getvalue(), expected)
             self.assertEqual(result.exit_code, 2)
 
-    def test_input_stream(self):
+    @patch('gitlint.cli.stdin_has_data', return_value=True)
+    def test_input_stream(self, _):
         """ Test for linting when a message is passed via stdin """
         expected_output = u"1: T2 Title has trailing whitespace: \"WIP: tïtle \"\n" + \
                           u"1: T5 Title contains the word 'WIP' (case-insensitive): \"WIP: tïtle \"\n" + \
@@ -133,15 +131,17 @@ def test_input_stream(self):
             self.assertEqual(result.exit_code, 3)
             self.assertEqual(result.output, "")
 
-    def test_silent_mode(self):
+    @patch('gitlint.cli.stdin_has_data', return_value=True)
+    def test_silent_mode(self, _):
         """ Test for --silent option """
         with patch('gitlint.display.stderr', new=StringIO()) as stderr:
             result = self.cli.invoke(cli.cli, ["--silent"], input=u"WIP: tïtle \n")
             self.assertEqual(stderr.getvalue(), "")
             self.assertEqual(result.exit_code, 3)
             self.assertEqual(result.output, "")
 
-    def test_verbosity(self):
+    @patch('gitlint.cli.stdin_has_data', return_value=True)
+    def test_verbosity(self, _):
         """ Test for --verbosity option """
         # We only test -v and -vv, more testing is really not required here
         # -v
@@ -169,13 +169,11 @@ def test_verbosity(self):
             self.assertEqual(result.exit_code, CLITests.CONFIG_ERROR_CODE)
             self.assertEqual(result.output, "Config Error: Option 'verbosity' must be set between 0 and 3\n")
 
+    @patch('gitlint.cli.stdin_has_data', return_value=False)
     @patch('gitlint.git.sh')
-    @patch('gitlint.cli.sys')
-    def test_debug(self, sys, sh):
+    def test_debug(self, sh, _):
         """ Test for --debug option """
 
-        sys.stdin.isatty.return_value = True
-
         sh.git.side_effect = ["6f29bf81a8322a04071bb794666e48c443a90360\n"  # git rev-list <SHA>
                               "25053ccec5e28e1bb8f7551fdbb5ab213ada2401\n"
                               "4da2656b0dadc76c7ee3fd0243a96cb64007f125\n",
@@ -221,7 +219,8 @@ def test_debug(self, sys, sh):
 
             self.assert_logged(expected_logs)
 
-    def test_extra_path(self):
+    @patch('gitlint.cli.stdin_has_data', return_value=True)
+    def test_extra_path(self, _):
         """ Test for --extra-path flag """
         # Test extra-path pointing to a directory
         with patch('gitlint.display.stderr', new=StringIO()) as stderr:
@@ -241,7 +240,8 @@ def test_extra_path(self):
             self.assertEqual(stderr.getvalue(), expected_output)
             self.assertEqual(result.exit_code, 2)
 
-    def test_config_file(self):
+    @patch('gitlint.cli.stdin_has_data', return_value=True)
+    def test_config_file(self, _):
         """ Test for --config option """
         with patch('gitlint.display.stderr', new=StringIO()) as stderr:
             config_path = self.get_sample_path("config/gitlintconfig")
@@ -273,10 +273,9 @@ def test_config_file_negative(self):
         result = self.cli.invoke(cli.cli, ["--config", config_path])
         self.assertEqual(result.exit_code, self.CONFIG_ERROR_CODE)
 
-    @patch('gitlint.cli.sys')
-    def test_target(self, sys):
+    @patch('gitlint.cli.stdin_has_data', return_value=False)
+    def test_target(self, _):
         """ Test for the --target option """
-        sys.stdin.isatty.return_value = True
         result = self.cli.invoke(cli.cli, ["--target", "/tmp"])
         # We expect gitlint to tell us that /tmp is not a git repo (this proves that it takes the target parameter
         # into account).
@@ -327,20 +326,18 @@ def test_generate_config_negative(self):
                        "Error: File \"{0}\" already exists.\n".format(sample_path)
         self.assertEqual(result.output, expected_msg)
 
+    @patch('gitlint.cli.stdin_has_data', return_value=False)
     @patch('gitlint.git.sh')
-    @patch('gitlint.cli.sys')
-    def test_git_error(self, sys, sh):
+    def test_git_error(self, sh, _):
         """ Tests that the cli handles git errors properly """
-        sys.stdin.isatty.return_value = True
         sh.git.side_effect = CommandNotFound("git")
         result = self.cli.invoke(cli.cli)
         self.assertEqual(result.exit_code, self.GIT_CONTEXT_ERROR_CODE)
 
+    @patch('gitlint.cli.stdin_has_data', return_value=False)
     @patch('gitlint.git.sh')
-    @patch('gitlint.cli.sys')
-    def test_no_commits_in_range(self, sys, sh):
+    def test_no_commits_in_range(self, sh, _):
         """ Test for --commits with the specified range being empty. """
-        sys.stdin.isatty.return_value = True
         sh.git.side_effect = lambda *_args, **_kwargs: ""
         result = self.cli.invoke(cli.cli, ["--commits", "master...HEAD"])
         expected = u'No commits in range "master...HEAD".\n'

gitlint/tests/test_config_precedence.py

@@ -19,7 +19,8 @@ class LintConfigPrecedenceTests(BaseTestCase):
     def setUp(self):
         self.cli = CliRunner()
 
-    def test_config_precedence(self):
+    @patch('gitlint.cli.stdin_has_data', return_value=True)
+    def test_config_precedence(self, _):
         # TODO(jroovers): this test really only test verbosity, we need to do some refactoring to gitlint.cli
         # to more easily test everything
         # Test that the config precedence is followed:
@@ -55,7 +56,8 @@ def test_config_precedence(self):
             self.assertEqual(result.output, "")
             self.assertEqual(stderr.getvalue(), "1: T5 Title contains the word 'WIP' (case-insensitive): \"WIP\"\n")
 
-    def test_ignore_precedence(self):
+    @patch('gitlint.cli.stdin_has_data', return_value=True)
+    def test_ignore_precedence(self, _):
         with patch('gitlint.display.stderr', new=StringIO()) as stderr:
             # --ignore takes precedence over -c general.ignore
             result = self.cli.invoke(cli.cli, ["-c", "general.ignore=T5", "--ignore", "B6"],

qa/base.py

@@ -48,7 +48,7 @@ def create_tmp_git_repo(cls):
         git("config", "core.precomposeunicode", "true", _cwd=tmp_git_repo)
         return tmp_git_repo
 
-    def _create_simple_commit(self, message, out=None, ok_code=None, env=None, git_repo=None):
+    def _create_simple_commit(self, message, out=None, ok_code=None, env=None, git_repo=None, tty_in=False):
         """ Creates a simple commit with an empty test file.
             :param message: Commit message for the commit. """
 
@@ -69,7 +69,8 @@ def _create_simple_commit(self, message, out=None, ok_code=None, env=None, git_r
         if not ok_code:
             ok_code = [0]
 
-        git("commit", "-m", message, _cwd=git_repo, _tty_in=True, _out=out, _ok_code=ok_code, _env=environment)
+        git("commit", "-m", message, _cwd=git_repo, _err_to_out=True, _out=out, _tty_in=tty_in,
+            _ok_code=ok_code, _env=environment)
         return test_filename
 
     @staticmethod
@@ -84,11 +85,11 @@ def get_sample_path(filename=""):
 
     def get_last_commit_short_hash(self, git_repo=None):
         git_repo = self.tmp_git_repo if git_repo is None else git_repo
-        return git("rev-parse", "--short", "HEAD", _cwd=git_repo, _tty_in=True).replace("\n", "")
+        return git("rev-parse", "--short", "HEAD", _cwd=git_repo, _err_to_out=True).replace("\n", "")
 
     def get_last_commit_hash(self, git_repo=None):
         git_repo = self.tmp_git_repo if git_repo is None else git_repo
-        return git("rev-parse", "HEAD", _cwd=git_repo, _tty_in=True).replace("\n", "")
+        return git("rev-parse", "HEAD", _cwd=git_repo, _err_to_out=True).replace("\n", "")
 
     @staticmethod
     def get_expected(filename="", variable_dict=None):

qa/test_commits.py

@@ -16,7 +16,7 @@ def test_successful(self):
         self._create_simple_commit(u"Sïmple title2\n\nSimple bödy describing the commit2")
         self._create_simple_commit(u"Sïmple title3\n\nSimple bödy describing the commit3")
         output = gitlint("--commits", "test-branch-commits-base...test-branch-commits",
-                         _cwd=self.tmp_git_repo, _tty_in=True)
+                         _cwd=self.tmp_git_repo, _err_to_out=True)
         self.assertEqual(output, "")
 
     def test_violations(self):
@@ -30,7 +30,7 @@ def test_violations(self):
         self._create_simple_commit(u"Sïmple title3.\n")
         commit_sha2 = self.get_last_commit_hash()[:10]
         output = gitlint("--commits", "test-branch-commits-violations-base...test-branch-commits-violations",
-                         _cwd=self.tmp_git_repo, _tty_in=True, _ok_code=[4])
+                         _cwd=self.tmp_git_repo, _err_to_out=True, _ok_code=[4])
         expected = (u"Commit {0}:\n".format(commit_sha2) +
                     u"1: T3 Title has trailing punctuation (.): \"Sïmple title3.\"\n" +
                     u"3: B6 Body message is missing\n"
@@ -48,7 +48,7 @@ def test_lint_single_commit(self):
         commit_sha = self.get_last_commit_hash()
         refspec = "{0}^...{0}".format(commit_sha)
         self._create_simple_commit(u"Sïmple title3.\n")
-        output = gitlint("--commits", refspec, _cwd=self.tmp_git_repo, _tty_in=True, _ok_code=[2])
+        output = gitlint("--commits", refspec, _cwd=self.tmp_git_repo, _err_to_out=True, _ok_code=[2])
         expected = (u"1: T3 Title has trailing punctuation (.): \"Sïmple title2.\"\n" +
                     u"3: B6 Body message is missing\n")
         self.assertEqual(output.exit_code, 2)
@@ -60,8 +60,8 @@ def test_lint_head(self):
         self._create_simple_commit(u"Sïmple title.\n\nSimple bödy describing the commit", git_repo=tmp_git_repo)
         self._create_simple_commit(u"Sïmple title", git_repo=tmp_git_repo)
         self._create_simple_commit(u"WIP: Sïmple title\n\nSimple bödy describing the commit", git_repo=tmp_git_repo)
-        output = gitlint("--commits", "HEAD", _cwd=tmp_git_repo, _tty_in=True, _ok_code=[3])
-        revlist = git("rev-list", "HEAD", _tty_in=True, _cwd=tmp_git_repo).split()
+        output = gitlint("--commits", "HEAD", _cwd=tmp_git_repo, _err_to_out=True, _ok_code=[3])
+        revlist = git("rev-list", "HEAD", _err_to_out=True, _cwd=tmp_git_repo).split()
 
         expected = (
             u"Commit {0}:\n".format(revlist[0][:10]) +

qa/test_config.py

@@ -14,30 +14,30 @@ class ConfigTests(BaseTestCase):
 
     def test_ignore_by_id(self):
         self._create_simple_commit(u"WIP: Thïs is a title.\nContënt on the second line")
-        output = gitlint("--ignore", "T5,B4", _cwd=self.tmp_git_repo, _tty_in=True, _ok_code=[1])
+        output = gitlint("--ignore", "T5,B4", _cwd=self.tmp_git_repo, _err_to_out=True, _ok_code=[1])
         expected = u"1: T3 Title has trailing punctuation (.): \"WIP: Thïs is a title.\"\n"
         self.assertEqual(output, expected)
 
     def test_ignore_by_name(self):
         self._create_simple_commit(u"WIP: Thïs is a title.\nContënt on the second line")
         output = gitlint("--ignore", "title-must-not-contain-word,body-first-line-empty",
-                         _cwd=self.tmp_git_repo, _tty_in=True, _ok_code=[1])
+                         _cwd=self.tmp_git_repo, _err_to_out=True, _ok_code=[1])
         expected = u"1: T3 Title has trailing punctuation (.): \"WIP: Thïs is a title.\"\n"
         self.assertEqual(output, expected)
 
     def test_verbosity(self):
         self._create_simple_commit(u"WIP: Thïs is a title.\nContënt on the second line")
-        output = gitlint("-v", _cwd=self.tmp_git_repo, _tty_in=True, _ok_code=[3])
+        output = gitlint("-v", _cwd=self.tmp_git_repo, _err_to_out=True, _ok_code=[3])
         expected = u"1: T3\n1: T5\n2: B4\n"
         self.assertEqual(output, expected)
 
-        output = gitlint("-vv", _cwd=self.tmp_git_repo, _tty_in=True, _ok_code=[3])
+        output = gitlint("-vv", _cwd=self.tmp_git_repo, _err_to_out=True, _ok_code=[3])
         expected = u"1: T3 Title has trailing punctuation (.)\n" + \
                    u"1: T5 Title contains the word 'WIP' (case-insensitive)\n" + \
                    u"2: B4 Second line is not empty\n"
         self.assertEqual(output, expected)
 
-        output = gitlint("-vvv", _cwd=self.tmp_git_repo, _tty_in=True, _ok_code=[3])
+        output = gitlint("-vvv", _cwd=self.tmp_git_repo, _err_to_out=True, _ok_code=[3])
         expected = u"1: T3 Title has trailing punctuation (.): \"WIP: Thïs is a title.\"\n" + \
                    u"1: T5 Title contains the word 'WIP' (case-insensitive): \"WIP: Thïs is a title.\"\n" + \
                    u"2: B4 Second line is not empty: \"Contënt on the second line\"\n"
@@ -49,7 +49,7 @@ def test_verbosity(self):
 
     def test_set_rule_option(self):
         self._create_simple_commit(u"This ïs a title.")
-        output = gitlint("-c", "title-max-length.line-length=5", _cwd=self.tmp_git_repo, _tty_in=True, _ok_code=[3])
+        output = gitlint("-c", "title-max-length.line-length=5", _cwd=self.tmp_git_repo, _err_to_out=True, _ok_code=[3])
         expected = u"1: T1 Title exceeds max length (16>5): \"This ïs a title.\"\n" + \
                    u"1: T3 Title has trailing punctuation (.): \"This ïs a title.\"\n" + \
                    "3: B6 Body message is missing\n"
@@ -60,7 +60,7 @@ def test_config_from_file(self):
                      "This line of the body is here because we need it"
         self._create_simple_commit(commit_msg)
         config_path = self.get_sample_path("config/gitlintconfig")
-        output = gitlint("--config", config_path, _cwd=self.tmp_git_repo, _tty_in=True, _ok_code=[5])
+        output = gitlint("--config", config_path, _cwd=self.tmp_git_repo, _err_to_out=True, _ok_code=[5])
 
         expected = "1: T1 Title exceeds max length (42>20)\n" + \
                    "1: T5 Title contains the word 'WIP' (case-insensitive)\n" + \
@@ -76,7 +76,7 @@ def test_config_from_file_debug(self):
         self._create_simple_commit(commit_msg)
         commit_sha = self.get_last_commit_hash()
         config_path = self.get_sample_path("config/gitlintconfig")
-        output = gitlint("--config", config_path, "--debug", _cwd=self.tmp_git_repo, _tty_in=True, _ok_code=[5])
+        output = gitlint("--config", config_path, "--debug", _cwd=self.tmp_git_repo, _err_to_out=True, _ok_code=[5])
 
         expected_date = git("log", "-1", "--pretty=%ai", _cwd=self.tmp_git_repo)
         expected_date = arrow.get(str(expected_date), "YYYY-MM-DD HH:mm:ss Z").datetime