All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] stash drops the stash even if creating the branch fails because it already exists
@ 2010-09-28 11:25 Tomas Carnecky
  2010-09-28 13:19 ` [PATCH 0/2] stash: test and fix git stash branch regression Jon Seymour
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Tomas Carnecky @ 2010-09-28 11:25 UTC (permalink / raw)
  To: git; +Cc: tla, Tomas Carnecky

This bug was disovered by someone on IRC when he tried to 'git stash branch <branch> <stash>'
while <branch> already existed. In that case the stash is dropped even though it isn't
applied on any branch, so the stash is effectively lost. I think that shouldn't happen,
so here is a test.

---
 t/t3903-stash.sh |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 9ed2396..0f6b2e4 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -545,4 +545,15 @@ test_expect_success 'invalid ref of the form stash@{n}, n >= N' '
 	git stash drop
 '
 
+test_expect_failure 'stash branch should not drop the stash if the branch exists' '
+	git stash clear &&
+	echo foo > file &&
+	git add file &&
+	git commit -m initial &&
+	echo bar > file &&
+	git stash &&
+	test_must_fail git stash branch master stash@{0} &&
+	git rev-parse stash@{0} --
+'
+
 test_done
-- 
1.7.3.3.gd2416

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 0/2] stash: test and fix git stash branch regression
  2010-09-28 11:25 [PATCH] stash drops the stash even if creating the branch fails because it already exists Tomas Carnecky
@ 2010-09-28 13:19 ` Jon Seymour
  2010-09-28 13:19 ` [PATCH 1/2] stash drops the stash even if creating the branch fails because it already exists Jon Seymour
  2010-09-28 13:19 ` [PATCH 2/2] stash: fix git stash branch regression when branch creation fails Jon Seymour
  2 siblings, 0 replies; 6+ messages in thread
From: Jon Seymour @ 2010-09-28 13:19 UTC (permalink / raw)
  To: git; +Cc: tla, tom, gitster, Jon Seymour

This series fixes another (sorry!) regression I introduced with my detached-stash series.

I have included Tomas' test, so that my tweak to the test script applies cleanly.

Jon Seymour (1):
  stash: fix git stash branch regression when branch creation fails

Tomas Carnecky (1):
  stash drops the stash even if creating the branch fails because it
    already exists

 git-stash.sh     |    6 +++---
 t/t3903-stash.sh |   11 +++++++++++
 2 files changed, 14 insertions(+), 3 deletions(-)

-- 
1.7.3.4.g787b.dirty

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] stash drops the stash even if creating the branch fails because it already exists
  2010-09-28 11:25 [PATCH] stash drops the stash even if creating the branch fails because it already exists Tomas Carnecky
  2010-09-28 13:19 ` [PATCH 0/2] stash: test and fix git stash branch regression Jon Seymour
@ 2010-09-28 13:19 ` Jon Seymour
  2010-09-28 13:21   ` Tomas Carnecky
  2010-09-28 13:19 ` [PATCH 2/2] stash: fix git stash branch regression when branch creation fails Jon Seymour
  2 siblings, 1 reply; 6+ messages in thread
From: Jon Seymour @ 2010-09-28 13:19 UTC (permalink / raw)
  To: git; +Cc: tla, tom, gitster

From: Tomas Carnecky <tom@dbservice.com>

This bug was disovered by someone on IRC when he tried to 'git stash branch <branch> <stash>'
while <branch> already existed. In that case the stash is dropped even though it isn't
applied on any branch, so the stash is effectively lost. I think that shouldn't happen,
so here is a test.
---
 t/t3903-stash.sh |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 9ed2396..0f6b2e4 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -545,4 +545,15 @@ test_expect_success 'invalid ref of the form stash@{n}, n >= N' '
 	git stash drop
 '
 
+test_expect_failure 'stash branch should not drop the stash if the branch exists' '
+	git stash clear &&
+	echo foo > file &&
+	git add file &&
+	git commit -m initial &&
+	echo bar > file &&
+	git stash &&
+	test_must_fail git stash branch master stash@{0} &&
+	git rev-parse stash@{0} --
+'
+
 test_done
-- 
1.7.3.4.g787b.dirty

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] stash: fix git stash branch regression when branch creation fails
  2010-09-28 11:25 [PATCH] stash drops the stash even if creating the branch fails because it already exists Tomas Carnecky
  2010-09-28 13:19 ` [PATCH 0/2] stash: test and fix git stash branch regression Jon Seymour
  2010-09-28 13:19 ` [PATCH 1/2] stash drops the stash even if creating the branch fails because it already exists Jon Seymour
@ 2010-09-28 13:19 ` Jon Seymour
  2 siblings, 0 replies; 6+ messages in thread
From: Jon Seymour @ 2010-09-28 13:19 UTC (permalink / raw)
  To: git; +Cc: tla, tom, gitster, Jon Seymour

Tomas Carnecky reported a regression in the behaviour
of git stash branch, when the branch creation fails.

This patch fixes that regression by restoring the
pre-condition for dropping that previously existed.

This patch assumes Tomas Carnecky's patch has already been applied.

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 git-stash.sh     |    6 +++---
 t/t3903-stash.sh |    2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 23a9ab5..5fb1245 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -432,9 +432,9 @@ apply_to_branch () {
 	assert_stash_like "$@"
 
 	git checkout -b $branch $REV^ &&
-	apply_stash "$@"
-
-	test -z "$IS_STASH_REF" || drop_stash "$@"
+	apply_stash "$@" && {
+		test -z "$IS_STASH_REF" || drop_stash "$@"
+	}
 }
 
 PARSE_CACHE='--not-parsed'
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 0f6b2e4..336e244 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -545,7 +545,7 @@ test_expect_success 'invalid ref of the form stash@{n}, n >= N' '
 	git stash drop
 '
 
-test_expect_failure 'stash branch should not drop the stash if the branch exists' '
+test_expect_success 'stash branch should not drop the stash if the branch exists' '
 	git stash clear &&
 	echo foo > file &&
 	git add file &&
-- 
1.7.3.4.g787b.dirty

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] stash drops the stash even if creating the branch fails because it already exists
  2010-09-28 13:19 ` [PATCH 1/2] stash drops the stash even if creating the branch fails because it already exists Jon Seymour
@ 2010-09-28 13:21   ` Tomas Carnecky
  2010-09-29 13:53     ` Jon Seymour
  0 siblings, 1 reply; 6+ messages in thread
From: Tomas Carnecky @ 2010-09-28 13:21 UTC (permalink / raw)
  To: Jon Seymour; +Cc: git, tla, gitster

On 9/28/10 3:19 PM, Jon Seymour wrote:
> From: Tomas Carnecky <tom@dbservice.com>
> 
> This bug was disovered by someone on IRC when he tried to 'git stash branch <branch> <stash>'
> while <branch> already existed. In that case the stash is dropped even though it isn't
> applied on any branch, so the stash is effectively lost. I think that shouldn't happen,
> so here is a test.

This line was missing from my original patch, sorry about that:
Signed-off-by: Tomas Carnecky <tom@dbservice.com>

> ---
>  t/t3903-stash.sh |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index 9ed2396..0f6b2e4 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -545,4 +545,15 @@ test_expect_success 'invalid ref of the form stash@{n}, n >= N' '
>  	git stash drop
>  '
>  
> +test_expect_failure 'stash branch should not drop the stash if the branch exists' '
> +	git stash clear &&
> +	echo foo > file &&
> +	git add file &&
> +	git commit -m initial &&
> +	echo bar > file &&
> +	git stash &&
> +	test_must_fail git stash branch master stash@{0} &&
> +	git rev-parse stash@{0} --
> +'
> +
>  test_done

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] stash drops the stash even if creating the branch fails because it already exists
  2010-09-28 13:21   ` Tomas Carnecky
@ 2010-09-29 13:53     ` Jon Seymour
  0 siblings, 0 replies; 6+ messages in thread
From: Jon Seymour @ 2010-09-29 13:53 UTC (permalink / raw)
  To: Tomas Carnecky; +Cc: git, tla, gitster

Junio,

Let me know if you want me to re-roll this series with Tomas'
sign-off. I can also add my simplification of the Brian's fix and
another fix I have made to have git stash save/create fail early in
case the index contains merge conflicts.

jon.

On Tue, Sep 28, 2010 at 11:21 PM, Tomas Carnecky <tom@dbservice.com> wrote:
> On 9/28/10 3:19 PM, Jon Seymour wrote:
>> From: Tomas Carnecky <tom@dbservice.com>
>>
>> This bug was disovered by someone on IRC when he tried to 'git stash branch <branch> <stash>'
>> while <branch> already existed. In that case the stash is dropped even though it isn't
>> applied on any branch, so the stash is effectively lost. I think that shouldn't happen,
>> so here is a test.
>
> This line was missing from my original patch, sorry about that:
> Signed-off-by: Tomas Carnecky <tom@dbservice.com>
>
>> ---
>>  t/t3903-stash.sh |   11 +++++++++++
>>  1 files changed, 11 insertions(+), 0 deletions(-)
>>
>> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
>> index 9ed2396..0f6b2e4 100755
>> --- a/t/t3903-stash.sh
>> +++ b/t/t3903-stash.sh
>> @@ -545,4 +545,15 @@ test_expect_success 'invalid ref of the form stash@{n}, n >= N' '
>>       git stash drop
>>  '
>>
>> +test_expect_failure 'stash branch should not drop the stash if the branch exists' '
>> +     git stash clear &&
>> +     echo foo > file &&
>> +     git add file &&
>> +     git commit -m initial &&
>> +     echo bar > file &&
>> +     git stash &&
>> +     test_must_fail git stash branch master stash@{0} &&
>> +     git rev-parse stash@{0} --
>> +'
>> +
>>  test_done
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-09-29 13:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-28 11:25 [PATCH] stash drops the stash even if creating the branch fails because it already exists Tomas Carnecky
2010-09-28 13:19 ` [PATCH 0/2] stash: test and fix git stash branch regression Jon Seymour
2010-09-28 13:19 ` [PATCH 1/2] stash drops the stash even if creating the branch fails because it already exists Jon Seymour
2010-09-28 13:21   ` Tomas Carnecky
2010-09-29 13:53     ` Jon Seymour
2010-09-28 13:19 ` [PATCH 2/2] stash: fix git stash branch regression when branch creation fails Jon Seymour

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.