Patchwork [STABLE] discovery: prevent crash caused by prune marker having no parent data

login
register
mail settings
Submitter Yuya Nishihara
Date April 19, 2017, 2:45 p.m.
Message ID <42fd230386fc31d24f3d.1492613151@mimosa>
Download mbox | patch
Permalink /patch/20265/
State Accepted
Headers show

Comments

Yuya Nishihara - April 19, 2017, 2:45 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1492611005 -32400
#      Wed Apr 19 23:10:05 2017 +0900
# Branch stable
# Node ID 42fd230386fc31d24f3d580a6f09efe596cd516c
# Parent  59afb0750aecaff6c2b2e4edaab04eb91eca8a88
discovery: prevent crash caused by prune marker having no parent data

If a marker has no parent information, parents field is set to None, which
caused TypeError. I think this shouldn't normally happen, but somehow
I have such marker in my repo.
Pierre-Yves David - April 19, 2017, 2:57 p.m.
On 04/19/2017 04:45 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1492611005 -32400
> #      Wed Apr 19 23:10:05 2017 +0900
> # Branch stable
> # Node ID 42fd230386fc31d24f3d580a6f09efe596cd516c
> # Parent  59afb0750aecaff6c2b2e4edaab04eb91eca8a88
> discovery: prevent crash caused by prune marker having no parent data
>
> If a marker has no parent information, parents field is set to None, which
> caused TypeError. I think this shouldn't normally happen, but somehow

This patch looks good to me.

It looks like you needs to run "debugrecordpruneparents" (from 
evolve.legacy) on your repo.

> I have such marker in my repo.
>
> diff --git a/mercurial/discovery.py b/mercurial/discovery.py
> --- a/mercurial/discovery.py
> +++ b/mercurial/discovery.py
> @@ -511,7 +511,7 @@ def pushingmarkerfor(obsstore, pushset,
>          for m in markers:
>              nexts = m[1] # successors
>              if not nexts: # this is a prune marker
> -                nexts = m[5] # parents
> +                nexts = m[5] or () # parents
>              for n in nexts:
>                  if n not in seen:
>                      seen.add(n)
> diff --git a/tests/test-obsolete-checkheads.t b/tests/test-obsolete-checkheads.t
> --- a/tests/test-obsolete-checkheads.t
> +++ b/tests/test-obsolete-checkheads.t
> @@ -281,3 +281,32 @@ Pulling the missing data makes it work
>    adding manifests
>    adding file changes
>    added 1 changesets with 1 changes to 1 files (+1 heads)
> +
> +Old head is pruned without parent data and new unrelated head added
> +===================================================================
> +
> +setup
> +
> +  $ cd ..
> +  $ rm -R remote local
> +  $ cp -R backup1 remote
> +  $ hg clone remote local -qr c70b08862e08
> +  $ cd local
> +  $ hg up -q '.^'
> +  $ mkcommit new-unrelated
> +  created new head
> +  $ hg debugobsolete `getid old`
> +  $ hg log -G --hidden
> +  @  350a93b716be (draft) add new-unrelated
> +  |
> +  | x  c70b08862e08 (draft) add old
> +  |/
> +  o  b4952fcf48cf (public) add base
> +
> +
> +  $ hg push
> +  pushing to $TESTTMP/remote (glob)
> +  searching for changes
> +  abort: push creates new remote head 350a93b716be!
> +  (merge or see 'hg help push' for details about pushing new heads)
> +  [255]
Sean Farley - April 20, 2017, 4:50 p.m.
Pierre-Yves David <pierre-yves.david@ens-lyon.org> writes:

> On 04/19/2017 04:45 PM, Yuya Nishihara wrote:
>> # HG changeset patch
>> # User Yuya Nishihara <yuya@tcha.org>
>> # Date 1492611005 -32400
>> #      Wed Apr 19 23:10:05 2017 +0900
>> # Branch stable
>> # Node ID 42fd230386fc31d24f3d580a6f09efe596cd516c
>> # Parent  59afb0750aecaff6c2b2e4edaab04eb91eca8a88
>> discovery: prevent crash caused by prune marker having no parent data
>>
>> If a marker has no parent information, parents field is set to None, which
>> caused TypeError. I think this shouldn't normally happen, but somehow
>
> This patch looks good to me.

Yeah, looks good to me, too.

Patch

diff --git a/mercurial/discovery.py b/mercurial/discovery.py
--- a/mercurial/discovery.py
+++ b/mercurial/discovery.py
@@ -511,7 +511,7 @@  def pushingmarkerfor(obsstore, pushset, 
         for m in markers:
             nexts = m[1] # successors
             if not nexts: # this is a prune marker
-                nexts = m[5] # parents
+                nexts = m[5] or () # parents
             for n in nexts:
                 if n not in seen:
                     seen.add(n)
diff --git a/tests/test-obsolete-checkheads.t b/tests/test-obsolete-checkheads.t
--- a/tests/test-obsolete-checkheads.t
+++ b/tests/test-obsolete-checkheads.t
@@ -281,3 +281,32 @@  Pulling the missing data makes it work
   adding manifests
   adding file changes
   added 1 changesets with 1 changes to 1 files (+1 heads)
+
+Old head is pruned without parent data and new unrelated head added
+===================================================================
+
+setup
+
+  $ cd ..
+  $ rm -R remote local
+  $ cp -R backup1 remote
+  $ hg clone remote local -qr c70b08862e08
+  $ cd local
+  $ hg up -q '.^'
+  $ mkcommit new-unrelated
+  created new head
+  $ hg debugobsolete `getid old`
+  $ hg log -G --hidden
+  @  350a93b716be (draft) add new-unrelated
+  |
+  | x  c70b08862e08 (draft) add old
+  |/
+  o  b4952fcf48cf (public) add base
+  
+
+  $ hg push
+  pushing to $TESTTMP/remote (glob)
+  searching for changes
+  abort: push creates new remote head 350a93b716be!
+  (merge or see 'hg help push' for details about pushing new heads)
+  [255]