[PATCH 1/2] rebase: check for errors from git-commit

[PATCH 1/2] rebase: check for errors from git-commit

[Eric Wong]

commit does not always succeed, so we'll have to check for
it in the absence of set -e.  This fixes a regression
introduced in 9e4bc7dd1bb9d92491c475cec55147fa0b3f954d

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---

 I've grown used to having 'set -e' at the beginning of my shell
 scripts.  IMHO it'd be a good idea to start moving towards this
 eventually (even though shell scripts seem to be getting phased-out
 somewhat).

 git-rebase.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 9ad1c44..47c8e8f 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -59,8 +59,8 @@ continue_merge () {
 
 	if test -n "`git-diff-index HEAD`"
 	then
+		git-commit -C "`cat $dotest/current`" || die 'Commit failed'
 		printf "Committed: %0${prec}d" $msgnum
-		git-commit -C "`cat $dotest/current`"
 	else
 		printf "Already applied: %0${prec}d" $msgnum
 	fi
-- 
1.4.1.rc1.g3cc8

[PATCH 2/2] rebase: get rid of outdated MRESOLVEMSG

[Eric Wong]

There was a time when rebase --skip didn't work when used with
--merge, but that is no more so we don't need that message
anymore.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-rebase.sh |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 47c8e8f..16fe6e0 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -34,11 +34,6 @@ When you have resolved this problem run 
 If you would prefer to skip this patch, instead run \"git rebase --skip\".
 To restore the original branch and stop rebasing run \"git rebase --abort\".
 "
-
-MRESOLVEMSG="
-When you have resolved this problem run \"git rebase --continue\".
-To restore the original branch and stop rebasing run \"git rebase --abort\".
-"
 unset newbase
 strategy=recursive
 do_merge=
@@ -54,7 +49,7 @@ continue_merge () {
 	then
 		echo "You still have unmerged paths in your index"
 		echo "did you forget update-index?"
-		die "$MRESOLVEMSG"
+		die "$RESOLVEMSG"
 	fi
 
 	if test -n "`git-diff-index HEAD`"
@@ -87,11 +82,11 @@ call_merge () {
 		;;
 	1)
 		test -d "$GIT_DIR/rr-cache" && git-rerere
-		die "$MRESOLVEMSG"
+		die "$RESOLVEMSG"
 		;;
 	2)
 		echo "Strategy: $rv $strategy failed, try another" 1>&2
-		die "$MRESOLVEMSG"
+		die "$RESOLVEMSG"
 		;;
 	*)
 		die "Unknown exit code ($rv) from command:" \
-- 
1.4.1.rc1.g3cc8

Re: [PATCH 1/2] rebase: check for errors from git-commit

[Junio C Hamano]

Eric Wong <normalperson@yhbt.net> writes:

>  I've grown used to having 'set -e' at the beginning of my shell
>  scripts.  IMHO it'd be a good idea to start moving towards this
>  eventually (even though shell scripts seem to be getting phased-out
>  somewhat).
>
>  git-rebase.sh |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 9ad1c44..47c8e8f 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -59,8 +59,8 @@ continue_merge () {
>  
>  	if test -n "`git-diff-index HEAD`"
>  	then
> +		git-commit -C "`cat $dotest/current`" || die 'Commit failed'
>  		printf "Committed: %0${prec}d" $msgnum
> -		git-commit -C "`cat $dotest/current`"

Anticipating failure from "git-commit" is the right thing to do,
but this is a "Now what?" situation.  What is the expected
course of action to recover from this for the end user, and how
can we phrase the error message to help that process?

Re: [PATCH 1/2] rebase: check for errors from git-commit

[Eric Wong]

Junio C Hamano <junkio@cox.net> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> 
> >  I've grown used to having 'set -e' at the beginning of my shell
> >  scripts.  IMHO it'd be a good idea to start moving towards this
> >  eventually (even though shell scripts seem to be getting phased-out
> >  somewhat).
> >
> >  git-rebase.sh |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/git-rebase.sh b/git-rebase.sh
> > index 9ad1c44..47c8e8f 100755
> > --- a/git-rebase.sh
> > +++ b/git-rebase.sh
> > @@ -59,8 +59,8 @@ continue_merge () {
> >  
> >  	if test -n "`git-diff-index HEAD`"
> >  	then
> > +		git-commit -C "`cat $dotest/current`" || die 'Commit failed'
> >  		printf "Committed: %0${prec}d" $msgnum
> > -		git-commit -C "`cat $dotest/current`"
> 
> Anticipating failure from "git-commit" is the right thing to do,
> but this is a "Now what?" situation.  What is the expected
> course of action to recover from this for the end user, and how
> can we phrase the error message to help that process?

I would expect git-commit to show the correct error message (or the
pre-commit hook), die "$RESOLVEMSG" might be a better option, though.

-- 
Eric Wong

Re: [PATCH 1/2] rebase: check for errors from git-commit

[Junio C Hamano]

Eric Wong <normalperson@yhbt.net> writes:

> Junio C Hamano <junkio@cox.net> wrote:
>
>> Anticipating failure from "git-commit" is the right thing to do,
>> but this is a "Now what?" situation.  What is the expected
>> course of action to recover from this for the end user, and how
>> can we phrase the error message to help that process?
>
> I would expect git-commit to show the correct error message (or the
> pre-commit hook), die "$RESOLVEMSG" might be a better option, though.

It would say 'resolve conflicts, mark them with update-index as
necessary, and say git-commit' or somesuch.  I am not sure you
would want the user to make a commit like suggested -- instead
you would want him to say 'git rebase --continue' wouldn't you?

Re: [PATCH 1/2] rebase: check for errors from git-commit

[Eric Wong]

Junio C Hamano <junkio@cox.net> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> 
> > Junio C Hamano <junkio@cox.net> wrote:
> >
> >> Anticipating failure from "git-commit" is the right thing to do,
> >> but this is a "Now what?" situation.  What is the expected
> >> course of action to recover from this for the end user, and how
> >> can we phrase the error message to help that process?
> >
> > I would expect git-commit to show the correct error message (or the
> > pre-commit hook), die "$RESOLVEMSG" might be a better option, though.
> 
> It would say 'resolve conflicts, mark them with update-index as
> necessary, and say git-commit' or somesuch.  I am not sure you
> would want the user to make a commit like suggested -- instead
> you would want him to say 'git rebase --continue' wouldn't you?

Actually RESOLVEMSG in git-rebase just tells the user about the
--continue, --skip, --abort options.

-- 
Eric Wong

Re: [PATCH 1/2] rebase: check for errors from git-commit

[Junio C Hamano]

Eric Wong <normalperson@yhbt.net> writes:

> Junio C Hamano <junkio@cox.net> wrote:
>> Eric Wong <normalperson@yhbt.net> writes:
>> 
>> > Junio C Hamano <junkio@cox.net> wrote:
>> >
>> >> Anticipating failure from "git-commit" is the right thing to do,
>> >> but this is a "Now what?" situation.  What is the expected
>> >> course of action to recover from this for the end user, and how
>> >> can we phrase the error message to help that process?
>> >
>> > I would expect git-commit to show the correct error message (or the
>> > pre-commit hook), die "$RESOLVEMSG" might be a better option, though.
>> 
>> It would say 'resolve conflicts, mark them with update-index as
>> necessary, and say git-commit' or somesuch.  I am not sure you
>> would want the user to make a commit like suggested -- instead
>> you would want him to say 'git rebase --continue' wouldn't you?
>
> Actually RESOLVEMSG in git-rebase just tells the user about the
> --continue, --skip, --abort options.

Yup, so telling the user to disregard what commit might say (but
do not hide it by redirecting 2>/dev/null, please) and dying
with "$RESOLVEMSG" might be a better way.