[RFC/PATH 0/4] remoe-hg: switch to SHA-1 ids

[RFC/PATH 0/4] remoe-hg: switch to SHA-1 ids

[Felipe Contreras]

Hi,

As some people suggested this is necessary for some use-cases, because revision
id numbers can change and point to different revisions.

Seems to work fine, but I wouldn't merge it just yet.

Felipe Contreras (4):
  remote-hg: shuffle some code
  remote-hg: improve node traversing
  remote-hg: add version checks to the marks
  remote-hg: switch from revisions to SHA-1 noteids

 contrib/remote-helpers/git-remote-hg | 77 ++++++++++++++++++++++--------------
 1 file changed, 48 insertions(+), 29 deletions(-)

-- 
1.8.2

[RFC/PATH 1/4] remote-hg: shuffle some code

[Felipe Contreras]

In preparation to shift to SHA-1's.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-hg | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index d82eb2d..a8591a2 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -63,6 +63,9 @@ def hgmode(mode):
 def hghex(node):
     return hg.node.hex(node)
 
+def hgbin(node):
+    return hg.node.bin(node)
+
 def get_config(config):
     cmd = ['git', 'config', '--get', config]
     process = subprocess.Popen(cmd, stdout=subprocess.PIPE)
@@ -207,7 +210,7 @@ def get_filechanges(repo, ctx, parent):
     removed = set()
 
     cur = ctx.manifest()
-    prev = repo[parent].manifest().copy()
+    prev = parent.manifest().copy()
 
     for fn in cur:
         if fn in prev:
@@ -326,7 +329,7 @@ def export_ref(repo, name, kind, head):
         else:
             committer = author
 
-        parents = [p for p in repo.changelog.parentrevs(rev) if p >= 0]
+        parents = [repo[p] for p in repo.changelog.parentrevs(rev) if p >= 0]
 
         if len(parents) == 0:
             modified = c.manifest().keys()
@@ -372,9 +375,9 @@ def export_ref(repo, name, kind, head):
         print desc
 
         if len(parents) > 0:
-            print "from :%s" % (rev_to_mark(parents[0]))
+            print "from :%s" % (rev_to_mark(parents[0].rev()))
             if len(parents) > 1:
-                print "merge :%s" % (rev_to_mark(parents[1]))
+                print "merge :%s" % (rev_to_mark(parents[1].rev()))
 
         for f in modified:
             export_file(c.filectx(f))
@@ -389,10 +392,10 @@ def export_ref(repo, name, kind, head):
 
     # make sure the ref is updated
     print "reset %s/%s" % (prefix, ename)
-    print "from :%u" % rev_to_mark(rev)
+    print "from :%u" % rev_to_mark(head.rev())
     print
 
-    marks.set_tip(ename, rev)
+    marks.set_tip(ename, head.rev())
 
 def export_tag(repo, tag):
     export_ref(repo, tag, 'tags', repo[tag])
@@ -651,7 +654,7 @@ def parse_commit(parser):
     tmp = encoding.encoding
     encoding.encoding = 'utf-8'
 
-    node = repo.commitctx(ctx)
+    node = hghex(repo.commitctx(ctx))
 
     encoding.encoding = tmp
 
@@ -675,7 +678,7 @@ def parse_reset(parser):
     parser.next()
 
     node = parser.repo.changelog.node(mark_to_rev(from_mark))
-    parsed_refs[ref] = node
+    parsed_refs[ref] = hghex(node)
 
 def parse_tag(parser):
     name = parser[1]
@@ -720,10 +723,10 @@ def do_export(parser):
         elif ref.startswith('refs/tags/'):
             tag = ref[len('refs/tags/'):]
             if mode == 'git':
-                msg = 'Added tag %s for changeset %s' % (tag, hghex(node[:6]));
-                parser.repo.tag([tag], node, msg, False, None, {})
+                msg = 'Added tag %s for changeset %s' % (tag, node[:6]);
+                parser.repo.tag([tag], hgbin(node), msg, False, None, {})
             else:
-                parser.repo.tag([tag], node, None, True, None, {})
+                parser.repo.tag([tag], hgbin(node), None, True, None, {})
             print "ok %s" % ref
         else:
             # transport-helper/fast-export bugs
@@ -735,7 +738,7 @@ def do_export(parser):
     # handle bookmarks
     for bmark, node in p_bmarks:
         ref = 'refs/heads' + bmark
-        new = hghex(node)
+        new = node
 
         if bmark in bmarks:
             old = bmarks[bmark].hex()
-- 
1.8.2

[RFC/PATH 2/4] remote-hg: improve node traversing

[Felipe Contreras]

We won't be able to count the unmarked commits, but we are not going to
be able to do that anyway when we switch to SHA-1 ids.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-hg | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index a8591a2..02fda2d 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -314,12 +314,16 @@ def export_ref(repo, name, kind, head):
     revs = xrange(tip, head.rev() + 1)
     count = 0
 
-    revs = [rev for rev in revs if not marks.is_marked(rev)]
-
     for rev in revs:
 
         c = repo[rev]
-        (manifest, user, (time, tz), files, desc, extra) = repo.changelog.read(c.node())
+        node = c.node()
+
+        if marks.is_marked(c.hex()):
+            count += 1
+            continue
+
+        (manifest, user, (time, tz), files, desc, extra) = repo.changelog.read(node)
         rev_branch = extra['branch']
 
         author = "%s %d %s" % (fixup_user(user), time, gittz(tz))
-- 
1.8.2

[RFC/PATH 3/4] remote-hg: add version checks to the marks

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-hg | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 02fda2d..9e124e1 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -43,6 +43,8 @@ AUTHOR_RE = re.compile('^([^<>]+?)? ?<([^<>]*)>$')
 AUTHOR_HG_RE = re.compile('^(.*?) ?<(.*?)(?:>(.+)?)?$')
 RAW_AUTHOR_RE = re.compile('^(\w+) (?:(.+)? )?<(.*)> (\d+) ([+-]\d+)')
 
+VERSION = 1
+
 def die(msg, *args):
     sys.stderr.write('ERROR: %s\n' % (msg % args))
     sys.exit(1)
@@ -76,12 +78,19 @@ class Marks:
 
     def __init__(self, path):
         self.path = path
+        self.clear()
+        self.load()
+
+        if self.version < VERSION:
+            self.clear()
+            self.version = VERSION
+
+    def clear(self):
         self.tips = {}
         self.marks = {}
         self.rev_marks = {}
         self.last_mark = 0
-
-        self.load()
+        self.version = 0
 
     def load(self):
         if not os.path.exists(self.path):
@@ -92,12 +101,13 @@ class Marks:
         self.tips = tmp['tips']
         self.marks = tmp['marks']
         self.last_mark = tmp['last-mark']
+        self.version = tmp.get('version', 1)
 
         for rev, mark in self.marks.iteritems():
             self.rev_marks[mark] = int(rev)
 
     def dict(self):
-        return { 'tips': self.tips, 'marks': self.marks, 'last-mark' : self.last_mark }
+        return { 'tips': self.tips, 'marks': self.marks, 'last-mark' : self.last_mark, 'version' : self.version }
 
     def store(self):
         json.dump(self.dict(), open(self.path, 'w'))
-- 
1.8.2

[RFC/PATH 4/4] remote-hg: switch from revisions to SHA-1 noteids

[Felipe Contreras]

Otherwise we won't know if revisions are replaced.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-hg | 40 +++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 9e124e1..162dabc 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -43,7 +43,7 @@ AUTHOR_RE = re.compile('^([^<>]+?)? ?<([^<>]*)>$')
 AUTHOR_HG_RE = re.compile('^(.*?) ?<(.*?)(?:>(.+)?)?$')
 RAW_AUTHOR_RE = re.compile('^(\w+) (?:(.+)? )?<(.*)> (\d+) ([+-]\d+)')
 
-VERSION = 1
+VERSION = 2
 
 def die(msg, *args):
     sys.stderr.write('ERROR: %s\n' % (msg % args))
@@ -104,7 +104,7 @@ class Marks:
         self.version = tmp.get('version', 1)
 
         for rev, mark in self.marks.iteritems():
-            self.rev_marks[mark] = int(rev)
+            self.rev_marks[mark] = rev
 
     def dict(self):
         return { 'tips': self.tips, 'marks': self.marks, 'last-mark' : self.last_mark, 'version' : self.version }
@@ -116,23 +116,23 @@ class Marks:
         return str(self.dict())
 
     def from_rev(self, rev):
-        return self.marks[str(rev)]
+        return self.marks[rev]
 
     def to_rev(self, mark):
         return self.rev_marks[mark]
 
     def get_mark(self, rev):
         self.last_mark += 1
-        self.marks[str(rev)] = self.last_mark
+        self.marks[rev] = self.last_mark
         return self.last_mark
 
     def new_mark(self, rev, mark):
-        self.marks[str(rev)] = mark
+        self.marks[rev] = mark
         self.rev_marks[mark] = rev
         self.last_mark = mark
 
     def is_marked(self, rev):
-        return self.marks.has_key(str(rev))
+        return self.marks.has_key(rev)
 
     def get_tip(self, branch):
         return self.tips.get(branch, 0)
@@ -305,7 +305,7 @@ def get_repo(url, alias):
 
 def rev_to_mark(rev):
     global marks
-    return marks.from_rev(rev)
+    return marks.from_rev(rev.hex())
 
 def mark_to_rev(mark):
     global marks
@@ -316,6 +316,10 @@ def export_ref(repo, name, kind, head):
 
     ename = '%s/%s' % (kind, name)
     tip = marks.get_tip(ename)
+    if tip in repo:
+        tip = repo[tip].rev()
+    else:
+        tip = 0
 
     # mercurial takes too much time checking this
     if tip and tip == head.rev():
@@ -382,16 +386,16 @@ def export_ref(repo, name, kind, head):
             print 'reset %s/%s' % (prefix, ename)
 
         print "commit %s/%s" % (prefix, ename)
-        print "mark :%d" % (marks.get_mark(rev))
+        print "mark :%d" % (marks.get_mark(c.hex()))
         print "author %s" % (author)
         print "committer %s" % (committer)
         print "data %d" % (len(desc))
         print desc
 
         if len(parents) > 0:
-            print "from :%s" % (rev_to_mark(parents[0].rev()))
+            print "from :%s" % (rev_to_mark(parents[0]))
             if len(parents) > 1:
-                print "merge :%s" % (rev_to_mark(parents[1].rev()))
+                print "merge :%s" % (rev_to_mark(parents[1]))
 
         for f in modified:
             export_file(c.filectx(f))
@@ -406,10 +410,10 @@ def export_ref(repo, name, kind, head):
 
     # make sure the ref is updated
     print "reset %s/%s" % (prefix, ename)
-    print "from :%u" % rev_to_mark(head.rev())
+    print "from :%u" % rev_to_mark(head)
     print
 
-    marks.set_tip(ename, head.rev())
+    marks.set_tip(ename, head.hex())
 
 def export_tag(repo, tag):
     export_ref(repo, tag, 'tags', repo[tag])
@@ -626,12 +630,12 @@ def parse_commit(parser):
         extra['committer'] = "%s %u %u" % committer
 
     if from_mark:
-        p1 = repo.changelog.node(mark_to_rev(from_mark))
+        p1 = mark_to_rev(from_mark)
     else:
         p1 = '\0' * 20
 
     if merge_mark:
-        p2 = repo.changelog.node(mark_to_rev(merge_mark))
+        p2 = mark_to_rev(merge_mark)
     else:
         p2 = '\0' * 20
 
@@ -672,10 +676,8 @@ def parse_commit(parser):
 
     encoding.encoding = tmp
 
-    rev = repo[node].rev()
-
     parsed_refs[ref] = node
-    marks.new_mark(rev, commit_mark)
+    marks.new_mark(node, commit_mark)
 
 def parse_reset(parser):
     global parsed_refs
@@ -691,8 +693,8 @@ def parse_reset(parser):
     from_mark = parser.get_mark()
     parser.next()
 
-    node = parser.repo.changelog.node(mark_to_rev(from_mark))
-    parsed_refs[ref] = hghex(node)
+    rev = mark_to_rev(from_mark)
+    parsed_refs[ref] = rev
 
 def parse_tag(parser):
     name = parser[1]
-- 
1.8.2

Re: [RFC/PATH 3/4] remote-hg: add version checks to the marks

[Jed Brown]

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

> @@ -76,12 +78,19 @@ class Marks:
>  
>      def __init__(self, path):
>          self.path = path
> +        self.clear()
> +        self.load()
> +
> +        if self.version < VERSION:
> +            self.clear()

It's friendlier to just upgrade the marks in-place. This takes less than
one second to run on repositories where full re-import would take half
an hour:

    def upgrade_marks(self, hgrepo):
        if self.marks_version == 1: # Convert from integer reversions to hgsha1
            warn("Upgrading marks-hg from hg sequence number to SHA1")
            self.marks_to_revisions = dict(
                (mark, hghex(hgrepo.changelog.node(int(rev))))
                for mark, rev in self.marks_to_revisions.iteritems())
            self.revisions_to_marks = dict(
                (hghex(hgrepo.changelog.node(int(rev))), mark)
                for rev, mark in self.revisions_to_marks.iteritems())
            self.marks_version = 2
            warn("Upgrade complete")

https://github.com/buchuki/gitifyhg/commit/23a6709efd14f3e058e3a846624b7677d1e8b497#L0R195

Re: [RFC/PATH 3/4] remote-hg: add version checks to the marks

[Felipe Contreras]

On Fri, Apr 5, 2013 at 6:46 AM, Jed Brown <jed@59a2.org> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> @@ -76,12 +78,19 @@ class Marks:
>>
>>      def __init__(self, path):
>>          self.path = path
>> +        self.clear()
>> +        self.load()
>> +
>> +        if self.version < VERSION:
>> +            self.clear()
>
> It's friendlier to just upgrade the marks in-place. This takes less than
> one second to run on repositories where full re-import would take half
> an hour:

Yeah, but that's riskier, and only works for this particular version.
Besides, it only happens one time per repository at best. Also, it
would mess up the current organization of the code. I'm not sure it's
worth worrying about it at this point, and we could always add a patch
on top.

-- 
Felipe Contreras