git pull fails to exit with non-zero status after fatal error

git pull fails to exit with non-zero status after fatal error

[Larry Streepy]

We have just upgraded to 1.5 (don't know if this is specific to 1.5, but 
just in case).  When I perform a git-pull on a working repo that has a 
modified file, git pull refuses to do the pull, as shown below:

$ git pull
Updating b5d9263..506b347
tools/Pvt/Pvt.py: needs update
fatal: Entry 'tools/Pvt/Pvt.py' not uptodate. Cannot merge.
$ echo $?
0

Notice the exit status of 0.  The driver scripts I use rely on a 
non-zero status to send me email when the git-pull fails, but this case 
eludes detection.

I'm assuming this is a bug, but if it's intended behavior I'd like to 
understand why it would use a 0 exit status after a issuing a fatal error.

Thanks,
Larry.

Re: git pull fails to exit with non-zero status after fatal error

[Johannes Schindelin]

Hi,

On Sat, 3 Mar 2007, Larry Streepy wrote:

> We have just upgraded to 1.5 (don't know if this is specific to 1.5, but just
> in case).  When I perform a git-pull on a working repo that has a modified
> file, git pull refuses to do the pull, as shown below:
> 
> $ git pull
> Updating b5d9263..506b347
> tools/Pvt/Pvt.py: needs update
> fatal: Entry 'tools/Pvt/Pvt.py' not uptodate. Cannot merge.
> $ echo $?
> 0
> 
> Notice the exit status of 0.

Does this fix it?

diff --git a/git-merge.sh b/git-merge.sh
index 498c938..6b23bf5 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -295,8 +295,9 @@ f,*)
 	new_head=$(git-rev-parse --verify "$1^0") &&
 	git-read-tree -v -m -u --exclude-per-directory=.gitignore $head "$new_head" &&
 	finish "$new_head" "Fast forward"
+	ret=$?
 	dropsave
-	exit 0
+	exit $ret
 	;;
 ?,1,?*"$LF"?*,*)
 	# We are not doing octopus and not fast forward.  Need a

Re: git pull fails to exit with non-zero status after fatal error

[Larry Streepy]

I'll give it a try and let you know.

Thanks for the quick feedback.

Larry.

Johannes Schindelin wrote:
> Does this fix it?
>
> diff --git a/git-merge.sh b/git-merge.sh
> index 498c938..6b23bf5 100755
> --- a/git-merge.sh
> +++ b/git-merge.sh
> @@ -295,8 +295,9 @@ f,*)
>  	new_head=$(git-rev-parse --verify "$1^0") &&
>  	git-read-tree -v -m -u --exclude-per-directory=.gitignore $head "$new_head" &&
>  	finish "$new_head" "Fast forward"
> +	ret=$?
>  	dropsave
> -	exit 0
> +	exit $ret
>  	;;
>  ?,1,?*"$LF"?*,*)
>  	# We are not doing octopus and not fast forward.  Need a
>   

Re: git pull fails to exit with non-zero status after fatal error

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Sat, 3 Mar 2007, Larry Streepy wrote:
>
>> $ git pull
>> Updating b5d9263..506b347
>> tools/Pvt/Pvt.py: needs update
>> fatal: Entry 'tools/Pvt/Pvt.py' not uptodate. Cannot merge.
>> $ echo $?
>> 0
>> 
>> Notice the exit status of 0.
>
> Does this fix it?
>
> diff --git a/git-merge.sh b/git-merge.sh
> index 498c938..6b23bf5 100755
> --- a/git-merge.sh
> +++ b/git-merge.sh
> @@ -295,8 +295,9 @@ f,*)
>  	new_head=$(git-rev-parse --verify "$1^0") &&
>  	git-read-tree -v -m -u --exclude-per-directory=.gitignore $head "$new_head" &&
>  	finish "$new_head" "Fast forward"
> +	ret=$?
>  	dropsave
> -	exit 0
> +	exit $ret
>  	;;
>  ?,1,?*"$LF"?*,*)
>  	# We are not doing octopus and not fast forward.  Need a

The cation "dropsave" takes is to remove the files that are not
needed after a successful merge, so I think it is better to fail
if the && chain that ends with finish, like this.

diff --git a/git-merge.sh b/git-merge.sh
index 498c938..4afcd95 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -294,7 +294,7 @@ f,*)
 	git-update-index --refresh 2>/dev/null
 	new_head=$(git-rev-parse --verify "$1^0") &&
 	git-read-tree -v -m -u --exclude-per-directory=.gitignore $head "$new_head" &&
-	finish "$new_head" "Fast forward"
+	finish "$new_head" "Fast forward" || exit
 	dropsave
 	exit 0
 	;;

Re: git pull fails to exit with non-zero status after fatal error

[Johannes Schindelin]

Hi,

On Sat, 3 Mar 2007, Junio C Hamano wrote:

> -	finish "$new_head" "Fast forward"
> +	finish "$new_head" "Fast forward" || exit

Doesn't "exit" default to "exit 0"?

Ciao,
Dscho

Re: git pull fails to exit with non-zero status after fatal error

[Shawn O. Pearce]

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Sat, 3 Mar 2007, Junio C Hamano wrote:
> 
> > -	finish "$new_head" "Fast forward"
> > +	finish "$new_head" "Fast forward" || exit
> 
> Doesn't "exit" default to "exit 0"?

No, it carries the exit status of the prior failed command if the
prior command failed.  ;-)

For what its worth, I learned that only while hacking on git-merge.
I think it is highly non-obvious.

-- 
Shawn.

Re: git pull fails to exit with non-zero status after fatal error

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>> On Sat, 3 Mar 2007, Junio C Hamano wrote:
>> 
>> > -	finish "$new_head" "Fast forward"
>> > +	finish "$new_head" "Fast forward" || exit
>> 
>> Doesn't "exit" default to "exit 0"?
>
> No, it carries the exit status of the prior failed command if the
> prior command failed.  ;-)
>
> For what its worth, I learned that only while hacking on git-merge.
> I think it is highly non-obvious.

Sorry to be a traditionalist.  "sequence && of && commands || exit"
is an established bourne shell idiom.

I do not mind if somebody proposed to change that to "... || exit $?"
to unconfuse uninitiated, though.

Re: git pull fails to exit with non-zero status after fatal error

[Johannes Schindelin]

Hi,

On Sat, 3 Mar 2007, Shawn O. Pearce wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > On Sat, 3 Mar 2007, Junio C Hamano wrote:
> > 
> > > -	finish "$new_head" "Fast forward"
> > > +	finish "$new_head" "Fast forward" || exit
> > 
> > Doesn't "exit" default to "exit 0"?
> 
> No, it carries the exit status of the prior failed command if the
> prior command failed.  ;-)

You live and learn, live and learn...

Thanks,
Dscho