threads / patch / 31630

patchcommit: configure submodules

Subject: [PATCH] commit: configure submodules

## tl;dr

14 messages between Sep 23, 2012 and Sep 24, 2012. Diffs are folded; open one to read it.

replies: 13people: 3as markdown or json

Orgad Shaneh· Sep 23, 2012, 07:37 UTC · lore

As reported on the mailing list[1], ignore submodule config is not respected on commit.

[1] http://git.661346.n2.nabble.com/submodule-ignore-is-not-respected-on-commit-td7539238.html

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
 builtin/commit.c |    1 +
 1 file changed, 1 insertion(+)

Show changes to builtin/commit.c +1 −0

diff --git a/builtin/commit.c b/builtin/commit.c
index 62028e7..7a83cae 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1452,6 +1452,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		usage_with_options(builtin_commit_usage, builtin_commit_options);
 
 	wt_status_prepare(&s);
+	gitmodules_config();
 	git_config(git_commit_config, &s);
 	determine_whence(&s);
 	s.colopts = 0;

-- 
1.7.10.4

Junio C Hamano· Sep 23, 2012, 08:37 UTC · re: Orgad Shaneh · lore

Re: [PATCH] commit: configure submodules

Orgad Shaneh <orgads@gmail.com> writes:

Show 7 quoted lines

> As reported on the mailing list[1], ignore submodule config is not
> respected on commit.
>
> [1] http://git.661346.n2.nabble.com/submodule-ignore-is-not-respected-on-commit-td7539238.html
>
> Signed-off-by: Orgad Shaneh <orgads@gmail.com>
> ---

Thanks.

Please do not force people to go to external website like that while reading the "git log" output. You could have just said

	"git status" does not list a submodule with uncommitted
	working tree files as modified when "submodule.$name.ignore"
	configuration is set to "dirty", but "git commit" forgets
        to take this configuration variable into account.

or something. It would also be good to say "I reported it earlier in this message" with the URL _after_ the three-dash line.

I see Jens added with 302ad7a (Submodules: Use "ignore" settings from .gitmodules too for diff and status, 2010-08-06) the call to gitmodules_config() to "git status" and "git diff" family, but I suspect that was a huge mistake. Once a submodule is initialized with "submodule init", the default set of configuration should be copied to the user's $GIT_DIR/config and subsequent run-time invocation should read $GIT_DIR/config and $GIT_DIR/config alone, to honor user's customization.

Instead, I think git_commit_config() and git_status_config() should call submodule_config() function to read submodule.$name.ignore not from .gitmodules file but from $GIT_DIR/config.

Jens, what do you think? I see no reason for anybody other than "submodule init" to call gitmodules_config() that reads from the in-tree .gitmodules file.

Show 15 quoted lines

>  builtin/commit.c |    1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 62028e7..7a83cae 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1452,6 +1452,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  		usage_with_options(builtin_commit_usage, builtin_commit_options);
>  
>  	wt_status_prepare(&s);
> +	gitmodules_config();
>  	git_config(git_commit_config, &s);
>  	determine_whence(&s);
>  	s.colopts = 0;

Jens Lehmann· Sep 23, 2012, 18:13 UTC · re: Junio C Hamano · lore

Re: [PATCH] commit: configure submodules

Am 23.09.2012 10:37, schrieb Junio C Hamano:

Show 8 quoted lines

> I see Jens added with 302ad7a (Submodules: Use "ignore" settings
> from .gitmodules too for diff and status, 2010-08-06) the call to
> gitmodules_config() to "git status" and "git diff" family, but I
> suspect that was a huge mistake.  Once a submodule is initialized
> with "submodule init", the default set of configuration should be
> copied to the user's $GIT_DIR/config and subsequent run-time
> invocation should read $GIT_DIR/config and $GIT_DIR/config alone, to
> honor user's customization.

Not honoring the user's customization would be a big mistake, but this is not what happens here. A setting in $GIT_DIR/config always overrides the one in .gitmodules (that's why gitmodules_config() is called before git_config()).

Show 7 quoted lines

> Instead, I think git_commit_config() and git_status_config() should
> call submodule_config() function to read submodule.$name.ignore not
> from .gitmodules file but from $GIT_DIR/config.
> 
> Jens, what do you think?  I see no reason for anybody other than
> "submodule init" to call gitmodules_config() that reads from the
> in-tree .gitmodules file.

I think the copying on init is not what we should do here because it sets the user's customization to what ever happened to be in .gitmodules at the time he initialized the submodule. Later changes from upstream to such a setting would not be honored unless the user copies that new setting herself (which I think is The Right Thing for the URL, but not for the other work tree related settings like 'ignore').

Imagine you have a submodule containing a huge media file which is set to be ignored for performance reasons. When upstream later decides it should rather use .gitattributes to just disable diffing that file and removes the submodule ignore so the users see changes to other files of the submodule again, that will just work the way it is done now, but won't when we copy that setting on init.

So it is either "honor upstream unless the user decides otherwise" or "take what upstream configured at init time as the users choice (until he actively changes it)". And I think the former is more flexible as it allows upstream to change settings without user intervention, which is why I did it that way.

And as I understand that .gitattributes follow the same principle: Unless the user configured something different in his $GIT_DIR/info/attributes file, git will use the settings from the .gitattributes file of the currently checked out commit.

So I think Orgad's change is sane and should go in.

Junio C Hamano· Sep 24, 2012, 05:57 UTC · re: Jens Lehmann · lore

Re: [PATCH] commit: configure submodules

Jens Lehmann <Jens.Lehmann@web.de> writes:

Show 7 quoted lines

>> Jens, what do you think?  I see no reason for anybody other than
>> "submodule init" to call gitmodules_config() that reads from the
>> in-tree .gitmodules file.
>
> I think the copying on init is not what we should do here because
> it sets the user's customization to what ever happened to be in
> .gitmodules at the time he initialized the submodule.

Hrm, why does the user have submodule.$name.$whatever customized before saying "submodule init $whategver" for that copying to be problematic?

> So I think Orgad's change is sane and should go in.

Matching what cmd_commit() does to what cmd_status() does, i.e. grab submodule.$name.ignore from somewhere, is not something I questioned. The patch is a good change to make them consistent.

What I was wondering was if that is a consistently wrong thing to do to read from .gitmodules not $GIT_DIR/config.

In any case, the log message I suggested in the review needs to be updated in the reroll to make it clear that this is about reading from .gitmodules, not "configuration". AFAICS, gitmodule_config() does not even read from $GIT_DIR/config, right?

Junio C Hamano· Sep 24, 2012, 16:27 UTC · re: Junio C Hamano · lore

Re: [PATCH] commit: configure submodules

Junio C Hamano <gitster@pobox.com> writes:

Show 27 quoted lines

> Jens Lehmann <Jens.Lehmann@web.de> writes:
>
>>> Jens, what do you think?  I see no reason for anybody other than
>>> "submodule init" to call gitmodules_config() that reads from the
>>> in-tree .gitmodules file.
>>
>> I think the copying on init is not what we should do here because
>> it sets the user's customization to what ever happened to be in
>> .gitmodules at the time he initialized the submodule.
>
> Hrm, why does the user have submodule.$name.$whatever customized
> before saying "submodule init $whategver" for that copying to be
> problematic?
>
>> So I think Orgad's change is sane and should go in.
>
> Matching what cmd_commit() does to what cmd_status() does, i.e. grab
> submodule.$name.ignore from somewhere, is not something I questioned.
> The patch is a good change to make them consistent.
>
> What I was wondering was if that is a consistently wrong thing to do
> to read from .gitmodules not $GIT_DIR/config.
>
> In any case, the log message I suggested in the review needs to be
> updated in the reroll to make it clear that this is about reading
> from .gitmodules, not "configuration".  AFAICS, gitmodule_config()
> does not even read from $GIT_DIR/config, right?

OK. gitmodule_config() does not read $GIT_DIR/config, but cmd_status() and cmd_commit() call git_diff_basic_config() that is called from git_diff_ui_config() to read submodule.$name.ignore from it. So Orgad's patch is _only_ about submodule.$name.ignore that is in in-tree .gitmodule; the log message shouldn't mention "config", as setting configuration variables work for both status and commit just fine.

Jens Lehmann· Sep 24, 2012, 18:15 UTC · re: Junio C Hamano · lore

Re: [PATCH] commit: configure submodules

Am 24.09.2012 18:27, schrieb Junio C Hamano:

Show 13 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>> In any case, the log message I suggested in the review needs to be
>> updated in the reroll to make it clear that this is about reading
>> from .gitmodules, not "configuration".  AFAICS, gitmodule_config()
>> does not even read from $GIT_DIR/config, right?
> 
> OK.  gitmodule_config() does not read $GIT_DIR/config, but
> cmd_status() and cmd_commit() call git_diff_basic_config() that is
> called from git_diff_ui_config() to read submodule.$name.ignore from
> it.  So Orgad's patch is _only_ about submodule.$name.ignore that is
> in in-tree .gitmodule; the log message shouldn't mention "config",
> as setting configuration variables work for both status and commit
> just fine.

Yes, I was just checking that call path too to make sure the user settings from $GIT_DIR/config really override those found in .gitmodules. And of course you are right, while the change to the code is sane the commit message still needs some work.

Junio C Hamano· Sep 24, 2012, 18:34 UTC · re: Jens Lehmann · lore

Re: [PATCH] commit: configure submodules

Jens Lehmann <Jens.Lehmann@web.de> writes:

Show 19 quoted lines

> Am 24.09.2012 18:27, schrieb Junio C Hamano:
>> Junio C Hamano <gitster@pobox.com> writes:
>>> In any case, the log message I suggested in the review needs to be
>>> updated in the reroll to make it clear that this is about reading
>>> from .gitmodules, not "configuration".  AFAICS, gitmodule_config()
>>> does not even read from $GIT_DIR/config, right?
>> 
>> OK.  gitmodule_config() does not read $GIT_DIR/config, but
>> cmd_status() and cmd_commit() call git_diff_basic_config() that is
>> called from git_diff_ui_config() to read submodule.$name.ignore from
>> it.  So Orgad's patch is _only_ about submodule.$name.ignore that is
>> in in-tree .gitmodule; the log message shouldn't mention "config",
>> as setting configuration variables work for both status and commit
>> just fine.
>
> Yes, I was just checking that call path too to make sure the user
> settings from $GIT_DIR/config really override those found in
> .gitmodules. And of course you are right, while the change to the
> code is sane the commit message still needs some work.

Here is what I tentatively queued on 'pu' (not pushed out yet).

-- >8 --
From: Orgad Shaneh <orgads@gmail.com>
Date: Sun, 23 Sep 2012 09:37:47 +0200
Subject: [PATCH] commit: pay attention to submodule.$name.ignore in
 .gitmodules

"git status" does not list a submodule with uncommitted working tree files as modified when "submodule.$name.ignore" is set to "dirty" in in-tree ".gitmodules" file. Both status and commit honor the setting in $GIT_DIR/config, but "commit" does not pick it up from .gitmodules, which is inconsistent.

Teach "git commit" to pay attention to the setting in .gitmodules as well.

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/commit.c | 1 +
 1 file changed, 1 insertion(+)

Show changes to builtin/commit.c +1 −0

diff --git a/builtin/commit.c b/builtin/commit.c
index 66fdd22..3cb1ef7 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1256,6 +1256,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	struct wt_status s;
 
 	wt_status_prepare(&s);
+	gitmodules_config();
 	git_config(git_commit_config, &s);
 	in_merge = file_exists(git_path("MERGE_HEAD"));
 	s.in_merge = in_merge;

-- 
1.7.12.1.441.g794a63b

Orgad Shaneh· Sep 24, 2012, 18:49 UTC · re: Junio C Hamano · lore

Re: [PATCH] commit: configure submodules

On Mon, Sep 24, 2012 at 8:34 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 61 quoted lines

>
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>
> > Am 24.09.2012 18:27, schrieb Junio C Hamano:
> >> Junio C Hamano <gitster@pobox.com> writes:
> >>> In any case, the log message I suggested in the review needs to be
> >>> updated in the reroll to make it clear that this is about reading
> >>> from .gitmodules, not "configuration".  AFAICS, gitmodule_config()
> >>> does not even read from $GIT_DIR/config, right?
> >>
> >> OK.  gitmodule_config() does not read $GIT_DIR/config, but
> >> cmd_status() and cmd_commit() call git_diff_basic_config() that is
> >> called from git_diff_ui_config() to read submodule.$name.ignore from
> >> it.  So Orgad's patch is _only_ about submodule.$name.ignore that is
> >> in in-tree .gitmodule; the log message shouldn't mention "config",
> >> as setting configuration variables work for both status and commit
> >> just fine.
> >
> > Yes, I was just checking that call path too to make sure the user
> > settings from $GIT_DIR/config really override those found in
> > .gitmodules. And of course you are right, while the change to the
> > code is sane the commit message still needs some work.
>
> Here is what I tentatively queued on 'pu' (not pushed out yet).
>
> -- >8 --
> From: Orgad Shaneh <orgads@gmail.com>
> Date: Sun, 23 Sep 2012 09:37:47 +0200
> Subject: [PATCH] commit: pay attention to submodule.$name.ignore in
>  .gitmodules
>
> "git status" does not list a submodule with uncommitted working tree
> files as modified when "submodule.$name.ignore" is set to "dirty" in
> in-tree ".gitmodules" file.  Both status and commit honor the setting
> in $GIT_DIR/config, but "commit" does not pick it up from .gitmodules,
> which is inconsistent.
>
> Teach "git commit" to pay attention to the setting in .gitmodules as
> well.
>
> Signed-off-by: Orgad Shaneh <orgads@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin/commit.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 66fdd22..3cb1ef7 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1256,6 +1256,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>         struct wt_status s;
>
>         wt_status_prepare(&s);
> +       gitmodules_config();
>         git_config(git_commit_config, &s);
>         in_merge = file_exists(git_path("MERGE_HEAD"));
>         s.in_merge = in_merge;
> --
> 1.7.12.1.441.g794a63b
>

That is not correct. git-config is ignored as well for commit. Maybe another fix is needed?

- Orgad

Junio C Hamano· Sep 24, 2012, 19:06 UTC · re: Orgad Shaneh · lore

Re: [PATCH] commit: configure submodules

Orgad Shaneh <orgads@gmail.com> writes:

> That is not correct. git-config is ignored as well for commit.

What do you mean?  As far as I can tell, if you have

    [submodule "var"]
        path = var
	ignore = dirty

in $GIT_DIR/config, a work-tree-dirty submodule "var" is not reported by "git status" and "git commit" without your patch, and your patch does not seem to break that. The only difference your patch makes is that if you had the above three-line block in the .gitmodules file and not in $GIT_DIR/config, "git status" ignored the dirtyness in the working tree, but "git commit" did notice and report it.

What am I missing?

Orgad Shaneh· Sep 24, 2012, 19:16 UTC · re: Junio C Hamano · lore

Re: [PATCH] commit: configure submodules

On Mon, Sep 24, 2012 at 9:06 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 21 quoted lines

> Orgad Shaneh <orgads@gmail.com> writes:
>
>> That is not correct. git-config is ignored as well for commit.
>
> What do you mean?  As far as I can tell, if you have
>
>     [submodule "var"]
>         path = var
>         ignore = dirty
>
> in $GIT_DIR/config, a work-tree-dirty submodule "var" is not
> reported by "git status" and "git commit" without your patch, and
> your patch does not seem to break that.  The only difference your
> patch makes is that if you had the above three-line block in
> the .gitmodules file and not in $GIT_DIR/config, "git status"
> ignored the dirtyness in the working tree, but "git commit" did
> notice and report it.
>
> What am I missing?
>
>

I have:
[submodule "mod"]
        url = [...]
        ignore = dirty

in .git/config, and I removed the ignore part from .gitmodules to be even.

I made a change inside mod, git status doesn't report its dirtiness, while git commit does.

git status: # On branch master # Changes to be committed: # (use "git reset HEAD <file>..." to unstage) # # modified: foo # # Changes not staged for commit: # (use "git add <file>..." to update what will be committed) # (use "git checkout -- <file>..." to discard changes in working directory) # # modified: .gitmodules #

git commit: # Please enter the commit message for your changes. Lines starting # with '#' will be ignored, and an empty message aborts the commit. # On branch master # Changes to be committed: # (use "git reset HEAD <file>..." to unstage) # # modified: foo # # Changes not staged for commit: # (use "git add <file>..." to update what will be committed) # (use "git checkout -- <file>..." to discard changes in working directory) # (commit or discard the untracked or modified content in submodules) # # modified: .gitmodules # modified: mod (modified content) #

Now I get it! That's because I don't have submodule.mod.path! config_name_for_path only gets initialized if path exists. Apparently git submodule init doesn't configure 'path', so it stays uninitialized.

- Orgad

Jens Lehmann· Sep 24, 2012, 19:56 UTC · re: Orgad Shaneh · lore

Re: [PATCH] commit: configure submodules

Am 24.09.2012 21:16, schrieb Orgad Shaneh:

Show 69 quoted lines

> On Mon, Sep 24, 2012 at 9:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Orgad Shaneh <orgads@gmail.com> writes:
>>
>>> That is not correct. git-config is ignored as well for commit.
>>
>> What do you mean?  As far as I can tell, if you have
>>
>>     [submodule "var"]
>>         path = var
>>         ignore = dirty
>>
>> in $GIT_DIR/config, a work-tree-dirty submodule "var" is not
>> reported by "git status" and "git commit" without your patch, and
>> your patch does not seem to break that.  The only difference your
>> patch makes is that if you had the above three-line block in
>> the .gitmodules file and not in $GIT_DIR/config, "git status"
>> ignored the dirtyness in the working tree, but "git commit" did
>> notice and report it.
>>
>> What am I missing?
>>
>>
> 
> I have:
> [submodule "mod"]
>         url = [...]
>         ignore = dirty
> 
> in .git/config, and I removed the ignore part from .gitmodules to be even.
> 
> I made a change inside mod, git status doesn't report its dirtiness,
> while git commit does.
> 
> git status:
> # On branch master
> # Changes to be committed:
> #   (use "git reset HEAD <file>..." to unstage)
> #
> #       modified:   foo
> #
> # Changes not staged for commit:
> #   (use "git add <file>..." to update what will be committed)
> #   (use "git checkout -- <file>..." to discard changes in working directory)
> #
> #       modified:   .gitmodules
> #
> 
> git commit:
> # Please enter the commit message for your changes. Lines starting
> # with '#' will be ignored, and an empty message aborts the commit.
> # On branch master
> # Changes to be committed:
> #   (use "git reset HEAD <file>..." to unstage)
> #
> #       modified:   foo
> #
> # Changes not staged for commit:
> #   (use "git add <file>..." to update what will be committed)
> #   (use "git checkout -- <file>..." to discard changes in working directory)
> #   (commit or discard the untracked or modified content in submodules)
> #
> #       modified:   .gitmodules
> #       modified:   mod (modified content)
> #
> 
> Now I get it! That's because I don't have submodule.mod.path!
> config_name_for_path only gets initialized if path exists. Apparently
> git submodule init doesn't configure 'path', so it stays
> uninitialized.

But submodule.mod.path should only be set in .gitmodules, not in $GIT_DIR/config. Did you just remove the ignore setting from .gitmodules or the path too?

Orgad Shaneh· Sep 24, 2012, 19:59 UTC · re: Jens Lehmann · lore

Re: [PATCH] commit: configure submodules

On Mon, Sep 24, 2012 at 9:56 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:

Show 74 quoted lines

> Am 24.09.2012 21:16, schrieb Orgad Shaneh:
>> On Mon, Sep 24, 2012 at 9:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Orgad Shaneh <orgads@gmail.com> writes:
>>>
>>>> That is not correct. git-config is ignored as well for commit.
>>>
>>> What do you mean?  As far as I can tell, if you have
>>>
>>>     [submodule "var"]
>>>         path = var
>>>         ignore = dirty
>>>
>>> in $GIT_DIR/config, a work-tree-dirty submodule "var" is not
>>> reported by "git status" and "git commit" without your patch, and
>>> your patch does not seem to break that.  The only difference your
>>> patch makes is that if you had the above three-line block in
>>> the .gitmodules file and not in $GIT_DIR/config, "git status"
>>> ignored the dirtyness in the working tree, but "git commit" did
>>> notice and report it.
>>>
>>> What am I missing?
>>>
>>>
>>
>> I have:
>> [submodule "mod"]
>>         url = [...]
>>         ignore = dirty
>>
>> in .git/config, and I removed the ignore part from .gitmodules to be even.
>>
>> I made a change inside mod, git status doesn't report its dirtiness,
>> while git commit does.
>>
>> git status:
>> # On branch master
>> # Changes to be committed:
>> #   (use "git reset HEAD <file>..." to unstage)
>> #
>> #       modified:   foo
>> #
>> # Changes not staged for commit:
>> #   (use "git add <file>..." to update what will be committed)
>> #   (use "git checkout -- <file>..." to discard changes in working directory)
>> #
>> #       modified:   .gitmodules
>> #
>>
>> git commit:
>> # Please enter the commit message for your changes. Lines starting
>> # with '#' will be ignored, and an empty message aborts the commit.
>> # On branch master
>> # Changes to be committed:
>> #   (use "git reset HEAD <file>..." to unstage)
>> #
>> #       modified:   foo
>> #
>> # Changes not staged for commit:
>> #   (use "git add <file>..." to update what will be committed)
>> #   (use "git checkout -- <file>..." to discard changes in working directory)
>> #   (commit or discard the untracked or modified content in submodules)
>> #
>> #       modified:   .gitmodules
>> #       modified:   mod (modified content)
>> #
>>
>> Now I get it! That's because I don't have submodule.mod.path!
>> config_name_for_path only gets initialized if path exists. Apparently
>> git submodule init doesn't configure 'path', so it stays
>> uninitialized.
>
> But submodule.mod.path should only be set in .gitmodules, not in
> $GIT_DIR/config. Did you just remove the ignore setting from
> .gitmodules or the path too?

Just the ignore, and my patch of course.

If it is not set in $GIT_DIR/config, then config_name_for_path is not initialized, and if it is not initialized, then set_diffopt_flags_from_submodule_config does nothing (handle_ignore_submodules_arg is not called). That is the main problem.

- Orgad

Jens Lehmann· Sep 24, 2012, 20:10 UTC · re: Orgad Shaneh · lore

Re: [PATCH] commit: configure submodules

Am 24.09.2012 21:59, schrieb Orgad Shaneh:

Show 83 quoted lines

> On Mon, Sep 24, 2012 at 9:56 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>> Am 24.09.2012 21:16, schrieb Orgad Shaneh:
>>> On Mon, Sep 24, 2012 at 9:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> Orgad Shaneh <orgads@gmail.com> writes:
>>>>
>>>>> That is not correct. git-config is ignored as well for commit.
>>>>
>>>> What do you mean?  As far as I can tell, if you have
>>>>
>>>>     [submodule "var"]
>>>>         path = var
>>>>         ignore = dirty
>>>>
>>>> in $GIT_DIR/config, a work-tree-dirty submodule "var" is not
>>>> reported by "git status" and "git commit" without your patch, and
>>>> your patch does not seem to break that.  The only difference your
>>>> patch makes is that if you had the above three-line block in
>>>> the .gitmodules file and not in $GIT_DIR/config, "git status"
>>>> ignored the dirtyness in the working tree, but "git commit" did
>>>> notice and report it.
>>>>
>>>> What am I missing?
>>>>
>>>>
>>>
>>> I have:
>>> [submodule "mod"]
>>>         url = [...]
>>>         ignore = dirty
>>>
>>> in .git/config, and I removed the ignore part from .gitmodules to be even.
>>>
>>> I made a change inside mod, git status doesn't report its dirtiness,
>>> while git commit does.
>>>
>>> git status:
>>> # On branch master
>>> # Changes to be committed:
>>> #   (use "git reset HEAD <file>..." to unstage)
>>> #
>>> #       modified:   foo
>>> #
>>> # Changes not staged for commit:
>>> #   (use "git add <file>..." to update what will be committed)
>>> #   (use "git checkout -- <file>..." to discard changes in working directory)
>>> #
>>> #       modified:   .gitmodules
>>> #
>>>
>>> git commit:
>>> # Please enter the commit message for your changes. Lines starting
>>> # with '#' will be ignored, and an empty message aborts the commit.
>>> # On branch master
>>> # Changes to be committed:
>>> #   (use "git reset HEAD <file>..." to unstage)
>>> #
>>> #       modified:   foo
>>> #
>>> # Changes not staged for commit:
>>> #   (use "git add <file>..." to update what will be committed)
>>> #   (use "git checkout -- <file>..." to discard changes in working directory)
>>> #   (commit or discard the untracked or modified content in submodules)
>>> #
>>> #       modified:   .gitmodules
>>> #       modified:   mod (modified content)
>>> #
>>>
>>> Now I get it! That's because I don't have submodule.mod.path!
>>> config_name_for_path only gets initialized if path exists. Apparently
>>> git submodule init doesn't configure 'path', so it stays
>>> uninitialized.
>>
>> But submodule.mod.path should only be set in .gitmodules, not in
>> $GIT_DIR/config. Did you just remove the ignore setting from
>> .gitmodules or the path too?
> 
> Just the ignore, and my patch of course.
> 
> If it is not set in $GIT_DIR/config, then config_name_for_path is not
> initialized, and if it is not initialized, then
> set_diffopt_flags_from_submodule_config does nothing
> (handle_ignore_submodules_arg is not called). That is the main
> problem.

But config_name_for_path can only be set via .gitmodules. It is set in parse_submodule_config_option() called by submodule_config() which is called from gitmodules_config() ... but only if .gitmodules doesn't have a merge conflict.

So either your .gitmodules has a merge conflict or the logic setting gitmodules_is_unmerged in gitmodules_config() is buggy.

Junio C Hamano· Sep 24, 2012, 20:36 UTC · re: Jens Lehmann · lore

Re: [PATCH] commit: configure submodules

Jens Lehmann <Jens.Lehmann@web.de> writes:

> But submodule.mod.path should only be set in .gitmodules, not in
> $GIT_DIR/config. Did you just remove the ignore setting from
> .gitmodules or the path too?

Without that in $GIT_DIR/config, how would path<->name mapping correctly work???

Confused...