Re: git-mergetool: wrap tools with 3 files only to use the BASE file instead of MERGED

Re: git-mergetool: wrap tools with 3 files only to use the BASE file instead of MERGED

[Jonathan Nieder]

(+cc: some relevant people)
Hi,

Tanguy Ortolo wrote[1]:

> git-mergetool ideally uses tools that work with 4 files: BASE, LOCAL,
> REMOTE, which are the usual original and two new version of the file,
> and MERGED, which is where the tool is supposed to write the result of
> the merge.
>
> The problem is that most tools, at least graphical ones, specifically
> meld, can only work with three files, as they save the result to the
> original file.
>
> git-mergetool currently handles this situation by passing MERGED LOCAL
> REMOTE to the tool. This could be fine, but unfortunately MERGE contains
> the conflicts, formatted for manual resolution, so it is not really
> appropriate as an original file.
>
> I think it would be better to wrap such merge tools by:
> 1. passing them BASE LOCAL REMOTE;
> 2. checking whether or not BASE hase been modified:
>    * if it has, then copying it to MERGED,
>    * if it has not, exiting with return code 1 (merge failed).
> This check can be by either saving and comparing the mdate, or perhaps
> the SHA-1 hash of the BASE file.
>
> If this sounds good enough, I can dive into git-mergetoo--lib and
> implement it. In the meantime, here is an example of a custom merge tool
> that wraps meld for that purpose.

I think you forgot to include the example.  Anyway, at first glance it
sounds like a sensible idea.  David et al: thoughts?

Regards,
Jonathan

[1] http://bugs.debian.org/637355

Re: git-mergetool: wrap tools with 3 files only to use the BASE file instead of MERGED

[Tanguy Ortolo]

[-- Attachment #1.1: Type: text/plain, Size: 133 bytes --]

Jonathan Nieder, 2011-08-10 11:12 UTC-0500:
> I think you forgot to include the example.

Yes, sorry. Here it is.

-- 
Tanguy Ortolo

[-- Attachment #1.2: meld-mergetool --]
[-- Type: text/plain, Size: 385 bytes --]

#! /bin/sh

BASE="$1"
LOCAL="$2"
REMOTE="$3"
MERGED="$4"

MTIME_BEFORE="$(stat --format='%Y' "$BASE")"
meld "$LOCAL" "$BASE" "$REMOTE"
MTIME_AFTER="$(stat --format="%Y" "$BASE")"

if [ "$MTIME_BEFORE" != "$MTIME_AFTER" ]
then
    # The base file was modified, which means the user saved it
    cp "$BASE" "$MERGED"
else
    # The user did not save the file: merge failed
    exit 1
fi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: git-mergetool: wrap tools with 3 files only to use the BASE file instead of MERGED

[Sebastian Schuberth]

On Wed, Aug 10, 2011 at 18:12, Jonathan Nieder <jrnieder@gmail.com> wrote:

>> I think it would be better to wrap such merge tools by:
>> 1. passing them BASE LOCAL REMOTE;
>> 2. checking whether or not BASE hase been modified:
>>    * if it has, then copying it to MERGED,
>>    * if it has not, exiting with return code 1 (merge failed).
>> This check can be by either saving and comparing the mdate, or perhaps
>> the SHA-1 hash of the BASE file.
>>
>> If this sounds good enough, I can dive into git-mergetoo--lib and
>> implement it. In the meantime, here is an example of a custom merge tool
>> that wraps meld for that purpose.
>
> I think you forgot to include the example.  Anyway, at first glance it
> sounds like a sensible idea.  David et al: thoughts?

Sounds sensible to me, too. (Although I'm not affected, as fortunately
the GUI merge tools I've been using so far, namely ECMerge and Beyond
Compare 3, both allow to specify a separate merge output.)

-- 
Sebastian Schuberth

Re: git-mergetool: wrap tools with 3 files only to use the BASE file instead of MERGED

[David Aguilar]

On Wed, Aug 10, 2011 at 11:12:11AM -0500, Jonathan Nieder wrote:
> (+cc: some relevant people)
> Hi,
> 
> Tanguy Ortolo wrote[1]:
> 
> > git-mergetool ideally uses tools that work with 4 files: BASE, LOCAL,
> > REMOTE, which are the usual original and two new version of the file,
> > and MERGED, which is where the tool is supposed to write the result of
> > the merge.
> >
> > The problem is that most tools, at least graphical ones, specifically
> > meld, can only work with three files, as they save the result to the
> > original file.
> >
> > git-mergetool currently handles this situation by passing MERGED LOCAL
> > REMOTE to the tool. This could be fine, but unfortunately MERGE contains
> > the conflicts, formatted for manual resolution, so it is not really
> > appropriate as an original file.
> >
> > I think it would be better to wrap such merge tools by:
> > 1. passing them BASE LOCAL REMOTE;
> > 2. checking whether or not BASE hase been modified:
> >    * if it has, then copying it to MERGED,
> >    * if it has not, exiting with return code 1 (merge failed).
> > This check can be by either saving and comparing the mdate, or perhaps
> > the SHA-1 hash of the BASE file.
> >
> > If this sounds good enough, I can dive into git-mergetoo--lib and
> > implement it. In the meantime, here is an example of a custom merge tool
> > that wraps meld for that purpose.
> 
> I think you forgot to include the example.  Anyway, at first glance it
> sounds like a sensible idea.  David et al: thoughts?

I think it sounds like a good thing for certain tools.
Sebastian mentioned it being fine in ecmerge and bc3.
xxdiff also lets you specify the output file, so it
probably wouldn't need it either, I think.

If the patch touches individual tools that need it, e.g. meld,
I say go for it.

> Regards,
> Jonathan
> 
> [1] http://bugs.debian.org/637355

cheers,
-- 
					David

Re: git-mergetool: wrap tools with 3 files only to use the BASE file instead of MERGED

[Jonathan Nieder]

David Aguilar wrote:

> I think it sounds like a good thing for certain tools.
> Sebastian mentioned it being fine in ecmerge and bc3.
> xxdiff also lets you specify the output file, so it
> probably wouldn't need it either, I think.

At the risk of taking away the itch for a good feature: meld joined
the crowd of tools with -o to specify an output file in v1.5.0.

 http://thread.gmane.org/gmane.comp.gnome.meld.general/1270

Re: git-mergetool: wrap tools with 3 files only to use the BASE file instead of MERGED

[David Aguilar]

On Thu, Aug 11, 2011 at 03:49:55AM -0500, Jonathan Nieder wrote:
> David Aguilar wrote:
> 
> > I think it sounds like a good thing for certain tools.
> > Sebastian mentioned it being fine in ecmerge and bc3.
> > xxdiff also lets you specify the output file, so it
> > probably wouldn't need it either, I think.
> 
> At the risk of taking away the itch for a good feature: meld joined
> the crowd of tools with -o to specify an output file in v1.5.0.
> 
>  http://thread.gmane.org/gmane.comp.gnome.meld.general/1270

What's the best way to use this and not break existing users?

meld v1.5.0 was released in December.
We could parse `meld --version` and use the new --output flag on
newer setups, leaving old setups alone.  That's a lot of
code to carry around but it's nicest to users.

Here's another idea..
mergetool--lib has vimdiff and vimdiff2.  Maybe we can add a new
meld3 tool that uses the --output flag?  Users with older setups
are unaffected. Documentation is changed to mention meld v1.5.0
and the meld3 tool.  New users have to configure :-/

Alternatively, rename the existing meld to meld2 and let the
new style call take over the current meld tool.
Older setups with new git that use meld can adjust their
config.

(and the final option: refactor mergetool--lib into separate
 files.  Yes, we should do that too! ;-))

what do you think?
-- 
					David

Re: git-mergetool: wrap tools with 3 files only to use the BASE file instead of MERGED

[Sebastian Schuberth]

On Thu, Aug 11, 2011 at 11:37, David Aguilar <davvid@gmail.com> wrote:

> What's the best way to use this and not break existing users?
>
> meld v1.5.0 was released in December.
> We could parse `meld --version` and use the new --output flag on
> newer setups, leaving old setups alone.  That's a lot of
> code to carry around but it's nicest to users.

I'd definitely prefer this solution, as it's the most convenient for users.

-- 
Sebastian Schuberth