Re: [PATCH] Ignore dirty submodule states during stash

[Vasily Titskiy <qehgt0@gmail.com>]

Hi Stefan,

> > 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.


> >> 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