git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Aguilar <davvid@gmail.com>
To: Ciaran <ciaranj@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: p4Merge bundled command and the behaviour with files (same name) added on different branches.
Date: Thu, 7 Apr 2011 02:43:49 -0700	[thread overview]
Message-ID: <20110407094347.GB7593@gmail.com> (raw)
In-Reply-To: <BANLkTimpsg=26C0mq=feVT7mt0nwZBoBUA@mail.gmail.com>

On Mon, Apr 04, 2011 at 09:55:41AM +0100, Ciaran wrote:
> [...]
> We would expect a 'both added' merge conflict (both the other branch,
> and the master branch added the file named bar.txt, but with different
> content.)  This is all good and right.  So in a system configured to
> use p4merge as the mergetool, one fires up with 'git mergetool'
> 
> What happens now is p4merge starts and tells us:
> 
> Base: bar.txt.LOCAL.<num1>.txt
> Left: bar.txt.LOCAL.<num1>.txt Differences from base: 0
> Right: bar.txt.LOCAL.<num2>.txt Differences from base: 1
> Merge: bar.txt Conflicts:0
> 
> Presenting the left + right options on top of each other in the result
> window (which may be correct) and leaving the save button disabled
> (grayed out)
> 
> If at this point one closes the window without editing the presented
> (apparently merged) file, then nothing will be saved to disk and we
> will see:
> 
> bar.txt seems unchanged.
> Was the merge successful? [y/n]
> 
> In the console.  Which Git wise is correct, that is exactly right, the
> p4merge tool hasn't made any actual changes to the underlying file.
> 
> This behaviour seems confusing to me (the p4merge client behaviour,
> *not* Git's)   I believe it is because in the case where there is no
> logical base between two files the local one is arbritrarily chosen,
> and p4merge *thinks* that this is equal to the merge result and has
> nothing to persist.
> 
> I have attached a patch that resolves the issue for me (e.g.
> introduces the behaviour I expect) by passing a reference to an empty
> file in the case where there is no meaningful base.  Unfortunately I
> don't understand enough to say whether this change is correct or not
> and would value feedback on it.
> 
> Many thanks
>  - Cj.

Thanks.  If this patch were for actual consideration you would
inline the patch instead of sending an attachment as described
in Documentation/SubmittingPatches.  Marking the subject line
with "[RFC PATCH]" lets us know that you're interested in
feedback.  I have a few questions below.

> index fb3f52b..3e486dc 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -262,7 +262,9 @@ run_merge_tool () {
>  			if $base_present; then
>  				"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" "$MERGED"
>  			else
> -				"$merge_tool_path" "$LOCAL" "$LOCAL" "$REMOTE" "$MERGED"
> +				touch ".empty"
> +				"$merge_tool_path" ".empty" "$LOCAL" "$REMOTE" "$MERGED"
> +				rm ".empty"
>  			fi
>  			check_unchanged
>  		else
> -- 

What if the user has a file called '.empty' in their repository?

What if the user Ctrl-C's out of mergetool -- does a stale
.empty file get left behind?

Does it work if we pass /dev/null instead?
Is such a strategy portable to Windows?

If /dev/null doesn't work, would it be better if the
empty file were given a different name?
Maybe something like foo.EMPTY.<num>.txt?
-- 
		David

  reply	other threads:[~2011-04-07  9:44 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-04  8:55 p4Merge bundled command and the behaviour with files (same name) added on different branches Ciaran
2011-04-07  9:43 ` David Aguilar [this message]
2011-04-07  9:48   ` Ciaran

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110407094347.GB7593@gmail.com \
    --to=davvid@gmail.com \
    --cc=ciaranj@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).