Subject: [PATCH] diff: do not show submodule with untracked files as "-dirty"

From: "Sangeeta via GitGitGadget" <[email-masked]>
Date: Thu, 15 Oct 2020 17:08:43 +0000

From: sangu09 <[email-masked]>

Git diff reports a submodule directory as -dirty even when there are only untracked files in the submodule directory.
This is inconsistent with what `git describe --dirty` says when run in the submodule directory in that state.

So this patch makes `git diff HEAD` consistent with `git describe --dirty` in case of untracked files. This can be solved by including the "--ignore-submodules=untracked" feature in git diff by default. So in order to make this as default behaviour I have added the behaviour of `ignore-submodules` in `repo_diff_setup()` function. Also, to avoid `ignoreSubmodules` in user config being overwritten, I have made a  global variable `diff_ignore_submodule_config` to keep a track whether `handle_ignore_submodules_arg` is called before or not.

Signed-off-by: Sangeeta Jain <[email-masked]>
---
    [Outreachy] diff: do not show submodule with untracked files as "-dirty"
    
    Git diff reports a submodule directory as -dirty even when there are
    only untracked files in the submodule directory. This is inconsistent
    with what git describe --dirty says when run in the submodule directory
    in that state.
    
    So this patch makes git diff HEAD consistent with git describe --dirty 
    in case of untracked files. This can be solved by including the
    "--ignore-submodules=untracked" feature in git diff by default. So in
    order to make this as default behaviour I have added the behaviour of 
    ignore-submodules in repo_diff_setup() function. Also, to avoid 
    ignoreSubmodules in user config being overwritten, I have made a global
    variable diff_ignore_submodule_config to keep a track whether 
    handle_ignore_submodules_arg is called before or not.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-751%2Fsangu09%2Fhandle_untracked-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-751/sangu09/handle_untracked-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/751

 diff.c                                       |  1 +
 submodule.c                                  |  2 ++
 t/t3701-add-interactive.sh                   |  2 +-
 t/t4027-diff-submodule.sh                    |  6 ++--
 t/t4041-diff-submodule-option.sh             | 16 ++++-----
 t/t4060-diff-submodule-option-diff-format.sh | 16 ++++-----
 t/t7064-wtstatus-pv2.sh                      |  8 ++---
 t/t7506-status-submodule.sh                  | 38 +++++---------------
 8 files changed, 36 insertions(+), 53 deletions(-)

diff --git a/diff.c b/diff.c
index 2bb2f8f57e..07d89e3e2b 100644
--- a/diff.c
+++ b/diff.c
@@ -4585,6 +4585,7 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
 		DIFF_XDL_SET(options, INDENT_HEURISTIC);
 
 	options->orderfile = diff_order_file_cfg;
+	options->flags.ignore_untracked_in_submodules = 1;
 
 	if (diff_no_prefix) {
 		options->a_prefix = options->b_prefix = "";
diff --git a/submodule.c b/submodule.c
index b3bb59f066..762298c010 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1678,6 +1678,8 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)
 	strvec_pushl(&cp.args, "status", "--porcelain=2", NULL);
 	if (ignore_untracked)
 		strvec_push(&cp.args, "-uno");
+	else
+        strvec_push (&cp.args, "--ignore-submodules=none");
 
 	prepare_submodule_repo_env(&cp.env_array);
 	cp.git_cmd = 1;
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index ca04fac417..98e46ad1ae 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -761,7 +761,7 @@ test_expect_success 'setup different kinds of dirty submodules' '
 		echo dirty >>initial &&
 		: >untracked
 	) &&
-	git -C for-submodules diff-files --name-only >actual &&
+	git -C for-submodules diff-files --name-only --ignore-submodules=none >actual &&
 	cat >expected <<-\EOF &&
 	dirty-both-ways
 	dirty-head
diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh
index d7145ccca4..7bf2cb9c88 100755
--- a/t/t4027-diff-submodule.sh
+++ b/t/t4027-diff-submodule.sh
@@ -93,7 +93,7 @@ test_expect_success 'git diff HEAD with dirty submodule (untracked)' '
 	) &&
 	git diff HEAD >actual &&
 	sed -e "1,/^@@/d" actual >actual.body &&
-	expect_from_to >expect.body $subtip $subprev-dirty &&
+	expect_from_to >expect.body $subtip $subprev &&
 	test_cmp expect.body actual.body
 '
 
@@ -168,13 +168,13 @@ test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match)'
 		git clean -qfdx &&
 		>cruft
 	) &&
-	git diff HEAD >actual &&
+	git diff --ignore-submodules=none HEAD >actual &&
 	sed -e "1,/^@@/d" actual >actual.body &&
 	expect_from_to >expect.body $subprev $subprev-dirty &&
 	test_cmp expect.body actual.body &&
 	git diff --ignore-submodules=all HEAD >actual2 &&
 	test_must_be_empty actual2 &&
-	git diff --ignore-submodules=untracked HEAD >actual3 &&
+	git diff HEAD >actual3 &&
 	test_must_be_empty actual3 &&
 	git diff --ignore-submodules=dirty HEAD >actual4 &&
 	test_must_be_empty actual4
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index f852136585..bb368b685d 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -262,7 +262,7 @@ test_expect_success 'submodule is up to date' '
 
 test_expect_success 'submodule contains untracked content' '
 	echo new > sm1/new-file &&
-	git diff-index -p --submodule=log HEAD >actual &&
+	git diff-index -p --submodule=log HEAD --ignore-submodules=none >actual &&
 	cat >expected <<-EOF &&
 	Submodule sm1 contains untracked content
 	EOF
@@ -270,7 +270,7 @@ test_expect_success 'submodule contains untracked content' '
 '
 
 test_expect_success 'submodule contains untracked content (untracked ignored)' '
-	git diff-index -p --ignore-submodules=untracked --submodule=log HEAD >actual &&
+	git diff-index -p --submodule=log HEAD >actual &&
 	test_must_be_empty actual
 '
 
@@ -286,7 +286,7 @@ test_expect_success 'submodule contains untracked content (all ignored)' '
 
 test_expect_success 'submodule contains untracked and modified content' '
 	echo new > sm1/foo6 &&
-	git diff-index -p --submodule=log HEAD >actual &&
+	git diff-index -p --submodule=log HEAD --ignore-submodules=none >actual &&
 	cat >expected <<-EOF &&
 	Submodule sm1 contains untracked content
 	Submodule sm1 contains modified content
@@ -296,7 +296,7 @@ test_expect_success 'submodule contains untracked and modified content' '
 
 test_expect_success 'submodule contains untracked and modified content (untracked ignored)' '
 	echo new > sm1/foo6 &&
-	git diff-index -p --ignore-submodules=untracked --submodule=log HEAD >actual &&
+	git diff-index -p --submodule=log HEAD >actual &&
 	cat >expected <<-EOF &&
 	Submodule sm1 contains modified content
 	EOF
@@ -337,7 +337,7 @@ test_expect_success 'submodule is modified' '
 
 test_expect_success 'modified submodule contains untracked content' '
 	echo new > sm1/new-file &&
-	git diff-index -p --submodule=log HEAD >actual &&
+	git diff-index -p  --ignore-submodules=none --submodule=log HEAD >actual &&
 	cat >expected <<-EOF &&
 	Submodule sm1 contains untracked content
 	Submodule sm1 $head6..$head8:
@@ -347,7 +347,7 @@ test_expect_success 'modified submodule contains untracked content' '
 '
 
 test_expect_success 'modified submodule contains untracked content (untracked ignored)' '
-	git diff-index -p --ignore-submodules=untracked --submodule=log HEAD >actual &&
+	git diff-index -p --submodule=log HEAD >actual &&
 	cat >expected <<-EOF &&
 	Submodule sm1 $head6..$head8:
 	  > change
@@ -371,7 +371,7 @@ test_expect_success 'modified submodule contains untracked content (all ignored)
 
 test_expect_success 'modified submodule contains untracked and modified content' '
 	echo modification >> sm1/foo6 &&
-	git diff-index -p --submodule=log HEAD >actual &&
+	git diff-index -p --ignore-submodules=none --submodule=log HEAD >actual &&
 	cat >expected <<-EOF &&
 	Submodule sm1 contains untracked content
 	Submodule sm1 contains modified content
@@ -383,7 +383,7 @@ test_expect_success 'modified submodule contains untracked and modified content'
 
 test_expect_success 'modified submodule contains untracked and modified content (untracked ignored)' '
 	echo modification >> sm1/foo6 &&
-	git diff-index -p --ignore-submodules=untracked --submodule=log HEAD >actual &&
+	git diff-index -p --submodule=log HEAD >actual &&
 	cat >expected <<-EOF &&
 	Submodule sm1 contains modified content
 	Submodule sm1 $head6..$head8:
diff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh
index fc8229c726..dc7b242697 100755
--- a/t/t4060-diff-submodule-option-diff-format.sh
+++ b/t/t4060-diff-submodule-option-diff-format.sh
@@ -409,7 +409,7 @@ test_expect_success 'submodule is up to date' '
 
 test_expect_success 'submodule contains untracked content' '
 	echo new > sm1/new-file &&
-	git diff-index -p --submodule=diff HEAD >actual &&
+	git diff-index -p --ignore-submodules=none --submodule=diff HEAD >actual &&
 	cat >expected <<-EOF &&
 	Submodule sm1 contains untracked content
 	EOF
@@ -417,7 +417,7 @@ test_expect_success 'submodule contains untracked content' '
 '
 
 test_expect_success 'submodule contains untracked content (untracked ignored)' '
-	git diff-index -p --ignore-submodules=untracked --submodule=diff HEAD >actual &&
+	git diff-index -p --submodule=diff HEAD >actual &&
 	test_must_be_empty actual
 '
 
@@ -433,7 +433,7 @@ test_expect_success 'submodule contains untracked content (all ignored)' '
 
 test_expect_success 'submodule contains untracked and modified content' '
 	echo new > sm1/foo6 &&
-	git diff-index -p --submodule=diff HEAD >actual &&
+	git diff-index -p --ignore-submodules=none --submodule=diff HEAD >actual &&
 	cat >expected <<-EOF &&
 	Submodule sm1 contains untracked content
 	Submodule sm1 contains modified content
@@ -451,7 +451,7 @@ test_expect_success 'submodule contains untracked and modified content' '
 # NOT OK
 test_expect_success 'submodule contains untracked and modified content (untracked ignored)' '
 	echo new > sm1/foo6 &&
-	git diff-index -p --ignore-submodules=untracked --submodule=diff HEAD >actual &&
+	git diff-index -p --submodule=diff HEAD >actual &&
 	cat >expected <<-EOF &&
 	Submodule sm1 contains modified content
 	diff --git a/sm1/foo6 b/sm1/foo6
@@ -512,7 +512,7 @@ test_expect_success 'submodule is modified' '
 
 test_expect_success 'modified submodule contains untracked content' '
 	echo new > sm1/new-file &&
-	git diff-index -p --submodule=diff HEAD >actual &&
+	git diff-index -p --ignore-submodules=none --submodule=diff HEAD >actual &&
 	cat >expected <<-EOF &&
 	Submodule sm1 contains untracked content
 	Submodule sm1 $head7..$head8:
@@ -528,7 +528,7 @@ test_expect_success 'modified submodule contains untracked content' '
 '
 
 test_expect_success 'modified submodule contains untracked content (untracked ignored)' '
-	git diff-index -p --ignore-submodules=untracked --submodule=diff HEAD >actual &&
+	git diff-index -p --submodule=diff HEAD >actual &&
 	cat >expected <<-EOF &&
 	Submodule sm1 $head7..$head8:
 	diff --git a/sm1/foo6 b/sm1/foo6
@@ -564,7 +564,7 @@ test_expect_success 'modified submodule contains untracked content (all ignored)
 
 test_expect_success 'modified submodule contains untracked and modified content' '
 	echo modification >> sm1/foo6 &&
-	git diff-index -p --submodule=diff HEAD >actual &&
+	git diff-index -p --ignore-submodules=none --submodule=diff HEAD >actual &&
 	cat >expected <<-EOF &&
 	Submodule sm1 contains untracked content
 	Submodule sm1 contains modified content
@@ -583,7 +583,7 @@ test_expect_success 'modified submodule contains untracked and modified content'
 
 test_expect_success 'modified submodule contains untracked and modified content (untracked ignored)' '
 	echo modification >> sm1/foo6 &&
-	git diff-index -p --ignore-submodules=untracked --submodule=diff HEAD >actual &&
+	git diff-index -p --submodule=diff HEAD >actual &&
 	cat >expected <<-EOF &&
 	Submodule sm1 contains modified content
 	Submodule sm1 $head7..$head8:
diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh
index 537787e598..200626251e 100755
--- a/t/t7064-wtstatus-pv2.sh
+++ b/t/t7064-wtstatus-pv2.sh
@@ -483,7 +483,7 @@ test_expect_success 'create and add submodule, submodule appears clean (A. S...)
 	)
 '
 
-test_expect_success 'untracked changes in added submodule (AM S..U)' '
+test_expect_success 'untracked changes in added submodule (A. S...))' '
 	(	cd super_repo &&
 		## create untracked file in the submodule.
 		(	cd sub1 &&
@@ -500,7 +500,7 @@ test_expect_success 'untracked changes in added submodule (AM S..U)' '
 		# branch.upstream origin/master
 		# branch.ab +0 -0
 		1 A. N... 000000 100644 100644 $ZERO_OID $HMOD .gitmodules
-		1 AM S..U 000000 160000 160000 $ZERO_OID $HSUB sub1
+		1 A. S... 000000 160000 160000 $ZERO_OID $HSUB sub1
 		EOF
 
 		git status --porcelain=v2 --branch --untracked-files=all >actual &&
@@ -560,7 +560,7 @@ test_expect_success 'staged and unstaged changes in added (AM S.M.)' '
 	)
 '
 
-test_expect_success 'staged and untracked changes in added submodule (AM S.MU)' '
+test_expect_success 'staged and untracked changes in added submodule (AM S.M.)' '
 	(	cd super_repo &&
 		(	cd sub1 &&
 			## stage new changes in tracked file.
@@ -579,7 +579,7 @@ test_expect_success 'staged and untracked changes in added submodule (AM S.MU)'
 		# branch.upstream origin/master
 		# branch.ab +0 -0
 		1 A. N... 000000 100644 100644 $ZERO_OID $HMOD .gitmodules
-		1 AM S.MU 000000 160000 160000 $ZERO_OID $HSUB sub1
+		1 AM S.M. 000000 160000 160000 $ZERO_OID $HSUB sub1
 		EOF
 
 		git status --porcelain=v2 --branch --untracked-files=all >actual &&
diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
index 3fcb44767f..b7ff7928fb 100755
--- a/t/t7506-status-submodule.sh
+++ b/t/t7506-status-submodule.sh
@@ -95,7 +95,7 @@ test_expect_success 'status with untracked file in submodule' '
 	(cd sub && git reset --hard) &&
 	echo "content" >sub/new-file &&
 	git status >output &&
-	test_i18ngrep "modified:   sub (untracked content)" output
+	test_i18ngrep "^nothing to commit" output
 '
 
 test_expect_success 'status -uno with untracked file in submodule' '
@@ -105,23 +105,19 @@ test_expect_success 'status -uno with untracked file in submodule' '
 
 test_expect_success 'status with untracked file in submodule (porcelain)' '
 	git status --porcelain >output &&
-	diff output - <<-\EOF
-	 M sub
-	EOF
+	test_must_be_empty output
 '
 
 test_expect_success 'status with untracked file in submodule (short)' '
 	git status --short >output &&
-	diff output - <<-\EOF
-	 ? sub
-	EOF
+	test_must_be_empty output
 '
 
 test_expect_success 'status with added and untracked file in submodule' '
 	(cd sub && git reset --hard && echo >foo && git add foo) &&
 	echo "content" >sub/new-file &&
 	git status >output &&
-	test_i18ngrep "modified:   sub (modified content, untracked content)" output
+	test_i18ngrep "modified:   sub (modified content)" output
 '
 
 test_expect_success 'status with added and untracked file in submodule (porcelain)' '
@@ -169,7 +165,7 @@ test_expect_success 'status with untracked file in modified submodule' '
 	(cd sub && git reset --hard) &&
 	echo "content" >sub/new-file &&
 	git status >output &&
-	test_i18ngrep "modified:   sub (new commits, untracked content)" output
+	test_i18ngrep "modified:   sub (new commits)" output
 '
 
 test_expect_success 'status with untracked file in modified submodule (porcelain)' '
@@ -183,7 +179,7 @@ test_expect_success 'status with added and untracked file in modified submodule'
 	(cd sub && git reset --hard && echo >foo && git add foo) &&
 	echo "content" >sub/new-file &&
 	git status >output &&
-	test_i18ngrep "modified:   sub (new commits, modified content, untracked content)" output
+	test_i18ngrep "modified:   sub (new commits, modified content)" output
 '
 
 test_expect_success 'status with added and untracked file in modified submodule (porcelain)' '
@@ -350,30 +346,17 @@ test_expect_success 'setup superproject with untracked file in nested submodule'
 
 test_expect_success 'status with untracked file in nested submodule (porcelain)' '
 	git -C super status --porcelain >output &&
-	diff output - <<-\EOF
-	 M sub1
-	 M sub2
-	 M sub3
-	EOF
+	test_must_be_empty output
 '
 
 test_expect_success 'status with untracked file in nested submodule (porcelain=2)' '
 	git -C super status --porcelain=2 >output &&
-	sanitize_output output &&
-	diff output - <<-\EOF
-	1 .M S..U 160000 160000 160000 HASH HASH sub1
-	1 .M S..U 160000 160000 160000 HASH HASH sub2
-	1 .M S..U 160000 160000 160000 HASH HASH sub3
-	EOF
+	test_must_be_empty output
 '
 
 test_expect_success 'status with untracked file in nested submodule (short)' '
 	git -C super status --short >output &&
-	diff output - <<-\EOF
-	 ? sub1
-	 ? sub2
-	 ? sub3
-	EOF
+	test_must_be_empty output
 '
 
 test_expect_success 'setup superproject with modified file in nested submodule' '
@@ -386,7 +369,6 @@ test_expect_success 'status with added file in nested submodule (porcelain)' '
 	diff output - <<-\EOF
 	 M sub1
 	 M sub2
-	 M sub3
 	EOF
 '
 
@@ -396,7 +378,6 @@ test_expect_success 'status with added file in nested submodule (porcelain=2)' '
 	diff output - <<-\EOF
 	1 .M S.M. 160000 160000 160000 HASH HASH sub1
 	1 .M S.M. 160000 160000 160000 HASH HASH sub2
-	1 .M S..U 160000 160000 160000 HASH HASH sub3
 	EOF
 '
 
@@ -405,7 +386,6 @@ test_expect_success 'status with added file in nested submodule (short)' '
 	diff output - <<-\EOF
 	 m sub1
 	 m sub2
-	 ? sub3
 	EOF
 '

base-commit: d98273ba77e1ab9ec755576bc86c716a97bf59d7
-- 
gitgitgadget