[PATCH v2] Ignore dirty submodule states during stash

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[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 <[hidden email]>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Junio C Hamano
Vasily Titskiy <[hidden email]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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 <[hidden email]> wrote:

>
> Vasily Titskiy <[hidden email]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Eric Sunshine
In reply to this post by Vasily Titskiy
On Tue, May 17, 2016 at 9:16 AM, Vasily Titskiy <[hidden email]> 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 <[hidden email]>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Junio C Hamano
In reply to this post by Vasily Titskiy
Vasily Titskiy <[hidden email]> 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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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 <[hidden email]> wrote:

> Vasily Titskiy <[hidden email]> 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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html