Subject: Re: [PATCH v2.1] commit: add --ignore-submodules[=<when>] parameter

From: Ronald Weiss <[email-masked]>
Date: Mon, 07 Apr 2014 23:46:32 +0200

On 6. 4. 2014 18:28, Jens Lehmann wrote:
> Am 02.04.2014 21:56, schrieb Ronald Weiss:
>> On 2. 4. 2014 20:53, Jens Lehmann wrote:
>>> Am 01.04.2014 23:59, schrieb Ronald Weiss:
>>>> On 1. 4. 2014 22:23, Jens Lehmann wrote:
>>>>> Am 01.04.2014 01:35, schrieb Ronald Weiss:
>>>>>> On 1. 4. 2014 0:50, Ronald Weiss wrote:
>>>>>>> On 31. 3. 2014 23:47, Ronald Weiss wrote:
>>>>>>>> On Mon, Mar 31, 2014 at 8:58 PM, Jens Lehmann <[email-masked]> wrote:
>>>>>>>>> As Junio mentioned it would be great if you could teach the add
>>>>>>>>> command also honor the --ignore-submodule command line option in
>>>>>>>>> a companion patch. In the course of doing so you'll easily see if
>>>>>>>>> I was right or not, then please just order them in the most logical
>>>>>>>>> way.
>>>>>>>>
>>>>>>>> Well, if You (or Junio) really don't want my patch without another one
>>>>>>>> for git add, I may try to do it. However, git add does not even honor
>>>>>>>> the submodules' ignore setting from .gitmodules (just tested with git
>>>>>>>> 1.9.1: "git add -u" doesn't honor it, while "git commit -a" does). So
>>>>>>>> teaching git add the --ignore-submodules switch in current state
>>>>>>>> doesn't seem right to me. You might propose to add also support for
>>>>>>>> the ignore setting, to make "add -u" and "commit -a" more consistent.
>>>>>>>> That seems like a good idea, but the effort needed is getting bigger,
>>>>>>>
>>>>>>> Well, now I actually looked at it, and it was pretty easy after all.
>>>>>>> The changes below seem to enable support for both ignore setting in
>>>>>>> .gitmodules, and also --ignore-submodules switch, for git add, on top
>>>>>>> of my patch for commit.
>>>>>>
>>>>>> There is a catch. With the changes below, submodules are ignored by add
>>>>>> even if explitely named on command line (eg. "git add x" does nothing
>>>>>> if x is submodule with new commits, but with ignore=all in .gitmodules).
>>>>>> That doesn't seem right.
>>>>>>
>>>>>> Any ideas, what to do about that? When exactly should such submodule be
>>>>>> actually ignored?
>>>>>
>>>>> Me thinks git add should require the '-f' option to add an ignored
>>>>> submodule (just like it does for files) unless the user uses the
>>>>> '--ignore-submodules=none' option. And if neither of these are given
>>>>> it should "fail with a list of ignored files" as the documentation
>>>>> states.
>>>>
>>>> It's still not clear, at least not to me. Should '-f' suppress the
>>>> ignore setting of all involved submodules? That would make it a
>>>> synonyme (or a superset) of --ignore-submodules=none. Or only if the
>>>> submodule is explicitly named on command line? That seems fuzzy to me,
>>>> and also more tricky to implement.
>>>
>>> Maybe my impression that doing "add" together with "commit" would be
>>> easy wasn't correct after all. I won't object if you try to tackle
>>> commit first (but I have the slight suspicion that similar questions
>>> will arise concerning the "add"ish functionality in commit too. So
>>> maybe after resolving those things might look clearer ;-)
>>
>> There is one big distinction. My patch for commit doesn't add any new
>> problems. It just adds the --ignore-submodules argument, which is easy
>> to implement and no unclear behavior decisions are needed.
>>
>> You are right that when specifying ignored submodules on commit's
>> command line, there is the same problem as with git add. However, it's
>> already there anyway. I don't feel in position to solve it, I'd just
>> like to have "git commit --ignore-submodules=none".
>>
>> With git add however, changing it to honor settings from .gitmodules
>> would change behavior people might be used to, so I would be afraid to
>> do that. Btw add also has the problem already, but only if somebody
>> configures the submodule's ignore setting in .git/config, rather than
>> .gitmodules. I don't know how much real use case that is.
>>
>> As I see it, there are now these rather easy possibilities (sorted
>> from the easiest):
>>
>> 1) Just teach commit the --ignore-submodules argument, as I proposed.
> 
> 1a) Teach commit to honor ignore from .git/config.

But commit already honors that. It honors submodule.<name>.ignore from
both .git/config and .gitmodules. It's just add which doesn't honor it
from .gitmodules, because cmd_add() function lacks a gitmodules_config()
call. Or do I miss something?

>> 2) Teach both add and commit to --ignore-submodules, but dont add that
>> problematic gitmodules_config() in add.c.
> 
> Why is that problematic after add learned --ignore-submodules=none?

First, because it changes current behaviour. Which is obviously
inconsistent currently, however I didn't find it easy to tell what's
the right thing to do.

And second, because the "-f implies --ignore-submodules=none" proposal,
which seems to be the easy cure for those accustomed to the current
behavior, seems non-trivial. Below You wrote that
--ignore-submodules=none should be implied by -f only for files
specified on the command line. OK. And what if a directory
containing the submodule is specified?

>> 3) Teach both add and commit to --ignore-submodules, and also let add
>> honor settings from .gitmodules, to make it more consistent with other
>> commands. And, make add --force imply --ignore-submodules=none.
>>
>> I like both 1) and 2). I don't like 3), the problem of add with
>> submodules' ignore setting is a bug IMHO (ignore=all in .git/config
>> causes strange behavior, while ignore=all in .gitmodules is ignored),
>> but not directly related to the --ignore-submodules param, and should
>> be solved separately.
> 
> I think the ignore config options and --ignore-submodules parameter
> are directly related, as you need the latter to override the former.
> In the long run commit should honor ignore=all in .git/config for
> unstaged submodules like add should honor the settings from the
> .gitmodules file. But we should always add the --ignore-submodules
> parameter first so that the user can override the configuration
> when needed. So I see these steps:
> 
> 1) Teach commit the --ignore-submodules option; then make it honor
>     ignore=all in .git/config in another commit.
> 
> 2) Teach add --ignore-submodules (which is implied by -f, but only
>     for the submodules given on the command line); then make it
>     honor the submodule.<name>.ignore option in another commit.
> 
> After that we'd have consistent ignore and override behavior. But
> it looks like getting -f right is not easy, so I'd prefer having
> 1) without 2) if the alternative is to get neither.

OK, I will try prepare that. However I'd more like to start with two
commits just adding the --ignore-submodules param to add and commit.
That should be easily acceptable, as there is no risk to break
anything, and it adds useful funcionality. Patching add first will
avoid having to touch add.c in patch for commit, which makes it more
clear and logical IMHO.

Then, on top of that, I'll prepare patches for add to honor ignore
from .gitmodules, and -f implying --ignore-submodules. That might need
more discussion, let's see.