[PATCH] Automerge fix

[PATCH] Automerge fix

[Petr Baudis]

Hello,

this patch fixes git-merge-one-file-script's automerge.

Signed-off-by: Petr Baudis <pasky@ucw.cz>

git-merge-one-file-script: 7ebf5dac4c69043cd2ff89bf7ee552152802f8d1
--- a/git-merge-one-file-script
+++ b/git-merge-one-file-script
@@ -43,7 +43,7 @@ case "${1:-.}${2:-.}${3:-.}" in
 	orig=$(unpack-file $1)
 	src1=$(unpack-file $2)
 	src2=$(unpack-file $3)
-	merge "$src2" "$orig" "$src1" || echo Leaving conflict merge in $src2 && exit 1
+	merge "$src2" "$orig" "$src1" || (echo Leaving conflict merge in $src2 && exit 1)
 	cp "$src2" "$4" && update-cache --add -- "$4" && exit 0
 	;;
 


-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

Re: [PATCH] Automerge fix

[Linus Torvalds]

On Tue, 19 Apr 2005, Petr Baudis wrote:
> 
> this patch fixes git-merge-one-file-script's automerge.

Nope, it doesn't. The original may not have worked, but neither does your 
vesion either: the reason for the exit 1 is that the _script_ should exit, 
but when you put it in a sub-shell with (..), now only the subshell exits 
with an error code, and we'll happily continue to do the following line 
which we should not do (since the merge failed).

> Signed-off-by: Petr Baudis <pasky@ucw.cz>
> 
> git-merge-one-file-script: 7ebf5dac4c69043cd2ff89bf7ee552152802f8d1
> --- a/git-merge-one-file-script
> +++ b/git-merge-one-file-script
> @@ -43,7 +43,7 @@ case "${1:-.}${2:-.}${3:-.}" in
>  	orig=$(unpack-file $1)
>  	src1=$(unpack-file $2)
>  	src2=$(unpack-file $3)
> -	merge "$src2" "$orig" "$src1" || echo Leaving conflict merge in $src2 && exit 1
> +	merge "$src2" "$orig" "$src1" || (echo Leaving conflict merge in $src2 && exit 1)
>  	cp "$src2" "$4" && update-cache --add -- "$4" && exit 0

What's the right way?

Maybe

	if merge "$src2" "$orig" "$src1"
	then 
		cp "$src2" "$4" && update-cache --add -- "$4" && exit 0
	fi
	echo Leaving conflict merge in $src2
	exit 1

would work?

		Linus

Re: [PATCH] Automerge fix

[Petr Baudis]

Dear diary, on Tue, Apr 19, 2005 at 04:48:09AM CEST, I got a letter
where Linus Torvalds <torvalds@osdl.org> told me that...
> > git-merge-one-file-script: 7ebf5dac4c69043cd2ff89bf7ee552152802f8d1
> > --- a/git-merge-one-file-script
> > +++ b/git-merge-one-file-script
> > @@ -43,7 +43,7 @@ case "${1:-.}${2:-.}${3:-.}" in
> >  	orig=$(unpack-file $1)
> >  	src1=$(unpack-file $2)
> >  	src2=$(unpack-file $3)
> > -	merge "$src2" "$orig" "$src1" || echo Leaving conflict merge in $src2 && exit 1
> > +	merge "$src2" "$orig" "$src1" || (echo Leaving conflict merge in $src2 && exit 1)
> >  	cp "$src2" "$4" && update-cache --add -- "$4" && exit 0
> 
> What's the right way?
> 
> Maybe
> 
> 	if merge "$src2" "$orig" "$src1"
> 	then 
> 		cp "$src2" "$4" && update-cache --add -- "$4" && exit 0
> 	fi
> 	echo Leaving conflict merge in $src2
> 	exit 1
> 
> would work?

Possibly. Or changing () to {} as suggested by Edgar Toernig.

FWIW, my fragment of this code now looks like:

        ret=0
        if ! merge "$src2" "$orig" "$src1"; then
                echo Conflicting merge!
                cat "$src2" >"$4"
                ret=1

        elif ! cat "$src2" >"$4" && update-cache --add -- "$4"; then
                echo "Choosing $src2 -> $4 failed"
                ret=1
        fi
        rm "$orig" "$src1" "$src2"
        exit $ret

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

Re: [PATCH] Automerge fix

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> What's the right way?

LT> Maybe

LT> 	if merge "$src2" "$orig" "$src1"
LT> 	then 
LT> 		cp "$src2" "$4" && update-cache --add -- "$4" && exit 0
LT> 	fi
LT> 	echo Leaving conflict merge in $src2
LT> 	exit 1

LT> would work?

Wouldn't this be more readable, short and sweet?

    merge "$src2" "$orig" "$src1" || {
        echo Leaving conflict merge in $src2 && exit 1
    }
    cp "$src2" "$4" && exec update-cache --add -- "$4"

You did not want subshell so I just changed the () pair to the
{} pair, and while I was at it I folded the "&& exit 0" into the
last command before it, which should be better.  You'd want to
know if update-cache --add failed for whatever reason.