Re: [PATCH] commit: configure submodules

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 23, 2012, 08:37 UTC
Message-ID: <7vr4pt16ep.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1348385867-17359-1-git-send-email-orgads@gmail.com>

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;