threads / patch / 122

patchAutomerge fix

Subject: [PATCH] Automerge fix

## tl;dr

4 messages between Apr 19, 2005 and Apr 19, 2005. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Petr Baudis· Apr 19, 2005, 01:02 UTC · lore
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
Show changes to git-merge-one-file-script +1 −1
--- 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
Linus Torvalds· Apr 19, 2005, 02:48 UTC · re: Petr Baudis · lore

Re: [PATCH] Automerge fix

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).

Show 12 quoted lines
> 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
Petr Baudis· Apr 19, 2005, 02:57 UTC · re: Linus Torvalds · lore

Re: [PATCH] Automerge fix

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...

Show 23 quoted lines
> > 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
Junio C Hamano· Apr 19, 2005, 05:26 UTC · re: Linus Torvalds · lore

Re: [PATCH] Automerge fix

>>>>> "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.

← back to recent threads