[REQ] Better error reporting when git rebase cannot rebase

[REQ] Better error reporting when git rebase cannot rebase

[Jake Goulding]

Right now, if you have modified files in the working directory and 
attempt to do a git rebase, you will see messages like:

$ git rebase origin/master
Makefile: needs update

However, it isn't always clear that that means the rebase *did not 
work*. This has bitten a few of my developers a few times when they 
think that a rebase worked and that they need to perform some sort of 
"update" to the files in question.

I would suggest making this more explicit:

$ git rebase origin/master
* error: rebasing branch master on branch origin/master failed
Makefile: uncommitted changes would be overwritten

Thanks!

-Jake

Re: [REQ] Better error reporting when git rebase cannot rebase

[Jeff King]

On Wed, Apr 02, 2008 at 11:44:10AM -0400, Jake Goulding wrote:

> Right now, if you have modified files in the working directory and  
> attempt to do a git rebase, you will see messages like:
>
> $ git rebase origin/master
> Makefile: needs update

I agree that message is terrible. I have long since lost my git
virginity, and it makes sense to me now, but I remember getting confused
by it quite a bit early on.

Unfortunately I am not sure of the best way to fix this message. It
happens at a very low level in read-cache.c, and so it shows up in
several different places. I am not even sure of all the places you can
see it, so coming up with different text that makes more sense is
non-trivial.

> However, it isn't always clear that that means the rebase *did not work*.
> This has bitten a few of my developers a few times when they think that a 
> rebase worked and that they need to perform some sort of "update" to the 
> files in question.

But this is, I think, a slightly separate issue, which is that rebase
just bails without saying why. And that's easy to fix. How about this?

-- >8 --
rebase: warn about uncommitted changes before bailing

If refreshing the index reports modified files, we used to
just bail without saying anything else, leaving the user
only with the cryptic "foo: needs update" message. Let's
also tell them that we are canceling the rebase.

Signed-off-by: Jeff King <peff@peff.net>
---
The message is patterned off the one below it. Maybe "uncommitted" is
not the right word; technically the changes are _unstaged_. And the
message for the index and head differing says the index is "not up to
date"; perhaps that should say "uncommitted changes".

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

diff --git a/git-rebase.sh b/git-rebase.sh
index 9b13b83..045b8ac 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -282,7 +282,10 @@ else
 fi
 
 # The tree must be really really clean.
-git update-index --refresh || exit
+if ! git update-index --refresh; then
+	echo >&2 cannot rebase: you have uncommitted changes
+	exit 1
+fi
 diff=$(git diff-index --cached --name-status -r HEAD --)
 case "$diff" in
 ?*)	echo "cannot rebase: your index is not up-to-date"
-- 
1.5.5.rc2.170.g6aa60.dirty

Re: [REQ] Better error reporting when git rebase cannot rebase

[Jake Goulding]

Jeff King wrote:
> 
> Unfortunately I am not sure of the best way to fix this message. It
> happens at a very low level in read-cache.c, and so it shows up in
> several different places. I am not even sure of all the places you can
> see it, so coming up with different text that makes more sense is
> non-trivial.
> 

I was afraid that something like that might be the case. Would it be 
worth putting in a second request to make that message more specific?

> But this is, I think, a slightly separate issue, which is that rebase
> just bails without saying why. And that's easy to fix. How about this?
> 
 > [.snip.]

That seems good to me. I think the important thing is to make sure that 
the user is aware that what they wanted to do did not happen.