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

[Michael Forney <mforney@mforney.org>]

On 2018-10-25, Stefan Beller <sbeller@google.com> wrote:
> 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/

Thanks for the links. Let me explain how I'm using
submodule.<name>.ignore. Maybe there's a better mechanism in git to
deal with this (if .ignore is a misfeature).

I have a git repository which contains a number of submodules that
refer to external repositories. Some of these repositories need to
patched in some way, so patches are stored alongside the submodules,
and are applied when building. This mostly works fine, but causes
submodules to show up as modified in `git status` and get updated with
`git commit -a`. To resolve this, I've added `ignore = all` to
.gitmodules for all the submodules that need patches applied. This
way, I can explicitly `git add` the submodule when I want to update
the base commit, but otherwise pretend that they are clean. This has
worked pretty well for me, but less so since git 2.15 when this issue
was introduced.

Of course, I could maintain and publish forks of those repositories
and use those as the remote for the submodules. However in many cases
these patches are just temporary until they get applied upstream and a
new release is made, and I don't really want to keep mirrors
unnecessarily, or keep switching the submodule URL between upstream
and my fork.

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

I just checked and if I don't specify a message on the command-line,
the status output in the message template *does* mention that `inner`
is getting updated.

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

Yes. Previously, `git commit -a` would not pick up the change (unless
I added it explicitly with `git add`), and `git log` would still show
changes to ignored submodules (which is the behavior I want).

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

Thanks, that's a good idea. I'll take a look through that series.

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

I just came across someone else affected by this issue:
https://github.com/git/git/commit/55568086#commitcomment-27137460