git status: ignoring untracked files must apply to submodules too

Since 1.7.0 submodules are considered dirty when they contain untracked files. But when git status is called with the "-uno" option, the user asked to ignore untracked files, so they must be ignored in submodules too. To achieve this, the new flag DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES is introduced.

---

This patch applies on top of current pu.

I'm open to suggestions for a shorter name for the new diff option IGNORE_UNTRACKED_IN_SUBMODULES; I did not manage to come up with a shorter yet still descriptive enough name.

 diff-lib.c                  |    2 +-
 diff.h                      |    1 +
 submodule.c                 |    9 +++++++--
 submodule.h                 |    2 +-
 t/t7506-status-submodule.sh |    5 +++++
 wt-status.c                 |    2 ++
 6 files changed, 17 insertions(+), 4 deletions(-)

index 4e30b8f..c9f6e05 100644
@@ -71,7 +71,7 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
 	if (S_ISGITLINK(ce->ce_mode)
 	    && !DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES)
 	    && (!changed || DIFF_OPT_TST(diffopt, DIRTY_SUBMODULES))) {
-		*dirty_submodule = is_submodule_modified(ce->name);
+		*dirty_submodule = is_submodule_modified(ce->name, DIFF_OPT_TST(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES));
 	}
 	return changed;
 }

index 95ed7f8..6a71013 100644
@@ -70,6 +70,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
 #define DIFF_OPT_DIFF_FROM_CONTENTS  (1 << 22)
 #define DIFF_OPT_SUBMODULE_LOG       (1 << 23)
 #define DIFF_OPT_DIRTY_SUBMODULES    (1 << 24)
+#define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 25)
 
 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)

index 714ca97..b3b8bc1 100644
@@ -130,7 +130,7 @@ void show_submodule_summary(FILE *f, const char *path,
 	strbuf_release(&sb);
 }
 
-unsigned is_submodule_modified(const char *path)
+unsigned is_submodule_modified(const char *path, int ignore_untracked)
 {
 	int i;
 	ssize_t len;

@@ -139,6 +139,7 @@ unsigned is_submodule_modified(const char *path)
 		"status",
 		"--porcelain",
 		NULL,
+		NULL,
 	};
 	const char *env[LOCAL_REPO_ENV_SIZE + 3];
 	struct strbuf buf = STRBUF_INIT;

@@ -163,6 +164,9 @@ unsigned is_submodule_modified(const char *path)
 	env[i++] = strbuf_detach(&buf, NULL);
 	env[i] = NULL;
 
+	if (ignore_untracked)
+		argv[2] = "-uno";
+
 	memset(&cp, 0, sizeof(cp));
 	cp.argv = argv;
 	cp.env = env;

@@ -181,7 +185,8 @@ unsigned is_submodule_modified(const char *path)
 				break;
 		} else {
 			dirty_submodule |= DIRTY_SUBMODULE_MODIFIED;
-			if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
+			if (ignore_untracked ||
+			    (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED))
 				break;
 		}
 		next_line = strchr(line, '\n');

index 267881c..dbda270 100644
@@ -5,6 +5,6 @@ void show_submodule_summary(FILE *f, const char *path,
 		unsigned char one[20], unsigned char two[20],
 		unsigned dirty_submodule,
 		const char *del, const char *add, const char *reset);
-unsigned is_submodule_modified(const char *path);
+unsigned is_submodule_modified(const char *path, int ignore_untracked);
 
 #endif

index dc9150a..aeec1f6 100755
@@ -67,6 +67,11 @@ test_expect_success 'status with untracked file in submodule' '
 	grep "modified:   sub (untracked content)" output
 '
 
+test_expect_success 'status -uno with untracked file in submodule' '
+	git status -uno >output &&
+	grep "^nothing to commit" output
+'
+
 test_expect_success 'status with untracked file in submodule (porcelain)' '
 	git status --porcelain >output &&
 	diff output - <<-\EOF

index cf90430..8ca59a2 100644
@@ -304,6 +304,8 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
 	setup_revisions(0, NULL, &rev, NULL);
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 	DIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);
+	if (!s->show_untracked_files)
+		DIFF_OPT_SET(&rev.diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
 	rev.diffopt.format_callback = wt_status_collect_changed_cb;
 	rev.diffopt.format_callback_data = s;
 	rev.prune_data = s->pathspec;

1.7.0.2.399.g53a88

Why do you even need that flag? Isn't it the matter of deciding to ignore or pay attention to the DIRTY_SUBMODULE_UNTRACKED bit in the return value of is_submodule_modified(), depending on whether the toplevel wt_status was called with -uno?

Am 13.03.2010 23:24, schrieb Junio C Hamano:

First: When called from "git status" run_diff_files() calls wt_status_collect_changed_cb() for every file it considers changed, so when the "-uno" option is given for a submodule with only untracked files i thought it cleaner to let is_submodule_modified() return 0 so this callback is not called at all and the submodule won't show up in the "change" list of wt_status (Yes, this could be done by not adding such a submodule to that list in the wt_status_collect_changed_cb() too).

Second: One of my next patches will be about adding an option to the git diff family to not show submodules with only untracked files in their work tree as modified, which calls for such a diff option AFAICS.

Third: We give is_submodule_modified() the possibility to stop parsing the output of the "git status" run in the submodule early, because as soon as it sees a modified file it will stop to parse the output further when untracked files shall be ignored.

This reminds me of another thing, perhaps independent, perhaps related. Why aren't we collecting the submodule status in wt_status_collect() to begin with? It examines the submodule status in wt_status_print(), but that feels quite against the way how the whole "struct wt_status" was designed to be used in the first place, I think. Would restuctuing the code that way make this easier to handle?

Am 14.03.2010 01:07, schrieb Junio C Hamano:

I'm not sure I understand that, but AFAICS in wt_status_print_changed() only the fact that at least one submodule is dirty is examined to be able to print the extra hint line. The status of each submodule is collected in wt_status_collect_changed_cb(), no?

And while not having being active when "struct wt_status" has been designed, i think adding submodules to the "change" list when they are dirty makes kind a sense ... but i might be wrong ;-)