Patchwork histedit: also handle locally missing node when reading obsolescence

login
register
mail settings
Submitter Pierre-Yves David
Date Feb. 24, 2016, 4:02 p.m.
Message ID <ed8cdb6c92b9025e067a.1456329752@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/13356/
State Accepted
Headers show

Comments

Pierre-Yves David - Feb. 24, 2016, 4:02 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1456329487 -3600
#      Wed Feb 24 16:58:07 2016 +0100
# Node ID ed8cdb6c92b9025e067a257218bbb965bea75979
# Parent  f6116114bc0b6ec4f1bfedb3e149b4c0d28e0be2
# Available At http://hg.netv6.net/marmoute-wip/mercurial/
#              hg pull http://hg.netv6.net/marmoute-wip/mercurial/ -r ed8cdb6c92b9
histedit: also handle locally missing node when reading obsolescence

The previous version of the code was interpreting markers to a missing node as
a prune in all cases. The expected way to handle such situation is to keep
reading marker. Only turning successors into "prune" if they are at the end of
a chain.

We update the code and add a test for this.
Kostia Balytskyi - Feb. 24, 2016, 4:18 p.m.
Looks good to me.




On 2/24/16, 4:02 PM, "Mercurial-devel on behalf of Pierre-Yves David" <mercurial-devel-bounces@mercurial-scm.org on behalf of pierre-yves.david@ens-lyon.org> wrote:

># HG changeset patch

># User Pierre-Yves David <pierre-yves.david@fb.com>

># Date 1456329487 -3600

>#      Wed Feb 24 16:58:07 2016 +0100

># Node ID ed8cdb6c92b9025e067a257218bbb965bea75979

># Parent  f6116114bc0b6ec4f1bfedb3e149b4c0d28e0be2

># Available At https://urldefense.proofpoint.com/v2/url?u=http-3A__hg.netv6.net_marmoute-2Dwip_mercurial_&d=CwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Pp-gQYFgs4tKlSFPF5kfCw&m=S-o7cclvYTOhi7p3VSTzXhZvmbhmfRKeEG9JkPT9tzc&s=bI-kbBVjuRSpyRdQwoZpZVq2wDbD_pozRO-KWMWWAo8&e= 

>#              hg pull https://urldefense.proofpoint.com/v2/url?u=http-3A__hg.netv6.net_marmoute-2Dwip_mercurial_&d=CwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Pp-gQYFgs4tKlSFPF5kfCw&m=S-o7cclvYTOhi7p3VSTzXhZvmbhmfRKeEG9JkPT9tzc&s=bI-kbBVjuRSpyRdQwoZpZVq2wDbD_pozRO-KWMWWAo8&e=  -r ed8cdb6c92b9

>histedit: also handle locally missing node when reading obsolescence

>

>The previous version of the code was interpreting markers to a missing node as

>a prune in all cases. The expected way to handle such situation is to keep

>reading marker. Only turning successors into "prune" if they are at the end of

>a chain.

>

>We update the code and add a test for this.

>

>diff --git a/hgext/histedit.py b/hgext/histedit.py

>--- a/hgext/histedit.py

>+++ b/hgext/histedit.py

>@@ -1395,26 +1395,26 @@ def adjustreplacementsfrommarkers(repo, 

>     data read from obsolescense markers"""

>     if not obsolete.isenabled(repo, obsolete.createmarkersopt):

>         return oldreplacements

> 

>     unfi = repo.unfiltered()

>+    nm = unfi.changelog.nodemap

>+    obsstore = repo.obsstore

>     newreplacements = list(oldreplacements)

>     oldsuccs = [r[1] for r in oldreplacements]

>     # successors that have already been added to succstocheck once

>     seensuccs = set().union(*oldsuccs) # create a set from an iterable of tuples

>     succstocheck = list(seensuccs)

>     while succstocheck:

>         n = succstocheck.pop()

>-        try:

>-            ctx = unfi[n]

>-        except error.RepoError:

>-            # XXX node unknown locally, we should properly follow marker

>+        missing = nm.get(n) is None

>+        markers = obsstore.successors.get(n, ())

>+        if missing and not markers:

>+            # dead end, mark it as such

>             newreplacements.append((n, ()))

>-            continue

>-

>-        for marker in obsolete.successormarkers(ctx):

>-            nsuccs = marker.succnodes()

>+        for marker in markers:

>+            nsuccs = marker[1]

>             newreplacements.append((n, nsuccs))

>             for nsucc in nsuccs:

>                 if nsucc not in seensuccs:

>                     seensuccs.add(nsucc)

>                     succstocheck.append(nsucc)

>diff --git a/tests/test-histedit-obsolete.t b/tests/test-histedit-obsolete.t

>--- a/tests/test-histedit-obsolete.t

>+++ b/tests/test-histedit-obsolete.t

>@@ -52,10 +52,44 @@ Test that histedit learns about obsolesc

>   $ hg debugobsolete

>   e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)

>   3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'user': 'test'} (glob)

>   1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'user': 'test'} (glob)

>   114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)

>+

>+With some node gone missing during the edit.

>+

>+  $ echo "pick `hg log -r 0 -T '{node|short}'`" > plan

>+  $ echo "pick `hg log -r 6 -T '{node|short}'`" >> plan

>+  $ echo "edit `hg log -r 5 -T '{node|short}'`" >> plan

>+  $ hg histedit -r 'all()' --commands plan

>+  Editing (49d44ab2be1b), you may commit or record as needed now.

>+  (hg histedit --continue to resume)

>+  [1]

>+  $ hg st

>+  A b

>+  A d

>+  ? plan

>+  $ hg commit --amend -X . -m XXXXXX

>+  $ hg commit --amend -X . -m b2

>+  $ hg --hidden --config extensions.strip= strip 'desc(XXXXXX)' --no-backup

>+  $ hg histedit --continue

>+  $ hg log -G

>+  @  9:273c1f3b8626 c

>+  |

>+  o  8:aba7da937030 b2

>+  |

>+  o  0:cb9a9f314b8b a

>+  

>+  $ hg debugobsolete

>+  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)

>+  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'user': 'test'} (glob)

>+  1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'user': 'test'} (glob)

>+  114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)

>+  76f72745eac0643d16530e56e2f86e36e40631f1 2ca853e48edbd6453a0674dc0fe28a0974c51b9c 0 (*) {'user': 'test'} (glob)

>+  2ca853e48edbd6453a0674dc0fe28a0974c51b9c aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'user': 'test'} (glob)

>+  49d44ab2be1b67a79127568a67c9c99430633b48 273c1f3b86267ed3ec684bb13af1fa4d6ba56e02 0 (*) {'user': 'test'} (glob)

>+  46abc7c4d8738e8563e577f7889e1b6db3da4199 aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'user': 'test'} (glob)

>   $ cd ..

> 

> Base setup for the rest of the testing

> ======================================

> 

>_______________________________________________

>Mercurial-devel mailing list

>Mercurial-devel@mercurial-scm.org

>https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=CwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Pp-gQYFgs4tKlSFPF5kfCw&m=S-o7cclvYTOhi7p3VSTzXhZvmbhmfRKeEG9JkPT9tzc&s=avtRmZH9piz4xya2X256znKozVrBxUhDCb0yuhxyOpM&e=
Augie Fackler - Feb. 24, 2016, 9:45 p.m.
On Wed, Feb 24, 2016 at 05:02:32PM +0100, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1456329487 -3600
> #      Wed Feb 24 16:58:07 2016 +0100
> # Node ID ed8cdb6c92b9025e067a257218bbb965bea75979
> # Parent  f6116114bc0b6ec4f1bfedb3e149b4c0d28e0be2
> # Available At http://hg.netv6.net/marmoute-wip/mercurial/
> #              hg pull http://hg.netv6.net/marmoute-wip/mercurial/ -r ed8cdb6c92b9
> histedit: also handle locally missing node when reading obsolescence

queued with some english tweaks, thanks

>
> The previous version of the code was interpreting markers to a missing node as
> a prune in all cases. The expected way to handle such situation is to keep
> reading marker. Only turning successors into "prune" if they are at the end of
> a chain.
>
> We update the code and add a test for this.
>
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -1395,26 +1395,26 @@ def adjustreplacementsfrommarkers(repo,
>      data read from obsolescense markers"""
>      if not obsolete.isenabled(repo, obsolete.createmarkersopt):
>          return oldreplacements
>
>      unfi = repo.unfiltered()
> +    nm = unfi.changelog.nodemap
> +    obsstore = repo.obsstore
>      newreplacements = list(oldreplacements)
>      oldsuccs = [r[1] for r in oldreplacements]
>      # successors that have already been added to succstocheck once
>      seensuccs = set().union(*oldsuccs) # create a set from an iterable of tuples
>      succstocheck = list(seensuccs)
>      while succstocheck:
>          n = succstocheck.pop()
> -        try:
> -            ctx = unfi[n]
> -        except error.RepoError:
> -            # XXX node unknown locally, we should properly follow marker
> +        missing = nm.get(n) is None
> +        markers = obsstore.successors.get(n, ())
> +        if missing and not markers:
> +            # dead end, mark it as such
>              newreplacements.append((n, ()))
> -            continue
> -
> -        for marker in obsolete.successormarkers(ctx):
> -            nsuccs = marker.succnodes()
> +        for marker in markers:
> +            nsuccs = marker[1]
>              newreplacements.append((n, nsuccs))
>              for nsucc in nsuccs:
>                  if nsucc not in seensuccs:
>                      seensuccs.add(nsucc)
>                      succstocheck.append(nsucc)
> diff --git a/tests/test-histedit-obsolete.t b/tests/test-histedit-obsolete.t
> --- a/tests/test-histedit-obsolete.t
> +++ b/tests/test-histedit-obsolete.t
> @@ -52,10 +52,44 @@ Test that histedit learns about obsolesc
>    $ hg debugobsolete
>    e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
>    3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'user': 'test'} (glob)
>    1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'user': 'test'} (glob)
>    114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
> +
> +With some node gone missing during the edit.
> +
> +  $ echo "pick `hg log -r 0 -T '{node|short}'`" > plan
> +  $ echo "pick `hg log -r 6 -T '{node|short}'`" >> plan
> +  $ echo "edit `hg log -r 5 -T '{node|short}'`" >> plan
> +  $ hg histedit -r 'all()' --commands plan
> +  Editing (49d44ab2be1b), you may commit or record as needed now.
> +  (hg histedit --continue to resume)
> +  [1]
> +  $ hg st
> +  A b
> +  A d
> +  ? plan
> +  $ hg commit --amend -X . -m XXXXXX
> +  $ hg commit --amend -X . -m b2
> +  $ hg --hidden --config extensions.strip= strip 'desc(XXXXXX)' --no-backup
> +  $ hg histedit --continue
> +  $ hg log -G
> +  @  9:273c1f3b8626 c
> +  |
> +  o  8:aba7da937030 b2
> +  |
> +  o  0:cb9a9f314b8b a
> +
> +  $ hg debugobsolete
> +  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
> +  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'user': 'test'} (glob)
> +  1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'user': 'test'} (glob)
> +  114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
> +  76f72745eac0643d16530e56e2f86e36e40631f1 2ca853e48edbd6453a0674dc0fe28a0974c51b9c 0 (*) {'user': 'test'} (glob)
> +  2ca853e48edbd6453a0674dc0fe28a0974c51b9c aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'user': 'test'} (glob)
> +  49d44ab2be1b67a79127568a67c9c99430633b48 273c1f3b86267ed3ec684bb13af1fa4d6ba56e02 0 (*) {'user': 'test'} (glob)
> +  46abc7c4d8738e8563e577f7889e1b6db3da4199 aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'user': 'test'} (glob)
>    $ cd ..
>
>  Base setup for the rest of the testing
>  ======================================
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -1395,26 +1395,26 @@  def adjustreplacementsfrommarkers(repo, 
     data read from obsolescense markers"""
     if not obsolete.isenabled(repo, obsolete.createmarkersopt):
         return oldreplacements
 
     unfi = repo.unfiltered()
+    nm = unfi.changelog.nodemap
+    obsstore = repo.obsstore
     newreplacements = list(oldreplacements)
     oldsuccs = [r[1] for r in oldreplacements]
     # successors that have already been added to succstocheck once
     seensuccs = set().union(*oldsuccs) # create a set from an iterable of tuples
     succstocheck = list(seensuccs)
     while succstocheck:
         n = succstocheck.pop()
-        try:
-            ctx = unfi[n]
-        except error.RepoError:
-            # XXX node unknown locally, we should properly follow marker
+        missing = nm.get(n) is None
+        markers = obsstore.successors.get(n, ())
+        if missing and not markers:
+            # dead end, mark it as such
             newreplacements.append((n, ()))
-            continue
-
-        for marker in obsolete.successormarkers(ctx):
-            nsuccs = marker.succnodes()
+        for marker in markers:
+            nsuccs = marker[1]
             newreplacements.append((n, nsuccs))
             for nsucc in nsuccs:
                 if nsucc not in seensuccs:
                     seensuccs.add(nsucc)
                     succstocheck.append(nsucc)
diff --git a/tests/test-histedit-obsolete.t b/tests/test-histedit-obsolete.t
--- a/tests/test-histedit-obsolete.t
+++ b/tests/test-histedit-obsolete.t
@@ -52,10 +52,44 @@  Test that histedit learns about obsolesc
   $ hg debugobsolete
   e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
   3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'user': 'test'} (glob)
   1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'user': 'test'} (glob)
   114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
+
+With some node gone missing during the edit.
+
+  $ echo "pick `hg log -r 0 -T '{node|short}'`" > plan
+  $ echo "pick `hg log -r 6 -T '{node|short}'`" >> plan
+  $ echo "edit `hg log -r 5 -T '{node|short}'`" >> plan
+  $ hg histedit -r 'all()' --commands plan
+  Editing (49d44ab2be1b), you may commit or record as needed now.
+  (hg histedit --continue to resume)
+  [1]
+  $ hg st
+  A b
+  A d
+  ? plan
+  $ hg commit --amend -X . -m XXXXXX
+  $ hg commit --amend -X . -m b2
+  $ hg --hidden --config extensions.strip= strip 'desc(XXXXXX)' --no-backup
+  $ hg histedit --continue
+  $ hg log -G
+  @  9:273c1f3b8626 c
+  |
+  o  8:aba7da937030 b2
+  |
+  o  0:cb9a9f314b8b a
+  
+  $ hg debugobsolete
+  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
+  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'user': 'test'} (glob)
+  1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'user': 'test'} (glob)
+  114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
+  76f72745eac0643d16530e56e2f86e36e40631f1 2ca853e48edbd6453a0674dc0fe28a0974c51b9c 0 (*) {'user': 'test'} (glob)
+  2ca853e48edbd6453a0674dc0fe28a0974c51b9c aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'user': 'test'} (glob)
+  49d44ab2be1b67a79127568a67c9c99430633b48 273c1f3b86267ed3ec684bb13af1fa4d6ba56e02 0 (*) {'user': 'test'} (glob)
+  46abc7c4d8738e8563e577f7889e1b6db3da4199 aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'user': 'test'} (glob)
   $ cd ..
 
 Base setup for the rest of the testing
 ======================================