[PATCH] histedit: add support for exec/x command to histedit (issue4036)

Pierre-Yves David pierre-yves.david at ens-lyon.org
Tue Sep 15 15:54:27 CDT 2015



On 09/14/2015 01:04 PM, Piotr Listkiewicz wrote:
>     This seems to be a resend, we usually add V2 (then V3, etc) to the
>     subject (using --flag V2 in patchbomb).
>
>
> My fault, Im new here and i probably omitted this information in
> contribution wiki

Not a big deal, but please do for the next ones, it's a bit confusing on 
the reviewer end otherwise.

>     Once we ensure we really release the lock (even when nested) we'll
>     have to ensure that we reacquires them with the same level of nesting.
>     (we probably want a context manager for this unlocking business)
>
>
> Honestly i don't know how can i release it properly and how to ensure
> that it reacquires them with the same level of nesting. I would be
> greateful for instruction/more information about it.

1) Check how the re-entrace is handled
2) Unlock the right amount of time
3) relock that amount of time

We probably want a context manager to handle that at the lock level.

Feel free to ask for more details if you need more guidance.

>     Also, if the commands changed the repository, do we need to forcibly
>     invalidate some cache to ensure a consistent view of the repository?
>     Or is this already taken care of by something else.
>
>  From tests it seems like it properly detects changes in repository
> other than unknown files.

My point here is that if some command changed stuff (new commit, 
bookmark location etc), some of it might not be detected by the current 
process that already parsed them from disk and naively assume they did 
not changed since it hold the lock.

I would like to see some investigation to know if we need to do 
something special (or if it is already handled, for example by the 
lock). to make sure we are safe here.


> But this behaviour is consistent with edit
> commands which doesn't detect unknown files also.

The edit command terminates the current process and starts a new one, so 
it is far more safe in this regards.

> In my view if we agree
> that unknown files should be detected by the exec command we should
> detect it in edit command as well.

I'm not familliar with this unknown file issue, do we something to fix 
there? Is there a bug tracking/explaining it?

>     This is going to break/misbehaver in in multiple cases.
>     1) if nothing changed, you code claim that changeset X replace
>     changesets X. If the current code does not explode with this yet,
>     this will probably bite use in the future
>     2) if anything else but an amend happened (fold, splitting, adding
>     more changesets) this is going to report wrong result and lead to
>     crash when moving bookmarks or creating obsolescence markers.
>
>
> It is most likely that i don't get it, but i have no idea why this would
> lead to such behaviour more than edit command. Edit command behaves same
> way if user amend to given commit, replacing it with new ammend version.

edit was initially intended to just edit the changeset and possibly 
split it in multiple parts. It has known issue with anything else. CF: 
http://bz.selenic.com/show_bug.cgi?id=4800

As exec will likely be heavily used for history edition we will hit this 
issues. not to talk about people dropping to a shell using exec

Moreover edit have special logic looking for replacement on --continue 
that track replacement. I do not see such logic in the exec code.


To conclude, thanks again for trackling this exec commands, I'm trying 
to point out at the know trap early on, because the topic is quite complex.

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list