On Wed, Nov 14, 2018 at 10:05 PM Michael Forney <mforney@xxxxxxxxxxx> wrote: > > +bmwill > > On 2018-11-14, Michael Forney <mforney@xxxxxxxxxxx> wrote: > > On 2018-10-25, Stefan Beller <sbeller@xxxxxxxxxx> wrote: > >> 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. > > Interesting. If I build git from master after reverting 55568086, I do > indeed observe the issue it claims to fix (unable to add ignored > submodules). However, if I build from 9ef23f91fc (the immediate parent > of 55568086), I do not see the issue. > > Investigating this further, it seems that 55568086 addresses an issue > that does not appear until later on in the series in ff6f1f564c > (submodule-config: lazy-load a repository's .gitmodules file). Perhaps > this was a result of reordering commits during a rebase. In other > words, I get correct behavior until 55568086, and in > 55568086..ff6f1f564c^ if I revert 55568086. > > Looking at ff6f1f564c, I don't really see anything that might be > related to git-add, git-reset, or git-diff, so I'm guessing that this > only worked before because the submodule config wasn't getting loaded > during `git add` or `git reset`. Now that the config is loaded > automatically, submodule.<name>.ignore started taking effect where it > shouldn't. > > Unfortunately, this doesn't really get me much closer to finding a fix. Maybe selectively unloading or overwriting the config? Or we can change is_submodule_ignored() in diff.c to be only applied selectively whether we are running the right command? For this approach we'd have to figure out the set of commands to which the ignore config should apply or not (and come up with a more concise documentation then) This approach sounds appealing to me as it would cover new commands as well and we'd only have a central point where the decision for ignoring is made. Stefan
