Improved file and path handling

- Replaced all open() statements with io.open(), this improves unicode file
  handling
- Removed a bunch of hard-coded forward slashes to use os.path.join instead,
  this is more OS agnostic
- Replaced os.path.abspath with os.path.realpath for improved symlink handling

This commit relates to jorisroovers#20

Author: jorisroovers

.pylintrc

@@ -24,7 +24,7 @@ variable-rgx=[a-z_][a-z0-9_]{0,30}$
 argument-rgx=[a-z_][a-z0-9_]{1,30}$
 
 # Method names should be at least 3 characters long
-# and be lowecased with underscores
+# and be lower-cased with underscores
 method-rgx=([a-z_][a-z0-9_]{2,50}|setUp|tearDown)$
 
 # Allow 'id' as variable name everywhere

gitlint/cli.py

@@ -285,7 +285,7 @@ def uninstall_hook(ctx):
 def generate_config(ctx):
     """ Generates a sample gitlint config file. """
     path = click.prompt('Please specify a location for the sample gitlint config file', default=DEFAULT_CONFIG_FILE)
-    path = os.path.abspath(path)
+    path = os.path.realpath(path)
     dir_name = os.path.dirname(path)
     if not os.path.exists(dir_name):
         click.echo(u"Error: Directory '{0}' does not exist.".format(dir_name), err=True)

gitlint/config.py

@@ -6,18 +6,13 @@
     from configparser import ConfigParser, Error as ConfigParserError  # pragma: no cover, pylint: disable=import-error
 
 import copy
+import io
 import re
 import os
 import shutil
 
-try:
-    # python >= 2.7
-    from collections import OrderedDict  # pylint: disable=no-name-in-module
-except ImportError:  # pragma: no cover
-    # python 2.4-2.6
-    from ordereddict import OrderedDict  # pragma: no cover
-
-from gitlint.utils import ustr
+from collections import OrderedDict
+from gitlint.utils import ustr, DEFAULT_ENCODING
 from gitlint import rules  # For some weird reason pylint complains about this, pylint: disable=unused-import
 from gitlint import options
 from gitlint import rule_finder
@@ -75,7 +70,7 @@ def __init__(self):
         self._debug = options.BoolOption('debug', False, "Enable debug mode")
         self._extra_path = None
         target_description = "Path of the target git repository (default=current working directory)"
-        self._target = options.PathOption('target', os.path.abspath(os.getcwd()), target_description)
+        self._target = options.PathOption('target', os.path.realpath(os.getcwd()), target_description)
         self._ignore = options.ListOption('ignore', [], 'List of rule-ids to ignore')
         self._contrib = options.ListOption('contrib', [], 'List of contrib-rules to enable')
         self._config_path = None
@@ -349,10 +344,13 @@ def set_from_config_file(self, filename):
         """ Loads lint config from a ini-style config file """
         if not os.path.exists(filename):
             raise LintConfigError(u"Invalid file path: {0}".format(filename))
-        self._config_path = os.path.abspath(filename)
+        self._config_path = os.path.realpath(filename)
         try:
             parser = ConfigParser()
-            parser.read(filename)
+
+            with io.open(filename, encoding=DEFAULT_ENCODING) as config_file:
+                # readfp() is deprecated in python 3.2+, but compatible with 2.7
+                parser.readfp(config_file, filename)  # pylint: disable=deprecated-method
 
             for section_name in parser.sections():
                 for option_name, option_value in parser.items(section_name):
@@ -394,7 +392,7 @@ def clone(self):
         return builder
 
 
-GITLINT_CONFIG_TEMPLATE_SRC_PATH = os.path.join(os.path.dirname(os.path.abspath(__file__)), "files/gitlint")
+GITLINT_CONFIG_TEMPLATE_SRC_PATH = os.path.join(os.path.dirname(os.path.realpath(__file__)), "files/gitlint")
 
 
 class LintConfigGenerator(object):

gitlint/hooks.py

@@ -1,10 +1,13 @@
+import io
 import shutil
 import os
 import stat
 
-COMMIT_MSG_HOOK_SRC_PATH = os.path.join(os.path.dirname(os.path.abspath(__file__)), "files/commit-msg")
-COMMIT_MSG_HOOK_DST_PATH = ".git/hooks/commit-msg"
-HOOKS_DIR_PATH = ".git/hooks"
+from gitlint.utils import DEFAULT_ENCODING
+
+COMMIT_MSG_HOOK_SRC_PATH = os.path.join(os.path.dirname(os.path.realpath(__file__)), "files/commit-msg")
+COMMIT_MSG_HOOK_DST_PATH = os.path.join(".git", "hooks", "commit-msg")
+HOOKS_DIR_PATH = os.path.join(".git", "hooks")
 GITLINT_HOOK_IDENTIFIER = "### gitlint commit-msg hook start ###\n"
 
 
@@ -17,12 +20,12 @@ class GitHookInstaller(object):
 
     @staticmethod
     def commit_msg_hook_path(lint_config):
-        return os.path.abspath(os.path.join(lint_config.target, COMMIT_MSG_HOOK_DST_PATH))
+        return os.path.realpath(os.path.join(lint_config.target, COMMIT_MSG_HOOK_DST_PATH))
 
     @staticmethod
     def _assert_git_repo(target):
         """ Asserts that a given target directory is a git repository """
-        hooks_dir = os.path.abspath(os.path.join(target, HOOKS_DIR_PATH))
+        hooks_dir = os.path.realpath(os.path.join(target, HOOKS_DIR_PATH))
         if not os.path.isdir(hooks_dir):
             raise GitHookInstallerError(u"{0} is not a git repository.".format(target))
 
@@ -48,8 +51,8 @@ def uninstall_commit_msg_hook(lint_config):
         if not os.path.exists(dest_path):
             raise GitHookInstallerError(u"There is no commit-msg hook present in {0}.".format(dest_path))
 
-        with open(dest_path) as file:
-            lines = file.readlines()
+        with io.open(dest_path, encoding=DEFAULT_ENCODING) as fp:
+            lines = fp.readlines()
             if len(lines) < 2 or lines[1] != GITLINT_HOOK_IDENTIFIER:
                 msg = u"The commit-msg hook in {0} was not installed by gitlint (or it was modified).\n" + \
                       u"Uninstallation of 3th party or modified gitlint hooks is not supported."

gitlint/options.py

@@ -116,4 +116,4 @@ def set(self, value):
         if error_msg:
             raise RuleOptionError(error_msg)
 
-        self.value = os.path.abspath(value)
+        self.value = os.path.realpath(value)

gitlint/tests/base.py

@@ -1,3 +1,4 @@
+import io
 import logging
 import os
 
@@ -9,7 +10,8 @@
     import unittest
 
 from gitlint.git import GitContext
-from gitlint.utils import ustr, LOG_FORMAT
+from gitlint.utils import ustr, LOG_FORMAT, DEFAULT_ENCODING
+
 
 # unittest2's assertRaisesRegex doesn't do unicode comparison.
 # Let's monkeypatch the str() function to point to unicode() so that it does :)
@@ -54,7 +56,7 @@ def get_sample_path(filename=""):
     def get_sample(filename=""):
         """ Read and return the contents of a file in gitlint/tests/samples """
         sample_path = BaseTestCase.get_sample_path(filename)
-        with open(sample_path) as content:
+        with io.open(sample_path, encoding=DEFAULT_ENCODING) as content:
             sample = ustr(content.read())
         return sample
 
@@ -63,7 +65,7 @@ def get_expected(filename="", variable_dict=None):
         """ Utility method to read an expected file from gitlint/tests/expected and return it as a string.
         Optionally replace template variables specified by variable_dict. """
         expected_path = os.path.join(BaseTestCase.EXPECTED_DIR, filename)
-        with open(expected_path) as content:
+        with io.open(expected_path, encoding=DEFAULT_ENCODING) as content:
             expected = ustr(content.read())
 
         if variable_dict:

gitlint/tests/test_cli.py

@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 
 import contextlib
+import io
 import os
 import sys
 import platform
@@ -12,7 +13,7 @@
     from StringIO import StringIO
 except ImportError:
     # python 3.x
-    from io import StringIO
+    from io import StringIO  # pylint: disable=ungrouped-imports
 
 from click.testing import CliRunner
 
@@ -30,6 +31,7 @@
 from gitlint import hooks
 from gitlint import __version__
 from gitlint import config
+from gitlint.utils import DEFAULT_ENCODING
 
 
 @contextlib.contextmanager
@@ -194,8 +196,8 @@ def test_msg_filename(self, _):
 
         with tempdir() as tmpdir:
             msg_filename = os.path.join(tmpdir, "msg")
-            with open(msg_filename, 'w') as f:
-                f.write("Commït title\n")
+            with io.open(msg_filename, 'w', encoding=DEFAULT_ENCODING) as f:
+                f.write(u"Commït title\n")
 
             with patch('gitlint.display.stderr', new=StringIO()) as stderr:
                 result = self.cli.invoke(cli.cli, ["--msg-filename", msg_filename])
@@ -260,7 +262,7 @@ def test_debug(self, sh, _):
                               u"file6.txt\npåth/to/file7.txt\n"]
 
         with patch('gitlint.display.stderr', new=StringIO()) as stderr:
-            config_path = self.get_sample_path("config/gitlintconfig")
+            config_path = self.get_sample_path(os.path.join("config", "gitlintconfig"))
             result = self.cli.invoke(cli.cli, ["--config", config_path, "--debug", "--commits",
                                                "foo...bar"])
 
@@ -276,8 +278,8 @@ def test_debug(self, sh, _):
                              u"DEBUG: gitlint.cli Python version: {0}".format(sys.version),
                              u"DEBUG: gitlint.cli Git version: git version 1.2.3",
                              u"DEBUG: gitlint.cli Gitlint version: {0}".format(__version__),
-                             self.get_expected('debug_configuration_output1', {'config_path': config_path,
-                                                                               'target': os.path.abspath(os.getcwd())}),
+                             self.get_expected('debug_configuration_output1',
+                                               {'config_path': config_path, 'target': os.path.realpath(os.getcwd())}),
                              u"DEBUG: gitlint.cli No --msg-filename flag, no or empty data passed to stdin. " +
                              u"Attempting to read from the local repo.",
                              u"DEBUG: gitlint.lint Linting commit 6f29bf81a8322a04071bb794666e48c443a90360",
@@ -307,7 +309,7 @@ def test_extra_path(self, _):
 
         # Test extra-path pointing to a file
         with patch('gitlint.display.stderr', new=StringIO()) as stderr:
-            extra_path = self.get_sample_path("user_rules/my_commit_rules.py")
+            extra_path = self.get_sample_path(os.path.join("user_rules", "my_commit_rules.py"))
             result = self.cli.invoke(cli.cli, ["--extra-path", extra_path, "--debug"])
             expected_output = u"1: UC1 Commit violåtion 1: \"Contënt 1\"\n" + \
                               "3: B6 Body message is missing\n"
@@ -333,7 +335,7 @@ def test_contrib_negative(self, _):
     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")
+            config_path = self.get_sample_path(os.path.join("config", "gitlintconfig"))
             result = self.cli.invoke(cli.cli, ["--config", config_path])
             self.assertEqual(result.output, "")
             self.assertEqual(stderr.getvalue(), "1: T5\n3: B6\n")
@@ -358,7 +360,7 @@ def test_config_file_negative(self):
         self.assertEqual(result.exit_code, self.USAGE_ERROR_CODE)
 
         # Invalid config file
-        config_path = self.get_sample_path("config/invalid-option-value")
+        config_path = self.get_sample_path(os.path.join("config", "invalid-option-value"))
         result = self.cli.invoke(cli.cli, ["--config", config_path])
         self.assertEqual(result.exit_code, self.CONFIG_ERROR_CODE)
 
@@ -382,7 +384,7 @@ def test_target_negative(self):
         self.assertEqual(result.output.split("\n")[3], expected_msg)
 
         # try setting a file as target
-        target_path = self.get_sample_path("config/gitlintconfig")
+        target_path = self.get_sample_path(os.path.join("config", "gitlintconfig"))
         result = self.cli.invoke(cli.cli, ["--target", target_path])
         self.assertEqual(result.exit_code, self.USAGE_ERROR_CODE)
         expected_msg = u"Error: Invalid value for \"--target\": Directory \"{0}\" is a file.".format(target_path)
@@ -394,9 +396,9 @@ def test_generate_config(self, generate_config):
         result = self.cli.invoke(cli.cli, ["generate-config"], input=u"tëstfile\n")
         self.assertEqual(result.exit_code, 0)
         expected_msg = u"Please specify a location for the sample gitlint config file [.gitlint]: tëstfile\n" + \
-                       u"Successfully generated {0}\n".format(os.path.abspath(u"tëstfile"))
+                       u"Successfully generated {0}\n".format(os.path.realpath(u"tëstfile"))
         self.assertEqual(result.output, expected_msg)
-        generate_config.assert_called_once_with(os.path.abspath(u"tëstfile"))
+        generate_config.assert_called_once_with(os.path.realpath(u"tëstfile"))
 
     def test_generate_config_negative(self):
         """ Negative test for the generate-config subcommand """
@@ -408,7 +410,7 @@ def test_generate_config_negative(self):
         self.assertEqual(result.output, expected_msg)
 
         # Existing file
-        sample_path = self.get_sample_path("config/gitlintconfig")
+        sample_path = self.get_sample_path(os.path.join("config", "gitlintconfig"))
         result = self.cli.invoke(cli.cli, ["generate-config"], input=sample_path)
         self.assertEqual(result.exit_code, self.USAGE_ERROR_CODE)
         expected_msg = "Please specify a location for the sample gitlint " + \
@@ -443,7 +445,7 @@ def test_install_hook(self, install_hook):
         self.assertEqual(result.exit_code, 0)
         self.assertEqual(result.output, expected)
         expected_config = config.LintConfig()
-        expected_config.target = os.path.abspath(os.getcwd())
+        expected_config.target = os.path.realpath(os.getcwd())
         install_hook.assert_called_once_with(expected_config)
 
     @patch('gitlint.hooks.GitHookInstaller.install_commit_msg_hook')
@@ -467,7 +469,7 @@ def test_install_hook_negative(self, install_hook):
         self.assertEqual(result.exit_code, self.GIT_CONTEXT_ERROR_CODE)
         self.assertEqual(result.output, u"tëst\n")
         expected_config = config.LintConfig()
-        expected_config.target = os.path.abspath(os.getcwd())
+        expected_config.target = os.path.realpath(os.getcwd())
         install_hook.assert_called_once_with(expected_config)
 
     @patch('gitlint.hooks.GitHookInstaller.uninstall_commit_msg_hook')
@@ -479,7 +481,7 @@ def test_uninstall_hook(self, uninstall_hook):
         self.assertEqual(result.exit_code, 0)
         self.assertEqual(result.output, expected)
         expected_config = config.LintConfig()
-        expected_config.target = os.path.abspath(os.getcwd())
+        expected_config.target = os.path.realpath(os.getcwd())
         uninstall_hook.assert_called_once_with(expected_config)
 
     @patch('gitlint.hooks.GitHookInstaller.uninstall_commit_msg_hook', side_effect=hooks.GitHookInstallerError(u"tëst"))
@@ -489,5 +491,5 @@ def test_uninstall_hook_negative(self, uninstall_hook):
         self.assertEqual(result.exit_code, self.GIT_CONTEXT_ERROR_CODE)
         self.assertEqual(result.output, u"tëst\n")
         expected_config = config.LintConfig()
-        expected_config.target = os.path.abspath(os.getcwd())
+        expected_config.target = os.path.realpath(os.getcwd())
         uninstall_hook.assert_called_once_with(expected_config)