Remove sh library (#476)

Removes the dependency on the `sh` library, which has been disabled and
deprecated since 0.18.0.

3 files changed, 58 insertions, 97 deletions

diff --git a/gitlint-core/gitlint/shell.py b/gitlint-core/gitlint/shell.py
index fddece0..36160a9 100644
--- a/gitlint-core/gitlint/shell.py
+++ b/gitlint-core/gitlint/shell.py
@@ -1,12 +1,11 @@
 """
-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 altogether in the future, but 'sh' does provide a few
-capabilities wrt dealing with more edge-case environments on *nix systems that are useful.
+This module implements a shim for the `sh` library (https://amoffat.github.io/sh/), which gitlint used to depend on.
+We still keep the `sh` API and semantics so the rest of the gitlint codebase doesn't need to be changed.
 """
 
 import subprocess
 
-from gitlint.utils import TERMINAL_ENCODING, USE_SH_LIB
+from gitlint.utils import TERMINAL_ENCODING
 
 
 def shell(cmd):
@@ -15,64 +14,58 @@ def shell(cmd):
         p.communicate()
 
 
-if USE_SH_LIB:
-    # import exceptions separately, this makes it a little easier to mock them out in the unit tests
-    from sh import (
-        CommandNotFound,
-        ErrorReturnCode,
-        git,
-    )
-else:
-
-    class CommandNotFound(Exception):
-        """Exception indicating a command was not found during execution"""
-
-    class ShResult:
-        """Result wrapper class. We use this to more easily migrate from using https://amoffat.github.io/sh/ to using
-        the builtin subprocess module"""
-
-        def __init__(self, full_cmd, stdout, stderr="", exitcode=0):
-            self.full_cmd = full_cmd
-            self.stdout = stdout
-            self.stderr = stderr
-            self.exit_code = exitcode
-
-        def __str__(self):
-            return self.stdout
-
-    class ErrorReturnCode(ShResult, Exception):
-        """ShResult subclass for unexpected results (acts as an exception)."""
-
-    def git(*command_parts, **kwargs):
-        """Git shell wrapper.
-        Implemented as separate function here, so we can do a 'sh' style imports:
-        `from shell import git`
-        """
-        args = ["git", *list(command_parts)]
-        return _exec(*args, **kwargs)
-
-    def _exec(*args, **kwargs):
-        pipe = subprocess.PIPE
-        popen_kwargs = {"stdout": pipe, "stderr": pipe, "shell": kwargs.get("_tty_out", False)}
-        if "_cwd" in kwargs:
-            popen_kwargs["cwd"] = kwargs["_cwd"]
-
-        try:
-            with subprocess.Popen(args, **popen_kwargs) as p:
-                result = p.communicate()
-        except FileNotFoundError as e:
-            raise CommandNotFound from e
-
-        exit_code = p.returncode
-        stdout = result[0].decode(TERMINAL_ENCODING)
-        stderr = result[1]  # 'sh' does not decode the stderr bytes to unicode
-        full_cmd = "" if args is None else " ".join(args)
-
-        # If not _ok_code is specified, then only a 0 exit code is allowed
-        ok_exit_codes = kwargs.get("_ok_code", [0])
-
-        if exit_code in ok_exit_codes:
-            return ShResult(full_cmd, stdout, stderr, exit_code)
-
-        # Unexpected error code => raise ErrorReturnCode
-        raise ErrorReturnCode(full_cmd, stdout, stderr, p.returncode)
+class CommandNotFound(Exception):
+    """Exception indicating a command was not found during execution"""
+
+
+class ShResult:
+    """Result wrapper class"""
+
+    def __init__(self, full_cmd, stdout, stderr="", exitcode=0):
+        self.full_cmd = full_cmd
+        self.stdout = stdout
+        self.stderr = stderr
+        self.exit_code = exitcode
+
+    def __str__(self):
+        return self.stdout
+
+
+class ErrorReturnCode(ShResult, Exception):
+    """ShResult subclass for unexpected results (acts as an exception)."""
+
+
+def git(*command_parts, **kwargs):
+    """Git shell wrapper.
+    Implemented as separate function here, so we can do a 'sh' style imports:
+    `from shell import git`
+    """
+    args = ["git", *list(command_parts)]
+    return _exec(*args, **kwargs)
+
+
+def _exec(*args, **kwargs):
+    pipe = subprocess.PIPE
+    popen_kwargs = {"stdout": pipe, "stderr": pipe, "shell": kwargs.get("_tty_out", False)}
+    if "_cwd" in kwargs:
+        popen_kwargs["cwd"] = kwargs["_cwd"]
+
+    try:
+        with subprocess.Popen(args, **popen_kwargs) as p:
+            result = p.communicate()
+    except FileNotFoundError as e:
+        raise CommandNotFound from e
+
+    exit_code = p.returncode
+    stdout = result[0].decode(TERMINAL_ENCODING)
+    stderr = result[1]  # 'sh' does not decode the stderr bytes to unicode
+    full_cmd = "" if args is None else " ".join(args)
+
+    # If not _ok_code is specified, then only a 0 exit code is allowed
+    ok_exit_codes = kwargs.get("_ok_code", [0])
+
+    if exit_code in ok_exit_codes:
+        return ShResult(full_cmd, stdout, stderr, exit_code)
+
+    # Unexpected error code => raise ErrorReturnCode
+    raise ErrorReturnCode(full_cmd, stdout, stderr, p.returncode)
diff --git a/gitlint-core/gitlint/tests/test_utils.py b/gitlint-core/gitlint/tests/test_utils.py
index d21ec3f..80883c4 100644
--- a/gitlint-core/gitlint/tests/test_utils.py
+++ b/gitlint-core/gitlint/tests/test_utils.py
@@ -10,22 +10,6 @@ class UtilsTests(BaseTestCase):
         # its value after we're done this doesn't influence other tests
         utils.PLATFORM_IS_WINDOWS = utils.platform_is_windows()
 
-    @patch("os.environ")
-    def test_use_sh_library(self, patched_env):
-        patched_env.get.return_value = "1"
-        self.assertEqual(utils.use_sh_library(), True)
-        patched_env.get.assert_called_once_with("GITLINT_USE_SH_LIB", None)
-
-        for invalid_val in ["0", "foöbar"]:
-            patched_env.get.reset_mock()  # reset mock call count
-            patched_env.get.return_value = invalid_val
-            self.assertEqual(utils.use_sh_library(), False, invalid_val)
-            patched_env.get.assert_called_once_with("GITLINT_USE_SH_LIB", None)
-
-        # Assert that when GITLINT_USE_SH_LIB is not set, we fallback to False (not using)
-        patched_env.get.return_value = None
-        self.assertEqual(utils.use_sh_library(), False)
-
     @patch("gitlint.utils.locale")
     def test_terminal_encoding_non_windows(self, mocked_locale):
         utils.PLATFORM_IS_WINDOWS = False
diff --git a/gitlint-core/gitlint/utils.py b/gitlint-core/gitlint/utils.py
index 3ccb78b..ba4f956 100644
--- a/gitlint-core/gitlint/utils.py
+++ b/gitlint-core/gitlint/utils.py
@@ -22,22 +22,6 @@ def platform_is_windows():
 PLATFORM_IS_WINDOWS = platform_is_windows()
 
 ########################################################################################################################
-# USE_SH_LIB
-# Determine whether to use the `sh` library
-# On windows we won't want to use the sh library since it's not supported - instead we'll use our own shell module.
-# However, we want to be able to overwrite this behavior for testing using the GITLINT_USE_SH_LIB env var.
-
-
-def use_sh_library():
-    gitlint_use_sh_lib_env = os.environ.get("GITLINT_USE_SH_LIB", None)
-    if gitlint_use_sh_lib_env:
-        return gitlint_use_sh_lib_env == "1"
-    return False
-
-
-USE_SH_LIB = use_sh_library()
-
-########################################################################################################################
 # TERMINAL_ENCODING
 # Encoding used for terminal encoding/decoding.