Invalidate cache if linter path or arguments change (#19)

Author: sieberst

gitlint/linters.py

@@ -73,7 +73,8 @@ def lint_command(name, program, arguments, filter_regex, filename, lines):
 
     Returns: dict: a dict with the extracted info from the message.
     """
-    output = utils.get_output_from_cache(name, filename)
+    linter_hash = utils.calculate_hash(program, arguments)
+    output = utils.get_output_from_cache(name, linter_hash, filename)
 
     if output is None:
         call_arguments = [program] + arguments + [filename]
@@ -91,7 +92,7 @@ def lint_command(name, program, arguments, filter_regex, filename, lines):
                 }
             }
         output = output.decode('utf-8')
-        utils.save_output_in_cache(name, filename, output)
+        utils.save_output_in_cache(name, linter_hash, filename, output)
 
     output_lines = output.split(os.linesep)
 

gitlint/utils.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 """Common function used across modules."""
 
+import hashlib
 import io
 import os
 import re
@@ -72,29 +73,48 @@ def _open_for_write(filename):
     return io.open(filename, 'w')
 
 
-def _get_cache_filename(name, filename):
-    """Returns the cache location for filename and linter name."""
+def _get_cache_filename(name, linter_hash, filename):
+    """Returns the cache location for filename and linter name and hash."""
     filename = os.path.abspath(filename)[1:]
+    linter_dir = "%s.%s" % (name, linter_hash)
     home_folder = os.path.expanduser('~')
     base_cache_dir = os.path.join(home_folder, '.git-lint', 'cache')
 
-    return os.path.join(base_cache_dir, name, filename)
+    return os.path.join(base_cache_dir, linter_dir, filename)
 
 
-def get_output_from_cache(name, filename):
+def calculate_hash(program, arguments):
+    """Calculate sha256 hash as hex string over program and arguments.
+
+    Args:
+      program: string: lint program.
+      arguments: list[string]: extra arguments for the program.
+
+    Returns: string: a string with the calculated sha256 hex value.
+    """
+    algorithm = hashlib.sha256()
+    algorithm.update(program)
+    for argument in arguments:
+        algorithm.update("|")
+        algorithm.update(argument)
+    return algorithm.hexdigest()
+
+
+def get_output_from_cache(name, linter_hash, filename):
     """Returns the output from the cache if still valid.
 
     It checks that the cache file is defined and that its modification time is
     after the modification time of the original file.
 
     Args:
       name: string: name of the linter.
+      linter_hash: string: hash representing linter binary and arguments.
       filename: string: path of the filename for which we are retrieving the
         output.
 
     Returns: a string with the output, if it is still valid, or None otherwise.
     """
-    cache_filename = _get_cache_filename(name, filename)
+    cache_filename = _get_cache_filename(name, linter_hash, filename)
     if (os.path.exists(cache_filename)
             and os.path.getmtime(filename) < os.path.getmtime(cache_filename)):
         with io.open(cache_filename) as f:
@@ -103,14 +123,15 @@ def get_output_from_cache(name, filename):
     return None
 
 
-def save_output_in_cache(name, filename, output):
+def save_output_in_cache(name, linter_hash, filename, output):
     """Saves output in the cache location.
 
     Args:
       name: string: name of the linter.
+      linter_hash: string: hash representing linter binary and arguments.
       filename: string: path of the filename for which we are saving the output.
       output: string: full output (not yet filetered) of the lint command.
     """
-    cache_filename = _get_cache_filename(name, filename)
+    cache_filename = _get_cache_filename(name, linter_hash, filename)
     with _open_for_write(cache_filename) as f:
         f.write(output)

test/unittest/test_utils.py

@@ -91,60 +91,74 @@ def test_open_for_write(self):
     def test_get_cache_filename(self):
         self.fs.create_dir('/abspath')
         os.chdir('/abspath')
+        dummy_hash = utils.calculate_hash("some_tool", [])
         with mock.patch('os.path.expanduser', return_value='/home/user'):
             self.assertEqual(
-                '/home/user/.git-lint/cache/linter1/abspath/bar/file.txt',
-                utils._get_cache_filename('linter1', 'bar/file.txt'))
+                '/home/user/.git-lint/cache/linter1.%s/abspath/bar/file.txt' %
+                dummy_hash,
+                utils._get_cache_filename('linter1', dummy_hash,
+                                          'bar/file.txt'))
 
             self.assertEqual(
-                '/home/user/.git-lint/cache/linter2/abspath/file.txt',
-                utils._get_cache_filename('linter2', 'file.txt'))
+                '/home/user/.git-lint/cache/linter2.%s/abspath/file.txt' %
+                dummy_hash,
+                utils._get_cache_filename('linter2', dummy_hash, 'file.txt'))
 
             self.assertEqual(
-                '/home/user/.git-lint/cache/linter3/bar/file.txt',
-                utils._get_cache_filename('linter3', '/bar/file.txt'))
+                '/home/user/.git-lint/cache/linter3.%s/bar/file.txt' %
+                dummy_hash,
+                utils._get_cache_filename('linter3', dummy_hash,
+                                          '/bar/file.txt'))
 
     @unittest.skipUnless(sys.version_info >= (3, 5),
                          'pyfakefs does not support pathlib2. See'
                          'https://github.com/jmcgeheeiv/pyfakefs/issues/408')
     def test_save_output_in_cache(self):
+        dummy_hash = utils.calculate_hash("some_tool", [])
         output = 'Some content'
         with mock.patch(
                 'gitlint.utils._get_cache_filename',
-                return_value='/cache/filename.txt'):
-            utils.save_output_in_cache('linter', 'filename', output)
+                return_value='/cache/linter.%s/filename.txt' % dummy_hash):
+            utils.save_output_in_cache('linter', dummy_hash, 'filename',
+                                       output)
 
-            with open(utils._get_cache_filename('linter', 'filename')) as f:
+            with open(
+                    utils._get_cache_filename('linter', dummy_hash,
+                                              'filename')) as f:
                 self.assertEqual(output, f.read())
 
     def test_get_output_from_cache_no_cache(self):
-        cache_filename = '/cache/filename.txt'
+        dummy_hash = utils.calculate_hash("some_tool", [])
+        cache_filename = '/cache/linter.%s/filename.txt' % dummy_hash
         with mock.patch(
                 'gitlint.utils._get_cache_filename',
                 return_value=cache_filename):
             self.assertIsNone(
-                utils.get_output_from_cache('linter', 'filename'))
+                utils.get_output_from_cache('linter', dummy_hash, 'filename'))
 
     def test_get_output_from_cache_cache_is_expired(self):
-        cache_filename = '/cache/filename.txt'
+        dummy_hash = utils.calculate_hash("some_tool", [])
+        cache_filename = '/cache/linter.%s/filename.txt' % dummy_hash
         self.fs.create_file(cache_filename)
         self.fs.create_file('filename')
         with mock.patch(
                 'gitlint.utils._get_cache_filename',
                 return_value=cache_filename):
             self.assertIsNone(
-                utils.get_output_from_cache('linter', 'filename'))
+                utils.get_output_from_cache('linter', dummy_hash, 'filename'))
 
     def test_get_output_from_cache_cache_is_valid(self):
-        cache_filename = '/cache/filename.txt'
+        dummy_hash = utils.calculate_hash("some_tool", [])
+        cache_filename = '/cache/linter.%s/filename.txt' % dummy_hash
         content = 'some_content'
         self.fs.create_file('filename')
         self.fs.create_file(cache_filename, contents=content)
         with mock.patch(
                 'gitlint.utils._get_cache_filename',
                 return_value=cache_filename):
-            self.assertEqual(content,
-                             utils.get_output_from_cache('linter', 'filename'))
+            self.assertEqual(
+                content,
+                utils.get_output_from_cache('linter', dummy_hash, 'filename'))
 
     def test_which_absolute_path(self):
         filename = '/foo/bar.sh'