Re: [PATCH] Ignore dirty submodule states during stash

From

Stefan Beller <sbeller@google.com>

Date

May 16, 2016, 16:09 UTC

Message-ID

<CAGZ79kZESuKiEt2RJJdWJPBySgbDA6abGkZMiTFgzaNCUP1_mA@mail.gmail.com>

In-Reply-To

<20160516154606.GA8797@gmail.com>

On Mon, May 16, 2016 at 8:46 AM, Vasily Titskiy <qehgt0@gmail.com> wrote:

> Hi Stefan,
>
> On Sun, May 15, 2016 at 11:37:20PM -0700, Stefan Beller wrote:
>> On Sun, May 15, 2016 at 7:07 PM, Vasily Titskiy <qehgt0@gmail.com> wrote:
>> > Do not save states of submodules as stash should ignore it.
>>
>> Can you explain why this is a good idea?
>> (It is not obvious to me either way.)
> Actually, submodules are already ignored by stash, but not fully (it was introduced in commit 6848d58).
> Current behavior is counter-intuitive, for example (if one has a project with a submodule):
>  $ cd sub1
>  $ edit .. commit .. edit .. commit. Alternatively, just checkout some other commit
>  $ cd .. # back to main project
>  $ git stash save
>    No local changes to save
>  $ # so, stash declares there are no changes
>  $ edit main.cpp
>  $ # For example, I need to update my working tree to latest master
>  $ git stash save # save local changes of 'main.cpp'...
>  $ git pull --recurse-submodules && git submodule update --recursive # update to latest
>  $ git stash pop # I expect to get stashed changes for 'main.cpp', but...
>    warning: Failed to merge submodule sub1 (commits don't follow merge-base)
>    Auto-merging sub1
>    CONFLICT (submodule): Merge conflict in sub1
>
> So, this is the issue. Instead of getting my local changes, I got a conflict (and stash is not
> poped out). The root cause is the 'stash' command does not know how to deal with submodules,
> but currently it tries to save the state of submodules, and even tries to re-apply the state
> (and it fails of course). The proposed solution fixes this behaviour.
>
> All internal tests work fine with the change.

Maybe we can put the test in t3903 instead?

>
>
>>
>> Do we need a test/documentation updates for this?
> I don't think so, 'stash' have never claimed submodule support.

But it also never explicitly claimed it doesn't support it.

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 92df596..b2649eb 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -41,6 +41,8 @@ the usual reflog syntax (e.g. `stash@{0}` is the most recently
 created stash, `stash@{1}` is the one before it, `stash@{2.hours.ago}`
 is also possible).

+Stashing ignores submodule operations completely.
+
 OPTIONS
 -------


Thanks,
Stefan



>
>>
>> >
>> > Signed-off-by: Vasily Titskiy <qehgt0@gmail.com>
>> > ---
>> >  git-stash.sh | 2 +-
>> >  1 file changed, 1 insertion(+), 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"
>> > --
>> > 2.1.4
>> >
>> > --
>> > 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