Teach "git status" the "--ignore-submodules" option

The first patch in this series lets "git submodule summary" and "git submodule status" ignore dirty submodules. I thought about teaching them a "--ignore" option too, but didn't for two reasons: First, the output of those commands is pretty much focused on commits. Second, when using "git status" with the config option 'status.submodulesummary' the submodule tree is scanned twice, once for "git status" and then again for "git submodule summary". With this patch the second run is gone, which is a big gain for users of 'status.submodulesummary' with large submodules.

The second patch teaches "git status" the same "--ignore-submodules" option that "git diff" recently learned. Ignoring all changes also suppresses the output of "git submodule summary" when 'status.submodulesummary' is set.

After this series I am planning to add a config option 'ignore' to .gitmodules, which can be set for each submodule to either "all", "dirty", "untracked" or "none" (the default). "git diff" and "git status" will use that config value for each submodule. Using "--ignore-submodule" overrides this default (and the new parameter "none" will be added there to able to override the config settings). And to avoid having to do "git submdule sync" every time that option changes, I would like to search for it in .git/config first. If it is not found there, it will be taken from .gitmodules, if present. So users can override the setting but if they don't, upstream can change it easily (e.g. when a submodules .gitignore has been updated so that "ignore=untracked" is no longer necessary anymore it can be removed). Also switching branches will have an effect instantly if the 'ignore' entry in .gitmodules is different between branches.

Opinions?

Jens Lehmann (2):
git submodule: ignore dirty submodules for summary and status Add the option "--ignore-submodules" to "git status"

 Documentation/git-status.txt |   11 ++++
 builtin/commit.c             |    7 ++-
 diff.c                       |   15 +----
 git-submodule.sh             |    6 +-
 submodule.c                  |   13 ++++
 submodule.h                  |    3 +
 t/t7508-status.sh            |  127 ++++++++++++++++++++++++++++++++++++++++++
 wt-status.c                  |   10 +++-
 wt-status.h                  |    1 +
 9 files changed, 177 insertions(+), 16 deletions(-)

1.7.1.757.g0ad3c.dirty

The summary and status commands only care about submodule commits, so it is rather pointless that they check for dirty work trees. This saves the time needed to scan the submodules work tree. Even "git status" profits from these savings when the status.submodulesummary config option is set, as this lead to traversing the submodule work trees twice, once for status and once again for the submodule summary. And if the submodule was just dirty, submodule summary produced rather meaningless output anyway:

* sub 1234567...1234567 (0):

---
 git-submodule.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

index 8c562a7..d9950c2 100755
@@ -580,7 +580,7 @@ cmd_summary() {
 
 	cd_to_toplevel
 	# Get modified modules cared by user
-	modules=$(git $diff_cmd $cached --raw $head -- "$@" |
+	modules=$(git $diff_cmd $cached --ignore-submodules=dirty --raw $head -- "$@" |
 		sane_egrep '^:([0-7]* )?160000' |
 		while read mod_src mod_dst sha1_src sha1_dst status name
 		do

@@ -594,7 +594,7 @@ cmd_summary() {
 
 	test -z "$modules" && return
 
-	git $diff_cmd $cached --raw $head -- $modules |
+	git $diff_cmd $cached --ignore-submodules=dirty --raw $head -- $modules |
 	sane_egrep '^:([0-7]* )?160000' |
 	cut -c2- |
 	while read mod_src mod_dst sha1_src sha1_dst status name

@@ -760,7 +760,7 @@ cmd_status()
 			continue;
 		fi
 		set_name_rev "$path" "$sha1"
-		if git diff-files --quiet -- "$path"
+		if git diff-files --ignore-submodules=dirty --quiet -- "$path"
 		then
 			say " $sha1 $displaypath$revname"
 		else

1.7.1.757.g0ad3c.dirty

In some use cases it is not desirable that "git status" considers submodules that only contain untracked content as dirty. This may happen e.g. when the submodule is not under the developers control and not all build generated files have been added to .gitignore by the upstream developers. Using the "untracked" parameter for the "--ignore-submodules" option disables checking for untracked content and lets git diff report them as changed only when they have new commits or modified content.

Sometimes it is not wanted to have submodules show up as changed when they just contain changes to their work tree (this was the behavior before 1.7.0). An example for that are scripts which just want to check for submodule commits while ignoring any changes to the work tree. Also users having large submodules known not to change might want to use this option, as the - sometimes substantial - time it takes to scan the submodule work tree(s) is saved when using the "dirty" parameter.

And if you want to ignore any changes to submodules, you can now do that by using this option without parameters or with "all" (when the config option status.submodulesummary is set, using "all" will also suppress the output of the submodule summary).

A new function handle_ignore_submodules_arg() is introduced to parse this option new to "git status" in a single location, as "git diff" already knew it.

---
 Documentation/git-status.txt |   11 ++++
 builtin/commit.c             |    7 ++-
 diff.c                       |   15 +----
 submodule.c                  |   13 ++++
 submodule.h                  |    3 +
 t/t7508-status.sh            |  127 ++++++++++++++++++++++++++++++++++++++++++
 wt-status.c                  |   10 +++-
 wt-status.h                  |    1 +
 8 files changed, 174 insertions(+), 13 deletions(-)

index fd0fe7c..2fd054c 100644
@@ -53,6 +53,17 @@ See linkgit:git-config[1] for configuration variable
 used to change the default for when the option is not
 specified.
 
+--ignore-submodules[=<when>]::
+	Ignore changes to submodules when looking for changes. <when> can be
+	either "untracked", "dirty" or "all", which is the default. When
+	"untracked" is used submodules are not considered dirty when they only
+	contain untracked content (but they are still scanned for modified
+	content). Using "dirty" ignores all changes to the work tree of submodules,
+	only changes to the commits stored in the superproject are shown (this was
+	the behavior before 1.7.0). Using "all" hides all changes to submodules
+	(and suppresses the output of submodule summaries when the config option
+	`status.submodulesummary` is set).
+
 -z::
 	Terminate entries with NUL, instead of LF.  This implies
 	the `--porcelain` output format if no other format is given.

index 3d99cf9..bd547f2 100644
@@ -72,7 +72,7 @@ static char *author_name, *author_email, *author_date;
 static int all, edit_flag, also, interactive, only, amend, signoff;
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
 static int no_post_rewrite, allow_empty_message;
-static char *untracked_files_arg, *force_date;
+static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
 /*
  * The default commit message cleanup mode will remove the lines
  * beginning with # (shell comments) and leading and trailing

@@ -1059,6 +1059,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		  PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
 		OPT_BOOLEAN(0, "ignored", &show_ignored_in_status,
 			    "show ignored files"),
+		{ OPTION_STRING, 0, "ignore-submodules", &ignore_submodule_arg, "when",
+		  "ignore changes to submodules, optional when: all, dirty, untracked. (Default: all)",
+		  PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
 		OPT_END(),
 	};
 

@@ -1089,6 +1092,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 
 	s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
 	s.in_merge = in_merge;
+	s.ignore_submodule_arg = ignore_submodule_arg;
 	wt_status_collect(&s);
 
 	if (s.relative_paths)

@@ -1107,6 +1111,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		break;
 	case STATUS_FORMAT_LONG:
 		s.verbose = verbose;
+		s.ignore_submodule_arg = ignore_submodule_arg;
 		wt_status_print(&s);
 		break;
 	}

index 9d70f9d..3aa695d 100644
@@ -3168,17 +3168,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 	else if (!strcmp(arg, "--no-textconv"))
 		DIFF_OPT_CLR(options, ALLOW_TEXTCONV);
 	else if (!strcmp(arg, "--ignore-submodules"))
-		DIFF_OPT_SET(options, IGNORE_SUBMODULES);
-	else if (!prefixcmp(arg, "--ignore-submodules=")) {
-		if (!strcmp(arg + 20, "all"))
-			DIFF_OPT_SET(options, IGNORE_SUBMODULES);
-		else if (!strcmp(arg + 20, "untracked"))
-			DIFF_OPT_SET(options, IGNORE_UNTRACKED_IN_SUBMODULES);
-		else if (!strcmp(arg + 20, "dirty"))
-			DIFF_OPT_SET(options, IGNORE_DIRTY_SUBMODULES);
-		else
-			die("bad --ignore-submodules argument: %s", arg + 20);
-	} else if (!strcmp(arg, "--submodule"))
+		handle_ignore_submodules_arg(options, "all");
+	else if (!prefixcmp(arg, "--ignore-submodules="))
+		handle_ignore_submodules_arg(options, arg + 20);
+	else if (!strcmp(arg, "--submodule"))
 		DIFF_OPT_SET(options, SUBMODULE_LOG);
 	else if (!prefixcmp(arg, "--submodule=")) {
 		if (!strcmp(arg + 12, "log"))

index 676d48f..61cb6e2 100644
@@ -46,6 +46,19 @@ done:
 	return ret;
 }
 
+void handle_ignore_submodules_arg(struct diff_options *diffopt,
+				  const char *arg)
+{
+	if (!strcmp(arg, "all"))
+		DIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);
+	else if (!strcmp(arg, "untracked"))
+		DIFF_OPT_SET(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
+	else if (!strcmp(arg, "dirty"))
+		DIFF_OPT_SET(diffopt, IGNORE_DIRTY_SUBMODULES);
+	else
+		die("bad --ignore-submodules argument: %s", arg);
+}
+
 void show_submodule_summary(FILE *f, const char *path,
 		unsigned char one[20], unsigned char two[20],
 		unsigned dirty_submodule,

index dbda270..6fd3bb4 100644
@@ -1,6 +1,9 @@
 #ifndef SUBMODULE_H
 #define SUBMODULE_H
 
+struct diff_options;
+
+void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
 void show_submodule_summary(FILE *f, const char *path,
 		unsigned char one[20], unsigned char two[20],
 		unsigned dirty_submodule,

index 9d9cb95..373213e 100644
@@ -10,6 +10,7 @@
 #include "run-command.h"
 #include "remote.h"
 #include "refs.h"
+#include "submodule.h"
 
 static char default_wt_status_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */

@@ -312,6 +313,8 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
 	DIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);
 	if (!s->show_untracked_files)
 		DIFF_OPT_SET(&rev.diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
+	if (s->ignore_submodule_arg)
+		handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);
 	rev.diffopt.format_callback = wt_status_collect_changed_cb;
 	rev.diffopt.format_callback_data = s;
 	rev.prune_data = s->pathspec;

@@ -328,6 +331,9 @@ static void wt_status_collect_changes_index(struct wt_status *s)
 	opt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference;
 	setup_revisions(0, NULL, &rev, &opt);
 
+	if (s->ignore_submodule_arg)
+		handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);
+
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = wt_status_collect_updated_cb;
 	rev.diffopt.format_callback_data = s;

@@ -646,7 +652,9 @@ void wt_status_print(struct wt_status *s)
 	wt_status_print_updated(s);
 	wt_status_print_unmerged(s);
 	wt_status_print_changed(s);
-	if (s->submodule_summary) {
+	if (s->submodule_summary &&
+	    (!s->ignore_submodule_arg ||
+	     strcmp(s->ignore_submodule_arg, "all"))) {
 		wt_status_print_submodule_summary(s, 0);  /* staged */
 		wt_status_print_submodule_summary(s, 1);  /* unstaged */
 	}

index 4cd74c4..9df9c9f 100644
@@ -45,6 +45,7 @@ struct wt_status {
 	int submodule_summary;
 	int show_ignored_files;
 	enum untracked_status_type show_untracked_files;
+	const char *ignore_submodule_arg;
 	char color_palette[WT_STATUS_REMOTE_BRANCH+1][COLOR_MAXLEN];
 
 	/* These are computed during processing of the individual sections */

1.7.1.757.g0ad3c.dirty

I think both patches make sense. It would be reassuring to hear from people who are heavier submodule users than me, though...

Thanks.

Speaking of which, what ever came of the submodule dogfood experiment? Given a patch to automatically resolve submodule-ejection/subtree-merge conflicts, do you think a patch series to split off gitk and git-gui as submodules could live in pu eventually?

Jonathan

Hi,

+1

They do sound like they'd fit into the submodule paradigm. Further, having git use it would be a good showcase of the submodule feature - or bug. :)

Cheers,
Ray Chuan

Am 26.06.2010 06:44, schrieb Tay Ray Chuan:

Yes, I think having them as a submodule makes lots of sense. But submodules are not there yet. Unless I overlooked something, the following issues must be resolved before having these two as a submodule, otherwise people will complain (and rightfully so!):

1) Switching branches, merging, rebasing and resetting in the superproject must result in a checkout of the matching submodule work tree (right now you always have to issue a "git submodule update" afterwards to get the submodules in sync).

2) On "git clone" the submodules must be cloned and checked out too (currently you have to do a "git submodule update --init" after cloning the superproject).

3) Switching between commits in the superproject where a directory is replaced by a submodule or vice versa doesn't work right now. Submodules should handle this situation, otherwise the commit putting gitk and git gui into submodules would become a barrier.

I am working on these issues, but that will take some time. But when they are solved, me too thinks that these two should become submodules.

Switching branches can change the HEAD commit of an already checked out submodule, and switching branches can change the list of submodules.

For the latter, maybe it would make sense to introduce ‘git checkout --recursive’ which runs ‘submodule update --init --recursive’ after checkout. IMHO, in git 2.0 this even ought to be made the default. One could use --no-recursive to access the more flexible traditional behavior.

Does ‘git clone --recursive’ take care of it?

The Makefile would need to be tweaked to give a reasonable message when the caller forgets to check out an important submodule.

Yes, this is the fatal problem imho since it would affect basic maintainance tasks. I consider maintainance of the branch (rather than consumption) the most important piece to begin with; for a branch not in next, usability problems can be resolved over time.

Thanks for the explanation.

Am 26.06.2010 20:31, schrieb Jonathan Nieder:

Yup, I am working on that (I did post a WIP patch some time ago where I added a '--ignore-submodules' option, but since then I changed my mind on the name and now too think that '--[no-]recursive' is a much better choice).

I applaud all the current submodule work -- thanks a ton!

I would like to ask submodule developers to please keep in mind scenarios where only a subset of the super-project's submodules are in use at one time.
In particular, please avoid forcing any blanket updates to all submodules.
I think the features being discussed would be much more useful if they only applied to submodules that are already initialized and updated by the user, and that any unintialized submodules should be left alone.

So, extending my request to this situation, I would say git should only update submodules that are already initialzied and updated.

Making clone do this automatically would be a show-stopper for us. The current '--recursive' option is fine (though we never use it).

It would be interesting if the super-project could configure which submodules to automatically clone.

(FYI, our build works along the lines of what Jonathan suggested: it instructs folks on how to obtain missing submodules.)

		M.

Am 28.06.2010 20:29, schrieb Marc Branchaud:

Thanks for your feedback!

Yes. I think we agreed some time ago that until the user can configure the wanted behavior only populated submodules should be touched, so I don't mention that explicitly anymore.

Yes, see above.

That's what I have in mind too. Maybe we could use the 'update' option of .gitmodules to clone all those submodules where it is set. Then different branches with different .gitmodules would behave differently.

That sounds fine to me.

Thanks again!

		M.