[PATCH v2] Ignore dirty submodule states during stash

[PATCH v2] Ignore dirty submodule states during stash

[Vasily Titskiy]

As stash does not know how to deal with submodules,
it should not try to save/restore their states
as it leads to redundant merge conflicts.

Added test checks if 'stash pop' does not trigger merge conflics
in submodules.

Signed-off-by: Vasily Titskiy <qehgt0@gmail.com>
---
 git-stash.sh     |  2 +-
 t/t3903-stash.sh | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/git-stash.sh b/git-stash.sh
index c7c65e2..b500c44 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -116,7 +116,7 @@ create_stash () {
 			git read-tree --index-output="$TMPindex" -m $i_tree &&
 			GIT_INDEX_FILE="$TMPindex" &&
 			export GIT_INDEX_FILE &&
-			git diff --name-only -z HEAD -- >"$TMP-stagenames" &&
+			git diff --name-only --ignore-submodules -z HEAD -- >"$TMP-stagenames" &&
 			git update-index -z --add --remove --stdin <"$TMP-stagenames" &&
 			git write-tree &&
 			rm -f "$TMPindex"
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 2142c1f..1be62f3 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -731,4 +731,38 @@ test_expect_success 'stash list --cc shows combined diff' '
 	test_cmp expect actual
 '
 
+test_expect_success 'stash ignores changes in submodules' '
+	git submodule init &&
+	git init sub1 &&
+	(
+		cd sub1 &&
+		echo "x" >file1 &&
+		git add file1 &&
+		git commit -a -m "initial sub1"
+	) &&
+	git submodule add ./. sub1 &&
+	echo "main" >file1 &&
+	git add file1 &&
+	git commit -a -m "initial main" &&
+	# make changes in submodule
+	(
+		cd sub1 &&
+		echo "y" >>file1 &&
+		git commit -a -m "change y"
+	) &&
+	git commit sub1 -m "update reference" &&
+	# switch submodule to another revision
+	(
+		cd sub1 &&
+		echo "z" >>file1 &&
+		git commit -a -m "change z"
+	) &&
+	# everything is prepared, check if changes in submodules are ignored
+	echo "local change" >>file1 &&
+	git stash save &&
+	git checkout HEAD~1 &&
+	git submodule update &&
+	git stash pop
+'
+
 test_done
-- 
2.1.4

Re: [PATCH v2] Ignore dirty submodule states during stash

[Junio C Hamano]

Vasily Titskiy <qehgt0@gmail.com> writes:

> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index 2142c1f..1be62f3 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -731,4 +731,38 @@ test_expect_success 'stash list --cc shows combined diff' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'stash ignores changes in submodules' '
> +	git submodule init &&

Hmmmm... what is this "submodule init" needed for at this point in
the test sequence?

> +	git init sub1 &&
> +	(
> +		cd sub1 &&
> +		echo "x" >file1 &&
> +		git add file1 &&
> +		git commit -a -m "initial sub1"
> +	) &&
> +	git submodule add ./. sub1 &&
> +	echo "main" >file1 &&
> +	git add file1 &&
> +	git commit -a -m "initial main" &&
> +	# make changes in submodule
> +	(
> +		cd sub1 &&
> +		echo "y" >>file1 &&
> +		git commit -a -m "change y"
> +	) &&
> +	git commit sub1 -m "update reference" &&
> +	# switch submodule to another revision
> +	(
> +		cd sub1 &&
> +		echo "z" >>file1 &&
> +		git commit -a -m "change z"
> +	) &&
> +	# everything is prepared, check if changes in submodules are ignored
> +	echo "local change" >>file1 &&
> +	git stash save &&
> +	git checkout HEAD~1 &&
> +	git submodule update &&
> +	git stash pop
> +'
> +
>  test_done

Re: [PATCH v2] Ignore dirty submodule states during stash

[Vasily Titskiy]

Hi Junio,

You're right, it's redundant here. Should I resubmit the path without this line?

--
  Regards,
  Vasily Titskiy

On Tue, May 17, 2016 at 12:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Vasily Titskiy <qehgt0@gmail.com> writes:
>
> > diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> > index 2142c1f..1be62f3 100755
> > --- a/t/t3903-stash.sh
> > +++ b/t/t3903-stash.sh
> > @@ -731,4 +731,38 @@ test_expect_success 'stash list --cc shows combined diff' '
> >       test_cmp expect actual
> >  '
> >
> > +test_expect_success 'stash ignores changes in submodules' '
> > +     git submodule init &&
>
> Hmmmm... what is this "submodule init" needed for at this point in
> the test sequence?
>
> > +     git init sub1 &&
> > +     (
> > +             cd sub1 &&
> > +             echo "x" >file1 &&
> > +             git add file1 &&
> > +             git commit -a -m "initial sub1"
> > +     ) &&
> > +     git submodule add ./. sub1 &&
> > +     echo "main" >file1 &&
> > +     git add file1 &&
> > +     git commit -a -m "initial main" &&
> > +     # make changes in submodule
> > +     (
> > +             cd sub1 &&
> > +             echo "y" >>file1 &&
> > +             git commit -a -m "change y"
> > +     ) &&
> > +     git commit sub1 -m "update reference" &&
> > +     # switch submodule to another revision
> > +     (
> > +             cd sub1 &&
> > +             echo "z" >>file1 &&
> > +             git commit -a -m "change z"
> > +     ) &&
> > +     # everything is prepared, check if changes in submodules are ignored
> > +     echo "local change" >>file1 &&
> > +     git stash save &&
> > +     git checkout HEAD~1 &&
> > +     git submodule update &&
> > +     git stash pop
> > +'
> > +
> >  test_done

Re: [PATCH v2] Ignore dirty submodule states during stash

[Junio C Hamano]

Vasily Titskiy <qehgt0@gmail.com> writes:

> You're right, it's redundant here. Should I resubmit the path without this line?

I wasn't pointing out that it was not needed.  I was only asking
what it was meant to do.

If you now think it shouldn't have been there, that merely means
your code was wrong.  It does not mean I'm right ;-)

With that line removed, would the patch now becomes right?  Are
there other bugs?

Re: [PATCH v2] Ignore dirty submodule states during stash

[Vasily Titskiy]

The command does nothing, so it's not needed. There is also a typo in
my patch description, so I'll resend it again with needed changes.
--
  Regards,
  Vasily Titskiy


On Tue, May 17, 2016 at 1:04 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Vasily Titskiy <qehgt0@gmail.com> writes:
>
>> You're right, it's redundant here. Should I resubmit the path without this line?
>
> I wasn't pointing out that it was not needed.  I was only asking
> what it was meant to do.
>
> If you now think it shouldn't have been there, that merely means
> your code was wrong.  It does not mean I'm right ;-)
>
> With that line removed, would the patch now becomes right?  Are
> there other bugs?

Re: [PATCH v2] Ignore dirty submodule states during stash

[Eric Sunshine]

On Tue, May 17, 2016 at 9:16 AM, Vasily Titskiy <qehgt0@gmail.com> wrote:
> As stash does not know how to deal with submodules,
> it should not try to save/restore their states
> as it leads to redundant merge conflicts.
>
> Added test checks if 'stash pop' does not trigger merge conflics

/conflics/conflicts/

> in submodules.
>
> Signed-off-by: Vasily Titskiy <qehgt0@gmail.com>