[PATCH 2/3] revision machinery: gentle submodule errors

[Stefan Beller <sbeller@google.com>]

When a submodule is broken (e.g. its git directory is deleted or is not
pointed to via the .git linking file), commands that deal with submodules
error out reporting the submodule error.

Sometimes users rather want to see the rest of the command succeed,
ignoring the submodule that is severely broken.

Introduce a flag for the revision machinery '--gentle-submodule-errors'
that will ask 'is_submodule_modified' to return the broken flag instead
of dying().

By switching the argument of is_submodule_modified to an unsigned bit
vector, we can signal more flags, such as the gentle bit introduced
in this patch. When this flag is given, avoid dying inside
'is_submodule_modified' for severe errors and return
DIRTY_SUBMODULE_BROKEN instead.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 diff-lib.c                                   | 10 ++++++++--
 diff.h                                       |  1 +
 diffcore.h                                   |  3 ++-
 revision.c                                   |  2 ++
 submodule.c                                  |  9 ++++++++-
 submodule.h                                  | 10 +++++++++-
 t/t4060-diff-submodule-option-diff-format.sh | 22 ++++++++++++++++++++++
 7 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 52447466b5..61d33b6d26 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -77,8 +77,14 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
 		if (DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES))
 			changed = 0;
 		else if (!DIFF_OPT_TST(diffopt, IGNORE_DIRTY_SUBMODULES)
-		    && (!changed || DIFF_OPT_TST(diffopt, DIRTY_SUBMODULES)))
-			*dirty_submodule = is_submodule_modified(ce->name, DIFF_OPT_TST(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES));
+		    && (!changed || DIFF_OPT_TST(diffopt, DIRTY_SUBMODULES))) {
+			unsigned flags = 0;
+			if (DIFF_OPT_TST(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES))
+				flags |= IS_SUBMODULE_MODIFIED_IGNORE_UNTRACKED;
+			if (diffopt->gentle_submodule_errors)
+				flags |= IS_SUBMODULE_MODIFIED_GENTLY;
+			*dirty_submodule = is_submodule_modified(ce->name, flags);
+		}
 		diffopt->flags = orig_flags;
 	}
 	return changed;
diff --git a/diff.h b/diff.h
index e9ccb38c26..d1e48de43c 100644
--- a/diff.h
+++ b/diff.h
@@ -164,6 +164,7 @@ struct diff_options {
 	const char *word_regex;
 	enum diff_words_type word_diff;
 	enum diff_submodule_format submodule_format;
+	int gentle_submodule_errors;
 
 	/* this is set by diffcore for DIFF_FORMAT_PATCH */
 	int found_changes;
diff --git a/diffcore.h b/diffcore.h
index 6230241354..ceef783570 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -40,9 +40,10 @@ struct diff_filespec {
 #define DIFF_FILE_VALID(spec) (((spec)->mode) != 0)
 	unsigned should_free : 1; /* data should be free()'ed */
 	unsigned should_munmap : 1; /* data should be munmap()'ed */
-	unsigned dirty_submodule : 2;  /* For submodules: its work tree is dirty */
+	unsigned dirty_submodule : 3;  /* For submodules: its work tree is dirty */
 #define DIRTY_SUBMODULE_UNTRACKED 1
 #define DIRTY_SUBMODULE_MODIFIED  2
+#define DIRTY_SUBMODULE_BROKEN 4
 	unsigned is_stdin : 1;
 	unsigned has_more_entries : 1; /* only appear in combined diff */
 	/* data should be considered "binary"; -1 means "don't know yet" */
diff --git a/revision.c b/revision.c
index 7ff61ff5f7..8ae1179b8d 100644
--- a/revision.c
+++ b/revision.c
@@ -2014,6 +2014,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->limited = 1;
 	} else if (!strcmp(arg, "--ignore-missing")) {
 		revs->ignore_missing = 1;
+	} else if (!strcmp(arg, "--gentle-submodule-errors")) {
+		revs->diffopt.gentle_submodule_errors = 1;
 	} else {
 		int opts = diff_opt_parse(&revs->diffopt, argv, argc, revs->prefix);
 		if (!opts)
diff --git a/submodule.c b/submodule.c
index 81d44cb7e9..d477297fab 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1039,13 +1039,18 @@ int fetch_populated_submodules(const struct argv_array *options,
 	return spf.result;
 }
 
-unsigned is_submodule_modified(const char *path, int ignore_untracked)
+/*
+ * Inspects a submodule and returns its state using flags
+ */
+unsigned is_submodule_modified(const char *path, unsigned flags)
 {
 	struct child_process cp = CHILD_PROCESS_INIT;
 	struct strbuf buf = STRBUF_INIT;
 	unsigned dirty_submodule = 0;
 	const char *git_dir;
 	int error_code = 0;
+	int ignore_untracked = flags & IS_SUBMODULE_MODIFIED_IGNORE_UNTRACKED;
+	int gently = flags & IS_SUBMODULE_MODIFIED_GENTLY;
 
 	strbuf_addf(&buf, "%s/.git", path);
 	git_dir = resolve_gitdir_gently(buf.buf, &error_code);
@@ -1069,6 +1074,8 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)
 			 * right here. Resolve again triggering die()
 			 * inside of the parsing.
 			 */
+			if (gently)
+				return DIRTY_SUBMODULE_BROKEN;
 			read_gitfile_gently(buf.buf, NULL);
 			die("BUG: read_gitfile_gently should have died.");
 		}
diff --git a/submodule.h b/submodule.h
index c8a0c9cb29..b7afac5e87 100644
--- a/submodule.h
+++ b/submodule.h
@@ -62,7 +62,15 @@ extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);
 extern int fetch_populated_submodules(const struct argv_array *options,
 			       const char *prefix, int command_line_option,
 			       int quiet, int max_parallel_jobs);
-extern unsigned is_submodule_modified(const char *path, int ignore_untracked);
+
+/* Ignore untracked files */
+#define IS_SUBMODULE_MODIFIED_IGNORE_UNTRACKED 1
+/*
+ * Report severe errors in the submodule (e.g. missing git dir) via
+ * DIRTY_SUBMODULE_BROKEN instead of dying.
+ */
+#define IS_SUBMODULE_MODIFIED_GENTLY 2
+extern unsigned is_submodule_modified(const char *path, unsigned flags);
 extern int submodule_uses_gitfile(const char *path);
 
 #define SUBMODULE_REMOVAL_DIE_ON_ERROR (1<<0)
diff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh
index 7e23b55ea4..e5e11a8378 100755
--- a/t/t4060-diff-submodule-option-diff-format.sh
+++ b/t/t4060-diff-submodule-option-diff-format.sh
@@ -746,4 +746,26 @@ test_expect_success 'diff --submodule=diff with .git file' '
 	test_cmp expected actual
 '
 
+test_expect_success 'setup -- break submodule' '
+	# the gitlink in sm2 points at a missing git dir now
+	mv .real .moved_real
+'
+
+test_expect_success 'diff with severely broken submodule' '
+	test_must_fail git diff &&
+
+	sm_hash=$(git rev-parse HEAD:sm2) &&
+	cat >expect <<-EOF &&
+	diff --git a/sm2 b/sm2
+	--- a/sm2
+	+++ b/sm2
+	@@ -1 +1 @@
+	-Subproject commit $sm_hash
+	+Subproject commit $sm_hash-dirty
+	EOF
+
+	git diff --gentle-submodule-errors -- sm2 >out &&
+	test_cmp expect out
+'
+
 test_done
-- 
2.12.0.402.g4b3201c2d6.dirty