Re: [PATCH] Ignore dirty submodule states during stash
- From
Vasily Titskiy <qehgt0@gmail.com>
- Date
- May 17, 2016, 03:17 UTC
- Message-ID
- <20160517031705.GA2617@gmail.com>
- In-Reply-To
- <CAGZ79kZESuKiEt2RJJdWJPBySgbDA6abGkZMiTFgzaNCUP1_mA@mail.gmail.com>
Hi Stefan,
Show 15 quoted lines
> > 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. > > I think you could take the example as above and make it into a test? > Showing that this change actually fixes a bug. > > Looking for a good place, I would have expected t/t3906-stash-submodule.sh > would be a good place to put your code, but I am not sure how to > properly integrate that test there. > > Maybe we can put the test in t3903 instead?
t3903 is better, as t3906 creates its submodule with forced 'ignore' option. It's not a default parameter and it actually hides the issue. I'll send the patches soon.
Show 20 quoted lines
> >> 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.
>
> Maybe we want to squash in something like
> (with better wording):
>
> 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
> -------Looks perfect to me.
-- Vasily