threads / patch / 35583

patchremote-hg: do not fail on invalid bookmarks

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

## tl;dr

7 messages between Dec 29, 2013 and Jan 6, 2014. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Antoine Pelisse· Dec 29, 2013, 11:30 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.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 contrib/remote-helpers/git-remote-hg | 3 +++
 1 file changed, 3 insertions(+)
Show changes to contrib/remote-helpers/git-remote-hg +3 −0
diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index eb89ef6..12d850e 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -625,6 +625,9 @@ def list_head(repo, cur):
 def do_list(parser):
     repo = parser.repo
     for bmark, node in bookmarks.listbookmarks(repo).iteritems():
+        if node == '0000000000000000000000000000000000000000':
+            warn("Ignoring invalid bookmark '%s'", bmark)
+            continue
         bmarks[bmark] = repo[node]
 
     cur = repo.dirstate.branch()
-- 
1.8.5.1.97.g8d784da.dirty
Mike Hommey· Dec 29, 2013, 22:24 UTC · re: Antoine Pelisse · lore

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

On Sun, Dec 29, 2013 at 12:30:02PM +0100, Antoine Pelisse wrote:
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.

It's not invalid, it's used to indicate deleted bookmarks. (Tags have the same property)

Mike
Antoine Pelisse· Dec 30, 2013, 07:41 UTC · re: Mike Hommey · lore

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

On Sun, Dec 29, 2013 at 11:24 PM, Mike Hommey <mh@glandium.org> wrote:
Show 12 quoted lines
> On Sun, Dec 29, 2013 at 12:30:02PM +0100, Antoine Pelisse wrote:
>> 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.
>
> It's not invalid, it's used to indicate deleted bookmarks. (Tags have
> the same property)

Hey Mike, Indeed, I don't know how I ended-up with such a bookmark, but it prevented me from git-cloning the repository (and the backtrace was not very helpful at first). But I'm still not sure what you mean by "deleted bookmarks" ? I guess it's not "hg bookmark --delete", as it would not be listed at all. Is it "hg strip some_changeset" that end-up deleting the bookmarked changeset ? I think I've tested this use-case and it moved the bookmark to a parent changeset.

Antoine,
Mike Hommey· Dec 30, 2013, 23:22 UTC · re: Antoine Pelisse · lore

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

On Mon, Dec 30, 2013 at 08:41:13AM +0100, Antoine Pelisse wrote:
Show 23 quoted lines
> On Sun, Dec 29, 2013 at 11:24 PM, Mike Hommey <mh@glandium.org> wrote:
> > On Sun, Dec 29, 2013 at 12:30:02PM +0100, Antoine Pelisse wrote:
> >> 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.
> >
> > It's not invalid, it's used to indicate deleted bookmarks. (Tags have
> > the same property)
> 
> Hey Mike,
> Indeed, I don't know how I ended-up with such a bookmark, but it
> prevented me from git-cloning the repository (and the backtrace was
> not very helpful at first).
> But I'm still not sure what you mean by "deleted bookmarks" ?
> I guess it's not "hg bookmark --delete", as it would not be listed at
> all. Is it "hg strip some_changeset" that end-up deleting the
> bookmarked changeset ? I think I've tested this use-case and it moved
> the bookmark to a parent changeset.

Mmmm after looking at the mercurial code, it looks like i was wrong and bookmarks are not handled like tags. You can actually create such a bookmark on purpose with:

$ hg bookmark -r null foo
Then, if you do, say:

$ hg up -r foo $ echo a > a $ hg add a $ hg commit -m a

Then you end up with a completely new head with no ancestors in common with the others.

In git terms,

$ hg bookmark -r null foo $ hg up -r foo

is equivalent to
$ git checkout --orphan foo
But git never creates an actual ref in that case.
Mike
Junio C Hamano· Dec 30, 2013, 19:12 UTC · re: Mike Hommey · lore

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

Mike Hommey <mh@glandium.org> writes:
Show 12 quoted lines
> On Sun, Dec 29, 2013 at 12:30:02PM +0100, Antoine Pelisse wrote:
>> 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.
>
> It's not invalid, it's used to indicate deleted bookmarks. (Tags have
> the same property)

So what should remote-hg behave in such a case, other than what Antoine's change does (which is to, IIRC, warn, ignore and continue).

Torsten Bögershausen· Jan 6, 2014, 13:52 UTC · re: Antoine Pelisse · lore

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

On 2013-12-29 12.30, Antoine Pelisse wrote:
Show 29 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.
> 
> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
> ---
>  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 eb89ef6..12d850e 100755
> --- a/contrib/remote-helpers/git-remote-hg
> +++ b/contrib/remote-helpers/git-remote-hg
> @@ -625,6 +625,9 @@ def list_head(repo, cur):
>  def do_list(parser):
>      repo = parser.repo
>      for bmark, node in bookmarks.listbookmarks(repo).iteritems():
> +        if node == '0000000000000000000000000000000000000000':
> +            warn("Ignoring invalid bookmark '%s'", bmark)
> +            continue
>          bmarks[bmark] = repo[node]
>  
>      cur = repo.dirstate.branch()
> 
(Side note: ap/remote-hg-skip-null-bookmarks)

When I run the test-suite like this: ~/projects/git/git.pu/contrib/remote-helpers$ debug=t verbose=t make test-hg-hg-git.sh

All 11 test cases fail on my systems (Debian Wheezy and Mac OS X):
[snip]
WARNING: Ignoring invalid bookmark 'master'
To hg::../hgrepo-git
 ! [remote rejected] master -> master
error: failed to push some refs to 'hg::../hgrepo-git'
not ok 1 - executable bit
#	
[snip]
Antoine Pelisse· Jan 6, 2014, 21:51 UTC · re: Torsten Bögershausen · lore

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

Thanks for noticing, I can reproduce at work, I will try to come-up with an improved version soon,

Cheers, Antoine

On Mon, Jan 6, 2014 at 2:52 PM, Torsten Bögershausen <tboegi@web.de> wrote:
Show 46 quoted lines
> On 2013-12-29 12.30, Antoine Pelisse wrote:
>> 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.
>>
>> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
>> ---
>>  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 eb89ef6..12d850e 100755
>> --- a/contrib/remote-helpers/git-remote-hg
>> +++ b/contrib/remote-helpers/git-remote-hg
>> @@ -625,6 +625,9 @@ def list_head(repo, cur):
>>  def do_list(parser):
>>      repo = parser.repo
>>      for bmark, node in bookmarks.listbookmarks(repo).iteritems():
>> +        if node == '0000000000000000000000000000000000000000':
>> +            warn("Ignoring invalid bookmark '%s'", bmark)
>> +            continue
>>          bmarks[bmark] = repo[node]
>>
>>      cur = repo.dirstate.branch()
>>
> (Side note: ap/remote-hg-skip-null-bookmarks)
>
> When I run the test-suite like this:
> ~/projects/git/git.pu/contrib/remote-helpers$ debug=t verbose=t make test-hg-hg-git.sh
>
> All 11 test cases fail on my systems (Debian Wheezy and Mac OS X):
> [snip]
> WARNING: Ignoring invalid bookmark 'master'
> To hg::../hgrepo-git
>  ! [remote rejected] master -> master
> error: failed to push some refs to 'hg::../hgrepo-git'
> not ok 1 - executable bit
> #
> [snip]
>
>

← back to recent threads