threads / discuss / 32467

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

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

## tl;dr

6 messages between Dec 27, 2012 and Dec 27, 2012.

replies: 5people: 3as markdown or json

Alex Vandiver· Dec 27, 2012, 18:07 UTC · lore
  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
Junio C Hamano· Dec 27, 2012, 18:51 UTC · re: Alex Vandiver · lore

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

Alex Vandiver <alex@chmrr.net> writes:
Show 16 quoted lines
> 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).

Alex Vandiver· Dec 27, 2012, 18:55 UTC · re: Junio C Hamano · lore

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

On Thu, 2012-12-27 at 10:51 -0800, Junio C Hamano wrote:
Show 14 quoted lines
> > $ 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
Jeff King· Dec 27, 2012, 19:05 UTC · re: Alex Vandiver · lore

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

On Thu, Dec 27, 2012 at 01:55:56PM -0500, Alex Vandiver wrote:
Show 19 quoted lines
> 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
Junio C Hamano· Dec 27, 2012, 20:11 UTC · re: Jeff King · lore

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

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...

Junio C Hamano· Dec 27, 2012, 19:21 UTC · re: Alex Vandiver · lore

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

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.

← back to recent threads