Junio C Hamano <junkio@xxxxxxx> wrote: > "Shawn O. Pearce" <spearce@xxxxxxxxxxx> writes: > > > diff --git a/git-merge.sh b/git-merge.sh > > index cb09438..7725908 100755 > > --- a/git-merge.sh > > +++ b/git-merge.sh > > @@ -203,7 +203,7 @@ f,*) > > git-update-index --refresh 2>/dev/null > > new_head=$(git-rev-parse --verify "$1^0") && > > git-read-tree -u -v -m $head "$new_head" && > > - finish "$new_head" "Fast forward" > > + finish "$new_head" "Fast forward" || exit 1 > > dropsave > > exit 0 > > ;; > > The shell function "finish" itself exits when there is an error; > is this needed? Yes. Without it: $ git checkout -b 931233bc666b^ $ echo broken >builtin-pickaxe.c $ git pull . next && echo good merge Updating c2e525d..522da27 builtin-pickaxe.c: needs update fatal: Entry 'builtin-pickaxe.c' not uptodate. Cannot merge. good merge Say what? There's no way that fast forward was good! Granted this use case is horrible but that fast forward went very, very badly, but the caller now thinks it was good. > > @@ -214,7 +214,7 @@ f,*) > > + git var GIT_COMMITTER_IDENT >/dev/null || exit 1 > > @@ -225,7 +225,7 @@ f,*) > > + ) || exit 1 > > @@ -253,7 +253,7 @@ f,*) > > +git var GIT_COMMITTER_IDENT >/dev/null || exit 1 > > @@ -327,7 +327,7 @@ done > > + result_commit=$(echo "$merge_msg" | git-commit-tree $result_tree $parents) || exit 1 > > Are these needed? Wouldn't "something || exit" already exit non-zero > when something exits non-zero? Hmm; apparently you are correct. But that seems like magic shell voodoo to me. I honestly didn't expect exit to behave like that. Please delete these 4 hunks and apply my patch without them. -- Shawn. - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html