Re: [PATCH] commit: configure submodules

[Junio C Hamano <gitster@pobox.com>]

Orgad Shaneh <orgads@gmail.com> writes:

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

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