Skip to contentSkip to conversation

Add optional parameters to the diff option "--ignore-submodules"

PATCH In review Jens Lehmann 5 participants 14 messages Jun 15, 2016 Documentation

lore
Share & tools
lore
#0 lore raw

cover Add optional parameters to the diff option "--ignore-submodules"

After thinking some time about peoples expectations and troubles with the recursive scanning of submodules, I came up with this:

What about expanding the "--ignore-submodules" option of the git diff family with three parameters:

--ignore-submodules=all : Same behavior as "--ignore-submodules",
submodules show never up as modified.

--ignore-submodules=untracked : Don't consider submodules as modified
when they only contain untracked files, but do if the commits in the superproject are different or tracked content is modified.

--ignore-submodules=dirty : Don't consider submodules as modified
when their work tree is dirty, no matter why. This is the pre 1.7.0 behavior and doesn't recurse into submodules at all.

The first patch is just a resend of a test case rename, the second contains the implementation.

To make that more useful the default could be controlled by the .git/config or .gitmodules file. So you could have two submodules:

[submodule "sub1"]
path = sub1
url = /home/me/sub1.git/
ignore = dirty
[submodule "sub2"]
path = sub2
url = /home/me/sub2.git/
ignore = untracked

Where sub1 will not be scanned for work tree modifications by "git diff" and "git status" and for sub2 any untracked files inside the submodule will be ignored. And then "git status" should learn the "--ignore-submodule" option too. With a fourth parameter "none" it would be possible to override the defaults from the configuration anyway you want.

Opinions?

Jens Lehmann (2):
git diff: rename test that had a conflicting name
Add optional parameters to the diff option "--ignore-submodules"

 Documentation/diff-options.txt                     |   10 ++-
 diff-lib.c                                         |    1 +
 diff.c                                             |   11 +++-
 diff.h                                             |    1 +
 t/t4027-diff-submodule.sh                          |   20 ++++-
 ...submodule.sh => t4041-diff-submodule-option.sh} |   81 ++++++++++++++++++++
 6 files changed, 118 insertions(+), 6 deletions(-)
 rename t/{t4041-diff-submodule.sh => t4041-diff-submodule-option.sh} (74%)
#1 lore raw
Quoted 9 lines · Jens Lehmann

After thinking some time about peoples expectations and troubles with the recursive scanning of submodules, I came up with this:

What about expanding the "--ignore-submodules" option of the git diff family with three parameters:

--ignore-submodules=all : Same behavior as "--ignore-submodules",
submodules show never up as modified.

Quoted 7 lines

--ignore-submodules=untracked : Don't consider submodules as modified
when they only contain untracked files, but do if the commits in the superproject are different or tracked content is modified.

--ignore-submodules=dirty : Don't consider submodules as modified
when their work tree is dirty, no matter why. This is the pre 1.7.0 behavior and doesn't recurse into submodules at all.

Pardon my ignorance: Does this make "dirty" a superset of "untracked"? Or are they orthogonal. And how does "all" compare to "dirty"? Are they synonyms, or is "all" a superset of "dirty"?

Quoted 12 lines

To make that more useful the default could be controlled by the .git/config or .gitmodules file. So you could have two submodules:

[submodule "sub1"]
path = sub1
url = /home/me/sub1.git/
ignore = dirty
[submodule "sub2"]
path = sub2
url = /home/me/sub2.git/
ignore = untracked

Quoted 3 lines

[...]

Opinions?

I agree with adding support for this in .gitmodules, to allow customizing on a per-submodule level.

...Johan

Signature · 2 lines
Johan Herland, <[email protected]>
www.herland.net
#2 lore raw

Am 09.06.2010 00:11, schrieb Johan Herland:

Quoted 14 lines · Jens Lehmann
Quoted 13 lines

After thinking some time about peoples expectations and troubles with the recursive scanning of submodules, I came up with this:

What about expanding the "--ignore-submodules" option of the git diff family with three parameters:

--ignore-submodules=all : Same behavior as "--ignore-submodules",
submodules show never up as modified.

--ignore-submodules=untracked : Don't consider submodules as modified
when they only contain untracked files, but do if the commits in the superproject are different or tracked content is modified.

Quoted 7 lines
Quoted 3 lines

--ignore-submodules=dirty : Don't consider submodules as modified
when their work tree is dirty, no matter why. This is the pre 1.7.0 behavior and doesn't recurse into submodules at all.

Pardon my ignorance: Does this make "dirty" a superset of "untracked"? Or are they orthogonal. And how does "all" compare to "dirty"? Are they synonyms, or is "all" a superset of "dirty"?

Sorry I didn't make that clear enough: "all" is a superset of "dirty" and "dirty" is a superset of "untracked".

There are currently (since 1.7.0) three reasons a submodule is considered dirty:

1) It contains untracked content
2) It contains modified tracked content
3) It contains newer commits than those committed in the superproject

"all" would ignore 1), 2) & 3)
"dirty" would ignore 1) & 2)
"untracked" would ignore 1)

Quoted 2 lines

I agree with adding support for this in .gitmodules, to allow customizing on a per-submodule level.

Yup, and different branches could handle that differently that way too.

#3 lore raw
Quoted 29 lines · Jens Lehmann

Am 09.06.2010 00:11, schrieb Johan Herland:

Quoted 16 lines · Jens Lehmann
Quoted 15 lines

After thinking some time about peoples expectations and troubles with the recursive scanning of submodules, I came up with this:

What about expanding the "--ignore-submodules" option of the git diff family with three parameters:

--ignore-submodules=all : Same behavior as "--ignore-submodules",

submodules show never up as modified.

--ignore-submodules=untracked : Don't consider submodules as modified

when they only contain untracked files, but do if the commits in the superproject are different or tracked content is modified.

Quoted 8 lines
Quoted 4 lines

--ignore-submodules=dirty : Don't consider submodules as modified

when their work tree is dirty, no matter why. This is the pre 1.7.0 behavior and doesn't recurse into submodules at all.

Pardon my ignorance: Does this make "dirty" a superset of "untracked"? Or are they orthogonal. And how does "all" compare to "dirty"? Are they synonyms, or is "all" a superset of "dirty"?

Sorry I didn't make that clear enough: "all" is a superset of "dirty" and "dirty" is a superset of "untracked".

Quoted 6 lines

There are currently (since 1.7.0) three reasons a submodule is considered dirty:

1) It contains untracked content
2) It contains modified tracked content
3) It contains newer commits than those committed in the superproject

I guess 3) really means that the submodule's HEAD points to a _different_ (not necessarily _newer_) commit than what's referenced in the superproject.

Quoted 3 lines

"all" would ignore 1), 2) & 3)
"dirty" would ignore 1) & 2)
"untracked" would ignore 1)

...and just to complete my understanding of this, 3) requires only checking the submodule's current HEAD, while 1) and 2) require traversing its work tree (i.e. the equivalent of a 'git status'), hence the potential expensiveness.

Also, I guess 2) includes both staged and unstaged modifications to tracked content?

Thanks for your help. All the ideas in your cover letter seem good to me.

...Johan

Signature · 2 lines
Johan Herland, <[email protected]>
www.herland.net
#4 lore raw

Am 09.06.2010 01:49, schrieb Johan Herland:

Quoted 10 lines · Jens Lehmann
Quoted 6 lines

There are currently (since 1.7.0) three reasons a submodule is considered dirty:

1) It contains untracked content
2) It contains modified tracked content
3) It contains newer commits than those committed in the superproject

I guess 3) really means that the submodule's HEAD points to a _different_ (not necessarily _newer_) commit than what's referenced in the superproject.

Sure, please replace my inaccurate description with yours ;-)

Quoted 8 lines
Quoted 3 lines

"all" would ignore 1), 2) & 3) "dirty" would ignore 1) & 2) "untracked" would ignore 1)

...and just to complete my understanding of this, 3) requires only checking the submodule's current HEAD, while 1) and 2) require traversing its work tree (i.e. the equivalent of a 'git status'), hence the potential expensiveness.

Thats correct.

Quoted 2 lines

Also, I guess 2) includes both staged and unstaged modifications to tracked content?

Yes (as it doesn't make a difference to the superproject if modifications inside the submodule are staged or not there is no distinction made between those two).

Quoted 1 line

Thanks for your help. All the ideas in your cover letter seem good to me.

Thank you for your comments, I'll start hacking to show some more patches.

#5 lore raw

PATCH 1/2 git diff: rename test that had a conflicting name

In reply to Jens Lehmann

In 86140d5 the new test t4041-diff-submodule.sh was introduced although t4027-diff-submodule.sh already existed. Rename the newer test to t4041-diff-submodule-option.sh to fix that.

---
 ...submodule.sh => t4041-diff-submodule-option.sh} |    0
 1 files changed, 0 insertions(+), 0 deletions(-)
 rename t/{t4041-diff-submodule.sh => t4041-diff-submodule-option.sh} (100%)
t/t4041-diff-submodule-option.sh renamed from t/t4041-diff-submodule.sh +0 -0
similarity index 100%
rename from t/t4041-diff-submodule.sh
rename to t/t4041-diff-submodule-option.sh
Signature · 1 line
1.7.1.447.g4d763
#6 lore raw

PATCH 2/2 Add optional parameters to the diff option "--ignore-submodules"

In reply to Jens Lehmann

In some use cases it is not desirable that the diff family 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. 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.

---
 Documentation/diff-options.txt   |   10 ++++-
 diff-lib.c                       |    1 +
 diff.c                           |   11 +++++-
 diff.h                           |    1 +
 t/t4027-diff-submodule.sh        |   20 ++++++++-
 t/t4041-diff-submodule-option.sh |   81 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 118 insertions(+), 6 deletions(-)
Documentation/diff-options.txt +8 -2
index e745a3c..2371262 100644
@@ -328,8 +328,14 @@ endif::git-format-patch[]
 --no-ext-diff::
 	Disallow external diff drivers.
 
---ignore-submodules::
-	Ignore changes to submodules in the diff generation.
+--ignore-submodules[=<when>]::
+	Ignore changes to submodules in the diff generation. <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 until 1.7.0). Using "all" hides all changes to submodules.
 
 --src-prefix=<prefix>::
 	Show the given source prefix instead of "a/".
diff-lib.c +1 -0
index c9f6e05..8b8978a 100644
@@ -70,6 +70,7 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
 	int changed = ce_match_stat(ce, st, ce_option);
 	if (S_ISGITLINK(ce->ce_mode)
 	    && !DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES)
+	    && !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));
 	}
diff.c +10 -1
index 2327cea..71fff7e 100644
@@ -3174,7 +3174,16 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		DIFF_OPT_CLR(options, ALLOW_TEXTCONV);
 	else if (!strcmp(arg, "--ignore-submodules"))
 		DIFF_OPT_SET(options, IGNORE_SUBMODULES);
-	else if (!strcmp(arg, "--submodule"))
+	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"))
 		DIFF_OPT_SET(options, SUBMODULE_LOG);
 	else if (!prefixcmp(arg, "--submodule=")) {
 		if (!strcmp(arg + 12, "log"))
diff.h +1 -0
index 48abe7a..5248cd8 100644
@@ -74,6 +74,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
 #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_IGNORE_DIRTY_SUBMODULES (1 << 26)
 
 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
t/t4027-diff-submodule.sh +17 -3
index 83c1914..559b41e 100755
@@ -103,9 +103,17 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)'
 	git diff HEAD >actual &&
 	sed -e "1,/^@@/d" actual >actual.body &&
 	expect_from_to >expect.body $subprev $subprev-dirty &&
-	test_cmp expect.body actual.body
+	test_cmp expect.body actual.body &&
+	git diff --ignore-submodules HEAD >actual2 &&
+	echo -n "" | test_cmp - actual2 &&
+	git diff --ignore-submodules=untracked HEAD >actual3 &&
+	sed -e "1,/^@@/d" actual3 >actual3.body &&
+	expect_from_to >expect.body $subprev $subprev-dirty &&
+	test_cmp expect.body actual3.body &&
+	git diff --ignore-submodules=dirty HEAD >actual4 &&
+	echo -n "" | test_cmp - actual4
 '
-
+test_done
 test_expect_success 'git diff HEAD with dirty submodule (index, refs match)' '
 	(
 		cd sub &&
@@ -129,7 +137,13 @@ test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match)'
 	git diff HEAD >actual &&
 	sed -e "1,/^@@/d" actual >actual.body &&
 	expect_from_to >expect.body $subprev $subprev-dirty &&
-	test_cmp expect.body actual.body
+	test_cmp expect.body actual.body &&
+	git diff --ignore-submodules=all HEAD >actual2 &&
+	echo -n "" | test_cmp - actual2 &&
+	git diff --ignore-submodules=untracked HEAD >actual3 &&
+	echo -n "" | test_cmp - actual3 &&
+	git diff --ignore-submodules=dirty HEAD >actual4 &&
+	echo -n "" | test_cmp - actual4
 '
 
 test_expect_success 'git diff (empty submodule dir)' '
t/t4041-diff-submodule-option.sh +81 -0

105 lines not on this page · raw message

Signature · 1 line
1.7.1.447.g4d763
#7 lore raw

On 06/08/2010 11:31 AM, Jens Lehmann wrote:

Two things...

t/t4027-diff-submodule.sh test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)' · line 108

Quoted 12 lines
t/t4027-diff-submodule.sh +3 -1
index 83c1914..559b41e 100755
@@ -103,9 +103,17 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)'
 	git diff HEAD >actual &&
 	sed -e "1,/^@@/d" actual >actual.body &&
 	expect_from_to >expect.body $subprev $subprev-dirty &&
-	test_cmp expect.body actual.body
+	test_cmp expect.body actual.body &&
+	git diff --ignore-submodules HEAD >actual2 &&
+	echo -n "" | test_cmp - actual2 &&

'echo -n' is not portable (here, and below). Please use printf instead. Ditto for t4041

t/t4027-diff-submodule.sh test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)' · line 116

Quoted 9 lines
t/t4027-diff-submodule.sh +7 -1
+	git diff --ignore-submodules=untracked HEAD >actual3 &&
+	sed -e "1,/^@@/d" actual3 >actual3.body &&
+	expect_from_to >expect.body $subprev $subprev-dirty &&
+	test_cmp expect.body actual3.body &&
+	git diff --ignore-submodules=dirty HEAD >actual4 &&
+	echo -n "" | test_cmp - actual4
 '
-
+test_done

^^^^^^^^^

Why is this test_done here? There are additional tests after this point, and an additional call to test_done.

Quoted 22 lines
t/t4027-diff-submodule.sh +7 -1
 test_expect_success 'git diff HEAD with dirty submodule (index, refs match)' '
 	(
 		cd sub &&
@@ -129,7 +137,13 @@ test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match)'
 	git diff HEAD >actual &&
 	sed -e "1,/^@@/d" actual >actual.body &&
 	expect_from_to >expect.body $subprev $subprev-dirty &&
-	test_cmp expect.body actual.body
+	test_cmp expect.body actual.body &&
+	git diff --ignore-submodules=all HEAD >actual2 &&
+	echo -n "" | test_cmp - actual2 &&
+	git diff --ignore-submodules=untracked HEAD >actual3 &&
+	echo -n "" | test_cmp - actual3 &&
+	git diff --ignore-submodules=dirty HEAD >actual4 &&
+	echo -n "" | test_cmp - actual4
 '
 
 test_expect_success 'git diff (empty submodule dir)' '
t/t4041-diff-submodule-option.sh +0 -0
index 019acb9..f44b906 100755
#8 lore raw

Am 24.06.2010 16:23, schrieb Brandon Casey:

Quoted 19 lines · Jens Lehmann

Two things...

t/t4027-diff-submodule.sh test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)' · line 108

Quoted 12 lines
t/t4027-diff-submodule.sh +3 -1
index 83c1914..559b41e 100755
@@ -103,9 +103,17 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)'
 	git diff HEAD >actual &&
 	sed -e "1,/^@@/d" actual >actual.body &&
 	expect_from_to >expect.body $subprev $subprev-dirty &&
-	test_cmp expect.body actual.body
+	test_cmp expect.body actual.body &&
+	git diff --ignore-submodules HEAD >actual2 &&
+	echo -n "" | test_cmp - actual2 &&

'echo -n' is not portable (here, and below). Please use printf instead. Ditto for t4041

Thanks, I haven't been aware of that.

Quoted 11 lines

t/t4027-diff-submodule.sh test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)' · line 116

Quoted 9 lines
t/t4027-diff-submodule.sh +7 -1
+	git diff --ignore-submodules=untracked HEAD >actual3 &&
+	sed -e "1,/^@@/d" actual3 >actual3.body &&
+	expect_from_to >expect.body $subprev $subprev-dirty &&
+	test_cmp expect.body actual3.body &&
+	git diff --ignore-submodules=dirty HEAD >actual4 &&
+	echo -n "" | test_cmp - actual4
 '
-
+test_done

^^^^^^^^^

Quoted 2 lines

Why is this test_done here? There are additional tests after this point, and an additional call to test_done.

Grmph, this is a leftover from debugging :-(
Thank you for spotting this one too!

As the patch introducing this is already in next, here is another one:

--------------------8<--------------------------
From: Jens Lehmann <[email protected]>
Date: Thu, 24 Jun 2010 19:01:27 +0200
Subject: [PATCH] t4027 & t4041: Fix use of "echo -n" and a premature "test_done"

Replace the non portable "echo -n" with "printf" and remove a premature "test_done".

---
 t/t4027-diff-submodule.sh        |   12 ++++++------
 t/t4041-diff-submodule-option.sh |   14 +++++++-------
 2 files changed, 13 insertions(+), 13 deletions(-)
t/t4027-diff-submodule.sh +6 -6
index 559b41e..3f2e591 100755
@@ -105,15 +105,15 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)'
 	expect_from_to >expect.body $subprev $subprev-dirty &&
 	test_cmp expect.body actual.body &&
 	git diff --ignore-submodules HEAD >actual2 &&
-	echo -n "" | test_cmp - actual2 &&
+	printf "" | test_cmp - actual2 &&
 	git diff --ignore-submodules=untracked HEAD >actual3 &&
 	sed -e "1,/^@@/d" actual3 >actual3.body &&
 	expect_from_to >expect.body $subprev $subprev-dirty &&
 	test_cmp expect.body actual3.body &&
 	git diff --ignore-submodules=dirty HEAD >actual4 &&
-	echo -n "" | test_cmp - actual4
+	printf "" | test_cmp - actual4
 '
-test_done
+
 test_expect_success 'git diff HEAD with dirty submodule (index, refs match)' '
 	(
 		cd sub &&
@@ -139,11 +139,11 @@ test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match)'
 	expect_from_to >expect.body $subprev $subprev-dirty &&
 	test_cmp expect.body actual.body &&
 	git diff --ignore-submodules=all HEAD >actual2 &&
-	echo -n "" | test_cmp - actual2 &&
+	printf "" | test_cmp - actual2 &&
 	git diff --ignore-submodules=untracked HEAD >actual3 &&
-	echo -n "" | test_cmp - actual3 &&
+	printf "" | test_cmp - actual3 &&
 	git diff --ignore-submodules=dirty HEAD >actual4 &&
-	echo -n "" | test_cmp - actual4
+	printf "" | test_cmp - actual4
 '
 
 test_expect_success 'git diff (empty submodule dir)' '
t/t4041-diff-submodule-option.sh +7 -7
index f44b906..7600ed1 100755
@@ -207,17 +207,17 @@ EOF
 
 test_expect_success 'submodule contains untracked content (untracked ignored)' "
 	git diff-index -p --ignore-submodules=untracked --submodule=log HEAD >actual &&
-	echo -n '' | diff actual -
+	printf '' | diff actual -
 "
 
 test_expect_success 'submodule contains untracked content (dirty ignored)' "
 	git diff-index -p --ignore-submodules=dirty --submodule=log HEAD >actual &&
-	echo -n '' | diff actual -
+	printf '' | diff actual -
 "
 
 test_expect_success 'submodule contains untracked content (all ignored)' "
 	git diff-index -p --ignore-submodules=all --submodule=log HEAD >actual &&
-	echo -n '' | diff actual -
+	printf '' | diff actual -
 "
 
 test_expect_success 'submodule contains untracked and modifed content' "
@@ -240,13 +240,13 @@ EOF
 test_expect_success 'submodule contains untracked and modifed content (dirty ignored)' "
 	echo new > sm1/foo6 &&
 	git diff-index -p --ignore-submodules=dirty --submodule=log HEAD >actual &&
-	echo -n '' | diff actual -
+	printf '' | diff actual -
 "
 
 test_expect_success 'submodule contains untracked and modifed content (all ignored)' "
 	echo new > sm1/foo6 &&
 	git diff-index -p --ignore-submodules --submodule=log HEAD >actual &&
-	echo -n '' | diff actual -
+	printf '' | diff actual -
 "
 
 test_expect_success 'submodule contains modifed content' "
@@ -295,7 +295,7 @@ EOF
 
 test_expect_success 'modified submodule contains untracked content (all ignored)' "
 	git diff-index -p --ignore-submodules=all --submodule=log HEAD >actual &&
-	echo -n '' | diff actual -
+	printf '' | diff actual -
 "
 
 test_expect_success 'modified submodule contains untracked and modifed content' "
@@ -331,7 +331,7 @@ EOF
 test_expect_success 'modified submodule contains untracked and modifed content (all ignored)' "
 	echo modification >> sm1/foo6 &&
 	git diff-index -p --ignore-submodules --submodule=log HEAD >actual &&
-	echo -n '' | diff actual -
+	printf '' | diff actual -
 "
 
 test_expect_success 'modified submodule contains modifed content' "
Signature · 1 line
1.7.1.716.g6c3d8
#9 lore raw
In reply to Jens Lehmann
Quoted 33 lines · Jens Lehmann

After thinking some time about peoples expectations and troubles with the recursive scanning of submodules, I came up with this:

What about expanding the "--ignore-submodules" option of the git diff family with three parameters:

--ignore-submodules=all : Same behavior as "--ignore-submodules",
submodules show never up as modified.

--ignore-submodules=untracked : Don't consider submodules as modified
when they only contain untracked files, but do if the commits in the superproject are different or tracked content is modified.

--ignore-submodules=dirty : Don't consider submodules as modified
when their work tree is dirty, no matter why. This is the pre 1.7.0 behavior and doesn't recurse into submodules at all.

The first patch is just a resend of a test case rename, the second contains the implementation.

To make that more useful the default could be controlled by the .git/config or .gitmodules file. So you could have two submodules:

[submodule "sub1"]
path = sub1
url = /home/me/sub1.git/
ignore = dirty
[submodule "sub2"]
path = sub2
url = /home/me/sub2.git/
ignore = untracked

Quoted 8 lines

Where sub1 will not be scanned for work tree modifications by "git diff" and "git status" and for sub2 any untracked files inside the submodule will be ignored. And then "git status" should learn the "--ignore-submodule" option too. With a fourth parameter "none" it would be possible to override the defaults from the configuration anyway you want.

Opinions?

The above outline (I haven't looked at the code) sounds sane. We might even want to make untracked (or dirty) the default, if nothing is given from the command line nor configuration.

I don't see a reason for patch 1/2, though.

Thanks.

#10 lore raw

Am 08.06.2010 19:28, schrieb Junio C Hamano:

Quoted 3 lines

The above outline (I haven't looked at the code) sounds sane. We might even want to make untracked (or dirty) the default, if nothing is given from the command line nor configuration.

Hm, as far as I interpreted the postings here, "none" should be a reasonable default for most users because it protects them from errors that are easy to make and hard to detect.

IIRC there are two main exceptions which want a different setting:

- Huge submodules which take a substantial portion of time to scan may want "dirty" to avoid spending the extra time (at least until we have an inotfiy daemon ;-).

- Submodules where the provider doesn't care or is not able to provide a proper .gitignore for other reasons want "untracked" to avoid the noise.

(Anything I missed?)

And these seem to fall into the category: If you are unlucky enough to have one of these included in your superproject, configure these to use "dirty" or "untracked" as appropriate. And AFAICT they are more the exception than the rule so I came up with this proposal of a per submodule configuration, so people won't have to turn off submodule checking for all submodules because of a single special one.

Quoted 1 line

I don't see a reason for patch 1/2, though.

Ok, I will drop that from the next round.

#11 lore raw

Hi,

Quoted 9 lines · Jens Lehmann

Am 08.06.2010 19:28, schrieb Junio C Hamano:

Quoted 3 lines

The above outline (I haven't looked at the code) sounds sane. We might even want to make untracked (or dirty) the default, if nothing is given from the command line nor configuration.

Hm, as far as I interpreted the postings here, "none" should be a reasonable default for most users because it protects them from errors that are easy to make and hard to detect.

Quoted 5 lines

IIRC there are two main exceptions which want a different setting:

- Huge submodules which take a substantial portion of time to scan may want "dirty" to avoid spending the extra time (at least until we have an inotfiy daemon ;-).

Not only huge submodules. Thrashing the disk cache (e.g. by the sheer number of submodules) is another reason. In my main project, the time for a simple "git status" went up from unnoticable (<0.1 seconds AFAICT) to 45 seconds.

Let me repeat that. Fourty-five seconds. For a simple "git status".

According to me, the fact that nobody noticed can only be explained by the lack of real submodule users. In particular, the people who invented the submodules, and pushed them into "official" Git, never used it, but precluded more appropriate solutions. I am not complaining, I am just stating observations.

It comes back to my argument that developers should not have beefy machines, lest they lose touch with the users and uses, and develop things that closely skirt reality just so.

Quoted 3 lines

- Submodules where the provider doesn't care or is not able to provide a proper .gitignore for other reasons want "untracked" to avoid the noise.

While this is a valid scenario, I do not think that any user was hurt by that case.

Quoted 6 lines

And these seem to fall into the category: If you are unlucky enough to have one of these included in your superproject, configure these to use "dirty" or "untracked" as appropriate. And AFAICT they are more the exception than the rule so I came up with this proposal of a per submodule configuration, so people won't have to turn off submodule checking for all submodules because of a single special one.

I agree that the basic reason for the default to check for dirty and untracked files is sound.

It is not your (Jens') fault that this does not integrate well into the Git context, and that users of the submodule feature were punished that heavily (just to reiterate, if anybody missed that number, the time on "git status" -- or for that matter, "git diff" -- went up by >4500%. In English words, that is four-thousand five hundred percent, and that is just a lower bound).

So I would actually argue (being a real submodule user, not just an imaginary one) that the default for dirty checking in the submodules should stay.

Quoted 3 lines
Quoted 1 line

I don't see a reason for patch 1/2, though.

Ok, I will drop that from the next round.

If I had not stopped caring about consistency in Git, I would strongly contest this dropping.

Ciao,
Dscho

#12 lore raw
Quoted 10 lines · Johannes Schindelin

I agree that the basic reason for the default to check for dirty and untracked files is sound.

It is not your (Jens') fault that this does not integrate well into the Git context, and that users of the submodule feature were punished that heavily (just to reiterate, if anybody missed that number, the time on "git status" -- or for that matter, "git diff" -- went up by >4500%. In English words, that is four-thousand five hundred percent, and that is just a lower bound).

Quoted 3 lines

So I would actually argue (being a real submodule user, not just an imaginary one) that the default for dirty checking in the submodules should stay.

Let me make sure I understand. You repeated three times that you suffered big time spending too many cycles due to extra checks (compared with older behaviour of not checking submodule working tree at all), but you would recommend the default to be the expensive one?

I am not trying to point out that these two statements are inconsistent. I am guessing that you are suggesting to "default to safe-but-expensive, let people who know what they are doing to tweak for performance", but you are as always a bit too terse for me to be sure, hence this question.

#13 lore raw

Hi,

Quoted 12 lines · Junio C Hamano
Quoted 10 lines · Johannes Schindelin

I agree that the basic reason for the default to check for dirty and untracked files is sound.

It is not your (Jens') fault that this does not integrate well into the Git context, and that users of the submodule feature were punished that heavily (just to reiterate, if anybody missed that number, the time on "git status" -- or for that matter, "git diff" -- went up by >4500%. In English words, that is four-thousand five hundred percent, and that is just a lower bound).

Quoted 8 lines
Quoted 3 lines

So I would actually argue (being a real submodule user, not just an imaginary one) that the default for dirty checking in the submodules should stay.

Let me make sure I understand. You repeated three times that you suffered big time spending too many cycles due to extra checks (compared with older behaviour of not checking submodule working tree at all), but you would recommend the default to be the expensive one?

I also pointed out that the suffering came from power usage of submodules.

If you only have a handful of submodules, and if they are relatively small, the current default makes much more sense than --ignore-submodules=dirty

Hth,
Johannes