threads / patch / 33611

v2, 9 partsremote-helpers: fixes and cleanups

Subject: [PATCH v2 0/9] remote-helpers: fixes and cleanups

## tl;dr

13 messages between Apr 26, 2013 and Apr 26, 2013. Diffs are folded; open one to read it.

replies: 12people: 2as markdown or json

Felipe Contreras· Apr 26, 2013, 00:08 UTC · lore
Hi,

Same as before, minus one fix. This time they are based on 'next' so there shouldn't be any conflicts.

Here's a bunch of cleanups mostly to synchronize remote-bzr and remote-hg.
Felipe Contreras (9):
  remote-helpers: trivial cleanups
  remote-hg: remove extra check
  remote-bzr: fix bad state issue
  remote-bzr: add support to push URLs
  remote-hg: use hashlib instead of hg sha1 util
  remote-bzr: store converted URL
  remote-bzr: tell bazaar to be quiet
  remote-bzr: strip extra newline
  remote-bzr: use proper push method
 contrib/remote-helpers/git-remote-bzr | 55 ++++++++++++++++++++++++++++-------
 contrib/remote-helpers/git-remote-hg  | 10 ++-----
 2 files changed, 48 insertions(+), 17 deletions(-)
-- 
1.8.2.1.884.g3532a8d
Felipe Contreras· Apr 26, 2013, 00:08 UTC · re: Felipe Contreras · lore

[PATCH v2 1/9] remote-helpers: trivial cleanups

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
Do I have to say it? If code is removed, it's because it's not used.
 contrib/remote-helpers/git-remote-bzr | 7 ++++---
 contrib/remote-helpers/git-remote-hg  | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)
Show changes to 2 files +5 −4

contrib/remote-helpers/git-remote-bzr, contrib/remote-helpers/git-remote-hg

diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index cc6609b..87c54bf 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -94,7 +94,7 @@ class Marks:
         return self.last_mark
 
     def is_marked(self, rev):
-        return str(rev) in self.marks
+        return rev in self.marks
 
     def new_mark(self, rev, mark):
         self.marks[rev] = mark
@@ -224,7 +224,7 @@ def export_files(tree, files):
             else:
                 mode = '100644'
 
-            # is the blog already exported?
+            # is the blob already exported?
             if h in filenodes:
                 mark = filenodes[h]
                 final.append((mode, mark, path))
@@ -521,7 +521,7 @@ def c_style_unescape(string):
     return string
 
 def parse_commit(parser):
-    global marks, blob_marks, bmarks, parsed_refs
+    global marks, blob_marks, parsed_refs
     global mode
 
     parents = []
@@ -643,6 +643,7 @@ def do_export(parser):
                 wt = repo.bzrdir.open_workingtree()
                 wt.update()
         print "ok %s" % ref
+
     print
 
 def do_capabilities(parser):
diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index fda4199..cfa96c1 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -12,7 +12,7 @@
 # For remote repositories a local clone is stored in
 # "$GIT_DIR/hg/origin/clone/.hg/".
 
-from mercurial import hg, ui, bookmarks, context, util, encoding, node, error, extensions
+from mercurial import hg, ui, bookmarks, context, encoding, node, error, extensions
 
 import re
 import sys
-- 
1.8.2.1.884.g3532a8d
Felipe Contreras· Apr 26, 2013, 00:08 UTC · re: Felipe Contreras · lore

[PATCH v2 2/9] remote-hg: remove extra check

Not needed since we use xrange ourselves.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-hg | 4 ----
 1 file changed, 4 deletions(-)
Show changes to contrib/remote-helpers/git-remote-hg +0 −4
diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index cfa96c1..80b3606 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -373,10 +373,6 @@ def export_ref(repo, name, kind, head):
     ename = '%s/%s' % (kind, name)
     tip = marks.get_tip(ename)
 
-    # mercurial takes too much time checking this
-    if tip and tip == head.rev():
-        # nothing to do
-        return
     revs = xrange(tip, head.rev() + 1)
     count = 0
 
-- 
1.8.2.1.884.g3532a8d
Felipe Contreras· Apr 26, 2013, 00:08 UTC · re: Felipe Contreras · lore

[PATCH v2 3/9] remote-bzr: fix bad state issue

Carried from remote-hg.

The problem reportedly happened after doing a push that fails, the abort causes the state of remote-hg to go bad, this happens because remote-hg's marks are not stored, but 'git fast-export' marks are.

Ensure that the marks are _always_ stored.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 6 ++++++
 1 file changed, 6 insertions(+)
Show changes to contrib/remote-helpers/git-remote-bzr +6 −0
diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 87c54bf..7b6584e 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -32,6 +32,7 @@ import os
 import json
 import re
 import StringIO
+import atexit
 
 NAME_RE = re.compile('^([^<>]+)')
 AUTHOR_RE = re.compile('^([^<>]+?)? ?<([^<>]*)>$')
@@ -728,6 +729,7 @@ def main(args):
     blob_marks = {}
     parsed_refs = {}
     files_cache = {}
+    marks = None
 
     gitdir = os.environ['GIT_DIR']
     dirname = os.path.join(gitdir, 'bzr', alias)
@@ -754,6 +756,10 @@ def main(args):
             die('unhandled command: %s' % line)
         sys.stdout.flush()
 
+def bye():
+    if not marks:
+        return
     marks.store()
 
+atexit.register(bye)
 sys.exit(main(sys.argv))
-- 
1.8.2.1.884.g3532a8d
Felipe Contreras· Apr 26, 2013, 00:08 UTC · re: Felipe Contreras · lore

[PATCH v2 4/9] remote-bzr: add support to push URLs

Just like in remote-hg.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
Show changes to contrib/remote-helpers/git-remote-bzr +13 −3
diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 7b6584e..f1d6d5e 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -32,7 +32,7 @@ import os
 import json
 import re
 import StringIO
-import atexit
+import atexit, shutil, hashlib
 
 NAME_RE = re.compile('^([^<>]+)')
 AUTHOR_RE = re.compile('^([^<>]+?)? ?<([^<>]*)>$')
@@ -719,11 +719,11 @@ def main(args):
     global blob_marks
     global parsed_refs
     global files_cache
+    global is_tmp
 
     alias = args[1]
     url = args[2]
 
-    prefix = 'refs/bzr/%s' % alias
     tags = {}
     filenodes = {}
     blob_marks = {}
@@ -731,6 +731,13 @@ def main(args):
     files_cache = {}
     marks = None
 
+    if alias[5:] == url:
+        is_tmp = True
+        alias = hashlib.sha1(alias).hexdigest()
+    else:
+        is_tmp = False
+
+    prefix = 'refs/bzr/%s' % alias
     gitdir = os.environ['GIT_DIR']
     dirname = os.path.join(gitdir, 'bzr', alias)
 
@@ -759,7 +766,10 @@ def main(args):
 def bye():
     if not marks:
         return
-    marks.store()
+    if not is_tmp:
+        marks.store()
+    else:
+        shutil.rmtree(dirname)
 
 atexit.register(bye)
 sys.exit(main(sys.argv))
-- 
1.8.2.1.884.g3532a8d
Felipe Contreras· Apr 26, 2013, 00:08 UTC · re: Felipe Contreras · lore

[PATCH v2 5/9] remote-hg: use hashlib instead of hg sha1 util

To be in sync with remote-bzr.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-hg | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to contrib/remote-helpers/git-remote-hg +2 −2
diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 80b3606..06920f2 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -22,7 +22,7 @@ import shutil
 import subprocess
 import urllib
 import atexit
-import urlparse
+import urlparse, hashlib
 
 #
 # If you want to switch to hg-git compatibility mode:
@@ -933,7 +933,7 @@ def main(args):
 
     if alias[4:] == url:
         is_tmp = True
-        alias = util.sha1(alias).hexdigest()
+        alias = hashlib.sha1(alias).hexdigest()
     else:
         is_tmp = False
 
-- 
1.8.2.1.884.g3532a8d
Felipe Contreras· Apr 26, 2013, 00:08 UTC · re: Felipe Contreras · lore

[PATCH v2 6/9] remote-bzr: store converted URL

Mercurial might convert the URL to something more appropriate, like an absolute path. Lets store that instead of the original URL, which won't work from a different working directory if it's relative.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
Show changes to contrib/remote-helpers/git-remote-bzr +12 −1
diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index f1d6d5e..dda2932 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -32,7 +32,7 @@ import os
 import json
 import re
 import StringIO
-import atexit, shutil, hashlib
+import atexit, shutil, hashlib, urlparse, subprocess
 
 NAME_RE = re.compile('^([^<>]+)')
 AUTHOR_RE = re.compile('^([^<>]+?)? ?<([^<>]*)>$')
@@ -713,6 +713,14 @@ def get_repo(url, alias):
 
     return branch
 
+def fix_path(alias, orig_url):
+    url = urlparse.urlparse(orig_url, 'file')
+    if url.scheme != 'file' or os.path.isabs(url.path):
+        return
+    abs_url = urlparse.urljoin("%s/" % os.getcwd(), orig_url)
+    cmd = ['git', 'config', 'remote.%s.url' % alias, "bzr::%s" % abs_url]
+    subprocess.call(cmd)
+
 def main(args):
     global marks, prefix, dirname
     global tags, filenodes
@@ -741,6 +749,9 @@ def main(args):
     gitdir = os.environ['GIT_DIR']
     dirname = os.path.join(gitdir, 'bzr', alias)
 
+    if not is_tmp:
+        fix_path(alias, url)
+
     if not os.path.exists(dirname):
         os.makedirs(dirname)
 
-- 
1.8.2.1.884.g3532a8d
John Szakmeister· Apr 26, 2013, 00:58 UTC · re: Felipe Contreras · lore

Re: [PATCH v2 6/9] remote-bzr: store converted URL

On Thu, Apr 25, 2013 at 8:08 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 8 quoted lines
> Mercurial might convert the URL to something more appropriate, like an
> absolute path. Lets store that instead of the original URL, which won't
> work from a different working directory if it's relative.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  contrib/remote-helpers/git-remote-bzr | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)

FWIW, it feels weird to be talking about Mercurial when you're patching git-remote-bzr. :-)

-John
Felipe Contreras· Apr 26, 2013, 01:12 UTC · re: John Szakmeister · lore

Re: [PATCH v2 6/9] remote-bzr: store converted URL

On Thu, Apr 25, 2013 at 7:58 PM, John Szakmeister <john@szakmeister.net> wrote:
Show 14 quoted lines
> On Thu, Apr 25, 2013 at 8:08 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> Mercurial might convert the URL to something more appropriate, like an
>> absolute path. Lets store that instead of the original URL, which won't
>> work from a different working directory if it's relative.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>  contrib/remote-helpers/git-remote-bzr | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>
>
> FWIW, it feels weird to be talking about Mercurial when you're
> patching git-remote-bzr. :-)

Yeah, it happens all the time in my local repos. I'm surprised it took so long to send a patch with this mistake =/

-- 
Felipe Contreras
Felipe Contreras· Apr 26, 2013, 00:08 UTC · re: Felipe Contreras · lore

[PATCH v2 7/9] remote-bzr: tell bazaar to be quiet

Otherwise we get notification, progress bars, and what not.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 3 +++
 1 file changed, 3 insertions(+)
Show changes to contrib/remote-helpers/git-remote-bzr +3 −0
diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index dda2932..8617e25 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -26,6 +26,7 @@ bzrlib.plugin.load_plugins()
 import bzrlib.generate_ids
 import bzrlib.transport
 import bzrlib.errors
+import bzrlib.ui
 
 import sys
 import os
@@ -755,6 +756,8 @@ def main(args):
     if not os.path.exists(dirname):
         os.makedirs(dirname)
 
+    bzrlib.ui.ui_factory.be_quiet(True)
+
     repo = get_repo(url, alias)
 
     marks_path = os.path.join(dirname, 'marks-int')
-- 
1.8.2.1.884.g3532a8d
John Szakmeister· Apr 26, 2013, 00:59 UTC · re: Felipe Contreras · lore

Re: [PATCH v2 7/9] remote-bzr: tell bazaar to be quiet

On Thu, Apr 25, 2013 at 8:08 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 25 quoted lines
> Otherwise we get notification, progress bars, and what not.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  contrib/remote-helpers/git-remote-bzr | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
> index dda2932..8617e25 100755
> --- a/contrib/remote-helpers/git-remote-bzr
> +++ b/contrib/remote-helpers/git-remote-bzr
> @@ -26,6 +26,7 @@ bzrlib.plugin.load_plugins()
>  import bzrlib.generate_ids
>  import bzrlib.transport
>  import bzrlib.errors
> +import bzrlib.ui
>
>  import sys
>  import os
> @@ -755,6 +756,8 @@ def main(args):
>      if not os.path.exists(dirname):
>          os.makedirs(dirname)
>
> +    bzrlib.ui.ui_factory.be_quiet(True)
> +
Nice change!
-John
Felipe Contreras· Apr 26, 2013, 00:08 UTC · re: Felipe Contreras · lore

[PATCH v2 8/9] remote-bzr: strip extra newline

It's added by fast-export, the user didn't type it.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 4 ++++
 1 file changed, 4 insertions(+)
Show changes to contrib/remote-helpers/git-remote-bzr +4 −0
diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 8617e25..9f56297 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -549,6 +549,10 @@ def parse_commit(parser):
         parents.append(parser.get_mark())
         parser.next()
 
+    # fast-export adds an extra newline
+    if data[-1] == '\n':
+        data = data[:-1]
+
     files = {}
 
     for line in parser:
-- 
1.8.2.1.884.g3532a8d
Felipe Contreras· Apr 26, 2013, 00:08 UTC · re: Felipe Contreras · lore

[PATCH v2 9/9] remote-bzr: use proper push method

Do not just randomly synchronize the revisions with no checks at all.

I don't have any evidence that there's anything wrong with the current code, which Bazaar seems to use, but for different purposes. Let's use the logic Bazaar UI uses to avoid surprises.

Also, add a non-ff check.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
Show changes to contrib/remote-helpers/git-remote-bzr +5 −5
diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 9f56297..c19ed0e 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -639,12 +639,12 @@ def do_export(parser):
     for ref, revid in parsed_refs.iteritems():
         if ref == 'refs/heads/master':
             repo.generate_revision_history(revid, marks.get_tip('master'))
-            revno, revid = repo.last_revision_info()
             if peer:
-                if hasattr(peer, "import_last_revision_info_and_tags"):
-                    peer.import_last_revision_info_and_tags(repo, revno, revid)
-                else:
-                    peer.import_last_revision_info(repo.repository, revno, revid)
+                try:
+                    repo.push(peer, stop_revision=revid)
+                except bzrlib.errors.DivergedBranches:
+                    print "error %s non-fast forward" % ref
+                    continue
             else:
                 wt = repo.bzrdir.open_workingtree()
                 wt.update()
-- 
1.8.2.1.884.g3532a8d

← back to recent threads