Merge lp:~jelmer/brz/git-config-branches into lp:brz

[Jelmer Vernooij]

Support reading related branches from git configs.

Support reading related branches from git configs.

[Martin Packman (gz)]

Looks good, see a couple of inline comments.

review: Approve

[The Breezy Bot (the-breezy-bot)]

Merging failed
https://ci.breezy-vcs.org/job/brz/job/brz-land/59/

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/brz/job/brz-land/64/

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/brz/job/brz-land/115/

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/brz/job/brz-land/259/

=== modified file 'breezy/git/branch.py'
--- breezy/git/branch.py	2019-01-19 17:13:53 +0000
+++ breezy/git/branch.py	2019-03-04 05:19:37 +0000
@@ -76,7 +76,10 @@
 from .unpeel_map import (
     UnpeelMap,
     )
-from .urls import git_url_to_bzr_url
+from .urls import (
+    git_url_to_bzr_url,
+    bzr_url_to_git_url,
+    )
 
 
 class GitPullResult(branch.PullResult):
@@ -464,49 +467,76 @@
         # Git doesn't do stacking (yet...)
         raise branch.UnstackableBranchFormat(self._format, self.base)
 
+    def _get_push_origin(self, cs):
+        """Get the name for the push origin.
+
+        The exact behaviour is documented in the git-config(1) manpage.
+        """
+        try:
+            return cs.get((b'branch', self.name.encode('utf-8')), b'pushRemote')
+        except KeyError:
+            try:
+                return cs.get((b'branch', ), b'remote')
+            except KeyError:
+                try:
+                    return cs.get((b'branch', self.name.encode('utf-8')), b'remote')
+                except KeyError:
+                    return b'origin'
+
+    def _get_origin(self, cs):
+        try:
+            return cs.get((b'branch', self.name.encode('utf-8')), b'remote')
+        except KeyError:
+            return b'origin'
+
+    def _get_related_push_branch(self, cs):
+        remote = self._get_push_origin(cs)
+        try:
+            location = cs.get((b"remote", remote), b"url")
+        except KeyError:
+            return None
+
+        return git_url_to_bzr_url(location.decode('utf-8'), ref=self.ref)
+
+    def _get_related_merge_branch(self, cs):
+        remote = self._get_origin(cs)
+        try:
+            location = cs.get((b"remote", remote), b"url")
+        except KeyError:
+            return None
+
+        try:
+            ref = cs.get((b"branch", remote), b"merge")
+        except KeyError:
+            ref = self.ref
+
+        return git_url_to_bzr_url(location.decode('utf-8'), ref=ref)
+
     def _get_parent_location(self):
         """See Branch.get_parent()."""
-        # FIXME: Set "origin" url from .git/config ?
         cs = self.repository._git.get_config_stack()
-        try:
-            location = cs.get((b"remote", b'origin'), b"url")
-        except KeyError:
-            return None
-
-        params = {}
-        try:
-            ref = cs.get((b"remote", b"origin"), b"merge")
-        except KeyError:
-            pass
-        else:
-            if ref != b'HEAD':
-                try:
-                    params['branch'] = urlutils.escape(ref_to_branch_name(ref))
-                except ValueError:
-                    params['ref'] = urlutils.quote_from_bytes(ref)
-
-        url = git_url_to_bzr_url(location.decode('utf-8'))
-        return urlutils.join_segment_parameters(url, params)
+        return self._get_related_merge_branch(cs)
+
+    def _write_git_config(self, cs):
+        f = BytesIO()
+        cs.write_to_file(f)
+        self.repository._git._put_named_file('config', f.getvalue())
 
     def set_parent(self, location):
-        # FIXME: Set "origin" url in .git/config ?
         cs = self.repository._git.get_config()
+        remote = self._get_origin(cs)
         this_url = urlutils.split_segment_parameters(self.user_url)[0]
-        target_url, target_params = urlutils.split_segment_parameters(location)
+        target_url, branch, ref = bzr_url_to_git_url(location)
         location = urlutils.relative_url(this_url, target_url)
-        cs.set((b"remote", b"origin"), b"url", location)
-        if 'branch' in target_params:
-            cs.set((b"remote", b"origin"), b"merge",
-                   branch_name_to_ref(target_params['branch']))
-        elif 'ref' in target_params:
-            cs.set((b"remote", b"origin"), b"merge",
-                   target_params['ref'])
+        cs.set((b"remote", remote), b"url", location)
+        if branch:
+            cs.set((b"branch", remote), b"merge", branch_name_to_ref(branch))
+        elif ref:
+            cs.set((b"branch", remote), b"merge", ref)
         else:
             # TODO(jelmer): Maybe unset rather than setting to HEAD?
-            cs.set((b"remote", b"origin"), b"merge", 'HEAD')
-        f = BytesIO()
-        cs.write_to_file(f)
-        self.repository._git._put_named_file('config', f.getvalue())
+            cs.set((b"branch", remote), b"merge", b'HEAD')
+        self._write_git_config(cs)
 
     def break_lock(self):
         raise NotImplementedError(self.break_lock)
@@ -720,7 +750,10 @@
     def get_push_location(self):
         """See Branch.get_push_location."""
         push_loc = self.get_config_stack().get('push_location')
-        return push_loc
+        if push_loc is not None:
+            return push_loc
+        cs = self.repository._git.get_config_stack()
+        return self._get_related_push_branch(cs)
 
     def set_push_location(self, location):
         """See Branch.set_push_location."""
 
=== modified file 'breezy/git/tests/test_urls.py'
--- breezy/git/tests/test_urls.py	2018-11-11 04:08:32 +0000
+++ breezy/git/tests/test_urls.py	2019-03-04 05:19:37 +0000
@@ -42,7 +42,26 @@
             ('git+ssh://user@foo/bar/path'))
 
     def test_path(self):
-        self.assertEqual(
-            git_url_to_bzr_url(
-                '/bar/path'),
-            ('/bar/path'))
+        self.assertEqual(git_url_to_bzr_url('/bar/path'), ('/bar/path'))
+
+    def test_with_ref(self):
+        self.assertEqual(
+            git_url_to_bzr_url('foo:bar/path', ref=b'HEAD'),
+            'git+ssh://foo/bar/path')
+        self.assertEqual(
+            git_url_to_bzr_url('foo:bar/path', ref=b'refs/heads/blah'),
+            'git+ssh://foo/bar/path,branch=blah')
+        self.assertEqual(
+            git_url_to_bzr_url('foo:bar/path', ref=b'refs/tags/blah'),
+            'git+ssh://foo/bar/path,ref=refs%2Ftags%2Fblah')
+
+    def test_with_branch(self):
+        self.assertEqual(
+            git_url_to_bzr_url('foo:bar/path', branch=''),
+            'git+ssh://foo/bar/path')
+        self.assertEqual(
+            git_url_to_bzr_url('foo:bar/path', branch='foo/blah'),
+            'git+ssh://foo/bar/path,branch=foo%2Fblah')
+        self.assertEqual(
+            git_url_to_bzr_url('foo:bar/path', branch='blah'),
+            'git+ssh://foo/bar/path,branch=blah')
 
=== modified file 'breezy/git/urls.py'
--- breezy/git/urls.py	2018-11-11 14:23:06 +0000
+++ breezy/git/urls.py	2019-03-04 05:19:37 +0000
@@ -18,7 +18,10 @@
 
 from __future__ import absolute_import
 
-from breezy.urlutils import URL, quote
+from .. import urlutils
+from .refs import (
+    ref_to_branch_name,
+    )
 
 from dulwich.client import parse_rsync_url
 
@@ -26,22 +29,46 @@
 KNOWN_GIT_SCHEMES = ['git+ssh', 'git', 'http', 'https', 'ftp']
 
 
-def git_url_to_bzr_url(location):
-    url = URL.from_string(location)
-    if (url.scheme not in KNOWN_GIT_SCHEMES
-            and not url.scheme.startswith('chroot-')):
+def git_url_to_bzr_url(location, branch=None, ref=None):
+    if branch is not None and ref is not None:
+        raise ValueError('only specify one of branch or ref')
+    url = urlutils.URL.from_string(location)
+    if (url.scheme not in KNOWN_GIT_SCHEMES and
+            not url.scheme.startswith('chroot-')):
         try:
             (username, host, path) = parse_rsync_url(location)
         except ValueError:
             return location
         else:
-            url = URL(
+            url = urlutils.URL(
                 scheme='git+ssh',
-                quoted_user=(quote(username) if username else None),
+                quoted_user=(urlutils.quote(username) if username else None),
                 quoted_password=None,
-                quoted_host=quote(host),
+                quoted_host=urlutils.quote(host),
                 port=None,
-                quoted_path=quote(path, safe="/~"))
-        return str(url)
-    else:
-        return location
+                quoted_path=urlutils.quote(path, safe="/~"))
+        location = str(url)
+    if ref == b'HEAD':
+        ref = branch = None
+    if ref:
+        try:
+            branch = ref_to_branch_name(ref)
+        except ValueError:
+            branch = None
+        else:
+            ref = None
+    if ref or branch:
+        params = {}
+        if ref:
+            params['ref'] = urlutils.quote_from_bytes(ref, safe='')
+        if branch:
+            params['branch'] = urlutils.escape(branch, safe='')
+        location = urlutils.join_segment_parameters(location, params)
+    return location
+
+
+def bzr_url_to_git_url(location):
+    target_url, target_params = urlutils.split_segment_parameters(location)
+    branch = target_params.get('branch')
+    ref = target_params.get('ref')
+    return target_url, branch, ref
 
=== modified file 'breezy/tests/per_branch/test_branch.py'
--- breezy/tests/per_branch/test_branch.py	2018-11-11 04:08:32 +0000
+++ breezy/tests/per_branch/test_branch.py	2019-03-04 05:19:37 +0000
@@ -164,7 +164,9 @@
 
         branch_b = wt_a.branch.controldir.sprout(
             'b', revision_id=rev1).open_branch()
-        self.assertEqual(wt_a.branch.user_url, branch_b.get_parent())
+        self.assertEqual(
+            urlutils.split_segment_parameters(wt_a.branch.user_url)[0],
+            urlutils.split_segment_parameters(branch_b.get_parent())[0])
         return branch_b
 
     def test_clone_branch_nickname(self):
 
=== modified file 'breezy/tests/per_branch/test_sprout.py'
--- breezy/tests/per_branch/test_sprout.py	2018-11-11 04:08:32 +0000
+++ breezy/tests/per_branch/test_sprout.py	2019-03-04 05:19:37 +0000
@@ -23,6 +23,7 @@
     osutils,
     revision as _mod_revision,
     tests,
+    urlutils,
     )
 from breezy.bzr import (
     branch as _mod_bzrbranch,
@@ -43,7 +44,9 @@
     def test_sprout_branch_parent(self):
         source = self.make_branch('source')
         target = source.controldir.sprout(self.get_url('target')).open_branch()
-        self.assertEqual(source.user_url, target.get_parent())
+        self.assertEqual(
+            urlutils.split_segment_parameters(source.user_url)[0],
+            urlutils.split_segment_parameters(target.get_parent())[0])
 
     def test_sprout_uses_bzrdir_branch_format(self):
         # branch.sprout(bzrdir) is defined as using the branch format selected
 
=== modified file 'breezy/urlutils.py'
--- breezy/urlutils.py	2018-11-12 01:41:38 +0000
+++ breezy/urlutils.py	2019-03-04 05:19:37 +0000
@@ -162,11 +162,11 @@
 unquote = urlparse.unquote
 
 
-def escape(relpath):
+def escape(relpath, safe='/~'):
     """Escape relpath to be a valid url."""
     if not isinstance(relpath, str) and sys.version_info[0] == 2:
         relpath = relpath.encode('utf-8')
-    return quote(relpath, safe='/~')
+    return quote(relpath, safe=safe)
 
 
 def file_relpath(base, path):