* [PATCH 1/3] remote-hg: do not fail on invalid bookmarks
@ 2014-03-19 12:33 Max Horn
2014-03-19 12:33 ` [PATCH 2/3] remote-hg: allow invalid bookmarks in a few edge cases Max Horn
2014-03-19 12:33 ` [PATCH 3/3] remote-hg: add test cases for null bookmarks Max Horn
0 siblings, 2 replies; 6+ messages in thread
From: Max Horn @ 2014-03-19 12:33 UTC (permalink / raw)
To: git; +Cc: Antoine Pelisse
From: Antoine Pelisse <apelisse@gmail.com>
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>
Signed-off-by: Max Horn <max@quendi.de>
---
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()
--
1.9.0.7.ga299b13
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] remote-hg: allow invalid bookmarks in a few edge cases
2014-03-19 12:33 [PATCH 1/3] remote-hg: do not fail on invalid bookmarks Max Horn
@ 2014-03-19 12:33 ` Max Horn
2014-03-19 13:07 ` Antoine Pelisse
2014-03-19 12:33 ` [PATCH 3/3] remote-hg: add test cases for null bookmarks Max Horn
1 sibling, 1 reply; 6+ messages in thread
From: Max Horn @ 2014-03-19 12:33 UTC (permalink / raw)
To: git; +Cc: Antoine Pelisse
Fix the previous commit to workaround issues with edge cases: Specifically,
remote-hg inserts a fake 'master' branch, unless the cloned hg repository
already contains a 'master' bookmark. If that 'master' bookmark happens
to reference the 'null' commit, the preceding fix ignores it. This
would leave us in an inconsistent state. Avoid this by NOT ignoring
null bookmarks named 'master' or 'default' under suitable circumstances.
Signed-off-by: Max Horn <max@quendi.de>
---
contrib/remote-helpers/git-remote-hg | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 12d850e..49b2c2e 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -626,8 +626,11 @@ 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
+ if fake_bmark == 'default' and bmark == 'master':
+ pass
+ else:
+ warn("Ignoring invalid bookmark '%s'", bmark)
+ continue
bmarks[bmark] = repo[node]
cur = repo.dirstate.branch()
--
1.9.0.7.ga299b13
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] remote-hg: add test cases for null bookmarks
2014-03-19 12:33 [PATCH 1/3] remote-hg: do not fail on invalid bookmarks Max Horn
2014-03-19 12:33 ` [PATCH 2/3] remote-hg: allow invalid bookmarks in a few edge cases Max Horn
@ 2014-03-19 12:33 ` Max Horn
1 sibling, 0 replies; 6+ messages in thread
From: Max Horn @ 2014-03-19 12:33 UTC (permalink / raw)
To: git; +Cc: Antoine Pelisse
Signed-off-by: Max Horn <max@quendi.de>
---
contrib/remote-helpers/test-hg.sh | 48 +++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] remote-hg: allow invalid bookmarks in a few edge cases
2014-03-19 12:33 ` [PATCH 2/3] remote-hg: allow invalid bookmarks in a few edge cases Max Horn
@ 2014-03-19 13:07 ` Antoine Pelisse
2014-03-19 15:00 ` Max Horn
0 siblings, 1 reply; 6+ messages in thread
From: Antoine Pelisse @ 2014-03-19 13:07 UTC (permalink / raw)
To: Max Horn; +Cc: git
Hi Max,
Thank you for working on this.
I believe it would be fair that you forget about patch 1/3 as you fix
it in this patch (2/3).
Also, I think it would be best NOT to integrate a patch (mine) that
breaks a test, as it
would make bisect harder to use.
Thanks,
Antoine
On Wed, Mar 19, 2014 at 1:33 PM, Max Horn <max@quendi.de> wrote:
> Fix the previous commit to workaround issues with edge cases: Specifically,
> remote-hg inserts a fake 'master' branch, unless the cloned hg repository
> already contains a 'master' bookmark. If that 'master' bookmark happens
> to reference the 'null' commit, the preceding fix ignores it. This
> would leave us in an inconsistent state. Avoid this by NOT ignoring
> null bookmarks named 'master' or 'default' under suitable circumstances.
>
> Signed-off-by: Max Horn <max@quendi.de>
> ---
> contrib/remote-helpers/git-remote-hg | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
> index 12d850e..49b2c2e 100755
> --- a/contrib/remote-helpers/git-remote-hg
> +++ b/contrib/remote-helpers/git-remote-hg
> @@ -626,8 +626,11 @@ 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
> + if fake_bmark == 'default' and bmark == 'master':
> + pass
> + else:
> + warn("Ignoring invalid bookmark '%s'", bmark)
> + continue
> bmarks[bmark] = repo[node]
>
> cur = repo.dirstate.branch()
> --
> 1.9.0.7.ga299b13
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] remote-hg: allow invalid bookmarks in a few edge cases
2014-03-19 13:07 ` Antoine Pelisse
@ 2014-03-19 15:00 ` Max Horn
2014-03-19 15:18 ` Antoine Pelisse
0 siblings, 1 reply; 6+ messages in thread
From: Max Horn @ 2014-03-19 15:00 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 2353 bytes --]
Hi Antoine,
On 19.03.2014, at 14:07, Antoine Pelisse <apelisse@gmail.com> wrote:
> Hi Max,
>
> Thank you for working on this.
> I believe it would be fair that you forget about patch 1/3 as you fix
> it in this patch (2/3).
> Also, I think it would be best NOT to integrate a patch (mine) that
> breaks a test, as it
> would make bisect harder to use.
OK, makes sense. I didn't want to step on anybodies feet by hijacking previously made work (however small or big it might be -- I've been burned by this before). Anyway, so I'll squash the first two commits together (or all three even?), and edit the message. But I'd like to properly attribute that you discovered the issue, so perhaps I can add something like "Reported-by: Antoine Pelisse" or so?
Max
>
> Thanks,
> Antoine
>
> On Wed, Mar 19, 2014 at 1:33 PM, Max Horn <max@quendi.de> wrote:
>> Fix the previous commit to workaround issues with edge cases: Specifically,
>> remote-hg inserts a fake 'master' branch, unless the cloned hg repository
>> already contains a 'master' bookmark. If that 'master' bookmark happens
>> to reference the 'null' commit, the preceding fix ignores it. This
>> would leave us in an inconsistent state. Avoid this by NOT ignoring
>> null bookmarks named 'master' or 'default' under suitable circumstances.
>>
>> Signed-off-by: Max Horn <max@quendi.de>
>> ---
>> contrib/remote-helpers/git-remote-hg | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
>> index 12d850e..49b2c2e 100755
>> --- a/contrib/remote-helpers/git-remote-hg
>> +++ b/contrib/remote-helpers/git-remote-hg
>> @@ -626,8 +626,11 @@ 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
>> + if fake_bmark == 'default' and bmark == 'master':
>> + pass
>> + else:
>> + warn("Ignoring invalid bookmark '%s'", bmark)
>> + continue
>> bmarks[bmark] = repo[node]
>>
>> cur = repo.dirstate.branch()
>> --
>> 1.9.0.7.ga299b13
>>
>
[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 235 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] remote-hg: allow invalid bookmarks in a few edge cases
2014-03-19 15:00 ` Max Horn
@ 2014-03-19 15:18 ` Antoine Pelisse
0 siblings, 0 replies; 6+ messages in thread
From: Antoine Pelisse @ 2014-03-19 15:18 UTC (permalink / raw)
To: Max Horn; +Cc: git
On Wed, Mar 19, 2014 at 4:00 PM, Max Horn <max@quendi.de> wrote:
>> Thank you for working on this.
>> I believe it would be fair that you forget about patch 1/3 as you fix
>> it in this patch (2/3).
>> Also, I think it would be best NOT to integrate a patch (mine) that
>> breaks a test, as it
>> would make bisect harder to use.
>
> OK, makes sense. I didn't want to step on anybodies feet by hijacking previously made work (however small or big it might be -- I've been burned by this before). Anyway, so I'll squash the first two commits together (or all three even?), and edit the message. But I'd like to properly attribute that you discovered the issue, so perhaps I can add something like "Reported-by: Antoine Pelisse" or so?
Yes,
I think you can squash all three commits into one, and use the
reported-by line that you mentioned.
Thanks,
Antoine
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-03-19 15:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-19 12:33 [PATCH 1/3] remote-hg: do not fail on invalid bookmarks Max Horn
2014-03-19 12:33 ` [PATCH 2/3] remote-hg: allow invalid bookmarks in a few edge cases Max Horn
2014-03-19 13:07 ` Antoine Pelisse
2014-03-19 15:00 ` Max Horn
2014-03-19 15:18 ` Antoine Pelisse
2014-03-19 12:33 ` [PATCH 3/3] remote-hg: add test cases for null bookmarks Max Horn
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 NNTP newsgroup(s).