# [PATCH v2] Ignore dirty submodule states during stash 6 messages from 2016-05-17 to 2016-05-17. Participants: Vasily Titskiy, Junio C Hamano, Eric Sunshine. Thread: https://gitlist.dev/t/42357 ## Vasily Titskiy, 2016-05-17 13:16 Subject: [PATCH v2] Ignore dirty submodule states during stash Message-ID: <20160517131635.GA5299@gmail.com> URL: https://gitlist.dev/e/20160517131635.GA5299%40gmail.com ``` 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 --- 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 ``` ## Junio C Hamano, 2016-05-17 16:15 Subject: Re: [PATCH v2] Ignore dirty submodule states during stash Message-ID: URL: https://gitlist.dev/e/xmqqmvnoy82r.fsf%40gitster.mtv.corp.google.com In-Reply-To: <20160517131635.GA5299@gmail.com> ``` Vasily Titskiy 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 ``` ## Vasily Titskiy, 2016-05-17 16:38 Subject: Re: [PATCH v2] Ignore dirty submodule states during stash Message-ID: URL: https://gitlist.dev/e/CABSveYgsRU%2BL5qvY_evwekaRU7-oL_2Mmxt6L1u2dYM7Xfq2rw%40mail.gmail.com In-Reply-To: ``` 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 wrote: > > Vasily Titskiy 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 ``` ## Eric Sunshine, 2016-05-17 16:58 Subject: Re: [PATCH v2] Ignore dirty submodule states during stash Message-ID: URL: https://gitlist.dev/e/CAPig%2BcSFD%2B4zhvxpqjGWomq1uW3JxCNi6pQP3f_bp4Tr1Kg5aA%40mail.gmail.com In-Reply-To: <20160517131635.GA5299@gmail.com> ``` On Tue, May 17, 2016 at 9:16 AM, Vasily Titskiy 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 ``` ## Junio C Hamano, 2016-05-17 17:04 Subject: Re: [PATCH v2] Ignore dirty submodule states during stash Message-ID: URL: https://gitlist.dev/e/xmqqa8joy5sv.fsf%40gitster.mtv.corp.google.com In-Reply-To: ``` Vasily Titskiy 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? ``` ## Vasily Titskiy, 2016-05-17 17:12 Subject: Re: [PATCH v2] Ignore dirty submodule states during stash Message-ID: URL: https://gitlist.dev/e/CABSveYjmqrPY4OVrYnWeCrjAqZdqOaciCzQ2_soB4akOcQgZpQ%40mail.gmail.com In-Reply-To: ``` 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 wrote: > Vasily Titskiy 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? ```