[PATCH 0/4] remote-hg: small fixes

Hi,

Just a few fixes, nothing major.

Felipe Contreras (3):
  remote-hg: fix compatibility with older versions of hg
  remote-hg: try the 'tip' if no checkout present
  remote-hg: avoid bad refs

Ramkumar Ramachandra (1):
  remote-hg: add missing config for basic tests

 contrib/remote-helpers/git-remote-hg | 7 ++++++-
 contrib/remote-helpers/test-hg.sh    | 9 +++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

-- 
1.8.0

From: Ramkumar Ramachandra <redacted>

'hg commit' fails otherwise in some versiosn of mercurial because of
the missing user information. Other versions simply throw a warning and
guess though.

Signed-off-by: Ramkumar Ramachandra <redacted>
Signed-off-by: Felipe Contreras <redacted>
---
 contrib/remote-helpers/test-hg.sh | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index 40e6e3c..031dcbd 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh

@@ -29,6 +29,15 @@ check () {
 	test_cmp expected actual
 }
 
+setup () {
+	(
+	echo "[ui]"
+	echo "username = A U Thor <author@example.com>"
+	) >> "$HOME"/.hgrc
+}
+
+setup
+
 test_expect_success 'cloning' '
   test_when_finished "rm -rf gitrepo*" &&
 

-- 
1.8.0

Turns out repo.revs was introduced quite late, and it doesn't do
anything fancy for our refspec; only list all the numbers in that range.

Signed-off-by: Felipe Contreras <redacted>
---
 contrib/remote-helpers/git-remote-hg | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 1d46838..c6d0367 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg

@@ -294,7 +294,7 @@ def export_ref(repo, name, kind, head):
     if tip and tip == head.rev():
         # nothing to do
         return
-    revs = repo.revs('%u:%u' % (tip, head))
+    revs = xrange(tip, head.rev() + 1)
     count = 0
 
     revs = [rev for rev in revs if not marks.is_marked(rev)]

-- 
1.8.0

There's no concept of HEAD in mercurial, but let's try our best to do
something sensible.

Signed-off-by: Felipe Contreras <redacted>
---
 contrib/remote-helpers/git-remote-hg | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index c6d0367..3cdc1e2 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg

@@ -440,6 +440,8 @@ def list_head(repo, cur):
         head = cur
         node = repo['.']
         if not node:
+            node = repo['tip']
+        if not node:
             return
         if head == 'default':
             head = 'master'

-- 
1.8.0

Turns out fast-export throws bad 'reset' commands because of a behavior
in transport-helper that is not even needed.

Either way, better to ignore them, otherwise the user will get warnings
when we OK them.

Signed-off-by: Felipe Contreras <redacted>
---
 contrib/remote-helpers/git-remote-hg | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 3cdc1e2..48f8f5d 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg

@@ -704,6 +704,9 @@ def do_export(parser):
         elif ref.startswith('refs/tags/'):
             tag = ref[len('refs/tags/'):]
             parser.repo.tag([tag], node, None, True, None, {})
+        else:
+            # transport-helper/fast-export bugs
+            continue
         print "ok %s" % ref
 
     print

-- 
1.8.0

On Mon, Nov 12, 2012 at 06:41:05PM +0100, Felipe Contreras wrote:

From: Ramkumar Ramachandra <redacted>

'hg commit' fails otherwise in some versiosn of mercurial because of

s/versiosn/versions/

+setup () {
+	(
+	echo "[ui]"
+	echo "username = A U Thor [off-list ref]"
+	) >> "$HOME"/.hgrc
+}

This makes sense, but I wonder if we should use something different from
the git author ident set up by the test scripts, just to double check
that we do not have any bugs in confusing the two during the import.

Something like "H G Wells [off-list ref]" would work, and satisfies
my deep-seated desire for bad puns.

-Peff

On Mon, Nov 12, 2012 at 9:32 PM, Jeff King [off-list ref] wrote:

On Mon, Nov 12, 2012 at 06:41:05PM +0100, Felipe Contreras wrote:

quoted

From: Ramkumar Ramachandra <redacted>

'hg commit' fails otherwise in some versiosn of mercurial because of

s/versiosn/versions/

quoted

+setup () {
+     (
+     echo "[ui]"
+     echo "username = A U Thor [off-list ref]"
+     ) >> "$HOME"/.hgrc
+}

This makes sense, but I wonder if we should use something different from
the git author ident set up by the test scripts, just to double check
that we do not have any bugs in confusing the two during the import.

I don't know, but these tests would not check for any of those issues.
When such tests are added I would prefer the author to use to be
explicitly defined, but lets see.

-- 
Felipe Contreras

This patch fixes my original problem. So,
Reported-by: Ramkumar Ramachandra <redacted>
Tested-by: Ramkumar Ramachandra <redacted>

However, test 4 in test-hg.sh still fails for me:

--- expected    2012-11-13 05:22:57.946637384 +0000
+++ actual      2012-11-13 05:22:57.946637384 +0000

@@ -1,2 +1,2 @@
-zero
-refs/heads/master
+feature-a
+refs/heads/feature-a

What is going on?

Ram

On Tue, Nov 13, 2012 at 04:46:36AM +0100, Felipe Contreras wrote:

quoted

quoted

+setup () {
+     (
+     echo "[ui]"
+     echo "username = A U Thor [off-list ref]"
+     ) >> "$HOME"/.hgrc
+}

This makes sense, but I wonder if we should use something different from
the git author ident set up by the test scripts, just to double check
that we do not have any bugs in confusing the two during the import.

I don't know, but these tests would not check for any of those issues.
When such tests are added I would prefer the author to use to be
explicitly defined, but lets see.

It's OK if we do not add more explicit tests at this point. I'd just
rather set a safer precedent on the off chance that it might catch
something in a later test, just as we use separate GIT_AUTHOR_* and
GIT_COMMITTER_* in the rest of the test suite. If the choice were not
completely arbitrary and had some maintenance cost, I might be more
concerned, but as far as I can tell, one name is as good as another at
this point.

Any objection to me marking it up as I apply?

-Peff

On Tue, Nov 13, 2012 at 6:25 AM, Ramkumar Ramachandra
[off-list ref] wrote:

Probably a bug in mercurial, but can be worked around:

--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg

@@ -429,10 +429,18 @@ def get_branch_tip(repo, branch):

     return heads[0]

+def get_current_bookmark(repo):
+    head = bookmarks.readcurrent(repo)
+    if not head:
+        return None
+    if repo[head] != repo['.']:
+        return None
+    return head
+
 def list_head(repo, cur):
     global g_head, bmarks

-    head = bookmarks.readcurrent(repo)
+    head = get_current_bookmark(repo)
     if head:
         node = repo[head]
     else:

-- 
Felipe Contreras

On Tue, Nov 13, 2012 at 6:48 AM, Jeff King [off-list ref] wrote:

Any objection to me marking it up as I apply?

Nope.

-- 
Felipe Contreras