Re: [PATCH] git stash: Avoid data loss when saving a stash

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 30, 2013, 19:14 UTC
Message-ID: <7vppv3jtrh.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130630132017.GQ12252@machine.or.cz>

Petr Baudis <pasky@ucw.cz> writes:

Show 18 quoted lines

>   Hi!
>
> On Fri, Jun 28, 2013 at 11:39:16AM -0700, Junio C Hamano wrote:
>> Thanks.  I'll queue it with a pair of fix-up commits on top, so that
>> they can later be squashed in.
>> 
>> The result of squashing the fix-ups would look like this.
>
>   Thanks! I agree with all of your changes.
>
>> -- >8 --
>> From: Petr Baudis <pasky@ucw.cz>
>> Date: Fri, 28 Jun 2013 17:05:32 +0200
>> Subject: [PATCH] git stash: avoid data loss when "git stash save" kills a directory
>
>   Hmm, it's a pity that the note that `git reset --hard` itself should
> perhaps also abort in that case got lost. I don't insist on mentioning
> it in the commit message, though.

I do not agree with your `git reset --hard` at all. With the command, the user demands "no matter what, I want get rid of any funny state in my working tree so that I can start my work from that specified commit (default to HEAD)".

Imagine that this is you did to arrive that "funny state":

	$ git rm foo ;# foo used to be tracked and in HEAD
        $ cp /somewhere/else/foo foo
	$ cp /somewhere/else/bar bar ;# bar is not in HEAD
	$ cp /somewhere/else/bar baz ;# baz is in HEAD
        ... do various other things ...

and then "git reset --hard". At that point, "foo" and "bar" are not tracked and completely unrelated to the project. "baz" is tracked and have unrelated contents from that of "HEAD".

In order to satisfy your desire to go back to the state of HEAD with minimal collateral amage, we need to get rid of the updated "foo" and "baz" and replace them with those from HEAD. We do not have to touch "bar" so we leave it as-is.

And the "killed" case is just like "foo" and "baz". If the state you want to go back to with "--hard" has a directory (a file) where your working tree's funny state has a file (a directory), the local cruft needs to go away to satisify your request.

I do not mind if you are proposing a different and new kind of reset that fails if it has to overwrite any local changes (be it tracked or untracked), but that is not "reset --hard". It is something else.

Show 15 quoted lines

> On Fri, Jun 28, 2013 at 02:30:15PM -0700, Junio C Hamano wrote:
>> -- >8 --
>> Subject: treat_directory(): do not declare submodules in index to be untracked
>
>   Oh, you are truly awesome! I admit that properly reviewing this patch
> is a little out of my depth right now as I'm not familiar with this
> infrastructure. I'd just like to note...
>
>>  	case index_gitdir:
>>  		if (dir->flags & DIR_SHOW_OTHER_DIRECTORIES)
>>  			return path_none;
>> -		return path_untracked;
>> +		return path_none;
>
> ...that the if-test can be removed now as both branches are the same.

Thanks for noticing. What was queued on 'pu' should already have fixed that one.