"fatal: git-write-tree: error building trees" from `git stash`

"fatal: git-write-tree: error building trees" from `git stash`

[Alex Vandiver]

  Heya,
I just ran into the following with `git stash`.  The set-up:

        git init
        echo "Initial" > foo
        git add .
        git commit -m 'Initial commit'
        echo "Rewrite" > foo
        git commit -am 'Second commit, rewrites content'
        echo "Stashed changes" >> foo
        git stash
        git co HEAD~
        
$ git stash pop
Auto-merging foo
CONFLICT (content): Merge conflict in foo
Recorded preimage for 'foo'

$ git stash
foo: needs merge
foo: needs merge
foo: unmerged (aeaa7e5e87cf309a7368d5d92a71c1f9e6a8c9e7)
foo: unmerged (a77fa514de2720c72c1a861de098595959a2c97a)
foo: unmerged (4a622d2b991f1a19ba7be313a46dc6f03692cd0a)
fatal: git-write-tree: error building trees
Cannot save the current index state

 - Alex

Re: "fatal: git-write-tree: error building trees" from `git stash`

[Junio C Hamano]

Alex Vandiver <alex@chmrr.net> writes:

> Heya,
> I just ran into the following with `git stash`.  The set-up:
> ...
> $ git stash pop
> Auto-merging foo
> CONFLICT (content): Merge conflict in foo
> Recorded preimage for 'foo'
>
> $ git stash
> foo: needs merge
> foo: needs merge
> foo: unmerged (aeaa7e5e87cf309a7368d5d92a71c1f9e6a8c9e7)
> foo: unmerged (a77fa514de2720c72c1a861de098595959a2c97a)
> foo: unmerged (4a622d2b991f1a19ba7be313a46dc6f03692cd0a)
> fatal: git-write-tree: error building trees
> Cannot save the current index state

This is totally expected, isn't it?

You do not save state in the middle of a conflict with "git stash"
(instead, you would "git stash" away your own work in progress
before you start operation that may create and leave conflicts).

Re: "fatal: git-write-tree: error building trees" from `git stash`

[Alex Vandiver]

On Thu, 2012-12-27 at 10:51 -0800, Junio C Hamano wrote:
> > $ git stash
> > foo: needs merge
> > foo: needs merge
> > foo: unmerged (aeaa7e5e87cf309a7368d5d92a71c1f9e6a8c9e7)
> > foo: unmerged (a77fa514de2720c72c1a861de098595959a2c97a)
> > foo: unmerged (4a622d2b991f1a19ba7be313a46dc6f03692cd0a)
> > fatal: git-write-tree: error building trees
> > Cannot save the current index state
> 
> This is totally expected, isn't it?
> 
> You do not save state in the middle of a conflict with "git stash"
> (instead, you would "git stash" away your own work in progress
> before you start operation that may create and leave conflicts).

Apologies for not being clear.  While being unable to stash is not
unexpected, perhaps, "Cannot stash while resolving conflicts" or similar
would be more understandable to the end user than the above.
 - Alex

Re: "fatal: git-write-tree: error building trees" from `git stash`

[Jeff King]

On Thu, Dec 27, 2012 at 01:55:56PM -0500, Alex Vandiver wrote:

> On Thu, 2012-12-27 at 10:51 -0800, Junio C Hamano wrote:
> > > $ git stash
> > > foo: needs merge
> > > foo: needs merge
> > > foo: unmerged (aeaa7e5e87cf309a7368d5d92a71c1f9e6a8c9e7)
> > > foo: unmerged (a77fa514de2720c72c1a861de098595959a2c97a)
> > > foo: unmerged (4a622d2b991f1a19ba7be313a46dc6f03692cd0a)
> > > fatal: git-write-tree: error building trees
> > > Cannot save the current index state
> > 
> > This is totally expected, isn't it?
> > 
> > You do not save state in the middle of a conflict with "git stash"
> > (instead, you would "git stash" away your own work in progress
> > before you start operation that may create and leave conflicts).
> 
> Apologies for not being clear.  While being unable to stash is not
> unexpected, perhaps, "Cannot stash while resolving conflicts" or similar
> would be more understandable to the end user than the above.

Yeah, I think the outcome is reasonable, but that message is just
horrible. Something like this might be better:

diff --git a/git-stash.sh b/git-stash.sh
index 688e259..7ea425c 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -217,6 +217,12 @@ save_stash () {
 
 	stash_msg="$*"
 
+	if ! git diff-index --cached --diff-filter=U --quiet HEAD; then
+		echo >&2 "fatal: unable to stash unmerged entries:"
+		git diff-index --cached --diff-filter=U --name-status HEAD
+		exit 1
+	fi
+
 	git update-index -q --refresh
 	if no_changes
 	then

but I suspect it is not sufficient:

  1. There are other code paths that will end up in write-tree which
     should probably be protected, too.

  2. Unmerged entries are only one reason that write-tree might fail.
     It's OK not to catch them all (since ultimately write-tree will
     complain if need be), but we may want to also handle intent-to-add
     entries with a nicer message.

-Peff

Re: "fatal: git-write-tree: error building trees" from `git stash`

[Junio C Hamano]

Alex Vandiver <alex@chmrr.net> writes:

> ... "Cannot stash while resolving conflicts" or similar would be
> more understandable to the end user than the above.

Interestingly enough, the "apply" side is protected with this one
liner:

        # current index state
        c_tree=$(git write-tree) ||
                die "$(gettext "Cannot apply a stash in the middle of a merge")"

since 5fd448f (git stash: Give friendlier errors when there is
nothing to apply, 2009-08-11).  I would think something in line with
that change on the "create" side is a welcome one.

Thanks.

Re: "fatal: git-write-tree: error building trees" from `git stash`

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> but I suspect it is not sufficient:
>
>   1. There are other code paths that will end up in write-tree which
>      should probably be protected, too.

Among 6 calls to write-tree, only the first ones in create_stash and
apply_stash are about the index the user originally had.  If the
only expected failure case is unmerged entries, it should be
sufficient to protect these two (and the one in apply_stash is
already covered, I think).

>   2. Unmerged entries are only one reason that write-tree might fail.
>      It's OK not to catch them all (since ultimately write-tree will
>      complain if need be), but we may want to also handle intent-to-add
>      entries with a nicer message.

Hrmph.

We used to fail write-tree when I-T-A entries existed and relied on
that behaviour to implement "no state lost"; as we broke write-tree
recently by allowing to write a tree out by pretending that I-T-A
entries do not exist, I think we broke it.  Stashing with I-T-A and
then unstashing it may lose the file.  Sigh...