threads / patch / 34814

patchremote-hg: skip ill-formed references

Subject: [PATCH] remote-hg: skip ill-formed references

## tl;dr

6 messages between Aug 31, 2013 and Aug 31, 2013. Diffs are folded; open one to read it.

replies: 5people: 2as markdown or json

Max Kirillov· Aug 31, 2013, 01:15 UTC · lore

References which fail check_refname_format() cause the whole import to fail. This might be undesirable if the references are not important.

A better solution would be to provide some mapping, either by some reversible encoding, or by generating and storing the associations locally.

But this is already going to allow working with many
existing repositories.
---
If there is no smarter solution ongoing maybe this could be
useful.
 contrib/remote-helpers/git-remote-hg | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)
Show changes to contrib/remote-helpers/git-remote-hg +14 −3
diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 0194c67..e32003b 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -591,6 +591,17 @@ def list_head(repo, cur):
     print "@refs/heads/%s HEAD" % head
     g_head = (head, node)
 
+def print_list_entry_if_valid(ref):
+    # same checks as in check_refname_format() in refs.c
+    if ref.startswith('.') or ref.find('/.') != -1 or \
+            ref.find('..') != -1 or \
+            any([(c <= ' ' or c in '~^:\\\177') for c in ref]) or \
+            ref.endswith('/') or \
+            ref.endswith('.lock'):
+        warn("Ill-named reference '%s' skipped" % ref)
+    else:
+        print "? %s" % ref
+
 def do_list(parser):
     global branches, bmarks, track_branches
 
@@ -611,15 +622,15 @@ def do_list(parser):
 
     if track_branches:
         for branch in branches:
-            print "? refs/heads/branches/%s" % gitref(branch)
+            print_list_entry_if_valid("refs/heads/branches/%s" % gitref(branch))
 
     for bmark in bmarks:
-        print "? refs/heads/%s" % gitref(bmark)
+        print_list_entry_if_valid("refs/heads/%s" % gitref(bmark))
 
     for tag, node in repo.tagslist():
         if tag == 'tip':
             continue
-        print "? refs/tags/%s" % gitref(tag)
+        print_list_entry_if_valid("refs/tags/%s" % gitref(tag))
 
     print
 

-- 
1.8.4.rc3.902.g80a4b9e
Felipe Contreras· Aug 31, 2013, 01:39 UTC · re: Max Kirillov · lore

Re: [PATCH] remote-hg: skip ill-formed references

On Fri, Aug 30, 2013 at 8:15 PM, Max Kirillov <max@max630.net> wrote:
Show 10 quoted lines
> References which fail check_refname_format() cause the whole
> import to fail. This might be undesirable if the references
> are not important.
>
> A better solution would be to provide some mapping, either
> by some reversible encoding, or by generating and storing
> the associations locally.
>
> But this is already going to allow working with many
> existing repositories.
Which repository triggered this?
Maybe we should do something similar as in git-remote-bzr:
def ref_is_valid(name):
    return not True in [c in name for c in '~^: \\']
        if not ref_is_valid(tag):
            continue
        print "? refs/tags/%s" % tag
-- 
Felipe Contreras
Max Kirillov· Aug 31, 2013, 13:58 UTC · re: Felipe Contreras · lore

Re: [PATCH] remote-hg: skip ill-formed references

Felipe Contreras <felipe.contreras <at> gmail.com> writes:

> Which repository triggered this?

Tha was some of the vim repositories, upstream https://code.google.com/p/vim/ or debian anonscm.debian.org/hg/pkg-vim/vim, or both. They contain tags with ~ symbol.

I don't have any experience with bazaar yet, so cannot say much about it.

Br,
-- 
Max
Felipe Contreras· Aug 31, 2013, 17:32 UTC · re: Max Kirillov · lore

Re: [PATCH] remote-hg: skip ill-formed references

On Sat, Aug 31, 2013 at 8:58 AM, Max Kirillov <max@max630.net> wrote:
Show 8 quoted lines
> Felipe Contreras <felipe.contreras <at>
> gmail.com> writes:
>> Which repository triggered this?
>
> Tha was some of the vim repositories, upstream
> https://code.google.com/p/vim/ or debian
> anonscm.debian.org/hg/pkg-vim/vim, or both.
> They contain tags with ~ symbol.
Thanks.
> I don't have any experience with bazaar yet, so
> cannot say much about it.

That code is independent from bazaar. the ref_is_valid() method should return True or False depending on whether Git thinks it's a valid ref or not, it's independent from Bazaar or Mercurial, and all remote-helpers could share the same one.

So this is what I have in mind:
Show changes to git-remote-hg.py +15 −3
--- a/git-remote-hg.py
+++ b/git-remote-hg.py
@@ -617,6 +617,9 @@ def list_head(repo, cur):
     print "@refs/heads/%s HEAD" % head
     g_head = (head, node)

+def ref_is_valid(name):
+    return not True in [c in name for c in '~^: \\']
+
 def do_list(parser):
     repo = parser.repo
     for bmark, node in bookmarks.listbookmarks(repo).iteritems():
@@ -635,15 +638,24 @@ def do_list(parser):

     if track_branches:
         for branch in branches:
-            print "? refs/heads/branches/%s" % gitref(branch)
+            branch = gitref(branch)
+            if not ref_is_valid(branch):
+                continue
+            print "? refs/heads/branches/%s" % branch

     for bmark in bmarks:
-        print "? refs/heads/%s" % gitref(bmark)
+        bmark = gitref(bmark)
+        if not ref_is_valid(bmark):
+            continue
+        print "? refs/heads/%s" % bmark

     for tag, node in repo.tagslist():
         if tag == 'tip':
             continue
-        print "? refs/tags/%s" % gitref(tag)
+        tag = gitref(tag)
+        if not ref_is_valid(tag):
+            continue
+        print "? refs/tags/%s" % tag

     print

-- 
Felipe Contreras
Felipe Contreras· Aug 31, 2013, 17:57 UTC · re: Max Kirillov · lore

Re: [PATCH] remote-hg: skip ill-formed references

On Sat, Aug 31, 2013 at 8:58 AM, Max Kirillov <max@max630.net> wrote:
Show 8 quoted lines
> Felipe Contreras <felipe.contreras <at>
> gmail.com> writes:
>> Which repository triggered this?
>
> Tha was some of the vim repositories, upstream
> https://code.google.com/p/vim/ or debian
> anonscm.debian.org/hg/pkg-vim/vim, or both.
> They contain tags with ~ symbol.
I can clone both fine. This is what I get with the debian one:

error: * Ignoring funny ref 'refs/tags/debian-7.2.436+hg~e12b9d992389-1' locally error: * Ignoring funny ref 'refs/tags/debian-7.2.436+hg~e12b9d992389-1+' locally error: * Ignoring funny ref 'refs/tags/debian-7.2.438+hg~d44112feb815-1' locally error: * Ignoring funny ref 'refs/tags/debian-7.2.438+hg~d44112feb815-2' locally error: * Ignoring funny ref 'refs/tags/debian-7.2.438+hg~d44112feb815-3' locally error: * Ignoring funny ref 'refs/tags/debian-7.2.438+hg~d44112feb815-4' locally error: * Ignoring funny ref 'refs/tags/debian-7.2.438+hg~d44112feb815-5' locally error: * Ignoring funny ref 'refs/tags/debian-7.2.445+hg~cb94c42c0e1a-1' locally error: * Ignoring funny ref 'refs/tags/debian-7.3b.20100720+hg~7b7508ee56f1-1' locally error: * Ignoring funny ref 'refs/tags/debian-7.3f.20100812+hg~20e83abf88b1-1' locally error: * Ignoring funny ref 'refs/tags/debian-7.3.000+hg~ee53a39d5896-1' locally error: * Ignoring funny ref 'refs/tags/debian-7.3.035+hg~8fdc12103333-1' locally error: * Ignoring funny ref 'refs/tags/debian-7.3.154+hg~74503f6ee649-1' locally error: * Ignoring funny ref 'refs/tags/debian-7.3.154+hg~74503f6ee649-2' locally error: * Ignoring funny ref 'refs/tags/debian-7.3.547-6~bpo60+1' locally error: * Ignoring funny ref 'refs/tags/debian-7.3.547-7~bpo60+1' locally

Maybe you need a newer version of Git. I'm using v1.8.4.
-- 
Felipe Contreras
Max Kirillov· Aug 31, 2013, 19:51 UTC · re: Felipe Contreras · lore

Re: [PATCH] remote-hg: skip ill-formed references

On Sat, Aug 31, 2013 at 12:57:34PM -0500, Felipe Contreras wrote:
Show 9 quoted lines
> On Sat, Aug 31, 2013 at 8:58 AM, Max Kirillov <max@max630.net> wrote:
>> Tha was some of the vim repositories, upstream
>> https://code.google.com/p/vim/ or debian
>> anonscm.debian.org/hg/pkg-vim/vim, or both.
>> They contain tags with ~ symbol.
> 
> I can clone both fine. This is what I get with the debian one:
> 
> error: * Ignoring funny ref 'refs/tags/debian-7.2.436+hg~e12b9d992389-1' locally

Yes, it really works with the new version. I used 1.7.10 before.

Since it is fixed already, just forget it. Sorry, should have checked it with the latest version.

-- 
Max

← back to recent threads