Merge branch 'master' of https://github.com/gitpython-developers/GitPython

10 files changed, 183 insertions, 42 deletions

diff --git a/git/cmd.py b/git/cmd.py
index b4f987ef..d8469565 100644
--- a/git/cmd.py
+++ b/git/cmd.py
@@ -14,7 +14,6 @@ import errno
 import mmap
 
 from git.odict import OrderedDict
-
 from contextlib import contextmanager
 import signal
 from subprocess import (
@@ -40,7 +39,8 @@ from git.compat import (
     PY3,
     bchr,
     # just to satisfy flake8 on py3
-    unicode
+    unicode,
+    safe_decode,
 )
 
 execute_kwargs = ('istream', 'with_keep_cwd', 'with_extended_output',
@@ -694,12 +694,12 @@ class Git(LazyMixin):
             cmdstr = " ".join(command)
 
             def as_text(stdout_value):
-                return not output_stream and stdout_value.decode(defenc) or '<OUTPUT_STREAM>'
+                return not output_stream and safe_decode(stdout_value) or '<OUTPUT_STREAM>'
             # end
 
             if stderr_value:
                 log.info("%s -> %d; stdout: '%s'; stderr: '%s'",
-                         cmdstr, status, as_text(stdout_value), stderr_value.decode(defenc))
+                         cmdstr, status, as_text(stdout_value), safe_decode(stderr_value))
             elif stdout_value:
                 log.info("%s -> %d; stdout: '%s'", cmdstr, status, as_text(stdout_value))
             else:
@@ -713,11 +713,11 @@ class Git(LazyMixin):
                 raise GitCommandError(command, status, stderr_value)
 
         if isinstance(stdout_value, bytes) and stdout_as_string:  # could also be output_stream
-            stdout_value = stdout_value.decode(defenc)
+            stdout_value = safe_decode(stdout_value)
 
         # Allow access to the command's status code
         if with_extended_output:
-            return (status, stdout_value, stderr_value.decode(defenc))
+            return (status, stdout_value, safe_decode(stderr_value))
         else:
             return stdout_value
 
diff --git a/git/compat.py b/git/compat.py
index 76509ba6..b3572474 100644
--- a/git/compat.py
+++ b/git/compat.py
@@ -35,6 +35,7 @@ if PY3:
         return d.values()
     range = xrange
     unicode = str
+    binary_type = bytes
 else:
     FileType = file
     # usually, this is just ascii, which might not enough for our encoding needs
@@ -44,6 +45,7 @@ else:
     byte_ord = ord
     bchr = chr
     unicode = unicode
+    binary_type = str
     range = xrange
     def mviter(d):
         return d.itervalues()
@@ -54,7 +56,7 @@ def safe_decode(s):
     if isinstance(s, unicode):
         return s
     elif isinstance(s, bytes):
-        return s.decode(defenc, errors='replace')
+        return s.decode(defenc, 'replace')
     raise TypeError('Expected bytes or text, but got %r' % (s,))
 
 
diff --git a/git/diff.py b/git/diff.py
index 9073767e..06193920 100644
--- a/git/diff.py
+++ b/git/diff.py
@@ -7,6 +7,7 @@ import re
 
 from gitdb.util import hex_to_bin
 
+from .compat import binary_type
 from .objects.blob import Blob
 from .objects.util import mode_str_to_int
 
@@ -245,18 +246,20 @@ class Diff(object):
     NULL_HEX_SHA = "0" * 40
     NULL_BIN_SHA = b"\0" * 20
 
-    __slots__ = ("a_blob", "b_blob", "a_mode", "b_mode", "a_path", "b_path",
-                 "new_file", "deleted_file", "rename_from", "rename_to", "diff")
+    __slots__ = ("a_blob", "b_blob", "a_mode", "b_mode", "a_rawpath", "b_rawpath",
+                 "new_file", "deleted_file", "raw_rename_from", "raw_rename_to", "diff")
 
-    def __init__(self, repo, a_path, b_path, a_blob_id, b_blob_id, a_mode,
-                 b_mode, new_file, deleted_file, rename_from,
-                 rename_to, diff):
+    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,
+                 raw_rename_to, diff):
 
         self.a_mode = a_mode
         self.b_mode = b_mode
 
-        self.a_path = a_path
-        self.b_path = b_path
+        assert a_rawpath is None or isinstance(a_rawpath, binary_type)
+        assert b_rawpath is None or isinstance(b_rawpath, binary_type)
+        self.a_rawpath = a_rawpath
+        self.b_rawpath = b_rawpath
 
         if self.a_mode:
             self.a_mode = mode_str_to_int(self.a_mode)
@@ -266,19 +269,21 @@ class Diff(object):
         if a_blob_id is None or a_blob_id == self.NULL_HEX_SHA:
             self.a_blob = None
         else:
-            self.a_blob = Blob(repo, hex_to_bin(a_blob_id), mode=self.a_mode, path=a_path)
+            self.a_blob = Blob(repo, hex_to_bin(a_blob_id), mode=self.a_mode, path=self.a_path)
 
         if b_blob_id is None or b_blob_id == self.NULL_HEX_SHA:
             self.b_blob = None
         else:
-            self.b_blob = Blob(repo, hex_to_bin(b_blob_id), mode=self.b_mode, path=b_path)
+            self.b_blob = Blob(repo, hex_to_bin(b_blob_id), mode=self.b_mode, path=self.b_path)
 
         self.new_file = new_file
         self.deleted_file = deleted_file
 
         # be clear and use None instead of empty strings
-        self.rename_from = rename_from or None
-        self.rename_to = rename_to or None
+        assert raw_rename_from is None or isinstance(raw_rename_from, binary_type)
+        assert raw_rename_to is None or isinstance(raw_rename_to, binary_type)
+        self.raw_rename_from = raw_rename_from or None
+        self.raw_rename_to = raw_rename_to or None
 
         self.diff = diff
 
@@ -345,6 +350,22 @@ class Diff(object):
         return res
 
     @property
+    def a_path(self):
+        return self.a_rawpath.decode(defenc, 'replace') if self.a_rawpath else None
+
+    @property
+    def b_path(self):
+        return self.b_rawpath.decode(defenc, 'replace') if self.b_rawpath else None
+
+    @property
+    def rename_from(self):
+        return self.raw_rename_from.decode(defenc, 'replace') if self.raw_rename_from else None
+
+    @property
+    def rename_to(self):
+        return self.raw_rename_to.decode(defenc, 'replace') if self.raw_rename_to else None
+
+    @property
     def renamed(self):
         """:returns: True if the blob of our diff has been renamed
         :note: This property is deprecated, please use ``renamed_file`` instead.
@@ -388,6 +409,7 @@ class Diff(object):
                 new_file_mode, deleted_file_mode, \
                 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)
 
             a_path = cls._pick_best_path(a_path, rename_from, a_path_fallback)
@@ -404,15 +426,15 @@ class Diff(object):
             a_mode = old_mode or deleted_file_mode or (a_path and (b_mode or new_mode or new_file_mode))
             b_mode = b_mode or new_mode or new_file_mode or (b_path and a_mode)
             index.append(Diff(repo,
-                              a_path and a_path.decode(defenc),
-                              b_path and b_path.decode(defenc),
+                              a_path,
+                              b_path,
                               a_blob_id and a_blob_id.decode(defenc),
                               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,
-                              rename_from and rename_from.decode(defenc),
-                              rename_to and rename_to.decode(defenc),
+                              rename_from,
+                              rename_to,
                               None))
 
             previous_header = header
@@ -438,8 +460,8 @@ class Diff(object):
             meta, _, path = line[1:].partition('\t')
             old_mode, new_mode, a_blob_id, b_blob_id, change_type = meta.split(None, 4)
             path = path.strip()
-            a_path = path
-            b_path = path
+            a_path = path.encode(defenc)
+            b_path = path.encode(defenc)
             deleted_file = False
             new_file = False
             rename_from = None
@@ -455,6 +477,8 @@ class Diff(object):
                 new_file = True
             elif change_type[0] == 'R':     # parses RXXX, where XXX is a confidence value
                 a_path, b_path = path.split('\t', 1)
+                a_path = a_path.encode(defenc)
+                b_path = b_path.encode(defenc)
                 rename_from, rename_to = a_path, b_path
             # END add/remove handling
 
diff --git a/git/ext/gitdb b/git/ext/gitdb
-Subproject 2389b75280efb1a63e6ea578eae7f897fd4beb1
+Subproject d1996e04dbf4841b853b60c1365f0f5fd28d170
diff --git a/git/index/base.py b/git/index/base.py
index 3e68f843..524b4568 100644
--- a/git/index/base.py
+++ b/git/index/base.py
@@ -931,19 +931,24 @@ class IndexFile(LazyMixin, diff.Diffable, Serializable):
         return out
 
     def commit(self, message, parent_commits=None, head=True, author=None,
-               committer=None, author_date=None, commit_date=None):
+               committer=None, author_date=None, commit_date=None,
+               skip_hooks=False):
         """Commit the current default index file, creating a commit object.
         For more information on the arguments, see tree.commit.
 
         :note: If you have manually altered the .entries member of this instance,
                don't forget to write() your changes to disk beforehand.
+               Passing skip_hooks=True is the equivalent of using `-n`
+               or `--no-verify` on the command line.
         :return: Commit object representing the new commit"""
-        run_commit_hook('pre-commit', self)
+        if not skip_hooks:
+            run_commit_hook('pre-commit', self)
         tree = self.write_tree()
         rval = Commit.create_from_tree(self.repo, tree, message, parent_commits,
                                        head, author=author, committer=committer,
                                        author_date=author_date, commit_date=commit_date)
-        run_commit_hook('post-commit', self)
+        if not skip_hooks:
+            run_commit_hook('post-commit', self)
         return rval
 
     @classmethod
diff --git a/git/remote.py b/git/remote.py
index e30debb7..c024030d 100644
--- a/git/remote.py
+++ b/git/remote.py
@@ -77,7 +77,6 @@ def to_progress_instance(progress):
 
 
 class PushInfo(object):
-
     """
     Carries information about the result of a push operation of a single head::
 
@@ -92,7 +91,7 @@ class PushInfo(object):
                         # it to local_ref.commit. Will be None if an error was indicated
         info.summary    # summary line providing human readable english text about the push
         """
-    __slots__ = ('local_ref', 'remote_ref_string', 'flags', 'old_commit', '_remote', 'summary')
+    __slots__ = ('local_ref', 'remote_ref_string', 'flags', '_old_commit_sha', '_remote', 'summary')
 
     NEW_TAG, NEW_HEAD, NO_MATCH, REJECTED, REMOTE_REJECTED, REMOTE_FAILURE, DELETED, \
         FORCED_UPDATE, FAST_FORWARD, UP_TO_DATE, ERROR = [1 << x for x in range(11)]
@@ -112,8 +111,12 @@ class PushInfo(object):
         self.local_ref = local_ref
         self.remote_ref_string = remote_ref_string
         self._remote = remote
-        self.old_commit = old_commit
+        self._old_commit_sha = old_commit
         self.summary = summary
+        
+    @property
+    def old_commit(self):
+        return self._old_commit_sha and self._remote.repo.commit(self._old_commit_sha) or None
 
     @property
     def remote_ref(self):
@@ -176,7 +179,7 @@ class PushInfo(object):
                 split_token = ".."
             old_sha, new_sha = summary.split(' ')[0].split(split_token)
             # have to use constructor here as the sha usually is abbreviated
-            old_commit = remote.repo.commit(old_sha)
+            old_commit = old_sha
         # END message handling
 
         return PushInfo(flags, from_ref, to_ref_string, remote, old_commit, summary)
@@ -203,7 +206,7 @@ class FetchInfo(object):
     NEW_TAG, NEW_HEAD, HEAD_UPTODATE, TAG_UPDATE, REJECTED, FORCED_UPDATE, \
         FAST_FORWARD, ERROR = [1 << x for x in range(8)]
 
-    re_fetch_result = re.compile("^\s*(.) (\[?[\w\s\.$@]+\]?)\s+(.+) -> ([/\w_\+\.\-$@#()]+)(    \(.*\)?$)?")
+    re_fetch_result = re.compile('^\s*(.) (\[?[\w\s\.$@]+\]?)\s+(.+) -> ([^\s]+)(    \(.*\)?$)?')
 
     _flag_map = {'!': ERROR,
                  '+': FORCED_UPDATE,
@@ -451,6 +454,54 @@ class Remote(LazyMixin, Iterable):
             yield Remote(repo, section[lbound + 1:rbound])
         # END for each configuration section
 
+    def set_url(self, new_url, old_url=None, **kwargs):
+        """Configure URLs on current remote (cf command git remote set_url)
+
+        This command manages URLs on the remote.
+
+        :param new_url: string being the URL to add as an extra remote URL
+        :param old_url: when set, replaces this URL with new_url for the remote
+        :return: self
+        """
+        scmd = 'set-url'
+        kwargs['insert_kwargs_after'] = scmd
+        if old_url:
+            self.repo.git.remote(scmd, self.name, old_url, new_url, **kwargs)
+        else:
+            self.repo.git.remote(scmd, self.name, new_url, **kwargs)
+        return self
+
+    def add_url(self, url, **kwargs):
+        """Adds a new url on current remote (special case of git remote set_url)
+
+        This command adds new URLs to a given remote, making it possible to have
+        multiple URLs for a single remote.
+
+        :param url: string being the URL to add as an extra remote URL
+        :return: self
+        """
+        return self.set_url(url, add=True)
+
+    def delete_url(self, url, **kwargs):
+        """Deletes a new url on current remote (special case of git remote set_url)
+
+        This command deletes new URLs to a given remote, making it possible to have
+        multiple URLs for a single remote.
+
+        :param url: string being the URL to delete from the remote
+        :return: self
+        """
+        return self.set_url(url, delete=True)
+
+    @property
+    def urls(self):
+        """:return: Iterator yielding all configured URL targets on a remote
+        as strings"""
+        remote_details = self.repo.git.remote("show", self.name)
+        for line in remote_details.split('\n'):
+            if '  Push  URL:' in line:
+                yield line.split(': ')[-1]
+
     @property
     def refs(self):
         """
diff --git a/git/test/fixtures/diff_patch_unsafe_paths b/git/test/fixtures/diff_patch_unsafe_paths
index 9ee6b834..1aad6754 100644
--- a/git/test/fixtures/diff_patch_unsafe_paths
+++ b/git/test/fixtures/diff_patch_unsafe_paths
@@ -68,6 +68,13 @@ index 0000000000000000000000000000000000000000..eaf5f7510320b6a327fb308379de2f94
 +++ "b/path/\360\237\222\251.txt"
 @@ -0,0 +1 @@
 +dummy content
+diff --git "a/path/\200-invalid-unicode-path.txt" "b/path/\200-invalid-unicode-path.txt"
+new file mode 100644
+index 0000000000000000000000000000000000000000..eaf5f7510320b6a327fb308379de2f94d8859a54
+--- /dev/null
++++ "b/path/\200-invalid-unicode-path.txt"
+@@ -0,0 +1 @@
++dummy content
 diff --git a/a/with spaces b/b/with some spaces
 similarity index 100%
 rename from a/with spaces
diff --git a/git/test/test_diff.py b/git/test/test_diff.py
index 8966351a..ba0d2d13 100644
--- a/git/test/test_diff.py
+++ b/git/test/test_diff.py
@@ -90,6 +90,8 @@ class TestDiff(TestBase):
         assert_true(diff.renamed)
         assert_equal(diff.rename_from, u'Jérôme')
         assert_equal(diff.rename_to, u'müller')
+        assert_equal(diff.raw_rename_from, b'J\xc3\xa9r\xc3\xb4me')
+        assert_equal(diff.raw_rename_to, b'm\xc3\xbcller')
         assert isinstance(str(diff), str)
 
         output = StringProcessAdapter(fixture('diff_rename_raw'))
@@ -129,7 +131,7 @@ class TestDiff(TestBase):
         output = StringProcessAdapter(fixture('diff_index_raw'))
         res = Diff._index_from_raw_format(None, output.stdout)
         assert res[0].deleted_file
-        assert res[0].b_path == ''
+        assert res[0].b_path is None
 
     def test_diff_initial_commit(self):
         initial_commit = self.rorepo.commit('33ebe7acec14b25c5f84f35a664803fcab2f7781')
@@ -162,16 +164,19 @@ class TestDiff(TestBase):
         self.assertEqual(res[7].b_path, u'path/with-question-mark?')
         self.assertEqual(res[8].b_path, u'path/¯\\_(ツ)_|¯')
         self.assertEqual(res[9].b_path, u'path/💩.txt')
+        self.assertEqual(res[9].b_rawpath, b'path/\xf0\x9f\x92\xa9.txt')
+        self.assertEqual(res[10].b_path, u'path/�-invalid-unicode-path.txt')
+        self.assertEqual(res[10].b_rawpath, b'path/\x80-invalid-unicode-path.txt')
 
         # The "Moves"
         # NOTE: The path prefixes a/ and b/ here are legit!  We're actually
         # verifying that it's not "a/a/" that shows up, see the fixture data.
-        self.assertEqual(res[10].a_path, u'a/with spaces')       # NOTE: path a/ here legit!
-        self.assertEqual(res[10].b_path, u'b/with some spaces')  # NOTE: path b/ here legit!
-        self.assertEqual(res[11].a_path, u'a/ending in a space ')
-        self.assertEqual(res[11].b_path, u'b/ending with space ')
-        self.assertEqual(res[12].a_path, u'a/"with-quotes"')
-        self.assertEqual(res[12].b_path, u'b/"with even more quotes"')
+        self.assertEqual(res[11].a_path, u'a/with spaces')       # NOTE: path a/ here legit!
+        self.assertEqual(res[11].b_path, u'b/with some spaces')  # NOTE: path b/ here legit!
+        self.assertEqual(res[12].a_path, u'a/ending in a space ')
+        self.assertEqual(res[12].b_path, u'b/ending with space ')
+        self.assertEqual(res[13].a_path, u'a/"with-quotes"')
+        self.assertEqual(res[13].b_path, u'b/"with even more quotes"')
 
     def test_diff_patch_format(self):
         # test all of the 'old' format diffs for completness - it should at least
diff --git a/git/test/test_docs.py b/git/test/test_docs.py
index 8dc08559..a4604c58 100644
--- a/git/test/test_docs.py
+++ b/git/test/test_docs.py
@@ -64,7 +64,9 @@ class Tutorials(TestBase):
         assert repo.head.ref == repo.heads.master                   # head is a symbolic reference pointing to master
         assert repo.tags['0.3.5'] == repo.tag('refs/tags/0.3.5')    # you can access tags in various ways too
         assert repo.refs.master == repo.heads['master']             # .refs provides access to all refs, i.e. heads ...
-        assert repo.refs['origin/master'] == repo.remotes.origin.refs.master  # ... remotes ...
+        
+        if 'TRAVIS' not in os.environ:
+            assert repo.refs['origin/master'] == repo.remotes.origin.refs.master  # ... remotes ...
         assert repo.refs['0.3.5'] == repo.tags['0.3.5']             # ... and tags
         # ![8-test_init_repo_object]
 
diff --git a/git/test/test_remote.py b/git/test/test_remote.py
index 9ca2f207..3c2e622d 100644
--- a/git/test/test_remote.py
+++ b/git/test/test_remote.py
@@ -9,7 +9,8 @@ from git.test.lib import (
     with_rw_repo,
     with_rw_and_rw_remote_repo,
     fixture,
-    GIT_DAEMON_PORT
+    GIT_DAEMON_PORT,
+    assert_raises
 )
 from git import (
     RemoteProgress,
@@ -62,7 +63,7 @@ class TestRemoteProgress(RemoteProgress):
         # check each stage only comes once
         op_id = op_code & self.OP_MASK
         assert op_id in (self.COUNTING, self.COMPRESSING, self.WRITING)
-        
+
         if op_code & self.WRITING > 0:
             if op_code & self.BEGIN > 0:
                 assert not message, 'should not have message when remote begins writing'
@@ -568,3 +569,47 @@ class TestRemote(TestBase):
         assert res[0].remote_ref_path == 'refs/pull/1/head'
         assert res[0].ref.path == 'refs/heads/pull/1/head'
         assert isinstance(res[0].ref, Head)
+
+    @with_rw_repo('HEAD', bare=False)
+    def test_multiple_urls(self, rw_repo):
+        # test addresses
+        test1 = 'https://github.com/gitpython-developers/GitPython'
+        test2 = 'https://github.com/gitpython-developers/gitdb'
+        test3 = 'https://github.com/gitpython-developers/smmap'
+
+        remote = rw_repo.remotes[0]
+        # Testing setting a single URL
+        remote.set_url(test1)
+        assert list(remote.urls) == [test1]
+
+        # Testing replacing that single URL
+        remote.set_url(test1)
+        assert list(remote.urls) == [test1]
+        # Testing adding new URLs
+        remote.set_url(test2, add=True)
+        assert list(remote.urls) == [test1, test2]
+        remote.set_url(test3, add=True)
+        assert list(remote.urls) == [test1, test2, test3]
+        # Testing removing an URL
+        remote.set_url(test2, delete=True)
+        assert list(remote.urls) == [test1, test3]
+        # Testing changing an URL
+        remote.set_url(test3, test2)
+        assert list(remote.urls) == [test1, test2]
+
+        # will raise: fatal: --add --delete doesn't make sense
+        assert_raises(GitCommandError, remote.set_url, test2, add=True, delete=True)
+
+        # Testing on another remote, with the add/delete URL
+        remote = rw_repo.create_remote('another', url=test1)
+        remote.add_url(test2)
+        assert list(remote.urls) == [test1, test2]
+        remote.add_url(test3)
+        assert list(remote.urls) == [test1, test2, test3]
+        # Testing removing all the URLs
+        remote.delete_url(test2)
+        assert list(remote.urls) == [test1, test3]
+        remote.delete_url(test1)
+        assert list(remote.urls) == [test3]
+        # will raise fatal: Will not delete all non-push URLs
+        assert_raises(GitCommandError, remote.delete_url, test3)