[PATCH] histedit: add support for exec/x command to histedit (issue4036)
durham at fb.com
Wed Sep 16 00:27:13 CDT 2015
On 9/15/15 1:54 PM, Pierre-Yves David wrote:
> 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
>> (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:
> 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
Regarding your initial points 1 and 2, he addresses this in the run()
function by recreating the commit before running the shell command. So
the hash will always be different (though I think the _commit function
may need to put some extra data in the extras field to ensure that the
More information about the Mercurial-devel