[PATCH 2 of 4 V2] histedit: switch state to store node instead of ctx

[Augie Fackler]

On Wed, Feb 4, 2015 at 3:45 PM, Durham Goode <durham at fb.com> wrote:
>
> On 2/3/15 3:17 AM, Pierre-Yves David wrote:
>>
>>
>>
>> On 02/03/2015 12:32 AM, Mateusz Kwapich wrote:
>>>
>>> # HG changeset patch
>>> # User Mateusz Kwapich <mitrandir at fb.com>
>>> # Date 1422314748 28800
>>> #      Mon Jan 26 15:25:48 2015 -0800
>>> # Node ID 5868827d6d1a2a75d9628d502281d0d7fad2b115
>>> # Parent  0c8cf9835334d76f52e7cebf9d70b5b0df646871
>>> histedit: switch state to store node instead of ctx
>>
>>
>> (I commented on V1 before seing V2, copy pasting comment)
>>
>> why is it necessary to change it on the state object itself? It is not
>> obvious to me how it helps.
>>
>> (If you want to clean stuff in this area, feel free to also nuke the last
>> on disk usage of pickle in the Mercurial codebase (beside convert madness)
>
> Currently, if the node no longer exists, the state object fails to load and
> pukes with an exception. Changing the state object to only store the node
> allows callers to handle these cases.  For instance, in bootstrapcontinue we
> can now detect that the node doesn't exist and exit gracefully.
>
> The alternative is to have the state object store something like None when
> the node doesn't exist, but then outside callers won't be able to access the
> old node for recovery (unless we store both the node and the ctx, but why
> bother).
>
> More importantly it allows us to detect this case when doing hg histedit
> --abort.  Currently this situation results in both --continue and --abort
> being broken and the user has to rm .hg/histedit-state to unfuck their repo.


I look forward to a resend that includes this excellent documentation
in the log message.

Thanks!
Augie