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

> On 8. 4. 2014 20:26, Jens Lehmann wrote:
>> Am 07.04.2014 23:46, schrieb Ronald Weiss:
>>> Then, on top of that, I'll prepare patches for add to honor ignore
>>> from .gitmodules, and -f implying --ignore-submodules. That might need
>>> more discussion, let's see.
>>
>> Makes sense.
> 
> I thought more about that, and also played with the code a bit.
> 
> First, I was confused when I wrote that git add doesn't honor
> submodules' ignore setting only from .gitmodules, but it does from
> .git/config. It doesn't, from neither. Sorry for the confusion. However,
> that doesn't change anything on the fact that it would be nice if add
> would honor the ignore setting, from both places.

> Second, there are some differences between adding standard ignored
> files, and ignored submodules:
> 
> 1) Already tracked files are never ignored, regardless of .gitignore.
>  However, tracked submodules should be ignored by "add -u", if told so
>  by their ignore setting.
> 
> 2) So .gitignore seems to only do something when adding new files to
>  the repo. However, when adding new submodules, they are probably never
>  ignored (setting the ignore setting for non existent submodule seems
>  like non-sense, although possible).

> 3) Ignored files can be ignored less explicitely (in global gitignore,
>  or using a wildcard, or by ignoring parent folder). So it makes sense
>  to warn the user if he tries to explicitely add an ignored file, as he
>  might not be aware that the file is ignored. Submodules, however, can
>  only be ignored explicitely. And when user explicitely specifies the
>  submodule in an add command, he most probably really wants to add it,
>  so I don't see the point in warning him and requiring the -f option.

> So, I think that the use cases are completely different, for submodules
> and ignored files. So trying to make add behave the same for both, might
> not be that good idea.
> 
> I would propose - let's make add honor the ignore setting by just
> parsing if from config like the other commands do, and pass it to
> underlying diff invocations. And at the same the, let's override it for
> submodules explicitely specified on the command line, to never ignore
> such submodules, without requiring the -f option. That seems to be
> pretty easy, see below.

> diff --git a/builtin/add.c b/builtin/add.c
> index 85f2110..f19e6c8 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -284,6 +284,10 @@ static int add_config(const char *var, const char *value, void *cb)
>  		ignore_add_errors = git_config_bool(var, value);
>  		return 0;
>  	}
> +
> +	if (starts_with(var, "submodule."))
> +		return parse_submodule_config_option(var, value);
> +
>  	return git_default_config(var, value, cb);
>  }
>  
> @@ -320,6 +324,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  	char *seen = NULL;
>  
>  	git_config(add_config, NULL);
> +	gitmodules_config();

>  	argc = parse_options(argc, argv, prefix, builtin_add_options,
>  			  builtin_add_usage, PARSE_OPT_KEEP_ARGV0);
> @@ -425,6 +430,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  			       PATHSPEC_EXCLUDE);
>  
>  		for (i = 0; i < pathspec.nr; i++) {
> +			int cachepos;
>  			const char *path = pathspec.items[i].match;
>  			if (pathspec.items[i].magic & PATHSPEC_EXCLUDE)
>  				continue;
> @@ -440,6 +446,18 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  					die(_("pathspec '%s' did not match any files"),
>  					    pathspec.items[i].original);
>  			}
> +
> +			/* disable ignore setting for any submodules specified explicitly in the pathspec */
> +			if (path[0] &&
> +				(cachepos = cache_name_pos(path, pathspec.items[i].len)) >= 0 &&
> +				S_ISGITLINK(active_cache[cachepos]->ce_mode)) {
> +				char *optname;
> +				int optnamelen = pathspec.items[i].len + 17;
> +				optname = xcalloc(optnamelen + 1, 1);
> +				snprintf(optname, optnamelen + 1, "submodule.%s.ignore", path);
> +				parse_submodule_config_option(optname, "none");

> +				free(optname);
> +			}
>  		}
>  		free(seen);
>  	}
> --  
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>