[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