Re: Re: Git issues with submodules

From: Heiko Voigt <hvoigt@hvoigt.net>
Date: Nov 22, 2013, 21:54 UTC
Message-ID: <20131122215454.GA4952@sandbox-ub>
In-Reply-To: <528FC638.5060403@web.de>

Hi,

On Fri, Nov 22, 2013 at 10:01:44PM +0100, Jens Lehmann wrote:

Show 48 quoted lines

> Hmm, looks like git show also needs to be fixed to honor the
> ignore setting from .gitmodules. It already does that for
> diff.ignoreSubmodules from either .git/config or git -c and
> also supports the --ignore-submodules command line option.
> The following fixes this inconsistency for me:
> 
> ---------------------->8-------------------
> diff --git a/builtin/log.c b/builtin/log.c
> index b708517..ca97cfb 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -25,6 +25,7 @@
>  #include "version.h"
>  #include "mailmap.h"
>  #include "gpg-interface.h"
> +#include "submodule.h"
> 
>  /* Set a default date-time format for git log ("log.date" config variable) */
>  static const char *default_date_mode = NULL;
> @@ -521,6 +522,7 @@ int cmd_show(int argc, const char **argv, const char *prefix
>         int i, count, ret = 0;
> 
>         init_grep_defaults();
> +       gitmodules_config();
>         git_config(git_log_config, NULL);
> 
>         memset(&match_all, 0, sizeof(match_all));
> ---------------------->8-------------------
> 
> But the question is if that is the right thing to do: should
> diff.ignoreSubmodules and submodule.<name>.ignore only affect
> the diff family or also git log & friends? That would make
> users blind for submodule history (which they already are
> when using diff & friends, so that might be ok here too).
> 
> > For some reason, the
> > `git add .` is adding the ignored submodule to the index.
> 
> The ignore setting is documented to only affect diff output
> (including what checkout, commit and status show as modified).
> While I agree that this behavior is confusing for Sergey and
> not optimal for the floating branch model he uses, git is
> currently doing exactly what it should. And for people using
> the ignore setting to not having to stat submodules with huge
> and/or many files that behavior is what they want: don't bother
> me with what changed, but commit what I did change on purpose.
> We may have to rethink what should happen for users of the
> floating branch model though.

This gets more nasty. When using 'git add .' you secretly add the submodule to the index. But it is neither shown in status nor diff --cached. commit actually complains there is nothing to add. But then once you add a local file to the index you can commit and secretly take the submodule change with you.

What I think needs fixing here first is that the ignore setting should not apply to any diffs between HEAD and index. IMO, it should only apply to the diff between worktree and index.

When we have that the user does not see the submodule changed when normally working. But after doing git add . the change to the submodule should be shown in status and diff regardless of the configuration.

I will have a look at that.

After that we can discuss whether add should add submodules that are tracked but not shown. How about commit -a ? Should it also ignore the change? I am undecided here. There does not seem to be any good decision. From the users point of view we should probably not add it since its not visible in status. What do others think?

Cheers Heiko