Fix how Diff handles commits that contain submodule changes

git/cmd.py

@@ -67,7 +67,7 @@
 
 def handle_process_output(process, stdout_handler, stderr_handler,
                           finalizer=None, decode_streams=True):
-    """Registers for notifications to lean that process output is ready to read, and dispatches lines to
+    """Registers for notifications to learn that process output is ready to read, and dispatches lines to
     the respective line handlers.
     This function returns once the finalizer returns
 

git/diff.py

@@ -278,6 +278,14 @@ def __init__(self, repo, a_rawpath, b_rawpath, a_blob_id, b_blob_id, a_mode,
         if self.b_mode:
             self.b_mode = mode_str_to_int(self.b_mode)
 
+        # Determine whether this diff references a submodule, if it does then
+        # we need to overwrite "repo" to the corresponding submodule's repo instead
+        if repo and a_rawpath:
+            for submodule in repo.submodules:
+                if submodule.path == a_rawpath.decode("utf-8"):
+                    repo = submodule.module()
+                    break
+
         if a_blob_id is None or a_blob_id == self.NULL_HEX_SHA:
             self.a_blob = None
         else:

git/test/test_diff.py

@@ -5,12 +5,15 @@
 # This module is part of GitPython and is released under
 # the BSD License: http://www.opensource.org/licenses/bsd-license.php
 import ddt
+import shutil
+import tempfile
 from git import (
     Repo,
     GitCommandError,
     Diff,
     DiffIndex,
     NULL_TREE,
+    Submodule,
 )
 from git.cmd import Git
 from git.test.lib import (
@@ -19,7 +22,6 @@
     fixture,
     assert_equal,
     assert_true,
-
 )
 from git.test.lib import with_rw_directory
 
@@ -29,9 +31,15 @@
 @ddt.ddt
 class TestDiff(TestBase):
 
+    def setUp(self):
+        self.repo_dir = tempfile.mkdtemp()
+        self.submodule_dir = tempfile.mkdtemp()
+
     def tearDown(self):
         import gc
         gc.collect()
+        shutil.rmtree(self.repo_dir)
+        shutil.rmtree(self.submodule_dir)
 
     def _assert_diff_format(self, diffs):
         # verify that the format of the diff is sane
@@ -68,7 +76,8 @@ def test_diff_with_staged_file(self, rw_dir):
         r.git.commit(all=True, message="change on topic branch")
 
         # there must be a merge-conflict
-        self.failUnlessRaises(GitCommandError, r.git.cherry_pick, 'master')
+        with self.assertRaises(GitCommandError):
+            r.git.cherry_pick('master')
 
         # Now do the actual testing - this should just work
         self.assertEqual(len(r.index.diff(None)), 2)
@@ -267,6 +276,43 @@ def test_diff_with_spaces(self):
         self.assertIsNone(diff_index[0].a_path, repr(diff_index[0].a_path))
         self.assertEqual(diff_index[0].b_path, u'file with spaces', repr(diff_index[0].b_path))
 
+    def test_diff_submodule(self):
+        """Test that diff is able to correctly diff commits that cover submodule changes"""
+        # Init a temp git repo that will be referenced as a submodule
+        sub = Repo.init(self.submodule_dir)
+        with open(self.submodule_dir + "/subfile", "w") as sub_subfile:
+            sub_subfile.write("")
+        sub.index.add(["subfile"])
+        sub.index.commit("first commit")
+
+        # Init a temp git repo that will incorporate the submodule
+        repo = Repo.init(self.repo_dir)
+        with open(self.repo_dir + "/test", "w") as foo_test:
+            foo_test.write("")
+        repo.index.add(['test'])
+        Submodule.add(repo, "subtest", "sub", url="file://" + self.submodule_dir)
+        repo.index.commit("first commit")
+        repo.create_tag('1')
+
+        # Add a commit to the submodule
+        submodule = repo.submodule('subtest')
+        with open(self.repo_dir + "/sub/subfile", "w") as foo_sub_subfile:
+            foo_sub_subfile.write("blub")
+        submodule.module().index.add(["subfile"])
+        submodule.module().index.commit("changed subfile")
+        submodule.binsha = submodule.module().head.commit.binsha
+
+        # Commit submodule updates in parent repo
+        repo.index.add([submodule])
+        repo.index.commit("submodule changed")
+        repo.create_tag('2')
+
+        diff = repo.commit('1').diff(repo.commit('2'))[0]
+        # If diff is unable to find the commit hashes (looks in wrong repo) the *_blob.size
+        # property will be a string containing exception text, an int indicates success
+        self.assertIsInstance(diff.a_blob.size, int)
+        self.assertIsInstance(diff.b_blob.size, int)
+
     def test_diff_interface(self):
         # test a few variations of the main diff routine
         assertion_map = {}

init-tests-after-clone.sh

@@ -12,4 +12,5 @@ git checkout master || git checkout -b master
 git reset --hard HEAD~1
 git reset --hard HEAD~1
 git reset --hard HEAD~1
-git reset --hard __testing_point__
+git reset --hard __testing_point__
+git submodule update --init --recursive