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

[Ronald Weiss <weiss.ronald@gmail.com>]

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 <Jens.Lehmann@web.de> 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?

>
> So I'm going to do some more testing, write tests for git add with
> ignoring submodules the various ways, and then post two patches
> rearranged, one for git add, and second for git commit on top of that,
> as you guys suggested. Including the change suggested by Jens, to not
> mess with index_differs_from() in diff-lib.c, that works fine too.
>
>
> diff --git a/builtin/add.c b/builtin/add.c
> index 1086294..9f70327 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -350,6 +350,8 @@ static int ignore_add_errors, intent_to_add, ignore_missing;
>   static int addremove = ADDREMOVE_DEFAULT;
>   static int addremove_explicit = -1; /* unspecified */
>
> +static char *ignore_submodule_arg;
> +
>   static int ignore_removal_cb(const struct option *opt, const char *arg, int unset)
>   {
>          /* if we are told to ignore, we are not adding removals */
> @@ -375,6 +377,9 @@ static struct option builtin_add_options[] = {
>          OPT_BOOL( 0 , "refresh", &refresh_only, N_("don't add, only refresh the index")),
>          OPT_BOOL( 0 , "ignore-errors", &ignore_add_errors, N_("just skip files which cannot be added because of errors")),
>          OPT_BOOL( 0 , "ignore-missing", &ignore_missing, N_("check if - even missing - files are ignored in dry run")),
> +       { OPTION_STRING, 0, "ignore-submodules", &ignore_submodule_arg, N_("when"),
> +         N_("ignore changes to submodules, optional when: all, none. (Default: all)"),
> +         PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
>          OPT_END(),
>   };
>
> @@ -422,6 +427,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>          int implicit_dot = 0;
>          struct update_callback_data update_data;
>
> +       gitmodules_config();
>          git_config(add_config, NULL);
>
>          argc = parse_options(argc, argv, prefix, builtin_add_options,
> @@ -584,7 +590,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>                  memset(&pathspec, 0, sizeof(pathspec));
>          }
>          update_data.flags = flags & ~ADD_CACHE_IMPLICIT_DOT;
> -       update_files_in_cache(prefix, &pathspec, &update_data, NULL);
> +       update_files_in_cache(prefix, &pathspec, &update_data, ignore_submodule_arg);
>
>          exit_status |= !!update_data.add_errors;
>          if (add_new_files)
>