Patchwork [v5] histedit: make histedit aware of obsolescense not stored in state (issue4800)

login
register
mail settings
Submitter Kostia Balytskyi
Date Feb. 23, 2016, 9:40 p.m.
Message ID <9129dbd7a1780c66bd21.1456263631@ikostia-mbp>
Download mbox | patch
Permalink /patch/13333/
State Accepted
Headers show

Comments

Kostia Balytskyi - Feb. 23, 2016, 9:40 p.m.
# HG changeset patch
# User Kostia Balytskyi <ikostia@fb.com>
# Date 1456263516 0
#      Tue Feb 23 21:38:36 2016 +0000
# Node ID 9129dbd7a1780c66bd212b16e2c08e2ecab20976
# Parent  28e1694ca60056d609ae2c8e0ad5cb2891416ea3
histedit: make histedit aware of obsolescense not stored in state (issue4800)

Before this change, when histedit exited to interactive session (during edit
command for example), user could introduce obsolescence markers that would not
be known to histedit. For example, user could've amended one of the commits.
The fact of this amendment would not be stored in histedit's state file
and later, when histedit would try to process all the replacements,
one of the final successors (in histedit's opinion) would turn out to be hidden.
This behavior is described in issue4800. This commit fixes it.
Pierre-Yves David - Feb. 23, 2016, 10:48 p.m.
On 02/23/2016 10:40 PM, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia@fb.com>
> # Date 1456263516 0
> #      Tue Feb 23 21:38:36 2016 +0000
> # Node ID 9129dbd7a1780c66bd212b16e2c08e2ecab20976
> # Parent  28e1694ca60056d609ae2c8e0ad5cb2891416ea3
> histedit: make histedit aware of obsolescense not stored in state (issue4800)
>
> Before this change, when histedit exited to interactive session (during edit
> command for example), user could introduce obsolescence markers that would not
> be known to histedit. For example, user could've amended one of the commits.
> The fact of this amendment would not be stored in histedit's state file
> and later, when histedit would try to process all the replacements,
> one of the final successors (in histedit's opinion) would turn out to be hidden.
> This behavior is described in issue4800. This commit fixes it.
>
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -173,6 +173,7 @@
>   import errno
>   import os
>   import sys
> +import itertools
>
>   from mercurial import bundle2
>   from mercurial import cmdutil
> @@ -1386,13 +1387,48 @@
>                   hint=_('use "drop %s" to discard, see also: '
>                          '"hg help -e histedit.config"') % missing[0][:12])
>
> +def adjustreplacementsfrommarkers(repo, oldreplacements):
> +    """Adjust replacements from obsolescense markers
> +
> +    Replacements structure is originally generated based on
> +    histedit's state and does not account for changes that are
> +    not recorded there. This function fixes that by adding
> +    data read from obsolescense markers"""
> +    if not obsolete.isenabled(repo, obsolete.createmarkersopt):
> +        return oldreplacements
> +
> +    unfi = repo.unfiltered()
> +    newreplacements = list(oldreplacements)
> +    oldsuccs = itertools.imap(lambda r: r[1], oldreplacements)

So the value of itertools is low

   oldsuccss = [r[1] for r in oldreplacements] #shorted and clearer.

Are you okay with see that replace inflight ?

> +    # successors that have already been added to succstocheck once
> +    seensuccs = set().union(*oldsuccs)

We should document your trick here. it's smart but fairly obscure.

A comment can probably be introduced in flight if you provide it here.

> +    succstocheck = list(seensuccs)
> +    while succstocheck:
> +        n = succstocheck.pop()
> +        try:
> +            ctx = unfi[n]
> +        except error.RepoError:
> +            # node unknown locally, should be treated as replaced
> +            newreplacements.append((n, ()))
> +            continue
> +
> +        for marker in obsolete.successormarkers(ctx):
> +            nsuccs = marker.succnodes()
> +            newreplacements.append((n, nsuccs))
> +            for nsucc in nsuccs:
> +                if nsucc not in seensuccs:
> +                    seensuccs.add(nsucc)
> +                    succstocheck.append(nsucc)
> +
> +    return newreplacements
> +
>   def processreplacement(state):
>       """process the list of replacements to return
>
>       1) the final mapping between original and created nodes
>       2) the list of temporary node created by histedit
>       3) the list of new commit created by histedit"""
> -    replacements = state.replacements
> +    replacements = adjustreplacementsfrommarkers(state.repo, state.replacements)
>       allsuccs = set()
>       replaced = set()
>       fullmapping = {}
> 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
> @@ -14,6 +14,36 @@
>     > rebase=
>     > EOF
>
> +Test that histedit learns about obsoletions not stored in histedit state
> +  $ hg init boo
> +  $ cd boo
> +  $ echo a > a
> +  $ hg ci -Am a
> +  adding a
> +  $ echo a > b
> +  $ echo a > c
> +  $ echo a > c
> +  $ hg ci -Am b
> +  adding b
> +  adding c
> +  $ echo a > d
> +  $ hg ci -Am c
> +  adding d
> +  $ echo "pick `hg log -r 0 -T '{node|short}'`" > plan
> +  $ echo "pick `hg log -r 2 -T '{node|short}'`" >> plan
> +  $ echo "edit `hg log -r 1 -T '{node|short}'`" >> plan
> +  $ hg histedit -r 'all()' --commands plan
> +  Editing (1b2d564fad96), you may commit or record as needed now.
> +  (hg histedit --continue to resume)
> +  [1]
> +  $ hg st
> +  A b
> +  A c
> +  ? plan
> +  $ hg commit --amend b
> +  $ hg histedit --continue
> +  $ cd ..
> +
>     $ hg init base
>     $ cd base
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Kostia Balytskyi - Feb. 24, 2016, 12:08 a.m.
On 2/23/16, 10:48 PM, "Pierre-Yves David" <pierre-yves.david@ens-lyon.org> wrote:

>

>

>On 02/23/2016 10:40 PM, Kostia Balytskyi wrote:

>> # HG changeset patch

>> # User Kostia Balytskyi <ikostia@fb.com>

>> # Date 1456263516 0

>> #      Tue Feb 23 21:38:36 2016 +0000

>> # Node ID 9129dbd7a1780c66bd212b16e2c08e2ecab20976

>> # Parent  28e1694ca60056d609ae2c8e0ad5cb2891416ea3

>> histedit: make histedit aware of obsolescense not stored in state (issue4800)

>>

>> Before this change, when histedit exited to interactive session (during edit

>> command for example), user could introduce obsolescence markers that would not

>> be known to histedit. For example, user could've amended one of the commits.

>> The fact of this amendment would not be stored in histedit's state file

>> and later, when histedit would try to process all the replacements,

>> one of the final successors (in histedit's opinion) would turn out to be hidden.

>> This behavior is described in issue4800. This commit fixes it.

>>

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

>> --- a/hgext/histedit.py

>> +++ b/hgext/histedit.py

>> @@ -173,6 +173,7 @@

>>   import errno

>>   import os

>>   import sys

>> +import itertools

>>

>>   from mercurial import bundle2

>>   from mercurial import cmdutil

>> @@ -1386,13 +1387,48 @@

>>                   hint=_('use "drop %s" to discard, see also: '

>>                          '"hg help -e histedit.config"') % missing[0][:12])

>>

>> +def adjustreplacementsfrommarkers(repo, oldreplacements):

>> +    """Adjust replacements from obsolescense markers

>> +

>> +    Replacements structure is originally generated based on

>> +    histedit's state and does not account for changes that are

>> +    not recorded there. This function fixes that by adding

>> +    data read from obsolescense markers"""

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

>> +        return oldreplacements

>> +

>> +    unfi = repo.unfiltered()

>> +    newreplacements = list(oldreplacements)

>> +    oldsuccs = itertools.imap(lambda r: r[1], oldreplacements)

>

>So the value of itertools is low

>

>   oldsuccss = [r[1] for r in oldreplacements] #shorted and clearer.

>

>Are you okay with see that replace inflight ?

Yes, go ahead.

>

>> +    # successors that have already been added to succstocheck once

>> +    seensuccs = set().union(*oldsuccs)

>

>We should document your trick here. it's smart but fairly obscure.

Martijn taught me that :)

>

>A comment can probably be introduced in flight if you provide it here.

Would this work: # create a set from an iterable of tuples
?
>

>> +    succstocheck = list(seensuccs)

>> +    while succstocheck:

>> +        n = succstocheck.pop()

>> +        try:

>> +            ctx = unfi[n]

>> +        except error.RepoError:

>> +            # node unknown locally, should be treated as replaced

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

>> +            continue

>> +

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

>> +            nsuccs = marker.succnodes()

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

>> +            for nsucc in nsuccs:

>> +                if nsucc not in seensuccs:

>> +                    seensuccs.add(nsucc)

>> +                    succstocheck.append(nsucc)

>> +

>> +    return newreplacements

>> +

>>   def processreplacement(state):

>>       """process the list of replacements to return

>>

>>       1) the final mapping between original and created nodes

>>       2) the list of temporary node created by histedit

>>       3) the list of new commit created by histedit"""

>> -    replacements = state.replacements

>> +    replacements = adjustreplacementsfrommarkers(state.repo, state.replacements)

>>       allsuccs = set()

>>       replaced = set()

>>       fullmapping = {}

>> 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

>> @@ -14,6 +14,36 @@

>>     > rebase=

>>     > EOF

>>

>> +Test that histedit learns about obsoletions not stored in histedit state

>> +  $ hg init boo

>> +  $ cd boo

>> +  $ echo a > a

>> +  $ hg ci -Am a

>> +  adding a

>> +  $ echo a > b

>> +  $ echo a > c

>> +  $ echo a > c

>> +  $ hg ci -Am b

>> +  adding b

>> +  adding c

>> +  $ echo a > d

>> +  $ hg ci -Am c

>> +  adding d

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

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

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

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

>> +  Editing (1b2d564fad96), you may commit or record as needed now.

>> +  (hg histedit --continue to resume)

>> +  [1]

>> +  $ hg st

>> +  A b

>> +  A c

>> +  ? plan

>> +  $ hg commit --amend b

>> +  $ hg histedit --continue

>> +  $ cd ..

>> +

>>     $ hg init base

>>     $ cd base

>>

>> _______________________________________________

>> 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=CwICaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Pp-gQYFgs4tKlSFPF5kfCw&m=ERwjPkiJFeqPT2xU66WWxTRzaY0ChyTf87dlkZxlZ34&s=8OsEmT6x_VnA-apVxFdXN24CF8vP1egZbeTxFJANX-8&e= 

>>

>

>-- 

>Pierre-Yves David
Pierre-Yves David - Feb. 24, 2016, 11:16 a.m.
Pushed to the clowncopter with the change we discussed.

On 02/24/2016 01:08 AM, Kostia Balytskyi wrote:
>
>
>
>
>
> On 2/23/16, 10:48 PM, "Pierre-Yves David" <pierre-yves.david@ens-lyon.org> wrote:
>
>>
>>
>> On 02/23/2016 10:40 PM, Kostia Balytskyi wrote:
>>> # HG changeset patch
>>> # User Kostia Balytskyi <ikostia@fb.com>
>>> # Date 1456263516 0
>>> #      Tue Feb 23 21:38:36 2016 +0000
>>> # Node ID 9129dbd7a1780c66bd212b16e2c08e2ecab20976
>>> # Parent  28e1694ca60056d609ae2c8e0ad5cb2891416ea3
>>> histedit: make histedit aware of obsolescense not stored in state (issue4800)
>>>
>>> Before this change, when histedit exited to interactive session (during edit
>>> command for example), user could introduce obsolescence markers that would not
>>> be known to histedit. For example, user could've amended one of the commits.
>>> The fact of this amendment would not be stored in histedit's state file
>>> and later, when histedit would try to process all the replacements,
>>> one of the final successors (in histedit's opinion) would turn out to be hidden.
>>> This behavior is described in issue4800. This commit fixes it.
>>>
>>> diff --git a/hgext/histedit.py b/hgext/histedit.py
>>> --- a/hgext/histedit.py
>>> +++ b/hgext/histedit.py
>>> @@ -173,6 +173,7 @@
>>>    import errno
>>>    import os
>>>    import sys
>>> +import itertools
>>>
>>>    from mercurial import bundle2
>>>    from mercurial import cmdutil
>>> @@ -1386,13 +1387,48 @@
>>>                    hint=_('use "drop %s" to discard, see also: '
>>>                           '"hg help -e histedit.config"') % missing[0][:12])
>>>
>>> +def adjustreplacementsfrommarkers(repo, oldreplacements):
>>> +    """Adjust replacements from obsolescense markers
>>> +
>>> +    Replacements structure is originally generated based on
>>> +    histedit's state and does not account for changes that are
>>> +    not recorded there. This function fixes that by adding
>>> +    data read from obsolescense markers"""
>>> +    if not obsolete.isenabled(repo, obsolete.createmarkersopt):
>>> +        return oldreplacements
>>> +
>>> +    unfi = repo.unfiltered()
>>> +    newreplacements = list(oldreplacements)
>>> +    oldsuccs = itertools.imap(lambda r: r[1], oldreplacements)
>>
>> So the value of itertools is low
>>
>>    oldsuccss = [r[1] for r in oldreplacements] #shorted and clearer.
>>
>> Are you okay with see that replace inflight ?
> Yes, go ahead.
>
>>
>>> +    # successors that have already been added to succstocheck once
>>> +    seensuccs = set().union(*oldsuccs)
>>
>> We should document your trick here. it's smart but fairly obscure.
> Martijn taught me that :)
>
>>
>> A comment can probably be introduced in flight if you provide it here.
> Would this work: # create a set from an iterable of tuples
> ?
>>
>>> +    succstocheck = list(seensuccs)
>>> +    while succstocheck:
>>> +        n = succstocheck.pop()
>>> +        try:
>>> +            ctx = unfi[n]
>>> +        except error.RepoError:
>>> +            # node unknown locally, should be treated as replaced
>>> +            newreplacements.append((n, ()))
>>> +            continue
>>> +
>>> +        for marker in obsolete.successormarkers(ctx):
>>> +            nsuccs = marker.succnodes()
>>> +            newreplacements.append((n, nsuccs))
>>> +            for nsucc in nsuccs:
>>> +                if nsucc not in seensuccs:
>>> +                    seensuccs.add(nsucc)
>>> +                    succstocheck.append(nsucc)
>>> +
>>> +    return newreplacements
>>> +
>>>    def processreplacement(state):
>>>        """process the list of replacements to return
>>>
>>>        1) the final mapping between original and created nodes
>>>        2) the list of temporary node created by histedit
>>>        3) the list of new commit created by histedit"""
>>> -    replacements = state.replacements
>>> +    replacements = adjustreplacementsfrommarkers(state.repo, state.replacements)
>>>        allsuccs = set()
>>>        replaced = set()
>>>        fullmapping = {}
>>> 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
>>> @@ -14,6 +14,36 @@
>>>      > rebase=
>>>      > EOF
>>>
>>> +Test that histedit learns about obsoletions not stored in histedit state
>>> +  $ hg init boo
>>> +  $ cd boo
>>> +  $ echo a > a
>>> +  $ hg ci -Am a
>>> +  adding a
>>> +  $ echo a > b
>>> +  $ echo a > c
>>> +  $ echo a > c
>>> +  $ hg ci -Am b
>>> +  adding b
>>> +  adding c
>>> +  $ echo a > d
>>> +  $ hg ci -Am c
>>> +  adding d
>>> +  $ echo "pick `hg log -r 0 -T '{node|short}'`" > plan
>>> +  $ echo "pick `hg log -r 2 -T '{node|short}'`" >> plan
>>> +  $ echo "edit `hg log -r 1 -T '{node|short}'`" >> plan
>>> +  $ hg histedit -r 'all()' --commands plan
>>> +  Editing (1b2d564fad96), you may commit or record as needed now.
>>> +  (hg histedit --continue to resume)
>>> +  [1]
>>> +  $ hg st
>>> +  A b
>>> +  A c
>>> +  ? plan
>>> +  $ hg commit --amend b
>>> +  $ hg histedit --continue
>>> +  $ cd ..
>>> +
>>>      $ hg init base
>>>      $ cd base
>>>
>>> _______________________________________________
>>> 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=CwICaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Pp-gQYFgs4tKlSFPF5kfCw&m=ERwjPkiJFeqPT2xU66WWxTRzaY0ChyTf87dlkZxlZ34&s=8OsEmT6x_VnA-apVxFdXN24CF8vP1egZbeTxFJANX-8&e=
>>>
>>
>> --
>> Pierre-Yves David
> _______________________________________________
> 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
@@ -173,6 +173,7 @@ 
 import errno
 import os
 import sys
+import itertools
 
 from mercurial import bundle2
 from mercurial import cmdutil
@@ -1386,13 +1387,48 @@ 
                 hint=_('use "drop %s" to discard, see also: '
                        '"hg help -e histedit.config"') % missing[0][:12])
 
+def adjustreplacementsfrommarkers(repo, oldreplacements):
+    """Adjust replacements from obsolescense markers
+
+    Replacements structure is originally generated based on
+    histedit's state and does not account for changes that are
+    not recorded there. This function fixes that by adding
+    data read from obsolescense markers"""
+    if not obsolete.isenabled(repo, obsolete.createmarkersopt):
+        return oldreplacements
+
+    unfi = repo.unfiltered()
+    newreplacements = list(oldreplacements)
+    oldsuccs = itertools.imap(lambda r: r[1], oldreplacements)
+    # successors that have already been added to succstocheck once
+    seensuccs = set().union(*oldsuccs)
+    succstocheck = list(seensuccs)
+    while succstocheck:
+        n = succstocheck.pop()
+        try:
+            ctx = unfi[n]
+        except error.RepoError:
+            # node unknown locally, should be treated as replaced
+            newreplacements.append((n, ()))
+            continue
+
+        for marker in obsolete.successormarkers(ctx):
+            nsuccs = marker.succnodes()
+            newreplacements.append((n, nsuccs))
+            for nsucc in nsuccs:
+                if nsucc not in seensuccs:
+                    seensuccs.add(nsucc)
+                    succstocheck.append(nsucc)
+
+    return newreplacements
+
 def processreplacement(state):
     """process the list of replacements to return
 
     1) the final mapping between original and created nodes
     2) the list of temporary node created by histedit
     3) the list of new commit created by histedit"""
-    replacements = state.replacements
+    replacements = adjustreplacementsfrommarkers(state.repo, state.replacements)
     allsuccs = set()
     replaced = set()
     fullmapping = {}
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
@@ -14,6 +14,36 @@ 
   > rebase=
   > EOF
 
+Test that histedit learns about obsoletions not stored in histedit state
+  $ hg init boo
+  $ cd boo
+  $ echo a > a
+  $ hg ci -Am a
+  adding a
+  $ echo a > b
+  $ echo a > c
+  $ echo a > c
+  $ hg ci -Am b
+  adding b
+  adding c
+  $ echo a > d
+  $ hg ci -Am c
+  adding d
+  $ echo "pick `hg log -r 0 -T '{node|short}'`" > plan
+  $ echo "pick `hg log -r 2 -T '{node|short}'`" >> plan
+  $ echo "edit `hg log -r 1 -T '{node|short}'`" >> plan
+  $ hg histedit -r 'all()' --commands plan
+  Editing (1b2d564fad96), you may commit or record as needed now.
+  (hg histedit --continue to resume)
+  [1]
+  $ hg st
+  A b
+  A c
+  ? plan
+  $ hg commit --amend b
+  $ hg histedit --continue
+  $ cd ..
+
   $ hg init base
   $ cd base