git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] Add new git-remote-hd helper
@ 2012-10-17 12:58 Felipe Contreras
  2012-10-17 16:03 ` Johannes Schindelin
                   ` (2 more replies)
  0 siblings, 3 replies; 27+ messages in thread
From: Felipe Contreras @ 2012-10-17 12:58 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow, Felipe Contreras

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

I've looked at many hg<->git tools and none satisfy me. Too complicated, or too
slow, or to difficult to setup, etc.

The only one I've liked so far is hg-fast-export[1], which is indeed fast,
relatively simple, and relatively easy to use. But it's not properly maintained
any more.

So, I decided to write my own from scratch, using hg-fast-export as
inspiration, and voila.

This one doesn't have any dependencies, just put it into your $PATH, and you
can clone and fetch hg repositories. More importantly to me; the code is
simple, and easy to maintain.

[1] http://repo.or.cz/w/fast-export.git

 contrib/remote-hd/git-remote-hg | 231 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 231 insertions(+)
 create mode 100755 contrib/remote-hd/git-remote-hg

diff --git a/contrib/remote-hd/git-remote-hg b/contrib/remote-hd/git-remote-hg
new file mode 100755
index 0000000..9157b30
--- /dev/null
+++ b/contrib/remote-hd/git-remote-hg
@@ -0,0 +1,231 @@
+#!/usr/bin/python2
+
+# Inspired by Rocco Rutte's hg-fast-export
+
+# Just copy to your ~/bin, or anywhere in your $PATH.
+# Then you can clone with:
+# hg::file:///path/to/mercurial/repo/
+
+from mercurial import hg, ui
+
+import re
+import sys
+import os
+import json
+
+def die(msg, *args):
+    print >> sys.stderr, 'ERROR:', msg % args
+    sys.exit(1)
+
+def gitmode(flags):
+    return 'l' in flags and '120000' or 'x' in flags and '100755' or '100644'
+
+def export_file(fc):
+    if fc.path() == '.hgtags':
+        return
+    d = fc.data()
+    print "M %s inline %s" % (gitmode(fc.flags()), fc.path())
+    print "data %d" % len(d)
+    print d
+
+def get_filechanges(repo, ctx, parents):
+    l = [repo.status(p, ctx)[:3] for p in parents]
+    changed, added, removed = [sum(e, []) for e in zip(*l)]
+    return added + changed, removed
+
+author_re = re.compile('^((.+?) )?(<.+?>)$')
+
+def fixup_user(user):
+    user = user.replace('"', '')
+    m = author_re.match(user)
+    if m:
+        name = m.group(1)
+        mail = m.group(3)
+    else:
+        name = user
+        mail = None
+
+    if not name:
+        name = 'Unknown'
+    if not mail:
+        mail = '<unknown>'
+
+    return '%s %s' % (name, mail)
+
+def get_repo(path, alias):
+    myui = ui.ui()
+    myui.setconfig('ui', 'interactive', 'off')
+    repo = hg.repository(myui, path)
+    return repo
+
+def hg_branch(b):
+    if b == 'master':
+        return 'default'
+    return b
+
+def git_branch(b):
+    if b == 'default':
+        return 'master'
+    return b
+
+def export_tag(repo, tag):
+    global prefix
+    print "reset %s/tags/%s" % (prefix, tag)
+    print "from :%s" % (repo[tag].rev() + 1)
+    print
+
+def export_branch(repo, branch):
+    global prefix, marks, cache, branches
+
+    heads = branches[hg_branch(branch)]
+
+    # verify there's only one head
+    if (len(heads) > 1):
+        die("Branch '%s' has more than one head" % hg_branch(branch))
+
+    head = repo[heads[0]]
+    tip = marks.get(branch, 0)
+    # mercurial takes too much time checking this
+    if tip == head.rev():
+        # nothing to do
+        return
+    revs = repo.revs('%u:%u' % (tip, head))
+    count = 0
+
+    revs = [rev for rev in revs if not cache.get(rev, False)]
+
+    for rev in revs:
+
+        c = repo[rev]
+        (manifest, user, (time, tz), files, desc, extra) = repo.changelog.read(c.node())
+        rev_branch = git_branch(extra['branch'])
+
+        tz = '%+03d%02d' % (-tz / 3600, -tz % 3600 / 60)
+
+        print "commit %s/branches/%s" % (prefix, rev_branch)
+        print "mark :%d" % (rev + 1)
+        print "committer %s %d %s" % (fixup_user(user), time, tz)
+        print "data %d" % (len(desc) + 1)
+        print desc
+        print
+
+        parents = [p for p in repo.changelog.parentrevs(rev) if p >= 0]
+
+        if len(parents) == 0:
+            modified = c.manifest().keys()
+            removed = []
+        else:
+            added = []
+            changed = []
+            print "from :%s" % (parents[0] + 1)
+            if len(parents) > 1:
+                print "merge :%s" % (parents[1] + 1)
+            modified, removed = get_filechanges(repo, c, parents)
+
+        for f in removed:
+            print "D %s" % (f)
+        for f in modified:
+            export_file(c.filectx(f))
+        print
+
+        count += 1
+        if (count % 100 == 0):
+            print "progress revision %d '%s' (%d/%d)" % (rev, branch, count, len(revs))
+            print "#############################################################"
+
+        cache[rev] = True
+
+    # store the latest revision
+    marks[branch] = rev
+
+def do_capabilities(repo, args):
+    global prefix, dirname
+
+    print "import"
+    print "refspec refs/heads/*:%s/branches/*" % prefix
+    print "refspec refs/tags/*:%s/tags/*" % prefix
+
+    path = os.path.join(dirname, 'marks-git')
+
+    print "*export-marks %s" % path
+    if os.path.exists(path):
+        print "*import-marks %s" % path
+
+    print
+
+def do_list(repo, args):
+    global branches
+
+    head = repo.dirstate.branch()
+    for branch in repo.branchmap():
+        heads = repo.branchheads(branch)
+        if len(heads):
+            branches[branch] = heads
+
+    for branch in branches:
+        print "? refs/heads/%s" % git_branch(branch)
+    for tag, node in repo.tagslist():
+        if tag == 'tip':
+            continue
+        print "? refs/tags/%s" % tag
+    print "@refs/heads/%s HEAD" % git_branch(head)
+    print
+
+def do_import(repo, args):
+    ref = args[1]
+
+    if (ref == 'HEAD'):
+        return
+
+    if ref.startswith('refs/heads/'):
+        branch = ref[len('refs/heads/'):]
+        export_branch(repo, branch)
+    elif ref.startswith('refs/tags/'):
+        tag = ref[len('refs/tags/'):]
+        export_tag(repo, tag)
+
+def main(args):
+    global prefix, dirname, marks, cache, branches
+
+    alias = args[1]
+    url = args[2]
+
+    gitdir = os.environ['GIT_DIR']
+    dirname = os.path.join(gitdir, 'hg')
+    cache = {}
+    branches = {}
+
+    repo = get_repo(url, alias)
+    prefix = 'refs/hg/%s' % alias
+
+    if not os.path.exists(dirname):
+        os.makedirs(dirname)
+
+    marks_path = os.path.join(dirname, 'marks-hg')
+    try:
+        fp = open(marks_path, 'r')
+        marks = json.load(fp)
+        fp.close()
+    except IOError:
+        marks = {}
+
+    line = True
+    while (line):
+        line = sys.stdin.readline().strip()
+        if line == '':
+            break
+        args = line.split()
+        cmd = args[0]
+        if cmd == 'capabilities':
+            do_capabilities(repo, args)
+        elif cmd == 'list':
+            do_list(repo, args)
+        elif cmd == 'import':
+            do_import(repo, args)
+        sys.stdout.flush()
+
+    fp = open(marks_path, 'w')
+    json.dump(marks, fp)
+    fp.close()
+
+sys.exit(main(sys.argv))
-- 
1.8.0.rc2.5.gccf4c94

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-17 12:58 [PATCH] Add new git-remote-hd helper Felipe Contreras
@ 2012-10-17 16:03 ` Johannes Schindelin
  2012-10-17 16:38   ` Felipe Contreras
  2012-10-17 22:59 ` Jeff King
  2012-10-18 13:18 ` Michael J Gruber
  2 siblings, 1 reply; 27+ messages in thread
From: Johannes Schindelin @ 2012-10-17 16:03 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Junio C Hamano, Sverre Rabbelier, Ilari Liusvaara,
	Daniel Barkalow

Hi,

On Wed, 17 Oct 2012, Felipe Contreras wrote:

> I've looked at many hg<->git tools and none satisfy me. Too complicated,
> or too slow, or to difficult to setup, etc.

The one I merged into Git for Windows (since that is what I install on all
my machines even if they run Linux) is rock-solid. It also comes with
tests. And it requires a fix I tried to get into git.git (but failed,
since I was asked to do much more in addition to what I needed for myself,
and I lack the time to address such requests these days).

So I have to admit that I do not quite see the point of avoiding to
enhance the existing work of Sverre (and a little bit of me, too, in a
hackathon for which I traveled half the continent back in July 2011).

Ciao,
Johannes

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-17 16:03 ` Johannes Schindelin
@ 2012-10-17 16:38   ` Felipe Contreras
  2012-10-17 17:39     ` Johannes Schindelin
  0 siblings, 1 reply; 27+ messages in thread
From: Felipe Contreras @ 2012-10-17 16:38 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: git, Junio C Hamano, Sverre Rabbelier, Ilari Liusvaara,
	Daniel Barkalow

On Wed, Oct 17, 2012 at 6:03 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Wed, 17 Oct 2012, Felipe Contreras wrote:
>
>> I've looked at many hg<->git tools and none satisfy me. Too complicated,
>> or too slow, or to difficult to setup, etc.
>
> The one I merged into Git for Windows (since that is what I install on all
> my machines even if they run Linux) is rock-solid. It also comes with
> tests. And it requires a fix I tried to get into git.git (but failed,
> since I was asked to do much more in addition to what I needed for myself,
> and I lack the time to address such requests these days).

Maybe, but who uses it? It's quite a lot of code, and it's quite
difficult to setup--you would need a non-vanilla version of git.

Compare this:
32 files changed, 3351 insertions(+), 289 deletions(-)

To this:
1 file changed, 231 insertions(+)

I would like to first get something that works in, and then step by
step work on top of that.

Anyway, I'm not even sure which version you are talking about, because
there's plenty out there:
https://github.com/SRabbelier/git/network

> So I have to admit that I do not quite see the point of avoiding to
> enhance the existing work of Sverre (and a little bit of me, too, in a
> hackathon for which I traveled half the continent back in July 2011).

It's way too much code, to be specific; 15x the code I just submitted.
It would be better to work together, but to me the code-styles are way
too different, the difference between night and day. If you are
interested in simplifying that code, get rid of the classes of classes
of classes and have something more consolidated, I could try to
contribute, but I doubt that's the case.

Anyway, this is 231 lines of code, and works just fine, which is
better than what we have in git.git for mercurial: basically nothing.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-17 16:38   ` Felipe Contreras
@ 2012-10-17 17:39     ` Johannes Schindelin
  2012-10-17 18:12       ` Felipe Contreras
  0 siblings, 1 reply; 27+ messages in thread
From: Johannes Schindelin @ 2012-10-17 17:39 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Junio C Hamano, Sverre Rabbelier, Ilari Liusvaara,
	Daniel Barkalow

Hi,

On Wed, 17 Oct 2012, Felipe Contreras wrote:

> On Wed, Oct 17, 2012 at 6:03 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > On Wed, 17 Oct 2012, Felipe Contreras wrote:
> >
> >> I've looked at many hg<->git tools and none satisfy me. Too
> >> complicated, or too slow, or to difficult to setup, etc.
> >
> > The one I merged into Git for Windows (since that is what I install on
> > all my machines even if they run Linux) is rock-solid. It also comes
> > with tests. And it requires a fix I tried to get into git.git (but
> > failed, since I was asked to do much more in addition to what I needed
> > for myself, and I lack the time to address such requests these days).
> 
> Maybe, but who uses it? It's quite a lot of code, and it's quite
> difficult to setup--you would need a non-vanilla version of git.

Okay, so the difficulty of setting it up is because it is not in mainline
git.git?

> Compare this:
> 32 files changed, 3351 insertions(+), 289 deletions(-)
> 
> To this:
> 1 file changed, 231 insertions(+)

Yeah, and that's also because of the severe lack of tests. And the lack of
possible code-sharing with other remote helpers.

As for who uses it:

	https://github.com/dscho/hg

> It would be better to work together, but to me the code-styles are way
> too different, the difference between night and day.

Aha. Well, okay, it was an offer to collaborate.

Ciao,
Johannes

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-17 17:39     ` Johannes Schindelin
@ 2012-10-17 18:12       ` Felipe Contreras
  2012-10-17 18:18         ` Sverre Rabbelier
  0 siblings, 1 reply; 27+ messages in thread
From: Felipe Contreras @ 2012-10-17 18:12 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: git, Junio C Hamano, Sverre Rabbelier, Ilari Liusvaara,
	Daniel Barkalow

On Wed, Oct 17, 2012 at 7:39 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Wed, 17 Oct 2012, Felipe Contreras wrote:
>
>> On Wed, Oct 17, 2012 at 6:03 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> > On Wed, 17 Oct 2012, Felipe Contreras wrote:
>> >
>> >> I've looked at many hg<->git tools and none satisfy me. Too
>> >> complicated, or too slow, or to difficult to setup, etc.
>> >
>> > The one I merged into Git for Windows (since that is what I install on
>> > all my machines even if they run Linux) is rock-solid. It also comes
>> > with tests. And it requires a fix I tried to get into git.git (but
>> > failed, since I was asked to do much more in addition to what I needed
>> > for myself, and I lack the time to address such requests these days).
>>
>> Maybe, but who uses it? It's quite a lot of code, and it's quite
>> difficult to setup--you would need a non-vanilla version of git.
>
> Okay, so the difficulty of setting it up is because it is not in mainline
> git.git?

My version:
1) cp contrib/remote-hg/git-remote-hg ~/bin

Your version? I don't know, but it certainly will be more than one
step... may more.

>> Compare this:
>> 32 files changed, 3351 insertions(+), 289 deletions(-)
>>
>> To this:
>> 1 file changed, 231 insertions(+)
>
> Yeah, and that's also because of the severe lack of tests. And the lack of
> possible code-sharing with other remote helpers.

What is there to share? It's 230 lines of code. And share it with
which remote helpers? And trying to do so will certainly make it
harder to setup.

As for the tests, I don't see any tests checking that the import of
tags succeeds. Oh, that's right, that is not implemented, so not
surprisingly; there are no tests for that. What does a dozen passing
tests tell you about the code? Not much. If my code had tests, the
test for importing tags would succeed, but I chose to spend my time
implementing features, and trying to make them accessible to as many
people as possible... rather than writing tests.

But fine, lets remove the tests out of the equation (150 lines), the
number of lines of code still exceeds 3000.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-17 18:12       ` Felipe Contreras
@ 2012-10-17 18:18         ` Sverre Rabbelier
  2012-10-17 18:33           ` Felipe Contreras
  0 siblings, 1 reply; 27+ messages in thread
From: Sverre Rabbelier @ 2012-10-17 18:18 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Johannes Schindelin, git, Junio C Hamano, Ilari Liusvaara,
	Daniel Barkalow

On Wed, Oct 17, 2012 at 11:12 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> But fine, lets remove the tests out of the equation (150 lines), the
> number of lines of code still exceeds 3000.

I don't think it's fair to just look at LOC, git-remote-hg when it was
just parsing was fairly simple. Most of the current code is our copy
of the python fast-import library which is only used to support
pushing to mercurial.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-17 18:18         ` Sverre Rabbelier
@ 2012-10-17 18:33           ` Felipe Contreras
  2012-10-18  8:47             ` Johannes Schindelin
  0 siblings, 1 reply; 27+ messages in thread
From: Felipe Contreras @ 2012-10-17 18:33 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Johannes Schindelin, git, Junio C Hamano, Ilari Liusvaara,
	Daniel Barkalow

On Wed, Oct 17, 2012 at 8:18 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> On Wed, Oct 17, 2012 at 11:12 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> But fine, lets remove the tests out of the equation (150 lines), the
>> number of lines of code still exceeds 3000.
>
> I don't think it's fair to just look at LOC, git-remote-hg when it was
> just parsing was fairly simple. Most of the current code is our copy
> of the python fast-import library which is only used to support
> pushing to mercurial.

Well, as a rule of thumb more code means more places for bugs to hide.
It is also quite frankly rather difficult to navigate; very
spaghetti-like. I have the feeling I can implement the same
fast-import functionality in a much simpler way, but for now I want to
concentrate on fetching, and hopefully making it easy for people to
actually use it.

I would like to cooperate as much as possible, but as I said, I would
rewrite a lot of that code. And also, I'm not even sure which
repository contains the latest version of this code. I've seen a
couple of them that are quite different, and none which are based on a
recent version of git.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-17 12:58 [PATCH] Add new git-remote-hd helper Felipe Contreras
  2012-10-17 16:03 ` Johannes Schindelin
@ 2012-10-17 22:59 ` Jeff King
  2012-10-18  3:44   ` Felipe Contreras
  2012-10-18 13:18 ` Michael J Gruber
  2 siblings, 1 reply; 27+ messages in thread
From: Jeff King @ 2012-10-17 22:59 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow

On Wed, Oct 17, 2012 at 02:58:41PM +0200, Felipe Contreras wrote:

> I've looked at many hg<->git tools and none satisfy me. Too complicated, or too
> slow, or to difficult to setup, etc.

I run into this every few months, evaluate all of the options, and come
to the same conclusion. So I am excited at the prospect of something
simple that just works out of the box.

Unfortunately, when I tried it, it did not work for me. :(

Details below.

>  contrib/remote-hd/git-remote-hg | 231 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 231 insertions(+)
>  create mode 100755 contrib/remote-hd/git-remote-hg

Is this "hd" a typo, or is there something clever I am missing?

> --- /dev/null
> +++ b/contrib/remote-hd/git-remote-hg
> @@ -0,0 +1,231 @@
> +#!/usr/bin/python2

I do not have /usr/bin/python2. I do have (on my Debian box):

  $ ls -l /usr/bin/python* | perl -lne 'print $& if m{/.*}'
  /usr/bin/python -> python2.7
  /usr/bin/python2.6
  /usr/bin/python2.7
  /usr/bin/python3 -> python3.2
  /usr/bin/python3.2 -> python3.2mu
  /usr/bin/python3.2mu
  /usr/bin/python3mu -> python3.2mu

Obviously a minor, easily fixable issue, but I wonder if it should ship
with a more portable default (like just "/usr/bin/python", or even
"/usr/bin/env python").

> +# Inspired by Rocco Rutte's hg-fast-export
> +
> +# Just copy to your ~/bin, or anywhere in your $PATH.
> +# Then you can clone with:
> +# hg::file:///path/to/mercurial/repo/

The first thing I tried was:

  $ git clone hg::https://code.google.com/p/dactyl/ 
  Cloning into 'dactyl'...
  fatal: Unable to find remote helper for 'hg'
  sigill:~/compile/dactyl$ git clone hg::https://code.google.com/p/dactyl/ 
  Cloning into 'dactyl'...
  Traceback (most recent call last):
    File "/home/peff/local/bin/git-remote-hg", line 231, in <module>
      sys.exit(main(sys.argv))
    File "/home/peff/local/bin/git-remote-hg", line 222, in main
      do_list(repo, args)
    File "/home/peff/local/bin/git-remote-hg", line 159, in do_list
      head = repo.dirstate.branch()
  AttributeError: 'httpsrepository' object has no attribute 'dirstate'

So we are failing here:

> +def do_list(repo, args):
> +    global branches
> +
> +    head = repo.dirstate.branch()
> +    for branch in repo.branchmap():
> +        heads = repo.branchheads(branch)
> +        if len(heads):
> +            branches[branch] = heads

Is there a way to get this information for remote repos?

I worked around it by doing an hg-clone and trying to git-clone from
that local clone. But that didn't work either:

  $ hg clone https://code.google.com/p/dactyl/ hg
  [... clone eventually completes ...]

  $ git clone hg::$PWD/hg git
  Cloning into 'git'...
  progress revision 99 'pentadactyl-1.0b5-branch' (100/5367)
  [... many more progress updates ...]
  progress revision 6766 'cpg-hack' (1400/1467)
  ERROR: Branch 'default' has more than one head
  error: refs/tags/VIMPERATOR_2_2_b1 does not point to a valid object!
  error: refs/tags/muttator-0.5 does not point to a valid object!
  error: refs/tags/pentadactyl-1.0 does not point to a valid object!
  error: refs/tags/pentadactyl-1.0b1 does not point to a valid object!
  error: refs/tags/pentadactyl-1.0b2 does not point to a valid object!
  error: refs/tags/pentadactyl-1.0b3 does not point to a valid object!
  error: refs/tags/pentadactyl-1.0b4 does not point to a valid object!
  error: refs/tags/pentadactyl-1.0b4.1 does not point to a valid object!
  error: refs/tags/pentadactyl-1.0b4.2 does not point to a valid object!
  error: refs/tags/pentadactyl-1.0b4.3 does not point to a valid object!
  error: refs/tags/pentadactyl-1.0b5 does not point to a valid object!
  error: refs/tags/pentadactyl-1.0b5.1 does not point to a valid object!
  error: refs/tags/pentadactyl-1.0b6 does not point to a valid object!
  error: refs/tags/pentadactyl-1.0b7 does not point to a valid object!
  error: refs/tags/pentadactyl-1.0b7.1 does not point to a valid object!
  error: refs/tags/pentadactyl-1.0rc1 does not point to a valid object!
  error: refs/tags/vimperator-0.4.1 does not point to a valid object!
  error: refs/tags/vimperator-0.5 does not point to a valid object!
  error: refs/tags/vimperator-0.5-branch-HEAD-merge-1 does not point to a valid object!
  error: refs/tags/vimperator-0.5.1 does not point to a valid object!
  error: refs/tags/vimperator-0.5.2 does not point to a valid object!
  error: refs/tags/vimperator-0.5.3 does not point to a valid object!
  error: refs/tags/vimperator-1.0 does not point to a valid object!
  error: refs/tags/vimperator-1.1 does not point to a valid object!
  error: refs/tags/vimperator-1.2 does not point to a valid object!
  error: refs/tags/vimperator-2.0 does not point to a valid object!
  error: refs/tags/vimperator-2.0a1 does not point to a valid object!
  error: refs/tags/vimperator-2.1 does not point to a valid object!
  error: refs/tags/vimperator-2.2 does not point to a valid object!
  error: refs/tags/vimperator-2.2b1 does not point to a valid object!
  error: refs/tags/xulmus-0.1 does not point to a valid object!

I seem to remember getting this with other importers, too (probably
because they were also based on the same script).

We do not need to fix every bug before bringing a script into git
(especially into contrib/), but I am wondering if this script errs too
much on the side of "simple" and not enough on "works out of the box".
Maybe this repo is really complex and unusual, and the multi-heads thing
is not common enough to worry about. But I feel cloning a remote is the
first thing most people are going to try, and it doesn't work.

-Peff

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-17 22:59 ` Jeff King
@ 2012-10-18  3:44   ` Felipe Contreras
  2012-10-18  5:18     ` Felipe Contreras
  2012-10-18  8:48     ` Felipe Contreras
  0 siblings, 2 replies; 27+ messages in thread
From: Felipe Contreras @ 2012-10-18  3:44 UTC (permalink / raw)
  To: Jeff King
  Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow

On Thu, Oct 18, 2012 at 12:59 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Oct 17, 2012 at 02:58:41PM +0200, Felipe Contreras wrote:
>
>> I've looked at many hg<->git tools and none satisfy me. Too complicated, or too
>> slow, or to difficult to setup, etc.
>
> I run into this every few months, evaluate all of the options, and come
> to the same conclusion. So I am excited at the prospect of something
> simple that just works out of the box.
>
> Unfortunately, when I tried it, it did not work for me. :(
>
> Details below.
>
>>  contrib/remote-hd/git-remote-hg | 231 ++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 231 insertions(+)
>>  create mode 100755 contrib/remote-hd/git-remote-hg
>
> Is this "hd" a typo, or is there something clever I am missing?

Yeah, I've fixed that now.

>> --- /dev/null
>> +++ b/contrib/remote-hd/git-remote-hg
>> @@ -0,0 +1,231 @@
>> +#!/usr/bin/python2
>
> I do not have /usr/bin/python2. I do have (on my Debian box):
>
>   $ ls -l /usr/bin/python* | perl -lne 'print $& if m{/.*}'
>   /usr/bin/python -> python2.7
>   /usr/bin/python2.6
>   /usr/bin/python2.7
>   /usr/bin/python3 -> python3.2
>   /usr/bin/python3.2 -> python3.2mu
>   /usr/bin/python3.2mu
>   /usr/bin/python3mu -> python3.2mu
>
> Obviously a minor, easily fixable issue, but I wonder if it should ship
> with a more portable default (like just "/usr/bin/python", or even
> "/usr/bin/env python").

Yeah, this has always been an issue in Arch Linux; I have to compile
git by exporting PYTHON_PATH.

I'm OK with using any of the two above suggestions above, as they are
more standard.

>> +# Inspired by Rocco Rutte's hg-fast-export
>> +
>> +# Just copy to your ~/bin, or anywhere in your $PATH.
>> +# Then you can clone with:
>> +# hg::file:///path/to/mercurial/repo/
>
> The first thing I tried was:
>
>   $ git clone hg::https://code.google.com/p/dactyl/

Right, doesn't look like it works for remote repositories. I think
that's the next feature I want to implement, but to be honest, I don't
think it's a big issue. To replace this:

git clone hg::https://code.google.com/p/dactyl/

With this

hg clone https://code.google.com/p/dactyl/
git clone hg::dactyl dactyl-git

We could do what other tools do, manually clone the repository and
store it internally, but I'll rather not trick the users this way.

> I worked around it by doing an hg-clone and trying to git-clone from
> that local clone. But that didn't work either:
>
>   $ hg clone https://code.google.com/p/dactyl/ hg
>   [... clone eventually completes ...]
>
>   $ git clone hg::$PWD/hg git
>   Cloning into 'git'...
>   progress revision 99 'pentadactyl-1.0b5-branch' (100/5367)
>   [... many more progress updates ...]
>   progress revision 6766 'cpg-hack' (1400/1467)
>   ERROR: Branch 'default' has more than one head

Yes, this is deliberate, we can't have more than one head per branch in git.

What you should do is go to the mercurial repo, and 'hg merge' (I think).

We could just pick the first head, and warn the user instead.

But at the moment it should fail at this point, I wonder why you get
the errors below.

>   error: refs/tags/VIMPERATOR_2_2_b1 does not point to a valid object!
>   error: refs/tags/muttator-0.5 does not point to a valid object!
>   error: refs/tags/pentadactyl-1.0 does not point to a valid object!
>   error: refs/tags/pentadactyl-1.0b1 does not point to a valid object!
>   error: refs/tags/pentadactyl-1.0b2 does not point to a valid object!
>   error: refs/tags/pentadactyl-1.0b3 does not point to a valid object!
>   error: refs/tags/pentadactyl-1.0b4 does not point to a valid object!
>   error: refs/tags/pentadactyl-1.0b4.1 does not point to a valid object!
>   error: refs/tags/pentadactyl-1.0b4.2 does not point to a valid object!
>   error: refs/tags/pentadactyl-1.0b4.3 does not point to a valid object!
>   error: refs/tags/pentadactyl-1.0b5 does not point to a valid object!
>   error: refs/tags/pentadactyl-1.0b5.1 does not point to a valid object!
>   error: refs/tags/pentadactyl-1.0b6 does not point to a valid object!
>   error: refs/tags/pentadactyl-1.0b7 does not point to a valid object!
>   error: refs/tags/pentadactyl-1.0b7.1 does not point to a valid object!
>   error: refs/tags/pentadactyl-1.0rc1 does not point to a valid object!
>   error: refs/tags/vimperator-0.4.1 does not point to a valid object!
>   error: refs/tags/vimperator-0.5 does not point to a valid object!
>   error: refs/tags/vimperator-0.5-branch-HEAD-merge-1 does not point to a valid object!
>   error: refs/tags/vimperator-0.5.1 does not point to a valid object!
>   error: refs/tags/vimperator-0.5.2 does not point to a valid object!
>   error: refs/tags/vimperator-0.5.3 does not point to a valid object!
>   error: refs/tags/vimperator-1.0 does not point to a valid object!
>   error: refs/tags/vimperator-1.1 does not point to a valid object!
>   error: refs/tags/vimperator-1.2 does not point to a valid object!
>   error: refs/tags/vimperator-2.0 does not point to a valid object!
>   error: refs/tags/vimperator-2.0a1 does not point to a valid object!
>   error: refs/tags/vimperator-2.1 does not point to a valid object!
>   error: refs/tags/vimperator-2.2 does not point to a valid object!
>   error: refs/tags/vimperator-2.2b1 does not point to a valid object!
>   error: refs/tags/xulmus-0.1 does not point to a valid object!

This is weird.

> I seem to remember getting this with other importers, too (probably
> because they were also based on the same script).

hg-fast-import also fails if there are multiple heads, also deliberately.

> We do not need to fix every bug before bringing a script into git
> (especially into contrib/), but I am wondering if this script errs too
> much on the side of "simple" and not enough on "works out of the box".
> Maybe this repo is really complex and unusual, and the multi-heads thing
> is not common enough to worry about. But I feel cloning a remote is the
> first thing most people are going to try, and it doesn't work.

Well, maybe the next time you try the branch will be merged. In
mercurial they even warn you about this when you do 'hg push', so it's
not a good practice that they are doing that in their own repo.

As for remote repos, I don't know what we should be doing at the moment.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-18  3:44   ` Felipe Contreras
@ 2012-10-18  5:18     ` Felipe Contreras
  2012-10-18  6:12       ` Sverre Rabbelier
  2012-10-18  8:48     ` Felipe Contreras
  1 sibling, 1 reply; 27+ messages in thread
From: Felipe Contreras @ 2012-10-18  5:18 UTC (permalink / raw)
  To: Jeff King
  Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow

On Thu, Oct 18, 2012 at 5:44 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Thu, Oct 18, 2012 at 12:59 AM, Jeff King <peff@peff.net> wrote:
>> On Wed, Oct 17, 2012 at 02:58:41PM +0200, Felipe Contreras wrote:
>>
>>> I've looked at many hg<->git tools and none satisfy me. Too complicated, or too
>>> slow, or to difficult to setup, etc.
>>
>> I run into this every few months, evaluate all of the options, and come
>> to the same conclusion. So I am excited at the prospect of something
>> simple that just works out of the box.
>>
>> Unfortunately, when I tried it, it did not work for me. :(

Ok, I've fixed all those issues:
http://github.com/felipec/git/blob/fc-remote-hg/contrib/remote-hg/git-remote-hg

Right now I've just added an error when using remote repositories. But
it seems there's no way around it; if we want to have support for
remote repos, we need to make a local clone.

> But at the moment it should fail at this point, I wonder why you get
> the errors below.
>
>>   error: refs/tags/VIMPERATOR_2_2_b1 does not point to a valid object!
>>   error: refs/tags/muttator-0.5 does not point to a valid object!
>>   error: refs/tags/pentadactyl-1.0 does not point to a valid object!
>>   error: refs/tags/pentadactyl-1.0b1 does not point to a valid object!
>>   error: refs/tags/pentadactyl-1.0b2 does not point to a valid object!
>>   error: refs/tags/pentadactyl-1.0b3 does not point to a valid object!
>>   error: refs/tags/pentadactyl-1.0b4 does not point to a valid object!
>>   error: refs/tags/pentadactyl-1.0b4.1 does not point to a valid object!
>>   error: refs/tags/pentadactyl-1.0b4.2 does not point to a valid object!
>>   error: refs/tags/pentadactyl-1.0b4.3 does not point to a valid object!
>>   error: refs/tags/pentadactyl-1.0b5 does not point to a valid object!
>>   error: refs/tags/pentadactyl-1.0b5.1 does not point to a valid object!
>>   error: refs/tags/pentadactyl-1.0b6 does not point to a valid object!
>>   error: refs/tags/pentadactyl-1.0b7 does not point to a valid object!
>>   error: refs/tags/pentadactyl-1.0b7.1 does not point to a valid object!
>>   error: refs/tags/pentadactyl-1.0rc1 does not point to a valid object!
>>   error: refs/tags/vimperator-0.4.1 does not point to a valid object!
>>   error: refs/tags/vimperator-0.5 does not point to a valid object!
>>   error: refs/tags/vimperator-0.5-branch-HEAD-merge-1 does not point to a valid object!
>>   error: refs/tags/vimperator-0.5.1 does not point to a valid object!
>>   error: refs/tags/vimperator-0.5.2 does not point to a valid object!
>>   error: refs/tags/vimperator-0.5.3 does not point to a valid object!
>>   error: refs/tags/vimperator-1.0 does not point to a valid object!
>>   error: refs/tags/vimperator-1.1 does not point to a valid object!
>>   error: refs/tags/vimperator-1.2 does not point to a valid object!
>>   error: refs/tags/vimperator-2.0 does not point to a valid object!
>>   error: refs/tags/vimperator-2.0a1 does not point to a valid object!
>>   error: refs/tags/vimperator-2.1 does not point to a valid object!
>>   error: refs/tags/vimperator-2.2 does not point to a valid object!
>>   error: refs/tags/vimperator-2.2b1 does not point to a valid object!
>>   error: refs/tags/xulmus-0.1 does not point to a valid object!
>
> This is weird.

I think I know why the errors above show up; even though the helper
dies, transport-helper doesn't check the status until the very end.

Something like this should do the trick:

diff --git a/run-command.c b/run-command.c
index 1101ef7..0a859ca 100644
--- a/run-command.c
+++ b/run-command.c
@@ -559,6 +559,21 @@ int run_command(struct child_process *cmd)
 	return finish_command(cmd);
 }

+int check_command(struct child_process *cmd)
+{
+	int status;
+	pid_t pid;
+
+	pid = waitpid(cmd->pid, &status, WNOHANG);
+
+	if (pid != cmd->pid)
+		return -1;
+	if (WIFSIGNALED(status))
+		return WTERMSIG(status);
+	if (WIFEXITED(status))
+		return WEXITSTATUS(status);
+}
+
 static void prepare_run_command_v_opt(struct child_process *cmd,
 				      const char **argv,
 				      int opt)
diff --git a/run-command.h b/run-command.h
index 44f7d2b..9019e38 100644
--- a/run-command.h
+++ b/run-command.h
@@ -45,6 +45,7 @@ struct child_process {
 int start_command(struct child_process *);
 int finish_command(struct child_process *);
 int run_command(struct child_process *);
+int check_command(struct child_process *cmd);

 extern int run_hook(const char *index_file, const char *name, ...);

diff --git a/transport-helper.c b/transport-helper.c
index cfe0988..bc1349d 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -441,6 +441,10 @@ static int fetch_with_import(struct transport *transport,

 	if (finish_command(&fastimport))
 		die("Error while running fast-import");
+
+	if (check_command(data->helper))
+		die("Error while running helper");
+
 	free(fastimport.argv);
 	fastimport.argv = NULL;

@@ -784,6 +788,10 @@ static int push_refs_with_export(struct transport
*transport,

 	if (finish_command(&exporter))
 		die("Error while running fast-export");
+
+	if (check_command(data->helper))
+		die("Error while running helper");
+
 	push_update_refs_status(data, remote_refs);
 	return 0;
 }

-- 
Felipe Contreras

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-18  5:18     ` Felipe Contreras
@ 2012-10-18  6:12       ` Sverre Rabbelier
  2012-10-18  9:10         ` Felipe Contreras
  2012-10-26  9:02         ` Felipe Contreras
  0 siblings, 2 replies; 27+ messages in thread
From: Sverre Rabbelier @ 2012-10-18  6:12 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Jeff King, git, Junio C Hamano, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow

On Wed, Oct 17, 2012 at 10:18 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Right now I've just added an error when using remote repositories. But
> it seems there's no way around it; if we want to have support for
> remote repos, we need to make a local clone.

My git-remote-hg does the local clone into .git/ using a hash of the
url (although you could just as well use urlencode, basically any way
to safely use a url as a directory name). Have a look if you want.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-17 18:33           ` Felipe Contreras
@ 2012-10-18  8:47             ` Johannes Schindelin
  2012-10-18  9:03               ` Felipe Contreras
  2012-10-21 18:03               ` Felipe Contreras
  0 siblings, 2 replies; 27+ messages in thread
From: Johannes Schindelin @ 2012-10-18  8:47 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Sverre Rabbelier, git, Junio C Hamano, Ilari Liusvaara,
	Daniel Barkalow

Hi Felipe,

On Wed, 17 Oct 2012, Felipe Contreras wrote:

> On Wed, Oct 17, 2012 at 8:18 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> > On Wed, Oct 17, 2012 at 11:12 AM, Felipe Contreras
> > <felipe.contreras@gmail.com> wrote:
> >> But fine, lets remove the tests out of the equation (150 lines), the
> >> number of lines of code still exceeds 3000.
> >
> > I don't think it's fair to just look at LOC, git-remote-hg when it was
> > just parsing was fairly simple. Most of the current code is our copy
> > of the python fast-import library which is only used to support
> > pushing to mercurial.
> 
> Well, as a rule of thumb more code means more places for bugs to hide.

Everybody on this list knows that. But it is equally true that more
functionality requires more code.

Besides, we are talking about concrete code, so there is no need at all
for handwaving arguments. GitHub makes it easy to point at exact line
numbers in exact file names in exact revisions, as you know, and we should
use that to discuss code.

> It is also quite frankly rather difficult to navigate; very
> spaghetti-like. I have the feeling [...]

Yours truly always welcomes constructive criticism. Other types of
criticism, not so much.

As to the functionality you seek: git-remote-hg found in
git://github.com/msysgit/git works. It has the following advantages over
every other solution, including the one proposed in this thread:

- it works

- no really, it works

- it supports pushes, too

- it matured over a long time

- there are tests

- whenever we fixed bugs, we also added tests for the bug fixes

- it is rock solid

- it is in constant use

Without push support, remote-hg is useless to me. Without regression tests
proving that it is rock solid, I will not use remote-hg. And I will not
indulge in efforts to duplicate work.

If there are concerns about code style or unnecessary code (insofar it is
really unnecessary, testgit for example is not, unless you want to avoid
robust regression tests), I will discuss issues and collaborate. If the
idea was not to collaborate, but to show off how much shorter code can be
when it lacks functionality and proof of robustness I require for my
everyday use of the program, dismissing existing code and concepts, less
so.

Hth,
Johannes

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-18  3:44   ` Felipe Contreras
  2012-10-18  5:18     ` Felipe Contreras
@ 2012-10-18  8:48     ` Felipe Contreras
  1 sibling, 0 replies; 27+ messages in thread
From: Felipe Contreras @ 2012-10-18  8:48 UTC (permalink / raw)
  To: Jeff King
  Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow

On Thu, Oct 18, 2012 at 5:44 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Thu, Oct 18, 2012 at 12:59 AM, Jeff King <peff@peff.net> wrote:

>> The first thing I tried was:
>>
>>   $ git clone hg::https://code.google.com/p/dactyl/
>
> Right, doesn't look like it works for remote repositories. I think
> that's the next feature I want to implement, but to be honest, I don't
> think it's a big issue. To replace this:

Done, now you should be able to clone and fetch remote repositories :)
https://github.com/felipec/git/commit/783e4b380ab4fabb4e2fb200722c92afc8494a83

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-18  8:47             ` Johannes Schindelin
@ 2012-10-18  9:03               ` Felipe Contreras
  2012-10-18  9:10                 ` Johannes Schindelin
  2012-10-18  9:26                 ` Junio C Hamano
  2012-10-21 18:03               ` Felipe Contreras
  1 sibling, 2 replies; 27+ messages in thread
From: Felipe Contreras @ 2012-10-18  9:03 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Sverre Rabbelier, git, Junio C Hamano, Ilari Liusvaara,
	Daniel Barkalow

On Thu, Oct 18, 2012 at 10:47 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Felipe,
>
> On Wed, 17 Oct 2012, Felipe Contreras wrote:
>
>> On Wed, Oct 17, 2012 at 8:18 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
>> > On Wed, Oct 17, 2012 at 11:12 AM, Felipe Contreras
>> > <felipe.contreras@gmail.com> wrote:
>> >> But fine, lets remove the tests out of the equation (150 lines), the
>> >> number of lines of code still exceeds 3000.
>> >
>> > I don't think it's fair to just look at LOC, git-remote-hg when it was
>> > just parsing was fairly simple. Most of the current code is our copy
>> > of the python fast-import library which is only used to support
>> > pushing to mercurial.
>>
>> Well, as a rule of thumb more code means more places for bugs to hide.
>
> Everybody on this list knows that. But it is equally true that more
> functionality requires more code.

Not necessarily. There's projects with more code and less
functionality than their alternatives.

>> It is also quite frankly rather difficult to navigate; very
>> spaghetti-like. I have the feeling [...]
>
> Yours truly always welcomes constructive criticism. Other types of
> criticism, not so much.
>
> As to the functionality you seek: git-remote-hg found in
> git://github.com/msysgit/git works. It has the following advantages over
> every other solution, including the one proposed in this thread:
>
> - it works
>
> - no really, it works

Not for me.

> - it supports pushes, too

I don't care. When I need that I'll implement that with probably much less code.

> - it matured over a long time

So has CVS.

> - there are tests

Only a dozen of them. If I write the same tests for my solution would
you be happier? I didn't think so.

> - whenever we fixed bugs, we also added tests for the bug fixes

Like this one?
https://github.com/msysgit/git/commit/9f934c9987981cbecf4ebaf8eb4a8e9f1d002caf

I don't see any tests for that.

> - it is in constant use

So you say, my impression is different.

> Without push support, remote-hg is useless to me.

Different people have different needs.

Without an easy way to setup, remote-hg is useless to me.

> If there are concerns about code style or unnecessary code (insofar it is
> really unnecessary, testgit for example is not, unless you want to avoid
> robust regression tests), I will discuss issues and collaborate. If the
> idea was not to collaborate, but to show off how much shorter code can be
> when it lacks functionality and proof of robustness I require for my
> everyday use of the program, dismissing existing code and concepts, less
> so.

So your idea of collaboration is accept that your code is awesome, and
my code sucks, and that I should fix your code, and throw my code to
the trash, while you do absolutely nothing but complain about the
whole situation. I have at least looked at your code. Have you even
looked at mine?

I've done my part in making my code easily available and ready for
review. I will not reply to you anymore until you show your
willingness to collaborate that you seem to demand for me, and:

1) Point to a remote-hg branch that is independent of msysgit stuff,
or any other irrelevant stuff
2) Is based on top of a recent version of git

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-18  9:03               ` Felipe Contreras
@ 2012-10-18  9:10                 ` Johannes Schindelin
  2012-10-18  9:26                 ` Junio C Hamano
  1 sibling, 0 replies; 27+ messages in thread
From: Johannes Schindelin @ 2012-10-18  9:10 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Sverre Rabbelier, git, Junio C Hamano, Ilari Liusvaara,
	Daniel Barkalow

Hi Felipe,

On Thu, 18 Oct 2012, Felipe Contreras wrote:

> On Thu, Oct 18, 2012 at 10:47 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>
> So your idea of collaboration is accept that your code is awesome, and
> my code sucks, and that I should fix your code, and throw my code to the
> trash, while you do absolutely nothing but complain about the whole
> situation.

I said no such thing. I like to keep things professional and keep emotions
out.

Hth,
Johannes

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-18  6:12       ` Sverre Rabbelier
@ 2012-10-18  9:10         ` Felipe Contreras
  2012-10-18  9:13           ` Johannes Schindelin
  2012-10-26  9:02         ` Felipe Contreras
  1 sibling, 1 reply; 27+ messages in thread
From: Felipe Contreras @ 2012-10-18  9:10 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Jeff King, git, Junio C Hamano, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow

On Thu, Oct 18, 2012 at 8:12 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> On Wed, Oct 17, 2012 at 10:18 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> Right now I've just added an error when using remote repositories. But
>> it seems there's no way around it; if we want to have support for
>> remote repos, we need to make a local clone.
>
> My git-remote-hg does the local clone into .git/ using a hash of the
> url (although you could just as well use urlencode, basically any way
> to safely use a url as a directory name). Have a look if you want.

Can you point to the version you are talking about? I've been checking
the remote-hg branch of fingolfin.

https://github.com/fingolfin/git/tree/remote-hg/

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-18  9:10         ` Felipe Contreras
@ 2012-10-18  9:13           ` Johannes Schindelin
  2012-10-18  9:22             ` Felipe Contreras
  0 siblings, 1 reply; 27+ messages in thread
From: Johannes Schindelin @ 2012-10-18  9:13 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Sverre Rabbelier, Jeff King, git, Junio C Hamano, Ilari Liusvaara,
	Daniel Barkalow

Hi,

On Thu, 18 Oct 2012, Felipe Contreras wrote:

> On Thu, Oct 18, 2012 at 8:12 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> > On Wed, Oct 17, 2012 at 10:18 PM, Felipe Contreras
> > <felipe.contreras@gmail.com> wrote:
> >> Right now I've just added an error when using remote repositories.
> >> But it seems there's no way around it; if we want to have support for
> >> remote repos, we need to make a local clone.
> >
> > My git-remote-hg does the local clone into .git/ using a hash of the
> > url (although you could just as well use urlencode, basically any way
> > to safely use a url as a directory name). Have a look if you want.
> 
> Can you point to the version you are talking about? I've been checking
> the remote-hg branch of fingolfin.
> 
> https://github.com/fingolfin/git/tree/remote-hg/

The code is in https://github.com/msysgit/git/ (Sverre does not have
enough time to work on remote-hg, and was okay with me hosting it in Git
for Windows, for all the reasons I mentioned earlier in this thread).

Hth,
Johannes

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-18  9:13           ` Johannes Schindelin
@ 2012-10-18  9:22             ` Felipe Contreras
  0 siblings, 0 replies; 27+ messages in thread
From: Felipe Contreras @ 2012-10-18  9:22 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Sverre Rabbelier, Jeff King, git, Junio C Hamano, Ilari Liusvaara,
	Daniel Barkalow

On Thu, Oct 18, 2012 at 11:13 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Thu, 18 Oct 2012, Felipe Contreras wrote:
>
>> On Thu, Oct 18, 2012 at 8:12 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
>> > On Wed, Oct 17, 2012 at 10:18 PM, Felipe Contreras
>> > <felipe.contreras@gmail.com> wrote:
>> >> Right now I've just added an error when using remote repositories.
>> >> But it seems there's no way around it; if we want to have support for
>> >> remote repos, we need to make a local clone.
>> >
>> > My git-remote-hg does the local clone into .git/ using a hash of the
>> > url (although you could just as well use urlencode, basically any way
>> > to safely use a url as a directory name). Have a look if you want.
>>
>> Can you point to the version you are talking about? I've been checking
>> the remote-hg branch of fingolfin.
>>
>> https://github.com/fingolfin/git/tree/remote-hg/
>
> The code is in https://github.com/msysgit/git/ (Sverre does not have
> enough time to work on remote-hg, and was okay with me hosting it in Git
> for Windows, for all the reasons I mentioned earlier in this thread).

Which branch? I don't see any remote-hg branch.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-18  9:03               ` Felipe Contreras
  2012-10-18  9:10                 ` Johannes Schindelin
@ 2012-10-18  9:26                 ` Junio C Hamano
  2012-10-18  9:38                   ` Felipe Contreras
  1 sibling, 1 reply; 27+ messages in thread
From: Junio C Hamano @ 2012-10-18  9:26 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Johannes Schindelin, Sverre Rabbelier, git, Ilari Liusvaara,
	Daniel Barkalow

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

>> As to the functionality you seek: git-remote-hg found in
>> git://github.com/msysgit/git works. It has the following advantages over
>> every other solution, including the one proposed in this thread:
>>
>> - it works
>>
>> - no really, it works
>
> Not for me.

Felipe, an argument along this line would not help very much.

Please elaborate a bit to describe what does not work and where you
are having problems more concretely.  Even for people who may want
to see if they agree with your "It does not work", "Not for me"
alone is too little for them to try out.  Others who may want to and
are capable of helping you won't know where to start.

Thanks.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-18  9:26                 ` Junio C Hamano
@ 2012-10-18  9:38                   ` Felipe Contreras
  2012-10-18  9:42                     ` Matthieu Moy
  0 siblings, 1 reply; 27+ messages in thread
From: Felipe Contreras @ 2012-10-18  9:38 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, Sverre Rabbelier, git, Ilari Liusvaara,
	Daniel Barkalow

On Thu, Oct 18, 2012 at 11:26 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>>> As to the functionality you seek: git-remote-hg found in
>>> git://github.com/msysgit/git works. It has the following advantages over
>>> every other solution, including the one proposed in this thread:
>>>
>>> - it works
>>>
>>> - no really, it works
>>
>> Not for me.
>
> Felipe, an argument along this line would not help very much.
>
> Please elaborate a bit to describe what does not work and where you
> are having problems more concretely.  Even for people who may want
> to see if they agree with your "It does not work", "Not for me"
> alone is too little for them to try out.  Others who may want to and
> are capable of helping you won't know where to start.

Basically what I already described:
1) You need a non-vanilla version of git
2) It's not easy to set up
3) It's not even clear which branch you should be using (in case you
are not using msysgit)

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-18  9:38                   ` Felipe Contreras
@ 2012-10-18  9:42                     ` Matthieu Moy
  0 siblings, 0 replies; 27+ messages in thread
From: Matthieu Moy @ 2012-10-18  9:42 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Junio C Hamano, Johannes Schindelin, Sverre Rabbelier, git,
	Ilari Liusvaara, Daniel Barkalow

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

> Basically what I already described:
> 1) You need a non-vanilla version of git
> 2) It's not easy to set up
> 3) It's not even clear which branch you should be using (in case you
> are not using msysgit)

I do not read that as "it does not work", but instead as "no one took
the time to push this code into git.git (although someone did in
msysgit)".

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-17 12:58 [PATCH] Add new git-remote-hd helper Felipe Contreras
  2012-10-17 16:03 ` Johannes Schindelin
  2012-10-17 22:59 ` Jeff King
@ 2012-10-18 13:18 ` Michael J Gruber
  2012-10-18 14:26   ` Felipe Contreras
  2 siblings, 1 reply; 27+ messages in thread
From: Michael J Gruber @ 2012-10-18 13:18 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow

Felipe Contreras venit, vidit, dixit 17.10.2012 14:58:
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> 
> I've looked at many hg<->git tools and none satisfy me. Too complicated, or too
> slow, or to difficult to setup, etc.

It's in an unsatisfying state, I agree. We have a great remote helper
infrastructure, but neither git-svn nor git-cvsimport/export use it, and
remote-hg is not in git.git.

git-svn used to be our killer feature! (It's becoming hard to maintain,
though.)

git-hg (in the shape of a remote helper) could be our next killer
feature, finally leading to our world domination ;)

> The only one I've liked so far is hg-fast-export[1], which is indeed fast,
> relatively simple, and relatively easy to use. But it's not properly maintained
> any more.
> 
> So, I decided to write my own from scratch, using hg-fast-export as
> inspiration, and voila.
> 
> This one doesn't have any dependencies, just put it into your $PATH, and you
> can clone and fetch hg repositories. More importantly to me; the code is
> simple, and easy to maintain.

Well, it still has hg as a dependency. All our remote
integration/helpers suffer from that. At least, every hg install comes
with the modules, whereas svn is a beast (apr and such) that often comes
without the language bindings.

> [1] http://repo.or.cz/w/fast-export.git
> 
>  contrib/remote-hd/git-remote-hg | 231 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 231 insertions(+)
>  create mode 100755 contrib/remote-hd/git-remote-hg

That diffstat looks great (sans tests, of course; it's contrib), but
there's no push functionality, and that is usually the most difficult
part all helpers have to implement: Not only the push interaction, but
also making sure that commits don't get duped in a roundtrip (git fetch
from vcs, push to vcs, git fetch from vcs).

Just cloning and fetching can be done most easily with a shared worktree
and scripting around "hg up" and "git commit -A" in some flavour.

> diff --git a/contrib/remote-hd/git-remote-hg b/contrib/remote-hd/git-remote-hg
> new file mode 100755
> index 0000000..9157b30
> --- /dev/null
> +++ b/contrib/remote-hd/git-remote-hg
> @@ -0,0 +1,231 @@
> +#!/usr/bin/python2
> +
> +# Inspired by Rocco Rutte's hg-fast-export
> +
> +# Just copy to your ~/bin, or anywhere in your $PATH.
> +# Then you can clone with:
> +# hg::file:///path/to/mercurial/repo/
> +
> +from mercurial import hg, ui
> +
> +import re
> +import sys
> +import os
> +import json
> +
> +def die(msg, *args):
> +    print >> sys.stderr, 'ERROR:', msg % args
> +    sys.exit(1)

While we don't have to code for py3, avoiding '>>' will help us later.
(It got removed in py3.) sys.sdterr.write() should be most portable.

> +def gitmode(flags):
> +    return 'l' in flags and '120000' or 'x' in flags and '100755' or '100644'
> +
> +def export_file(fc):
> +    if fc.path() == '.hgtags':

Is this always relative? Just wondering, dunno.

> +        return
> +    d = fc.data()
> +    print "M %s inline %s" % (gitmode(fc.flags()), fc.path())
> +    print "data %d" % len(d)
> +    print d
> +
> +def get_filechanges(repo, ctx, parents):
> +    l = [repo.status(p, ctx)[:3] for p in parents]
> +    changed, added, removed = [sum(e, []) for e in zip(*l)]
> +    return added + changed, removed
> +
> +author_re = re.compile('^((.+?) )?(<.+?>)$')
> +
> +def fixup_user(user):
> +    user = user.replace('"', '')
> +    m = author_re.match(user)
> +    if m:
> +        name = m.group(1)
> +        mail = m.group(3)
> +    else:
> +        name = user
> +        mail = None
> +
> +    if not name:
> +        name = 'Unknown'
> +    if not mail:
> +        mail = '<unknown>'
> +
> +    return '%s %s' % (name, mail)
> +
> +def get_repo(path, alias):
> +    myui = ui.ui()
> +    myui.setconfig('ui', 'interactive', 'off')
> +    repo = hg.repository(myui, path)
> +    return repo

Is there a reason to spell this out, e.g.: Why not

return hg.repository(myui, path)

> +
> +def hg_branch(b):
> +    if b == 'master':
> +        return 'default'
> +    return b
> +
> +def git_branch(b):
> +    if b == 'default':
> +        return 'master'
> +    return b
> +
> +def export_tag(repo, tag):
> +    global prefix
> +    print "reset %s/tags/%s" % (prefix, tag)
> +    print "from :%s" % (repo[tag].rev() + 1)
> +    print
> +
> +def export_branch(repo, branch):
> +    global prefix, marks, cache, branches
> +
> +    heads = branches[hg_branch(branch)]
> +
> +    # verify there's only one head
> +    if (len(heads) > 1):
> +        die("Branch '%s' has more than one head" % hg_branch(branch))

We have to deal with this at some point... Do you support "hg
bookmarks"? They'd be an option, or we implement better detached head
handling in git...

> +
> +    head = repo[heads[0]]
> +    tip = marks.get(branch, 0)
> +    # mercurial takes too much time checking this
> +    if tip == head.rev():
> +        # nothing to do
> +        return
> +    revs = repo.revs('%u:%u' % (tip, head))
> +    count = 0
> +

> +    revs = [rev for rev in revs if not cache.get(rev, False)]
> +
> +    for rev in revs:

Those lines set up the list just so that you iterate over it later.
Please don't do this (unless you know that revs is very small).

for rev in revs:
  if cache.get(rev, False):
    continue

is more performant. You can reduce this further to

count=0
for rev in repo.revs('%u:%u' % (tip, head)):
  if cache.get(rev, False):
    continue

which is even more performant generally, and especially if repo.revs()
returns an iterator rather than a list.

[Yes, you could use lambda+filter, but let's not get religious. The
above is simple and pythonic.]

> +
> +        c = repo[rev]
> +        (manifest, user, (time, tz), files, desc, extra) = repo.changelog.read(c.node())

Same here, you introduce c just for the next line (unless I'm mistaken).

> +        rev_branch = git_branch(extra['branch'])
> +
> +        tz = '%+03d%02d' % (-tz / 3600, -tz % 3600 / 60)
> +
> +        print "commit %s/branches/%s" % (prefix, rev_branch)
> +        print "mark :%d" % (rev + 1)
> +        print "committer %s %d %s" % (fixup_user(user), time, tz)
> +        print "data %d" % (len(desc) + 1)
> +        print desc
> +        print
> +
> +        parents = [p for p in repo.changelog.parentrevs(rev) if p >= 0]
> +
> +        if len(parents) == 0:
> +            modified = c.manifest().keys()
> +            removed = []
> +        else:
> +            added = []
> +            changed = []
> +            print "from :%s" % (parents[0] + 1)

What is this +1 offset that is appearing here and there?

> +            if len(parents) > 1:
> +                print "merge :%s" % (parents[1] + 1)
> +            modified, removed = get_filechanges(repo, c, parents)
> +
> +        for f in removed:
> +            print "D %s" % (f)
> +        for f in modified:
> +            export_file(c.filectx(f))
> +        print
> +
> +        count += 1
> +        if (count % 100 == 0):
> +            print "progress revision %d '%s' (%d/%d)" % (rev, branch, count, len(revs))
> +            print "#############################################################"
> +
> +        cache[rev] = True
> +
> +    # store the latest revision
> +    marks[branch] = rev
> +
> +def do_capabilities(repo, args):
> +    global prefix, dirname
> +
> +    print "import"
> +    print "refspec refs/heads/*:%s/branches/*" % prefix
> +    print "refspec refs/tags/*:%s/tags/*" % prefix
> +
> +    path = os.path.join(dirname, 'marks-git')
> +
> +    print "*export-marks %s" % path
> +    if os.path.exists(path):
> +        print "*import-marks %s" % path
> +
> +    print
> +
> +def do_list(repo, args):
> +    global branches
> +
> +    head = repo.dirstate.branch()
> +    for branch in repo.branchmap():
> +        heads = repo.branchheads(branch)
> +        if len(heads):
> +            branches[branch] = heads

I'm a bit confused here. repo.brancheads() is a list, no? Is this the
single head case only? I'd expect [0] of that, but you seem to be
getting branch names (strings).

Also, if len(heads) == 0 then branches[branch] is undefined or stale. No?

> +
> +    for branch in branches:
> +        print "? refs/heads/%s" % git_branch(branch)
> +    for tag, node in repo.tagslist():
> +        if tag == 'tip':
> +            continue
> +        print "? refs/tags/%s" % tag
> +    print "@refs/heads/%s HEAD" % git_branch(head)
> +    print
> +
> +def do_import(repo, args):
> +    ref = args[1]
> +
> +    if (ref == 'HEAD'):
> +        return
> +
> +    if ref.startswith('refs/heads/'):
> +        branch = ref[len('refs/heads/'):]
> +        export_branch(repo, branch)
> +    elif ref.startswith('refs/tags/'):
> +        tag = ref[len('refs/tags/'):]
> +        export_tag(repo, tag)
> +
> +def main(args):
> +    global prefix, dirname, marks, cache, branches
> +
> +    alias = args[1]
> +    url = args[2]
> +
> +    gitdir = os.environ['GIT_DIR']
> +    dirname = os.path.join(gitdir, 'hg')
> +    cache = {}
> +    branches = {}
> +
> +    repo = get_repo(url, alias)
> +    prefix = 'refs/hg/%s' % alias
> +
> +    if not os.path.exists(dirname):
> +        os.makedirs(dirname)
> +
> +    marks_path = os.path.join(dirname, 'marks-hg')
> +    try:
> +        fp = open(marks_path, 'r')
> +        marks = json.load(fp)
> +        fp.close()
> +    except IOError:
> +        marks = {}
> +
> +    line = True
> +    while (line):
> +        line = sys.stdin.readline().strip()
> +        if line == '':
> +            break
> +        args = line.split()
> +        cmd = args[0]
> +        if cmd == 'capabilities':
> +            do_capabilities(repo, args)
> +        elif cmd == 'list':
> +            do_list(repo, args)
> +        elif cmd == 'import':
> +            do_import(repo, args)
> +        sys.stdout.flush()
> +
> +    fp = open(marks_path, 'w')
> +    json.dump(marks, fp)
> +    fp.close()
> +
> +sys.exit(main(sys.argv))
> 

Overall, this looks like plain scripting in python rather than anything
oo'ish, but that's okay and probably dictated by the remote helper
interface and/or the existing exporter.

I'm all for an improvement in that area, but still feel we'd need a
combined effort rather than yet another start.

Michael

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-18 13:18 ` Michael J Gruber
@ 2012-10-18 14:26   ` Felipe Contreras
  0 siblings, 0 replies; 27+ messages in thread
From: Felipe Contreras @ 2012-10-18 14:26 UTC (permalink / raw)
  To: Michael J Gruber
  Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow

On Thu, Oct 18, 2012 at 3:18 PM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> Felipe Contreras venit, vidit, dixit 17.10.2012 14:58:
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>
>> I've looked at many hg<->git tools and none satisfy me. Too complicated, or too
>> slow, or to difficult to setup, etc.
>
> It's in an unsatisfying state, I agree. We have a great remote helper
> infrastructure, but neither git-svn nor git-cvsimport/export use it, and
> remote-hg is not in git.git.
>
> git-svn used to be our killer feature! (It's becoming hard to maintain,
> though.)
>
> git-hg (in the shape of a remote helper) could be our next killer
> feature, finally leading to our world domination ;)
>
>> The only one I've liked so far is hg-fast-export[1], which is indeed fast,
>> relatively simple, and relatively easy to use. But it's not properly maintained
>> any more.
>>
>> So, I decided to write my own from scratch, using hg-fast-export as
>> inspiration, and voila.
>>
>> This one doesn't have any dependencies, just put it into your $PATH, and you
>> can clone and fetch hg repositories. More importantly to me; the code is
>> simple, and easy to maintain.
>
> Well, it still has hg as a dependency. All our remote
> integration/helpers suffer from that. At least, every hg install comes
> with the modules, whereas svn is a beast (apr and such) that often comes
> without the language bindings.

Yeah, but there's no way around that, even if we write some code to
access the repository, it's quite likely that it will change. Unlike
git, mercurial developers expect people to access the repository
through mercurial API, not directly, and that's probably what we
should do if we want to avoid problems.

>>  contrib/remote-hd/git-remote-hg | 231 ++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 231 insertions(+)
>>  create mode 100755 contrib/remote-hd/git-remote-hg
>
> That diffstat looks great (sans tests, of course; it's contrib), but
> there's no push functionality, and that is usually the most difficult
> part all helpers have to implement: Not only the push interaction, but
> also making sure that commits don't get duped in a roundtrip (git fetch
> from vcs, push to vcs, git fetch from vcs).

Yeah, it will probably require much more code, but I think by far the
most important feature is fetching.

> Just cloning and fetching can be done most easily with a shared worktree
> and scripting around "hg up" and "git commit -A" in some flavour.

Yea, but that will be dead slow.

>> diff --git a/contrib/remote-hd/git-remote-hg b/contrib/remote-hd/git-remote-hg
>> new file mode 100755
>> index 0000000..9157b30
>> --- /dev/null
>> +++ b/contrib/remote-hd/git-remote-hg
>> @@ -0,0 +1,231 @@
>> +#!/usr/bin/python2
>> +
>> +# Inspired by Rocco Rutte's hg-fast-export
>> +
>> +# Just copy to your ~/bin, or anywhere in your $PATH.
>> +# Then you can clone with:
>> +# hg::file:///path/to/mercurial/repo/
>> +
>> +from mercurial import hg, ui
>> +
>> +import re
>> +import sys
>> +import os
>> +import json
>> +
>> +def die(msg, *args):
>> +    print >> sys.stderr, 'ERROR:', msg % args
>> +    sys.exit(1)
>
> While we don't have to code for py3, avoiding '>>' will help us later.
> (It got removed in py3.) sys.sdterr.write() should be most portable.

All right.

>> +def gitmode(flags):
>> +    return 'l' in flags and '120000' or 'x' in flags and '100755' or '100644'
>> +
>> +def export_file(fc):
>> +    if fc.path() == '.hgtags':
>
> Is this always relative? Just wondering, dunno.

AFAIK, why wouldn't it?

>> +def get_repo(path, alias):
>> +    myui = ui.ui()
>> +    myui.setconfig('ui', 'interactive', 'off')
>> +    repo = hg.repository(myui, path)
>> +    return repo
>
> Is there a reason to spell this out, e.g.: Why not
>
> return hg.repository(myui, path)

No reason, but I already have patches on top of this.

>> +def hg_branch(b):
>> +    if b == 'master':
>> +        return 'default'
>> +    return b
>> +
>> +def git_branch(b):
>> +    if b == 'default':
>> +        return 'master'
>> +    return b
>> +
>> +def export_tag(repo, tag):
>> +    global prefix
>> +    print "reset %s/tags/%s" % (prefix, tag)
>> +    print "from :%s" % (repo[tag].rev() + 1)
>> +    print
>> +
>> +def export_branch(repo, branch):
>> +    global prefix, marks, cache, branches
>> +
>> +    heads = branches[hg_branch(branch)]
>> +
>> +    # verify there's only one head
>> +    if (len(heads) > 1):
>> +        die("Branch '%s' has more than one head" % hg_branch(branch))
>
> We have to deal with this at some point... Do you support "hg
> bookmarks"? They'd be an option, or we implement better detached head
> handling in git...

I already updated this, I converted it to a warning and just picked a
random head.

Adding support for bookmarks should be easy, it's just a matter of
deciding how to differentiate branches from bookmarks. Perhaps
'refs/heads/bookmarks/foo'.

>> +    revs = [rev for rev in revs if not cache.get(rev, False)]
>> +
>> +    for rev in revs:
>
> Those lines set up the list just so that you iterate over it later.
> Please don't do this (unless you know that revs is very small).
>
> for rev in revs:
>   if cache.get(rev, False):
>     continue
>
> is more performant. You can reduce this further to
>
> count=0
> for rev in repo.revs('%u:%u' % (tip, head)):
>   if cache.get(rev, False):
>     continue
>
> which is even more performant generally, and especially if repo.revs()
> returns an iterator rather than a list.
>
> [Yes, you could use lambda+filter, but let's not get religious. The
> above is simple and pythonic.]

And we wouldn't be able to calculate the progress: len(revs).

>> +
>> +        c = repo[rev]
>> +        (manifest, user, (time, tz), files, desc, extra) = repo.changelog.read(c.node())
>
> Same here, you introduce c just for the next line (unless I'm mistaken).

The second line is already too big.

>> +            print "from :%s" % (parents[0] + 1)
>
> What is this +1 offset that is appearing here and there?

Marks start from 1, no? If not, this can be removed.

>> +def do_list(repo, args):
>> +    global branches
>> +
>> +    head = repo.dirstate.branch()
>> +    for branch in repo.branchmap():
>> +        heads = repo.branchheads(branch)
>> +        if len(heads):
>> +            branches[branch] = heads
>
> I'm a bit confused here. repo.brancheads() is a list, no? Is this the
> single head case only? I'd expect [0] of that, but you seem to be
> getting branch names (strings).

No, heads is a list of heads, as nodes.

> Also, if len(heads) == 0 then branches[branch] is undefined or stale. No?

Yes, but we don't care about those branches; they are closed.

> Overall, this looks like plain scripting in python rather than anything
> oo'ish, but that's okay and probably dictated by the remote helper
> interface and/or the existing exporter.

If there was an advantage of creating new classes, I would be all for
it, but I don't see any.

> I'm all for an improvement in that area, but still feel we'd need a
> combined effort rather than yet another start.

Me too, but as far a sverre's remote-hg code goes, there isn't even a
branch to work from.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-18  8:47             ` Johannes Schindelin
  2012-10-18  9:03               ` Felipe Contreras
@ 2012-10-21 18:03               ` Felipe Contreras
  2012-10-21 20:03                 ` Johannes Schindelin
  1 sibling, 1 reply; 27+ messages in thread
From: Felipe Contreras @ 2012-10-21 18:03 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Sverre Rabbelier, git, Junio C Hamano, Ilari Liusvaara,
	Daniel Barkalow

On Thu, Oct 18, 2012 at 10:47 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:

> Without push support, remote-hg is useless to me. Without regression tests
> proving that it is rock solid, I will not use remote-hg.

Done and done. My remote-hg now has support for pushing, all in less
than 500 lines of code. It also manages to pass all 14 of the
"extensive tests" of your remote-hg. Anything else?

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-21 18:03               ` Felipe Contreras
@ 2012-10-21 20:03                 ` Johannes Schindelin
  2012-10-21 20:31                   ` Felipe Contreras
  0 siblings, 1 reply; 27+ messages in thread
From: Johannes Schindelin @ 2012-10-21 20:03 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Sverre Rabbelier, git, Junio C Hamano, Ilari Liusvaara,
	Daniel Barkalow

Hi Felipe,

On Sun, 21 Oct 2012, Felipe Contreras wrote:

> On Thu, Oct 18, 2012 at 10:47 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> 
> > Without push support, remote-hg is useless to me. Without regression
> > tests proving that it is rock solid, I will not use remote-hg.
> 
> Done and done. My remote-hg now has support for pushing, all in less
> than 500 lines of code. It also manages to pass all 14 of the "extensive
> tests" of your remote-hg. Anything else?

While I think that a lot of effort was duplicated now, and while I am
still interested in less handwaving arguments than "I find the code
bloated", I will compare the performance on both hg and openjdk and if I
do not find any issues, have a look at the code, too.

That will have to wait until I am home in a bit more than a week, though.

Ciao,
Johannes

P.S.: Sverre's remote-hg does not really handle octopus merges. It is
incomplete. I had a good plan how to complete it (see the msysGit wiki
page about remote-hg) but lacked the time to implement it (the problem is
that hg does not have octopus merges, and we want things to be
bidirectional).

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-21 20:03                 ` Johannes Schindelin
@ 2012-10-21 20:31                   ` Felipe Contreras
  0 siblings, 0 replies; 27+ messages in thread
From: Felipe Contreras @ 2012-10-21 20:31 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Sverre Rabbelier, git, Junio C Hamano, Ilari Liusvaara,
	Daniel Barkalow

On Sun, Oct 21, 2012 at 10:03 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Sun, 21 Oct 2012, Felipe Contreras wrote:
>
>> On Thu, Oct 18, 2012 at 10:47 AM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>>
>> > Without push support, remote-hg is useless to me. Without regression
>> > tests proving that it is rock solid, I will not use remote-hg.
>>
>> Done and done. My remote-hg now has support for pushing, all in less
>> than 500 lines of code. It also manages to pass all 14 of the "extensive
>> tests" of your remote-hg. Anything else?
>
> While I think that a lot of effort was duplicated now, and while I am
> still interested in less handwaving arguments than "I find the code
> bloated",

The only way to avoid duplicated effort is to work together, and I've
yet to see where the remote-hg branch is supposed to be (without any
msysgit stuff), so that other people can give it a try, and propose
changes.

> P.S.: Sverre's remote-hg does not really handle octopus merges. It is
> incomplete. I had a good plan how to complete it (see the msysGit wiki
> page about remote-hg) but lacked the time to implement it (the problem is
> that hg does not have octopus merges, and we want things to be
> bidirectional).

Yeah, I'm aware mercurial doesn't have those, that's why I didn't
implement that, other tools do something similar as you mention in the
wiki, but the code is rather convoluted.

Note that this doesn't prevent things to be bidirectional, what it
prevents is using this tool to export git repositories to mercurial,
not the other way around. If you do an octopus merge on a repository
that you know is going to end in mercurial, that's just asking for
trouble, and complaints from the other users of that repo.

Anyway, I don't think that feature is that important, what is more
important is to make sure renames and branches are stored properly. I
have tests that check that the output is the the same as hg-git, but
I'm still not there.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add new git-remote-hd helper
  2012-10-18  6:12       ` Sverre Rabbelier
  2012-10-18  9:10         ` Felipe Contreras
@ 2012-10-26  9:02         ` Felipe Contreras
  1 sibling, 0 replies; 27+ messages in thread
From: Felipe Contreras @ 2012-10-26  9:02 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Jeff King, git, Junio C Hamano, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow

On Thu, Oct 18, 2012 at 8:12 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> On Wed, Oct 17, 2012 at 10:18 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> Right now I've just added an error when using remote repositories. But
>> it seems there's no way around it; if we want to have support for
>> remote repos, we need to make a local clone.
>
> My git-remote-hg does the local clone into .git/ using a hash of the
> url (although you could just as well use urlencode, basically any way
> to safely use a url as a directory name). Have a look if you want.

I can't find that code.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2012-10-26  9:02 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-17 12:58 [PATCH] Add new git-remote-hd helper Felipe Contreras
2012-10-17 16:03 ` Johannes Schindelin
2012-10-17 16:38   ` Felipe Contreras
2012-10-17 17:39     ` Johannes Schindelin
2012-10-17 18:12       ` Felipe Contreras
2012-10-17 18:18         ` Sverre Rabbelier
2012-10-17 18:33           ` Felipe Contreras
2012-10-18  8:47             ` Johannes Schindelin
2012-10-18  9:03               ` Felipe Contreras
2012-10-18  9:10                 ` Johannes Schindelin
2012-10-18  9:26                 ` Junio C Hamano
2012-10-18  9:38                   ` Felipe Contreras
2012-10-18  9:42                     ` Matthieu Moy
2012-10-21 18:03               ` Felipe Contreras
2012-10-21 20:03                 ` Johannes Schindelin
2012-10-21 20:31                   ` Felipe Contreras
2012-10-17 22:59 ` Jeff King
2012-10-18  3:44   ` Felipe Contreras
2012-10-18  5:18     ` Felipe Contreras
2012-10-18  6:12       ` Sverre Rabbelier
2012-10-18  9:10         ` Felipe Contreras
2012-10-18  9:13           ` Johannes Schindelin
2012-10-18  9:22             ` Felipe Contreras
2012-10-26  9:02         ` Felipe Contreras
2012-10-18  8:48     ` Felipe Contreras
2012-10-18 13:18 ` Michael J Gruber
2012-10-18 14:26   ` Felipe Contreras

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).