Re: [PATCH v5 1/1] mergetool: add automerge configuration

[Junio C Hamano <gitster@pobox.com>]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> +auto_merge () {
> +	git merge-file --diff3 --marker-size=7 -q -p "$LOCAL" "$BASE" "$REMOTE" >"$DIFF3"
> +	if test -s "$DIFF3"
> +	then

We do not want to ignore the exit status from the command.  IOW, I
think the above wants to be rather

	if git merge-file ... >"$DIFF3" &&
	   test -s "$DIFF3"
	then
		...

to catch a merge-file that writes halfway and then crashes (doing
the same check in different ways are probably possible, of course)

> +		sed -e '/^<<<<<<< /,/^||||||| /d' -e '/^=======\r\?$/,/^>>>>>>> /d' "$DIFF3" >"$BASE"

Does everybody's sed take "\?" and interprets it as zero-or-one?
POSIX uses BRE and it doesn't like \? as far as I recall, and "-E"
to force ERE is a GNUism.

> +		sed -e '/^||||||| /,/^>>>>>>> /d' -e '/^<<<<<<< /d' "$DIFF3" >"$LOCAL"
> +		sed -e '/^<<<<<<< /,/^=======\r\?$/d' -e '/^>>>>>>> /d' "$DIFF3" >"$REMOTE"

I'd feel safer if these resulting $BASE, $LOCAL and $REMOTE are
validated to be conflict-marker free (i.e. '^\([<|=>]\)\1\1\1\1\1\1'
does not appear) to make sure there was no funny virtual ancestor
that records a conflicted recursive merge result confused our logic.

When we see an unfortunate sign that it happened, we can revert the
automerge and let the tool handle the original input.

> +	fi
> +	rm -- "$DIFF3"
> +}
> +

"$DIFF3" is always created (unless shell redirection into it fails),
so "rm --" would be fine in practice, I guess, but "rm -f --" would
not hurt.

> +	DIFF3="$MERGETOOL_TMPDIR/${BASE}_DIFF3_$$$ext"

$MERGETOOL_TMPDIR is either "mktemp -d -t "git-mergetool-XXXXXX" or
".".  Also, we liberally pass "$DIFF3" to "sed" as an argument and
assume that the command would take it as a filename and not an
option.

For the above reason, "rm --", while it is not wrong per-se, can be
just a simple "rm", as there is no funny leading letters in "$DIFF3"
that requires disambiguation.