threads / patch / 36237

v2remote-hg: do not fail on invalid bookmarks

Subject: [PATCH v2] remote-hg: do not fail on invalid bookmarks

## tl;dr

2 messages between Mar 19, 2014 and Mar 19, 2014. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Max Horn· Mar 19, 2014, 21:42 UTC · lore

Mercurial can have bookmarks pointing to "nullid" (the empty root revision), while Git can not have references to it. When cloning or fetching from a Mercurial repository that has such a bookmark, the import will fail because git-remote-hg will not be able to create the corresponding reference.

Warn the user about the invalid reference, and continue the import, instead of stopping right away.

Also add some test cases for this issue.
Reported-by: Antoine Pelisse <apelisse@gmail.com>
Signed-off-by: Max Horn <max@quendi.de>
---
 contrib/remote-helpers/git-remote-hg |  6 +++++
 contrib/remote-helpers/test-hg.sh    | 48 ++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)
Show changes to 2 files +54 −0

contrib/remote-helpers/git-remote-hg, contrib/remote-helpers/test-hg.sh

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index eb89ef6..49b2c2e 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -625,6 +625,12 @@ def list_head(repo, cur):
 def do_list(parser):
     repo = parser.repo
     for bmark, node in bookmarks.listbookmarks(repo).iteritems():
+        if node == '0000000000000000000000000000000000000000':
+            if fake_bmark == 'default' and bmark == 'master':
+                pass
+            else:
+                warn("Ignoring invalid bookmark '%s'", bmark)
+                continue
         bmarks[bmark] = repo[node]
 
     cur = repo.dirstate.branch()
diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index a933b1e..8d01b32 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -772,4 +772,52 @@ test_expect_success 'remote double failed push' '
 	)
 '
 
+test_expect_success 'clone remote with master null bookmark' '
+	test_when_finished "rm -rf gitrepo* hgrepo*" &&
+
+	(
+	hg init hgrepo &&
+	cd hgrepo &&
+	echo a >a &&
+	hg add a &&
+	hg commit -m a &&
+	hg bookmark -r null master
+	) &&
+
+	git clone "hg::hgrepo" gitrepo &&
+	check gitrepo HEAD a
+'
+
+test_expect_success 'clone remote with default null bookmark' '
+	test_when_finished "rm -rf gitrepo* hgrepo*" &&
+
+	(
+	hg init hgrepo &&
+	cd hgrepo &&
+	echo a >a &&
+	hg add a &&
+	hg commit -m a &&
+	hg bookmark -r null -f default
+	) &&
+
+	git clone "hg::hgrepo" gitrepo &&
+	check gitrepo HEAD a
+'
+
+test_expect_success 'clone remote with generic null bookmark' '
+	test_when_finished "rm -rf gitrepo* hgrepo*" &&
+
+	(
+	hg init hgrepo &&
+	cd hgrepo &&
+	echo a >a &&
+	hg add a &&
+	hg commit -m a &&
+	hg bookmark -r null bmark
+	) &&
+
+	git clone "hg::hgrepo" gitrepo &&
+	check gitrepo HEAD a
+'
+
 test_done

-- 
1.9.0.7.ga299b13
Junio C Hamano· Mar 19, 2014, 21:51 UTC · re: Max Horn · lore

Re: [PATCH v2] remote-hg: do not fail on invalid bookmarks

Max Horn <max@quendi.de> writes:
Show 8 quoted lines
> Mercurial can have bookmarks pointing to "nullid" (the empty root
> revision), while Git can not have references to it.
> When cloning or fetching from a Mercurial repository that has such a
> bookmark, the import will fail because git-remote-hg will not be able to
> create the corresponding reference.
>
> Warn the user about the invalid reference, and continue the import,
> instead of stopping right away.

The text above is identical to what Antoine wrote in e8d48743 (remote-hg: do not fail on invalid bookmarks, 2013-12-29); I'd assume that this is to replace it.

But the code seems to do more, and I think that is related to the detailed analysis you dug up from the archive earlier and then summarised in your $gmane/244440. Can you say a bit more about these fake-bmark and bmark checking like you did in that original 3-patch series?

Thanks.
Show 83 quoted lines
> Also add some test cases for this issue.
>
> Reported-by: Antoine Pelisse <apelisse@gmail.com>
> Signed-off-by: Max Horn <max@quendi.de>
> ---
>  contrib/remote-helpers/git-remote-hg |  6 +++++
>  contrib/remote-helpers/test-hg.sh    | 48 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+)
>
> diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
> index eb89ef6..49b2c2e 100755
> --- a/contrib/remote-helpers/git-remote-hg
> +++ b/contrib/remote-helpers/git-remote-hg
> @@ -625,6 +625,12 @@ def list_head(repo, cur):
>  def do_list(parser):
>      repo = parser.repo
>      for bmark, node in bookmarks.listbookmarks(repo).iteritems():
> +        if node == '0000000000000000000000000000000000000000':
> +            if fake_bmark == 'default' and bmark == 'master':
> +                pass
> +            else:
> +                warn("Ignoring invalid bookmark '%s'", bmark)
> +                continue
>          bmarks[bmark] = repo[node]
>  
>      cur = repo.dirstate.branch()
> diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
> index a933b1e..8d01b32 100755
> --- a/contrib/remote-helpers/test-hg.sh
> +++ b/contrib/remote-helpers/test-hg.sh
> @@ -772,4 +772,52 @@ test_expect_success 'remote double failed push' '
>  	)
>  '
>  
> +test_expect_success 'clone remote with master null bookmark' '
> +	test_when_finished "rm -rf gitrepo* hgrepo*" &&
> +
> +	(
> +	hg init hgrepo &&
> +	cd hgrepo &&
> +	echo a >a &&
> +	hg add a &&
> +	hg commit -m a &&
> +	hg bookmark -r null master
> +	) &&
> +
> +	git clone "hg::hgrepo" gitrepo &&
> +	check gitrepo HEAD a
> +'
> +
> +test_expect_success 'clone remote with default null bookmark' '
> +	test_when_finished "rm -rf gitrepo* hgrepo*" &&
> +
> +	(
> +	hg init hgrepo &&
> +	cd hgrepo &&
> +	echo a >a &&
> +	hg add a &&
> +	hg commit -m a &&
> +	hg bookmark -r null -f default
> +	) &&
> +
> +	git clone "hg::hgrepo" gitrepo &&
> +	check gitrepo HEAD a
> +'
> +
> +test_expect_success 'clone remote with generic null bookmark' '
> +	test_when_finished "rm -rf gitrepo* hgrepo*" &&
> +
> +	(
> +	hg init hgrepo &&
> +	cd hgrepo &&
> +	echo a >a &&
> +	hg add a &&
> +	hg commit -m a &&
> +	hg bookmark -r null bmark
> +	) &&
> +
> +	git clone "hg::hgrepo" gitrepo &&
> +	check gitrepo HEAD a
> +'
> +
>  test_done

← back to recent threads