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

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

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