Merge branch 'master' into fix/deepsource-issues

12 files changed, 148 insertions, 21 deletions

diff --git a/AUTHORS b/AUTHORS
index 66a4329f..c8604bfc 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -35,5 +35,6 @@ Contributors are:
 -César Izurieta <cesar _at_ caih.org>
 -Arthur Milchior <arthur _at_ milchior.fr>
 -Anil Khatri <anil.soccer.khatri _at_ gmail.com>
+-JJ Graham <thetwoj _at_ gmail.com>
 
 Portions derived from other open source works and are clearly marked.
diff --git a/README.md b/README.md
index 3734019e..46867a4c 100644
--- a/README.md
+++ b/README.md
@@ -110,12 +110,13 @@ Please have a look at the [contributions file][contributing].
 
 ### How to make a new release
 
-* Update/verify the version in the `VERSION` file
-* Update/verify that the changelog has been updated
+* Update/verify the **version** in the `VERSION` file
+* Update/verify that the `doc/source/changes.rst` changelog file was updated
 * Commit everything
 * Run `git tag -s <version>` to tag the version in Git
 * Run `make release`
-* Finally, set the upcoming version in the `VERSION` file, usually be
+* Close the milestone mentioned in the _changelog_ and create a new one. _Do not reuse milestones by renaming them_.
+* set the upcoming version in the `VERSION` file, usually be
   incrementing the patch level, and possibly by appending `-dev`. Probably you
   want to `git push` once more.
 
diff --git a/VERSION b/VERSION
index 75a22a26..b0f2dcb3 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-3.0.3
+3.0.4
diff --git a/doc/source/changes.rst b/doc/source/changes.rst
index 356056e7..ff5c0459 100644
--- a/doc/source/changes.rst
+++ b/doc/source/changes.rst
@@ -2,6 +2,12 @@
 Changelog
 =========
 
+3.0.4 - Bugfixes
+=============================================
+
+see the following for details:
+https://github.com/gitpython-developers/gitpython/milestone/31?closed=1
+
 3.0.3 - Bugfixes
 =============================================
 
diff --git a/git/cmd.py b/git/cmd.py
index 661e9bb7..a13556aa 100644
--- a/git/cmd.py
+++ b/git/cmd.py
@@ -67,7 +67,7 @@ __all__ = ('Git',)
 
 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
 
@@ -330,6 +330,9 @@ class Git(LazyMixin):
             but git stops liking them as it will escape the backslashes.
             Hence we undo the escaping just to be sure.
             """
+            url = os.path.expandvars(url)
+            if url.startswith('~'):
+                url = os.path.expanduser(url)
             url = url.replace("\\\\", "\\").replace("\\", "/")
 
         return url
@@ -362,8 +365,11 @@ class Git(LazyMixin):
                 proc.stderr.close()
 
             # did the process finish already so we have a return code ?
-            if proc.poll() is not None:
-                return
+            try:
+                if proc.poll() is not None:
+                    return
+            except OSError as ex:
+                log.info("Ignored error after process had died: %r", ex)
 
             # can be that nothing really exists anymore ...
             if os is None or getattr(os, 'kill', None) is None:
diff --git a/git/diff.py b/git/diff.py
index 6a5d51cc..897228a7 100644
--- a/git/diff.py
+++ b/git/diff.py
@@ -167,7 +167,7 @@ class DiffIndex(list):
     # R = Renamed
     # M = Modified
     # T = Changed in the type
-    change_type = ("A", "D", "R", "M", "T")
+    change_type = ("A", "C", "D", "R", "M", "T")
 
     def iter_change_type(self, change_type):
         """
@@ -193,6 +193,8 @@ class DiffIndex(list):
                 yield diff
             elif change_type == "D" and diff.deleted_file:
                 yield diff
+            elif change_type == "C" and diff.copied_file:
+                yield diff
             elif change_type == "R" and diff.renamed:
                 yield diff
             elif change_type == "M" and diff.a_blob and diff.b_blob and diff.a_blob != diff.b_blob:
@@ -235,7 +237,7 @@ class Diff(object):
     # precompiled regex
     re_header = re.compile(br"""
                                 ^diff[ ]--git
-                                    [ ](?P<a_path_fallback>"?a/.+?"?)[ ](?P<b_path_fallback>"?b/.+?"?)\n
+                                    [ ](?P<a_path_fallback>"?[ab]/.+?"?)[ ](?P<b_path_fallback>"?[ab]/.+?"?)\n
                                 (?:^old[ ]mode[ ](?P<old_mode>\d+)\n
                                    ^new[ ]mode[ ](?P<new_mode>\d+)(?:\n|$))?
                                 (?:^similarity[ ]index[ ]\d+%\n
@@ -243,6 +245,9 @@ class Diff(object):
                                    ^rename[ ]to[ ](?P<rename_to>.*)(?:\n|$))?
                                 (?:^new[ ]file[ ]mode[ ](?P<new_file_mode>.+)(?:\n|$))?
                                 (?:^deleted[ ]file[ ]mode[ ](?P<deleted_file_mode>.+)(?:\n|$))?
+                                (?:^similarity[ ]index[ ]\d+%\n
+                                   ^copy[ ]from[ ].*\n
+                                   ^copy[ ]to[ ](?P<copied_file_name>.*)(?:\n|$))?
                                 (?:^index[ ](?P<a_blob_id>[0-9A-Fa-f]+)
                                     \.\.(?P<b_blob_id>[0-9A-Fa-f]+)[ ]?(?P<b_mode>.+)?(?:\n|$))?
                                 (?:^---[ ](?P<a_path>[^\t\n\r\f\v]*)[\t\r\f\v]*(?:\n|$))?
@@ -253,11 +258,11 @@ class Diff(object):
     NULL_BIN_SHA = b"\0" * 20
 
     __slots__ = ("a_blob", "b_blob", "a_mode", "b_mode", "a_rawpath", "b_rawpath",
-                 "new_file", "deleted_file", "raw_rename_from", "raw_rename_to",
-                 "diff", "change_type", "score")
+                 "new_file", "deleted_file", "copied_file", "raw_rename_from",
+                 "raw_rename_to", "diff", "change_type", "score")
 
     def __init__(self, repo, a_rawpath, b_rawpath, a_blob_id, b_blob_id, a_mode,
-                 b_mode, new_file, deleted_file, raw_rename_from,
+                 b_mode, new_file, deleted_file, copied_file, raw_rename_from,
                  raw_rename_to, diff, change_type, score):
 
         self.a_mode = a_mode
@@ -273,6 +278,14 @@ class Diff(object):
         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:
@@ -285,6 +298,7 @@ class Diff(object):
 
         self.new_file = new_file
         self.deleted_file = deleted_file
+        self.copied_file = copied_file
 
         # be clear and use None instead of empty strings
         assert raw_rename_from is None or isinstance(raw_rename_from, binary_type)
@@ -336,6 +350,8 @@ class Diff(object):
             msg += '\nfile deleted in rhs'
         if self.new_file:
             msg += '\nfile added in rhs'
+        if self.copied_file:
+            msg += '\nfile %r copied from %r' % (self.b_path, self.a_path)
         if self.rename_from:
             msg += '\nfile renamed from %r' % self.rename_from
         if self.rename_to:
@@ -420,11 +436,12 @@ class Diff(object):
             a_path_fallback, b_path_fallback, \
                 old_mode, new_mode, \
                 rename_from, rename_to, \
-                new_file_mode, deleted_file_mode, \
+                new_file_mode, deleted_file_mode, copied_file_name, \
                 a_blob_id, b_blob_id, b_mode, \
                 a_path, b_path = _header.groups()
 
-            new_file, deleted_file = bool(new_file_mode), bool(deleted_file_mode)
+            new_file, deleted_file, copied_file = \
+                bool(new_file_mode), bool(deleted_file_mode), bool(copied_file_name)
 
             a_path = cls._pick_best_path(a_path, rename_from, a_path_fallback)
             b_path = cls._pick_best_path(b_path, rename_to, b_path_fallback)
@@ -446,7 +463,7 @@ class Diff(object):
                               b_blob_id and b_blob_id.decode(defenc),
                               a_mode and a_mode.decode(defenc),
                               b_mode and b_mode.decode(defenc),
-                              new_file, deleted_file,
+                              new_file, deleted_file, copied_file,
                               rename_from,
                               rename_to,
                               None, None, None))
@@ -487,6 +504,7 @@ class Diff(object):
             b_path = path.encode(defenc)
             deleted_file = False
             new_file = False
+            copied_file = False
             rename_from = None
             rename_to = None
 
@@ -498,6 +516,11 @@ class Diff(object):
             elif change_type == 'A':
                 a_blob_id = None
                 new_file = True
+            elif change_type == 'C':
+                copied_file = True
+                a_path, b_path = path.split('\t', 1)
+                a_path = a_path.encode(defenc)
+                b_path = b_path.encode(defenc)
             elif change_type == 'R':
                 a_path, b_path = path.split('\t', 1)
                 a_path = a_path.encode(defenc)
@@ -509,8 +532,8 @@ class Diff(object):
             # END add/remove handling
 
             diff = Diff(repo, a_path, b_path, a_blob_id, b_blob_id, old_mode, new_mode,
-                        new_file, deleted_file, rename_from, rename_to, '',
-                        change_type, score)
+                        new_file, deleted_file, copied_file, rename_from, rename_to,
+                        '', change_type, score)
             index.append(diff)
 
         handle_process_output(proc, handle_diff_line, None, finalize_process, decode_streams=False)
diff --git a/git/objects/util.py b/git/objects/util.py
index 7b6a2763..5dbd9822 100644
--- a/git/objects/util.py
+++ b/git/objects/util.py
@@ -105,6 +105,9 @@ class tzoffset(tzinfo):
         self._offset = timedelta(seconds=-secs_west_of_utc)
         self._name = name or 'fixed'
 
+    def __reduce__(self):
+        return tzoffset, (-self._offset.total_seconds(), self._name)
+
     def utcoffset(self, dt):
         return self._offset
 
diff --git a/git/test/fixtures/diff_copied_mode b/git/test/fixtures/diff_copied_mode
new file mode 100644
index 00000000..60707afc
--- /dev/null
+++ b/git/test/fixtures/diff_copied_mode
@@ -0,0 +1,4 @@
+diff --git a/test1.txt b/test2.txt
+similarity index 100%
+copy from test1.txt
+copy to test2.txt
diff --git a/git/test/fixtures/diff_copied_mode_raw b/git/test/fixtures/diff_copied_mode_raw
new file mode 100644
index 00000000..7640f3ab
--- /dev/null
+++ b/git/test/fixtures/diff_copied_mode_raw
@@ -0,0 +1 @@
+:100644 100644 cfe9deac6e10683917e80f877566b58644aa21df cfe9deac6e10683917e80f877566b58644aa21df C100	test1.txt	test2.txt
diff --git a/git/test/test_diff.py b/git/test/test_diff.py
index e47b9331..e4e7556d 100644
--- a/git/test/test_diff.py
+++ b/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 @@ from git.test.lib import (
     fixture,
     assert_equal,
     assert_true,
-
 )
 from git.test.lib import with_rw_directory
 
@@ -29,9 +31,15 @@ import os.path as osp
 @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
@@ -60,7 +68,12 @@ class TestDiff(TestBase):
 
         with open(fp, 'w') as fs:
             fs.write("Hola Mundo")
-        r.git.commit(all=True, message="change on master")
+        r.git.add(Git.polish_url(fp))
+        self.assertEqual(len(r.index.diff("HEAD", create_patch=True)), 1,
+                         "create_patch should generate patch of diff to HEAD")
+        r.git.commit(message="change on master")
+        self.assertEqual(len(r.index.diff("HEAD", create_patch=True)), 0,
+                         "create_patch should generate no patch, already on HEAD")
 
         r.git.checkout('HEAD~1', b='topic')
         with open(fp, 'w') as fs:
@@ -68,7 +81,8 @@ class TestDiff(TestBase):
         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)
@@ -112,6 +126,29 @@ class TestDiff(TestBase):
         self.assertEqual(diff.score, 100)
         self.assertEqual(len(list(diffs.iter_change_type('R'))), 1)
 
+    def test_diff_with_copied_file(self):
+        output = StringProcessAdapter(fixture('diff_copied_mode'))
+        diffs = Diff._index_from_patch_format(self.rorepo, output)
+        self._assert_diff_format(diffs)
+
+        assert_equal(1, len(diffs))
+
+        diff = diffs[0]
+        assert_true(diff.copied_file)
+        assert_true(diff.a_path, u'test1.txt')
+        assert_true(diff.b_path, u'test2.txt')
+        assert isinstance(str(diff), str)
+
+        output = StringProcessAdapter(fixture('diff_copied_mode_raw'))
+        diffs = Diff._index_from_raw_format(self.rorepo, output)
+        self.assertEqual(len(diffs), 1)
+        diff = diffs[0]
+        self.assertEqual(diff.change_type, 'C')
+        self.assertEqual(diff.score, 100)
+        self.assertEqual(diff.a_path, u'test1.txt')
+        self.assertEqual(diff.b_path, u'test2.txt')
+        self.assertEqual(len(list(diffs.iter_change_type('C'))), 1)
+
     def test_diff_with_change_in_type(self):
         output = StringProcessAdapter(fixture('diff_change_in_type'))
         diffs = Diff._index_from_patch_format(self.rorepo, output)
@@ -244,6 +281,43 @@ class TestDiff(TestBase):
         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 = {}
diff --git a/git/test/test_util.py b/git/test/test_util.py
index b5f9d222..a4d9d7ad 100644
--- a/git/test/test_util.py
+++ b/git/test/test_util.py
@@ -4,6 +4,7 @@
 # This module is part of GitPython and is released under
 # the BSD License: http://www.opensource.org/licenses/bsd-license.php
 
+import pickle
 import tempfile
 import time
 from unittest import skipIf
@@ -280,3 +281,9 @@ class TestUtils(TestBase):
         # Wrong offset: UTC-9000, should return datetime + tzoffset(UTC)
         altz = utctz_to_altz('-9000')
         self.assertEqual(datetime.fromtimestamp(1522827734, tzoffset(0)), from_timestamp(1522827734, altz))
+
+    def test_pickle_tzoffset(self):
+        t1 = tzoffset(555)
+        t2 = pickle.loads(pickle.dumps(t1))
+        self.assertEqual(t1._offset, t2._offset)
+        self.assertEqual(t1._name, t2._name)
diff --git a/init-tests-after-clone.sh b/init-tests-after-clone.sh
index 0d445891..e852f3cd 100755
--- a/init-tests-after-clone.sh
+++ b/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__
\ No newline at end of file
+git reset --hard __testing_point__
+git submodule update --init --recursive
\ No newline at end of file