git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v4 2/5] stash: Add a test for when apply fails during stash branch

From
Junio C Hamano <gitster@pobox.com>
Date
Jun 13, 2017, 19:40 UTC
Message-ID
<xmqqefun65h0.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20170608005535.13080-3-joel@teichroeb.net>
Joel Teichroeb <joel@teichroeb.net> writes:
> If the return value of merge recurisve is not checked, the stash could end
> up being dropped even though it was not applied properly
s/recurisve/recursive/
Show 19 quoted lines
> Signed-off-by: Joel Teichroeb <joel@teichroeb.net>
> ---
>  t/t3903-stash.sh | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index cc923e6335..5399fb05ca 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -656,6 +656,20 @@ test_expect_success 'stash branch should not drop the stash if the branch exists
>  	git rev-parse stash@{0} --
>  '
>  
> +test_expect_success 'stash branch should not drop the stash if the apply fails' '
> +	git stash clear &&
> +	git reset HEAD~1 --hard &&
> +	echo foo >file &&
> +	git add file &&
> +	git commit -m initial &&
It's not quite intuitive to call a non-root commit "initial" ;-)
> +	echo bar >file &&
> +	git stash &&
> +	echo baz >file &&
OK, so 'file' has 'foo' in HEAD, 'bar' in the stash@{0}.
> +	test_when_finished "git checkout master" &&
> +	test_must_fail git stash branch new_branch stash@{0} &&

Hmph. Do we blindly checkout new_branch out of stash@{0}^1 and unstash, but because 'file' in the working tree is dirty, we fail to apply the stash and stop?

This sounds like a bug to me. Shouldn't we be staying on 'master', and fail without even creating 'new_branch', when this happens?

In any case we should be testing what branch we are on after this step. What branch should we be on after "git stash branch" fails?

Show 6 quoted lines
> +	git rev-parse stash@{0} --
> +'
> +
>  test_expect_success 'stash apply shows status same as git status (relative to current directory)' '
>  	git stash clear &&
>  	echo 1 >subdir/subfile1 &&
Previous: Joel TeichroebNext: Joel Teichroeb
Message 5 of 26 in “Implement git stash as a builtin command”
  1. 0/5 Implement git stash as a builtin commandJoel Teichroeb, Jun 8, 2017
  2. 1/5 stash: add test for stash create with no filesJoel Teichroeb, Jun 8, 2017
  3. Junio C HamanoJun 13, 2017
  4. 2/5 stash: Add a test for when apply fails during stash branchJoel Teichroeb, Jun 8, 2017
  5. Junio C HamanoJun 13, 2017
  6. Joel TeichroebJun 13, 2017
  7. 3/5 stash: add test for stashing in a detached stateJoel Teichroeb, Jun 8, 2017
  8. Junio C HamanoJun 13, 2017
  9. Joel TeichroebJun 13, 2017
  10. Junio C HamanoJun 13, 2017
  11. 4/5 merge: close the index lock when not writing the new indexJoel Teichroeb, Jun 8, 2017
  12. Junio C HamanoJun 13, 2017
  13. 5/5 stash: implement builtin stashJoel Teichroeb, Jun 8, 2017
  14. Thomas GummererJun 11, 2017
  15. Joel TeichroebJun 20, 2017
  16. Thomas GummererJun 25, 2017
  17. Matthieu MoyJun 26, 2017
  18. Thomas GummererJun 27, 2017
  19. Junio C HamanoJun 16, 2017
  20. Junio C HamanoJun 16, 2017
  21. Johannes SchindelinJun 19, 2017
  22. Jeff KingJun 19, 2017
  23. Joel TeichroebJun 20, 2017
  24. Junio C HamanoJun 22, 2017
  25. Junio C HamanoJun 22, 2017
  26. Joel TeichroebJun 11, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.