Re: Confusing behavior with ignored submodules and `git commit -a`

[Stefan Beller <sbeller@google.com>]

On Thu, Oct 25, 2018 at 11:03 AM Michael Forney <mforney@mforney.org> wrote:
>
> On 2018-03-16, Michael Forney <mforney@mforney.org> wrote:
> > Hi,
> >
> > In the past few months have noticed some confusing behavior with
> > ignored submodules. I finally got around to bisecting this to commit
> > 5556808690ea245708fb80383be5c1afee2fb3eb (add, reset: ensure
> > submodules can be added or reset).

Uh. :(

See the discussion starting at
https://public-inbox.org/git/20170725213928.125998-4-bmwill@google.com/
specifically
https://public-inbox.org/git/xmqqinieq49v.fsf@gitster.mtv.corp.google.com/


> >
> > Here is a demonstration of the problem:
> >
[...]
> > Up to here is all expected.

Makes sense.

> > However, if I go to update `foo.txt` and
> > commit with `git commit -a`, changes to inner get recorded
> > unexpectedly. What's worse is the shortstat output of `git commit -a`,
> > and the diff output of `git show` give no indication that the
> > submodule was changed.

This is really bad. git-status and git-commit share some code,
and we'll populate the commit message with a status output.
So it seems reasonable to expect the status and the commit to match,
i.e. if status tells me there is no change, then commit should not record
the submodule update.

> > $ git commit -a -m 'update foo.txt'
> > [master 6ec564c] update foo.txt
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > $ git show
> > commit 6ec564c15ddae099c71f01750b4c434557525653 (HEAD -> master)
> > Author: Michael Forney <mforney@mforney.org>
> > Date:   Fri Mar 16 20:18:37 2018 -0700
> >
> >     update foo.txt
> >
> > diff --git a/foo.txt b/foo.txt
> > index d00491f..0cfbf08 100644
> > --- a/foo.txt
> > +++ b/foo.txt
> > @@ -1 +1 @@
> > -1
> > +2
> > $
> >
> > There have been a couple occasions where I accidentally pushed local
> > changes to ignored submodules because of this. Since they don't show
> > up in the log output, it is difficult to figure out what actually has
> > gone wrong.

How was it prevented before? Just by git commit -a not picking up the
submodule change?

> >
> > Anyway, since the bisected commit (555680869) only mentions add and
> > reset, I'm assuming that this is a regression and not a deliberate
> > behavior change. The documentation for submodule.<name>.ignore states
> > that the setting should only affect `git status` and the diff family.
> > In terms of my expectations, I would go further and say it should only
> > affect `git status` and diffs against the working tree.
> >
> > I took a brief look through the relevant sources, and it wasn't clear
> > to me how to fix this without accidentally changing the behavior of
> > other subcommands.
> >
> > Any help with this issue is appreciated!

I guess reverting that commit is not a good idea now, as
I would expect something to break.

Maybe looking through the series 614ea03a71
(Merge branch 'bw/submodule-config-cleanup', 2017-08-26)
to understand why it happened in the context would be a good start.

> I accidentally pushed local changes to ignored submodules again due to this.
>
> Can anyone confirm whether this is the intended behavior of ignore? If
> it is, then at least the documentation needs an update saying that
> `commit -a` will commit all submodule changes, even if they are
> ignored.

The docs say "(but it will nonetheless show up in the output of
status and commit when it has been staged)" as well, so that commit
sounds like a regression?

Stefan