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

["Torsten Bögershausen" <tboegi@web.de>]

On 2014-03-21 12.36, Max Horn wrote:
All tests passed :-), thanks from my side.
comments inline, some are debatable
> 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 failed because git-remote-hg was not be able to create the
> corresponding reference.
>
> Warn the user about the invalid reference, and do not advertise these
> bookmarks as head refs, but otherwise continue the import. In
> particular, we still keep track of the fact that the remote repository
> has a bookmark of the given name, in case the user wants to modify that
> bookmark.
>
> Also add some test cases for this issue.
s/some test cases/test cases/
>
> Reported-by: Antoine Pelisse <apelisse@gmail.com>
> Signed-off-by: Max Horn <max@quendi.de>
> ---
> This is a different fix than in my previous attempts. I thought
> a bit more about the issue, and determined that the previous fix,
> while working, was not really correct: It is wrong to
> treat nullid bookmarks as if they are non-existent; if e.g.
> the user wants to modify the bookmark from git, we need to
> into account that the remote already has a bookmark with that name.
> Indeed, I extended the new test cases to cover this aspect.
> With the previous fix, the new tests would fail upon pushing,
> with the new one, they work.
>
>  contrib/remote-helpers/git-remote-hg |  5 ++-
>  contrib/remote-helpers/test-hg.sh    | 67 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 71 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
> index eb89ef6..36b5261 100755
> --- a/contrib/remote-helpers/git-remote-hg
> +++ b/contrib/remote-helpers/git-remote-hg
> @@ -643,7 +643,10 @@ def do_list(parser):
>              print "? refs/heads/branches/%s" % gitref(branch)
>  
>      for bmark in bmarks:
> -        print "? refs/heads/%s" % gitref(bmark)
> +        if  bmarks[bmark].hex() == '0000000000000000000000000000000000000000':
> +            warn("Ignoring invalid bookmark '%s'", bmark)
> +        else:
> +            print "? refs/heads/%s" % gitref(bmark)
>  
>      for tag, node in repo.tagslist():
>          if tag == 'tip':
> diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
> index a933b1e..f5d0d97 100755
> --- a/contrib/remote-helpers/test-hg.sh
> +++ b/contrib/remote-helpers/test-hg.sh
> @@ -772,4 +772,71 @@ test_expect_success 'remote double failed push' '
>  	)
>  '
>  
> +test_expect_success 'clone remote with master null bookmark, then push to the bookmark' '
> +	test_when_finished "rm -rf gitrepo* hgrepo*" &&
> +
> +	(
> +	hg init hgrepo &&
> +	cd hgrepo &&
Minor:
We can change the order here, to make the "cd hgrepo" the first line in the subshell:

+	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 &&
And here we do "cd", and this should be done in a subshell
> +	cd gitrepo &&
> +	git checkout --quiet -b master &&
> +	echo b >b &&
> +	git add b &&
> +	git commit -m b &&
> +	git push origin master
> +'
> +
> +test_expect_success 'clone remote with default null bookmark, then push to the bookmark' '
> +	test_when_finished "rm -rf gitrepo* hgrepo*" &&
> +
> +	(
> +	hg init hgrepo &&
> +	cd hgrepo &&
(Same minor as above)
> +	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 &&
> +	cd gitrepo &&
> +	git checkout --quiet -b default &&
> +	echo b >b &&
> +	git add b &&
> +	git commit -m b &&
> +	git push origin default
> +'
> +
> +test_expect_success 'clone remote with generic null bookmark, then push to the bookmark' '
> +	test_when_finished "rm -rf gitrepo* hgrepo*" &&
> +
> +	(
> +	hg init hgrepo &&
> +	cd hgrepo &&
(Same as above)
> +	echo a >a &&
> +	hg add a &&
> +	hg commit -m a &&
> +	hg bookmark -r null bmark
> +	) &&
> +
> +	git clone "hg::hgrepo" gitrepo &&
> +	check gitrepo HEAD a &&
> +	cd gitrepo &&
Sub-shell missing
> +	git checkout --quiet -b bmark &&
> +	git remote -v &&
> +	echo b >b &&
> +	git add b &&
> +	git commit -m b &&
> +	git push origin bmark
> +'
> +
>  test_done