Re: p4Merge bundled command and the behaviour with files (same name) added on different branches.
- From
David Aguilar <davvid@gmail.com>
- Date
- Apr 7, 2011, 09:43 UTC
- Message-ID
- <20110407094347.GB7593@gmail.com>
- In-Reply-To
- <BANLkTimpsg=26C0mq=feVT7mt0nwZBoBUA@mail.gmail.com>
On Mon, Apr 04, 2011 at 09:55:41AM +0100, Ciaran wrote:
Show 41 quoted lines
> [...] > 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.
Show 15 quoted lines
> 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