Re: [PATCH v2 1/3] stash bug: stash can lose data in a file removed from the index

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 18, 2010, 23:11 UTC
Message-ID: <7vvdbotkz0.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20100418182749.GA29329@hashpling.org>

Charles Bailey <charles@hashpling.org> writes:

Show 11 quoted lines

> +test_expect_success 'stash file to symlink' '
> +	git reset --hard &&
> +	rm file &&
> +	ln -s file2 file &&
> +	git stash save "file to symlink" &&
> +	test -f file &&
> +	test bar = "$(cat file)" &&
> +	git stash apply &&
> +	test -f file &&
> +	test file2 = "$(readlink file)"
> +'

It is likely that this needs to be protected with SYMLINKS prerequisite. Also I am a bit unhappy about the use of "readlink" which is not even in POSIX.1 here. We already have one use of it in the tests but that only happens while doing valgrind. Traditionally this has been more portably done by reading from "ls -l file", like so:

	case "$(ls -l file)" in *" file -> file2") :;; *) false;; esac

Also, whether "readlink file" or "ls -l file" is used to check the result, the "test -f file" is redundant.

Show 16 quoted lines

> +test_expect_failure 'stash directory to file' '
> +	git reset --hard &&
> +	mkdir dir &&
> +	echo foo >dir/file &&
> +	git add dir/file &&
> +	git commit -m "Add file in dir" &&
> +	rm dir/file &&
> +	rmdir dir &&
> +	echo bar >dir &&
> +	git stash save "directory to file" &&
> +	test -d dir &&
> +	test foo = "$(cat dir/file)" &&
> +	test_must_fail git stash apply &&
> +	test bar = "$(cat dir)" &&
> +	git reset --soft HEAD^
> +'

I have a feeling that this test is being a bit unfair.

What should a successful invocation of "stash apply" leave in the working tree in this case, especially when you consider that in a real life use case you may have other files in "dir" directory or changes to "dir/file"?