Re: [Monotone-devel] update multiple projects

[Stephen Leake]

Thomas Keller <address@hidden> writes:


> Currently, whether you do a normal or a
> workspace merge, as soon as a content conflict arises, mtn opens a
> merger process (might it be vim or kdiff3 or emacs or opendiff) and
> waits until this process ends, i.e. returns, to pick up the saved
> temporary file afterwards.

Right.

> The problem is now that the shell call which opens the merger
> process has to _block_ until the process actually ends, 

Right.

> if it spawns in the background and returns immediately, 

Why would it do this? Sounds like a bug in the merger hook, or the
merge tool, to me.

There aught to be a way to spawn a process and wait for it; that
certainly works with Emacs on Linux and Windows. 

But there are Windows apps that I would not be able to do that with;
Microsoft Outlook, for instance. I could not find a way to launch that
from a command line; I have to click on the icon. "modern" GUI OS
interfaces are broken in many ways!

> mtn cannot pick up the temp file and therefor not the manual merge
> result. Thats why it waits for a key stroke from the user to act
> upon further. And this is actually the default behaviour for the
> default merger "opendiff" on Mac OS X...

I see that code in std_hooks.lua mergers.opendiff. I'd call this a bug
in opendiff. But I'm not volunteering to fix it.

So 'mtn au stdio' 'l6updatee' with the standard hooks does require
prompting via stdio, if the best available merger is opendiff. So we
(I) have to do something. Sigh.

>>> [--non-interactive] is set to true in CMD(stdio) and CMD(remote_stdio), so
>>> it would just be great if the new update command would honor this
>>> option as well.
>> 
>> ...
>> 
>> I think we need to make the value of --non-interactive available in
>> Lua hooks, so the user overriding a hook can check it before doing
>> something that might prompt.
>
> This sounds more like a workaround for a problem similar to one which
> you already solved quite smartly. Wouldn't be a better solution to
> expand show_conflicts to work in workspace mode and give mtn automate
> update an option --resolve-conflicts-file to resolve these on the fly?

Yes. But that's a lot of work, and I don't think it's worth it. I'm
also not sure it's possible; I have looked at the way update finds and
handles conflicts, and it's more complicated than for an in-database
merge; there are many separate stages, and you can't always back out
and leave a clean workspace. I'm certainly not willing to hold up
'gds_update_all' for it.

Another alternative is to let update fail if merge3 is required, and
let the user commit, merge the heads, and then update. When there are
major changes, that is certainly the prefered method. But I don't
always do that now when there are only minor changes, that are easy
enough to handle during update. I don't want changing the internals of
the Emacs mtn front-end to be more efficient to force a change in my
style.

So I'd rather go with this solution/workaround:

Add another argument 'non_interactive' to the merge3 Lua hook. That
requires passing that same flag down from 'update' to the actual call
to 'merge3'.

The standard mergers.emacs function should ignore that flag, since it
doesn't prompt; mergers.opendiff should respect it, and fail if it is
true. The user can then use a better merger with update, or commit, or
revert.

This is upward-compatible with existing user-written merge3 hooks. 

That only addresses this one Lua hook. If there are other hooks that
should care about non-interactive, then we either need to do the same
thing with them, or add a general way to access options from Lua. But
merge3 is the only hook relevant to 'mtn au update' (I think), so this
workaround is good for now.

> ...
> Heh, well, the code which resets the options before every command
> execution is hairy and its not better that its used in similar (but not
> quite equal) versions in three places: the mtn_automate lua hook,
> CMD(stdio) and CMD(remote_stdio). 

True. I was thinking we should try to unify that.

> When I hacked on the remote stuff the last time I tried to unify
> this a bit, but this was not easily possible. In the end I still
> felt not comfortable enough to make major changes in this area
> without opening a big can of worms.

Right. But I am comfortable with 'change_workspace', and putting off
unifying the workspace options code until we encounter a real problem :).

>>>>> 3) It would be cool if you could test the options for automate update a
>>>>> bit more
>>>>
>>>> ...
>
> But if you're not up
> for the task because you say "its already tested for the normal
> commands" then I just have to remember that the next time I review some
> code which touches this area.

Ok. Feel free to add some comments to support this.

-- 
-- Stephe