Skip to contentSkip to conversation

Submodules: Add the new config option "ignore"

PATCH Superseded Jens Lehmann 3 participants 15 messages Jun 15, 2016 Documentation

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Jens Lehmann

author
#0 lore raw

cover Submodules: Add the new config option "ignore"

These two patches are extremly useful for those users who chose submodules because they wanted to reduce the time it takes for "git status" and "git diff" to recurse through the whole work tree by putting sometimes large amounts of files into submodules, which weren't scanned in the past.

Since 1.7.0 "git status" and "git diff" recurse through submodule work trees, which has a - sometimes drastic - performance impact when large submodules are present. Using the "ignore=dirty" config option restores the former behaviour.

This option can be set in the .gitmodules file or in .git/config, any settings found in the latter override those from .gitmodules. This enables the distribution of a default setting for this option by simply committing a modified .gitmodules file without each developer having to call "git submodule sync". When using "git status" or "git diff" with "--ignore-submodules=none" option the default behavior of scanning all submodules work trees can be restored temporarily.

Comments?

Jens Lehmann (2):
Submodules: Add the new "ignore" config option for diff and status
Submodules: Use "ignore" settings from .gitmodules too for diff and
status

 Documentation/config.txt       |   13 ++++
 Documentation/diff-options.txt |    6 ++-
 Documentation/git-status.txt   |    6 ++-
 Documentation/gitmodules.txt   |   15 ++++
 builtin/commit.c               |    2 +
 builtin/diff-files.c           |    2 +
 builtin/diff-index.c           |    2 +
 builtin/diff-tree.c            |    2 +
 builtin/diff.c                 |    2 +
 diff-lib.c                     |   15 +++--
 diff.c                         |   35 ++++++++--
 diff.h                         |    1 +
 submodule.c                    |   63 ++++++++++++++++-
 submodule.h                    |    4 +
 t/t4027-diff-submodule.sh      |  139 ++++++++++++++++++++++++++++++++++++
 t/t7508-status.sh              |  154 ++++++++++++++++++++++++++++++++++++++-
 wt-status.c                    |    8 ++-
 17 files changed, 449 insertions(+), 20 deletions(-)
Signature · 1 line
1.7.2.223.g830604.dirty
author
#1 lore raw

PATCH 1/2 Submodules: Add the new "ignore" config option for diff and status

The new "ignore" config option controls the default behavior for "git status" and the diff family. It specifies under what circumstances they consider submodules as modified and can be set separately for each submodule.

The command line option "--ignore-submodules=" has been extended to accept the new parameter "none" for both status and diff.

Users that chose submodules to get rid of long work tree scanning times might want to set the "dirty" option for those submodules. This brings back the pre 1.7.0 behavior, where submodule work trees were never scanned for modifications. By using "--ignore-submodules=none" on the command line the status and diff commands can be told to do a full scan.

This option can be set to the following values (which have the same name and meaning as for the "--ignore-submodules" option of status and diff):

"all": All changes to the submodule will be ignored.

"dirty": Only differences of the commit recorded in the superproject and
the submodules HEAD will be considered modifications, all changes to the work tree of the submodule will be ignored. When using this value, the submodule will not be scanned for work tree changes at all, leading to a performance benefit on large submodules.

"untracked": Only untracked files in the submodules work tree are ignored,
a changed HEAD and/or modified files in the submodule will mark it as modified.

"none" (which is the default): Either untracked or modified files in a
submodules work tree or a difference between the subdmodules HEAD and the commit recorded in the superproject will make it show up as changed. This value is added as a new parameter for the "--ignore-submodules" option of the diff family and "git status" so the user can override the settings in the configuration.

Signed-off-by Jens Lehmann <[email protected]>
---
 Documentation/config.txt       |   10 ++++
 Documentation/diff-options.txt |    6 ++-
 Documentation/git-status.txt   |    6 ++-
 diff-lib.c                     |   15 ++++--
 diff.c                         |   35 ++++++++++++---
 diff.h                         |    1 +
 submodule.c                    |   51 +++++++++++++++++++++-
 submodule.h                    |    3 +
 t/t4027-diff-submodule.sh      |   73 +++++++++++++++++++++++++++++++
 t/t7508-status.sh              |   93 ++++++++++++++++++++++++++++++++++++++--
 wt-status.c                    |    8 +++-
 11 files changed, 281 insertions(+), 20 deletions(-)
Documentation/config.txt +10 -0
index f81fb91..783f264 100644
@@ -1747,6 +1747,16 @@ submodule.<name>.update::
 	URL and other values found in the `.gitmodules` file.  See
 	linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
 
+submodule.<submodulename>.ignore::
+	Defines under what circumstances "git status" and the diff family show
+	a submodule as modified. When set to "all", it will never be considered
+	modified, "dirty" will ignore all changes to the submodules work tree and
+	takes only differences between the HEAD of the submodule and the commit
+	recorded in the superproject into account. "untracked" will additionally
+	let submodules with modified tracked files in their work tree show up.
+	Using "none" (the default when this option is not set) also shows
+	submodules that have untracked files in their work tree as changed.
+
 tar.umask::
 	This variable can be used to restrict the permission bits of
 	tar archive entries.  The default is 0002, which turns off the
Documentation/diff-options.txt +5 -1
index 2371262..9cf7506 100644
@@ -330,7 +330,11 @@ endif::git-format-patch[]
 
 --ignore-submodules[=<when>]::
 	Ignore changes to submodules in the diff generation. <when> can be
-	either "untracked", "dirty" or "all", which is the default. When
+	either "none", "untracked", "dirty" or "all", which is the default
+	Using "none" will consider the submodule modified when it either contains
+	untracked or modified files or its HEAD differs from the commit recorded
+	in the superproject and can be used to override any settings of the
+	'ignore' option in linkgit:git-config[1]. 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,
Documentation/git-status.txt +5 -1
index 2fd054c..a7a5d79 100644
@@ -55,7 +55,11 @@ 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
+	either "none", "untracked", "dirty" or "all", which is the default.
+	Using "none" will consider the submodule modified when it either contains
+	untracked or modified files or its HEAD differs from the commit recorded
+	in the superproject and can be used to override any settings of the
+	'ignore' option in linkgit:git-config[1]. 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,
diff-lib.c +10 -5
index 8b8978a..392ce2b 100644
@@ -68,11 +68,16 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
 				      unsigned ce_option, unsigned *dirty_submodule)
 {
 	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));
+	if (S_ISGITLINK(ce->ce_mode)) {
+		unsigned orig_flags = diffopt->flags;
+		if (!DIFF_OPT_TST(diffopt, OVERRIDE_SUBMODULE_CONFIG))
+			set_diffopt_flags_from_submodule_config(diffopt, ce->name);
+		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));
+		diffopt->flags = orig_flags;
 	}
 	return changed;
 }
diff.c +29 -6
index 17873f3..8206047 100644
@@ -141,6 +141,9 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
+	if (!prefixcmp(var, "submodule."))
+		return parse_submodule_config_option(var, value);
+
 	return git_color_default_config(var, value, cb);
 }
 

@@ -3166,11 +3169,13 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		DIFF_OPT_SET(options, ALLOW_TEXTCONV);
 	else if (!strcmp(arg, "--no-textconv"))
 		DIFF_OPT_CLR(options, ALLOW_TEXTCONV);
-	else if (!strcmp(arg, "--ignore-submodules"))
+	else if (!strcmp(arg, "--ignore-submodules")) {
+		DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
 		handle_ignore_submodules_arg(options, "all");
-	else if (!prefixcmp(arg, "--ignore-submodules="))
+	} else if (!prefixcmp(arg, "--ignore-submodules=")) {
+		DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
 		handle_ignore_submodules_arg(options, arg + 20);
-	else if (!strcmp(arg, "--submodule"))
+	} else if (!strcmp(arg, "--submodule"))
 		DIFF_OPT_SET(options, SUBMODULE_LOG);
 	else if (!prefixcmp(arg, "--submodule=")) {
 		if (!strcmp(arg + 12, "log"))

@@ -4103,6 +4108,24 @@ int diff_result_code(struct diff_options *opt, int status)
 	return result;
 }
 
+/*
+ * Shall changes to this submodule be ignored?
+ *
+ * Submodule changes can be configured to be ignored separately for each path,
+ * but that configuration can be overridden from the command line.
+ */
+static int is_submodule_ignored(const char *path, struct diff_options *options)
+{
+	int ignored = 0;
+	unsigned orig_flags = options->flags;
+	if (!DIFF_OPT_TST(options, OVERRIDE_SUBMODULE_CONFIG))
+		set_diffopt_flags_from_submodule_config(options, path);
+	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES))
+		ignored = 1;
+	options->flags = orig_flags;
+	return ignored;
+}
+
 void diff_addremove(struct diff_options *options,
 		    int addremove, unsigned mode,
 		    const unsigned char *sha1,

@@ -4110,7 +4133,7 @@ void diff_addremove(struct diff_options *options,
 {
 	struct diff_filespec *one, *two;
 
-	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(mode))
+	if (S_ISGITLINK(mode) && is_submodule_ignored(concatpath, options))
 		return;
 
 	/* This may look odd, but it is a preparation for

@@ -4157,8 +4180,8 @@ void diff_change(struct diff_options *options,
 {
 	struct diff_filespec *one, *two;
 
-	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(old_mode)
-			&& S_ISGITLINK(new_mode))
+	if (S_ISGITLINK(old_mode) && S_ISGITLINK(new_mode) &&
+	    is_submodule_ignored(concatpath, options))
 		return;
 
 	if (DIFF_OPT_TST(options, REVERSE_DIFF)) {
diff.h +1 -0
index 063d10a..53ad345 100644
@@ -77,6 +77,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
 #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_OVERRIDE_SUBMODULE_CONFIG (1 << 27)
 
 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
submodule.c +50 -1

69 lines not on this page · raw message

submodule.h +3 -0
index 6fd3bb4..185a5ce 100644
@@ -3,6 +3,9 @@
 
 struct diff_options;
 
+void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
+		const char *path);
+int parse_submodule_config_option(const char *var, const char *value);
 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],
t/t4027-diff-submodule.sh +73 -0

85 lines not on this page · raw message

t/t7508-status.sh +89 -4

200 lines not on this page · raw message

wt-status.c +6 -2
index 2f9e33c..54b6b03 100644
@@ -313,8 +313,10 @@ 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)
+	if (s->ignore_submodule_arg) {
+		DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
 		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;

@@ -331,8 +333,10 @@ 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)
+	if (s->ignore_submodule_arg) {
+		DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
 		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;
Signature · 1 line
1.7.2.223.g830604.dirty
author
#2 lore raw

PATCH 2/2 Submodules: Use "ignore" settings from .gitmodules too for diff and status

In reply to Jens Lehmann

The .gitmodules file is parsed for "submodule.<name>.ignore" entries before looking for them in .git/config. Thus settings found in .git/config will override those from .gitmodules, thereby allowing the local developer to ignore settings given by the remote side while also letting upstream set defaults for those users who don't have special needs.

Signed-off-by Jens Lehmann <[email protected]>
---
 Documentation/config.txt       |    3 ++
 Documentation/diff-options.txt |    2 +-
 Documentation/git-status.txt   |    2 +-
 Documentation/gitmodules.txt   |   15 +++++++++
 builtin/commit.c               |    2 +
 builtin/diff-files.c           |    2 +
 builtin/diff-index.c           |    2 +
 builtin/diff-tree.c            |    2 +
 builtin/diff.c                 |    2 +
 submodule.c                    |   12 +++++++
 submodule.h                    |    1 +
 t/t4027-diff-submodule.sh      |   66 ++++++++++++++++++++++++++++++++++++++++
 t/t7508-status.sh              |   63 +++++++++++++++++++++++++++++++++++++-
 13 files changed, 171 insertions(+), 3 deletions(-)
Documentation/config.txt +3 -0
index 783f264..06e70cb 100644
@@ -1756,6 +1756,9 @@ submodule.<submodulename>.ignore::
 	let submodules with modified tracked files in their work tree show up.
 	Using "none" (the default when this option is not set) also shows
 	submodules that have untracked files in their work tree as changed.
+	This setting overrides any setting made in .gitmodules for this submodule,
+	both settings can be overriden on the command line by using the
+	"--ignore-submodule" option.
 
 tar.umask::
 	This variable can be used to restrict the permission bits of
Documentation/diff-options.txt +1 -1
index 9cf7506..faa467b 100644
@@ -334,7 +334,7 @@ endif::git-format-patch[]
 	Using "none" will consider the submodule modified when it either contains
 	untracked or modified files or its HEAD differs from the commit recorded
 	in the superproject and can be used to override any settings of the
-	'ignore' option in linkgit:git-config[1]. When
+	'ignore' option in linkgit:git-config[1] or linkgit:gitmodules[5]. 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,
Documentation/git-status.txt +1 -1
index a7a5d79..dae190a 100644
@@ -59,7 +59,7 @@ specified.
 	Using "none" will consider the submodule modified when it either contains
 	untracked or modified files or its HEAD differs from the commit recorded
 	in the superproject and can be used to override any settings of the
-	'ignore' option in linkgit:git-config[1]. When
+	'ignore' option in linkgit:git-config[1] or linkgit:gitmodules[5]. 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,
Documentation/gitmodules.txt +15 -0
index 72a13d1..8ae107d 100644
@@ -44,6 +44,21 @@ submodule.<name>.update::
 	This config option is overridden if 'git submodule update' is given
 	the '--merge' or '--rebase' options.
 
+submodule.<name>.ignore::
+	Defines under what circumstances "git status" and the diff family show
+	a submodule as modified. When set to "all", it will never be considered
+	modified, "dirty" will ignore all changes to the submodules work tree and
+	takes only differences between the HEAD of the submodule and the commit
+	recorded in the superproject into account. "untracked" will additionally
+	let submodules with modified tracked files in their work tree show up.
+	Using "none" (the default when this option is not set) also shows
+	submodules that have untracked files in their work tree as changed.
+	If this option is also present in the submodules entry in .git/config of
+	the superproject, the setting there will override the one found in
+	.gitmodules.
+	Both settings can be overriden on the command line by using the
+	"--ignore-submodule" option.
+
 
 EXAMPLES
 --------
builtin/commit.c +2 -0
index a78dbd8..9456f4a 100644
@@ -25,6 +25,7 @@
 #include "rerere.h"
 #include "unpack-trees.h"
 #include "quote.h"
+#include "submodule.h"
 
 static const char * const builtin_commit_usage[] = {
 	"git commit [options] [--] <filepattern>...",

@@ -1073,6 +1074,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		status_format = STATUS_FORMAT_PORCELAIN;
 
 	wt_status_prepare(&s);
+	gitmodules_config();
 	git_config(git_status_config, &s);
 	in_merge = file_exists(git_path("MERGE_HEAD"));
 	argc = parse_options(argc, argv, prefix,
builtin/diff-files.c +2 -0
index 5b64011..951c7c8 100644
@@ -8,6 +8,7 @@
 #include "commit.h"
 #include "revision.h"
 #include "builtin.h"
+#include "submodule.h"
 
 static const char diff_files_usage[] =
 "git diff-files [-q] [-0/-1/2/3 |-c|--cc] [<common diff options>] [<path>...]"

@@ -20,6 +21,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
 	unsigned options = 0;
 
 	init_revisions(&rev, prefix);
+	gitmodules_config();
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 	rev.abbrev = 0;
builtin/diff-index.c +2 -0
index 0483749..2eb32bd 100644
@@ -3,6 +3,7 @@
 #include "commit.h"
 #include "revision.h"
 #include "builtin.h"
+#include "submodule.h"
 
 static const char diff_cache_usage[] =
 "git diff-index [-m] [--cached] "

@@ -17,6 +18,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
 	int result;
 
 	init_revisions(&rev, prefix);
+	gitmodules_config();
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 	rev.abbrev = 0;
builtin/diff-tree.c +2 -0
index 3c78bda..0d2a3e9 100644
@@ -3,6 +3,7 @@
 #include "commit.h"
 #include "log-tree.h"
 #include "builtin.h"
+#include "submodule.h"
 
 static struct rev_info log_tree_opt;
 

@@ -112,6 +113,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 	int read_stdin = 0;
 
 	init_revisions(opt, prefix);
+	gitmodules_config();
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 	opt->abbrev = 0;
 	opt->diff = 1;
builtin/diff.c +2 -0
index 89ae89c..a43d326 100644
@@ -13,6 +13,7 @@
 #include "revision.h"
 #include "log-tree.h"
 #include "builtin.h"
+#include "submodule.h"
 
 struct blobinfo {
 	unsigned char sha1[20];

@@ -279,6 +280,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	 */
 
 	prefix = setup_git_directory_gently(&nongit);
+	gitmodules_config();
 	git_config(git_diff_ui_config, NULL);
 
 	if (diff_use_color_default == -1)
submodule.c +12 -0
index d35a14d..1bcb0e9 100644
@@ -65,6 +65,18 @@ static int submodule_config(const char *var, const char *value, void *cb)
 	return 0;
 }
 
+void gitmodules_config()
+{
+	const char *work_tree = get_git_work_tree();
+	if (work_tree) {
+		struct strbuf gitmodules_path = STRBUF_INIT;
+		strbuf_addstr(&gitmodules_path, work_tree);
+		strbuf_addstr(&gitmodules_path, "/.gitmodules");
+		git_config_from_file(submodule_config, gitmodules_path.buf, NULL);
+		strbuf_release(&gitmodules_path);
+	}
+}
+
 int parse_submodule_config_option(const char *var, const char *value)
 {
 	int len;
submodule.h +1 -0
index 185a5ce..8ac4037 100644
@@ -5,6 +5,7 @@ struct diff_options;
 
 void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 		const char *path);
+void gitmodules_config();
 int parse_submodule_config_option(const char *var, const char *value);
 void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
 void show_submodule_summary(FILE *f, const char *path,
t/t4027-diff-submodule.sh +66 -0

84 lines not on this page · raw message

t/t7508-status.sh +62 -1

137 lines not on this page · raw message

Signature · 1 line
1.7.2.223.g830604.dirty
#3 lore raw

Re: [PATCH 2/2] Submodules: Use "ignore" settings from .gitmodules too for diff and status

Quoted 3 lines · Jens Lehmann

The .gitmodules file is parsed for "submodule.<name>.ignore" entries before looking for them in .git/config. Thus settings found in .git/config will override those from .gitmodules,...

Hmph.

The value of "submodule.<name>.path" does not have to be "<name>". There seems to be a bit of confusion here.

I thought that [PATCH 1/2] used the path to the submodule, not its name, as the key. Even though the patch to Documentation/config.txt talks about "submodule.<submodulename>.ignore", what the code actually seems to do in set-from-submodule-config is to use path.

But this patch seems to key off of <name>.

I suspect that it would make more sense to use <name> so that once the user configures ignore in .git/config, it will persist across moving of the submodule in the superproject tree.

In either case, we would want to be consistent between the two, no?

author
#4 lore raw

v2 cover Submodules: Add the new config option "ignore"

Am 28.07.2010 23:35, schrieb Junio C Hamano:

Quoted 10 lines
Quoted 3 lines · Jens Lehmann

The .gitmodules file is parsed for "submodule.<name>.ignore" entries before looking for them in .git/config. Thus settings found in .git/config will override those from .gitmodules,...

Hmph.

The value of "submodule.<name>.path" does not have to be "<name>". There seems to be a bit of confusion here.

Thanks for clearing up my confusion, this is v2 of this patch using the path configured for the submodule instead of its name.

Jens Lehmann (2):
Submodules: Add the new "ignore" config option for diff and status
Submodules: Use "ignore" settings from .gitmodules too for diff and
status

 Documentation/config.txt       |   13 +++
 Documentation/diff-options.txt |    6 +-
 Documentation/git-status.txt   |    6 +-
 Documentation/gitmodules.txt   |   15 ++++
 builtin/commit.c               |    2 +
 builtin/diff-files.c           |    2 +
 builtin/diff-index.c           |    2 +
 builtin/diff-tree.c            |    2 +
 builtin/diff.c                 |    2 +
 diff-lib.c                     |   15 +++-
 diff.c                         |   35 +++++++--
 diff.h                         |    1 +
 submodule.c                    |   76 +++++++++++++++++-
 submodule.h                    |    4 +
 t/t4027-diff-submodule.sh      |  152 ++++++++++++++++++++++++++++++++++
 t/t7508-status.sh              |  175 +++++++++++++++++++++++++++++++++++++++-
 wt-status.c                    |    8 ++-
 17 files changed, 496 insertions(+), 20 deletions(-)
Signature · 1 line
1.7.2.1.54.g6bed1
author
#5 lore raw

PATCH v2 1/2 Submodules: Add the new "ignore" config option for diff and status

The new "ignore" config option controls the default behavior for "git status" and the diff family. It specifies under what circumstances they consider submodules as modified and can be set separately for each submodule.

The command line option "--ignore-submodules=" has been extended to accept the new parameter "none" for both status and diff.

Users that chose submodules to get rid of long work tree scanning times might want to set the "dirty" option for those submodules. This brings back the pre 1.7.0 behavior, where submodule work trees were never scanned for modifications. By using "--ignore-submodules=none" on the command line the status and diff commands can be told to do a full scan.

This option can be set to the following values (which have the same name and meaning as for the "--ignore-submodules" option of status and diff):

"all": All changes to the submodule will be ignored.

"dirty": Only differences of the commit recorded in the superproject and
the submodules HEAD will be considered modifications, all changes to the work tree of the submodule will be ignored. When using this value, the submodule will not be scanned for work tree changes at all, leading to a performance benefit on large submodules.

"untracked": Only untracked files in the submodules work tree are ignored,
a changed HEAD and/or modified files in the submodule will mark it as modified.

"none" (which is the default): Either untracked or modified files in a
submodules work tree or a difference between the subdmodules HEAD and the commit recorded in the superproject will make it show up as changed. This value is added as a new parameter for the "--ignore-submodules" option of the diff family and "git status" so the user can override the settings in the configuration.

Signed-off-by Jens Lehmann <[email protected]>
---
 Documentation/config.txt       |   10 ++++
 Documentation/diff-options.txt |    6 ++-
 Documentation/git-status.txt   |    6 ++-
 diff-lib.c                     |   15 ++++--
 diff.c                         |   35 ++++++++++++--
 diff.h                         |    1 +
 submodule.c                    |   57 ++++++++++++++++++++++-
 submodule.h                    |    3 +
 t/t4027-diff-submodule.sh      |   76 ++++++++++++++++++++++++++++++
 t/t7508-status.sh              |  100 ++++++++++++++++++++++++++++++++++++++--
 wt-status.c                    |    8 ++-
 11 files changed, 297 insertions(+), 20 deletions(-)
Documentation/config.txt +10 -0
index f81fb91..06faa02 100644
@@ -1747,6 +1747,16 @@ submodule.<name>.update::
 	URL and other values found in the `.gitmodules` file.  See
 	linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
 
+submodule.<name>.ignore::
+	Defines under what circumstances "git status" and the diff family show
+	a submodule as modified. When set to "all", it will never be considered
+	modified, "dirty" will ignore all changes to the submodules work tree and
+	takes only differences between the HEAD of the submodule and the commit
+	recorded in the superproject into account. "untracked" will additionally
+	let submodules with modified tracked files in their work tree show up.
+	Using "none" (the default when this option is not set) also shows
+	submodules that have untracked files in their work tree as changed.
+
 tar.umask::
 	This variable can be used to restrict the permission bits of
 	tar archive entries.  The default is 0002, which turns off the
Documentation/diff-options.txt +5 -1
index 2371262..9cf7506 100644
@@ -330,7 +330,11 @@ endif::git-format-patch[]
 
 --ignore-submodules[=<when>]::
 	Ignore changes to submodules in the diff generation. <when> can be
-	either "untracked", "dirty" or "all", which is the default. When
+	either "none", "untracked", "dirty" or "all", which is the default
+	Using "none" will consider the submodule modified when it either contains
+	untracked or modified files or its HEAD differs from the commit recorded
+	in the superproject and can be used to override any settings of the
+	'ignore' option in linkgit:git-config[1]. 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,
Documentation/git-status.txt +5 -1
index 2fd054c..a7a5d79 100644
@@ -55,7 +55,11 @@ 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
+	either "none", "untracked", "dirty" or "all", which is the default.
+	Using "none" will consider the submodule modified when it either contains
+	untracked or modified files or its HEAD differs from the commit recorded
+	in the superproject and can be used to override any settings of the
+	'ignore' option in linkgit:git-config[1]. 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,
diff-lib.c +10 -5
index 8b8978a..392ce2b 100644
@@ -68,11 +68,16 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
 				      unsigned ce_option, unsigned *dirty_submodule)
 {
 	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));
+	if (S_ISGITLINK(ce->ce_mode)) {
+		unsigned orig_flags = diffopt->flags;
+		if (!DIFF_OPT_TST(diffopt, OVERRIDE_SUBMODULE_CONFIG))
+			set_diffopt_flags_from_submodule_config(diffopt, ce->name);
+		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));
+		diffopt->flags = orig_flags;
 	}
 	return changed;
 }
diff.c +29 -6
index 782896d..bf6b9b1 100644
@@ -141,6 +141,9 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
+	if (!prefixcmp(var, "submodule."))
+		return parse_submodule_config_option(var, value);
+
 	return git_color_default_config(var, value, cb);
 }
 

@@ -3165,11 +3168,13 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		DIFF_OPT_SET(options, ALLOW_TEXTCONV);
 	else if (!strcmp(arg, "--no-textconv"))
 		DIFF_OPT_CLR(options, ALLOW_TEXTCONV);
-	else if (!strcmp(arg, "--ignore-submodules"))
+	else if (!strcmp(arg, "--ignore-submodules")) {
+		DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
 		handle_ignore_submodules_arg(options, "all");
-	else if (!prefixcmp(arg, "--ignore-submodules="))
+	} else if (!prefixcmp(arg, "--ignore-submodules=")) {
+		DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
 		handle_ignore_submodules_arg(options, arg + 20);
-	else if (!strcmp(arg, "--submodule"))
+	} else if (!strcmp(arg, "--submodule"))
 		DIFF_OPT_SET(options, SUBMODULE_LOG);
 	else if (!prefixcmp(arg, "--submodule=")) {
 		if (!strcmp(arg + 12, "log"))

@@ -4102,6 +4107,24 @@ int diff_result_code(struct diff_options *opt, int status)
 	return result;
 }
 
+/*
+ * Shall changes to this submodule be ignored?
+ *
+ * Submodule changes can be configured to be ignored separately for each path,
+ * but that configuration can be overridden from the command line.
+ */
+static int is_submodule_ignored(const char *path, struct diff_options *options)
+{
+	int ignored = 0;
+	unsigned orig_flags = options->flags;
+	if (!DIFF_OPT_TST(options, OVERRIDE_SUBMODULE_CONFIG))
+		set_diffopt_flags_from_submodule_config(options, path);
+	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES))
+		ignored = 1;
+	options->flags = orig_flags;
+	return ignored;
+}
+
 void diff_addremove(struct diff_options *options,
 		    int addremove, unsigned mode,
 		    const unsigned char *sha1,

@@ -4109,7 +4132,7 @@ void diff_addremove(struct diff_options *options,
 {
 	struct diff_filespec *one, *two;
 
-	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(mode))
+	if (S_ISGITLINK(mode) && is_submodule_ignored(concatpath, options))
 		return;
 
 	/* This may look odd, but it is a preparation for

@@ -4156,8 +4179,8 @@ void diff_change(struct diff_options *options,
 {
 	struct diff_filespec *one, *two;
 
-	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(old_mode)
-			&& S_ISGITLINK(new_mode))
+	if (S_ISGITLINK(old_mode) && S_ISGITLINK(new_mode) &&
+	    is_submodule_ignored(concatpath, options))
 		return;
 
 	if (DIFF_OPT_TST(options, REVERSE_DIFF)) {
diff.h +1 -0
index 063d10a..53ad345 100644
@@ -77,6 +77,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
 #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_OVERRIDE_SUBMODULE_CONFIG (1 << 27)
 
 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
submodule.c +56 -1

75 lines not on this page · raw message

submodule.h +3 -0
index 6fd3bb4..185a5ce 100644
@@ -3,6 +3,9 @@
 
 struct diff_options;
 
+void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
+		const char *path);
+int parse_submodule_config_option(const char *var, const char *value);
 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],
t/t4027-diff-submodule.sh +76 -0

88 lines not on this page · raw message

t/t7508-status.sh +96 -4

207 lines not on this page · raw message

wt-status.c +6 -2
index 2f9e33c..54b6b03 100644
@@ -313,8 +313,10 @@ 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)
+	if (s->ignore_submodule_arg) {
+		DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
 		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;

@@ -331,8 +333,10 @@ 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)
+	if (s->ignore_submodule_arg) {
+		DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
 		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;
Signature · 1 line
1.7.2.1.54.g6bed1
author
#6 lore raw

PATCH v2 2/2 Submodules: Use "ignore" settings from .gitmodules too for diff and status

In reply to Jens Lehmann

The .gitmodules file is parsed for "submodule.<name>.ignore" entries before looking for them in .git/config. Thus settings found in .git/config will override those from .gitmodules, thereby allowing the local developer to ignore settings given by the remote side while also letting upstream set defaults for those users who don't have special needs.

Signed-off-by Jens Lehmann <[email protected]>
---
 Documentation/config.txt       |    3 +
 Documentation/diff-options.txt |    2 +-
 Documentation/git-status.txt   |    2 +-
 Documentation/gitmodules.txt   |   15 +++++++
 builtin/commit.c               |    2 +
 builtin/diff-files.c           |    2 +
 builtin/diff-index.c           |    2 +
 builtin/diff-tree.c            |    2 +
 builtin/diff.c                 |    2 +
 submodule.c                    |   19 ++++++++
 submodule.h                    |    1 +
 t/t4027-diff-submodule.sh      |   76 +++++++++++++++++++++++++++++++++
 t/t7508-status.sh              |   91 ++++++++++++++++++++++++++++++++++++----
 13 files changed, 209 insertions(+), 10 deletions(-)
Documentation/config.txt +3 -0
index 06faa02..d97b986 100644
@@ -1756,6 +1756,9 @@ submodule.<name>.ignore::
 	let submodules with modified tracked files in their work tree show up.
 	Using "none" (the default when this option is not set) also shows
 	submodules that have untracked files in their work tree as changed.
+	This setting overrides any setting made in .gitmodules for this submodule,
+	both settings can be overriden on the command line by using the
+	"--ignore-submodule" option.
 
 tar.umask::
 	This variable can be used to restrict the permission bits of
Documentation/diff-options.txt +1 -1
index 9cf7506..faa467b 100644
@@ -334,7 +334,7 @@ endif::git-format-patch[]
 	Using "none" will consider the submodule modified when it either contains
 	untracked or modified files or its HEAD differs from the commit recorded
 	in the superproject and can be used to override any settings of the
-	'ignore' option in linkgit:git-config[1]. When
+	'ignore' option in linkgit:git-config[1] or linkgit:gitmodules[5]. 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,
Documentation/git-status.txt +1 -1
index a7a5d79..dae190a 100644
@@ -59,7 +59,7 @@ specified.
 	Using "none" will consider the submodule modified when it either contains
 	untracked or modified files or its HEAD differs from the commit recorded
 	in the superproject and can be used to override any settings of the
-	'ignore' option in linkgit:git-config[1]. When
+	'ignore' option in linkgit:git-config[1] or linkgit:gitmodules[5]. 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,
Documentation/gitmodules.txt +15 -0
index 72a13d1..8ae107d 100644
@@ -44,6 +44,21 @@ submodule.<name>.update::
 	This config option is overridden if 'git submodule update' is given
 	the '--merge' or '--rebase' options.
 
+submodule.<name>.ignore::
+	Defines under what circumstances "git status" and the diff family show
+	a submodule as modified. When set to "all", it will never be considered
+	modified, "dirty" will ignore all changes to the submodules work tree and
+	takes only differences between the HEAD of the submodule and the commit
+	recorded in the superproject into account. "untracked" will additionally
+	let submodules with modified tracked files in their work tree show up.
+	Using "none" (the default when this option is not set) also shows
+	submodules that have untracked files in their work tree as changed.
+	If this option is also present in the submodules entry in .git/config of
+	the superproject, the setting there will override the one found in
+	.gitmodules.
+	Both settings can be overriden on the command line by using the
+	"--ignore-submodule" option.
+
 
 EXAMPLES
 --------
builtin/commit.c +2 -0
index 2bb30c0..269fa38 100644
@@ -25,6 +25,7 @@
 #include "rerere.h"
 #include "unpack-trees.h"
 #include "quote.h"
+#include "submodule.h"
 
 static const char * const builtin_commit_usage[] = {
 	"git commit [options] [--] <filepattern>...",

@@ -1073,6 +1074,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		status_format = STATUS_FORMAT_PORCELAIN;
 
 	wt_status_prepare(&s);
+	gitmodules_config();
 	git_config(git_status_config, &s);
 	in_merge = file_exists(git_path("MERGE_HEAD"));
 	argc = parse_options(argc, argv, prefix,
builtin/diff-files.c +2 -0
index 5b64011..951c7c8 100644
@@ -8,6 +8,7 @@
 #include "commit.h"
 #include "revision.h"
 #include "builtin.h"
+#include "submodule.h"
 
 static const char diff_files_usage[] =
 "git diff-files [-q] [-0/-1/2/3 |-c|--cc] [<common diff options>] [<path>...]"

@@ -20,6 +21,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
 	unsigned options = 0;
 
 	init_revisions(&rev, prefix);
+	gitmodules_config();
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 	rev.abbrev = 0;
builtin/diff-index.c +2 -0
index 0483749..2eb32bd 100644
@@ -3,6 +3,7 @@
 #include "commit.h"
 #include "revision.h"
 #include "builtin.h"
+#include "submodule.h"
 
 static const char diff_cache_usage[] =
 "git diff-index [-m] [--cached] "

@@ -17,6 +18,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
 	int result;
 
 	init_revisions(&rev, prefix);
+	gitmodules_config();
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 	rev.abbrev = 0;
builtin/diff-tree.c +2 -0
index 3c78bda..0d2a3e9 100644
@@ -3,6 +3,7 @@
 #include "commit.h"
 #include "log-tree.h"
 #include "builtin.h"
+#include "submodule.h"
 
 static struct rev_info log_tree_opt;
 

@@ -112,6 +113,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 	int read_stdin = 0;
 
 	init_revisions(opt, prefix);
+	gitmodules_config();
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 	opt->abbrev = 0;
 	opt->diff = 1;
builtin/diff.c +2 -0
index 89ae89c..a43d326 100644
@@ -13,6 +13,7 @@
 #include "revision.h"
 #include "log-tree.h"
 #include "builtin.h"
+#include "submodule.h"
 
 struct blobinfo {
 	unsigned char sha1[20];

@@ -279,6 +280,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	 */
 
 	prefix = setup_git_directory_gently(&nongit);
+	gitmodules_config();
 	git_config(git_diff_ui_config, NULL);
 
 	if (diff_use_color_default == -1)
submodule.c +19 -0
index ff28630..9ebafc1 100644
@@ -62,6 +62,25 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 	}
 }
 
+static int submodule_config(const char *var, const char *value, void *cb)
+{
+	if (!prefixcmp(var, "submodule."))
+		return parse_submodule_config_option(var, value);
+	return 0;
+}
+
+void gitmodules_config()
+{
+	const char *work_tree = get_git_work_tree();
+	if (work_tree) {
+		struct strbuf gitmodules_path = STRBUF_INIT;
+		strbuf_addstr(&gitmodules_path, work_tree);
+		strbuf_addstr(&gitmodules_path, "/.gitmodules");
+		git_config_from_file(submodule_config, gitmodules_path.buf, NULL);
+		strbuf_release(&gitmodules_path);
+	}
+}
+
 int parse_submodule_config_option(const char *var, const char *value)
 {
 	int len;
submodule.h +1 -0
index 185a5ce..8ac4037 100644
@@ -5,6 +5,7 @@ struct diff_options;
 
 void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 		const char *path);
+void gitmodules_config();
 int parse_submodule_config_option(const char *var, const char *value);
 void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
 void show_submodule_summary(FILE *f, const char *path,
t/t4027-diff-submodule.sh +76 -0

94 lines not on this page · raw message

t/t7508-status.sh +83 -8

171 lines not on this page · raw message

Signature · 1 line
1.7.2.1.54.g6bed1
#7 lore raw
In reply to Jens Lehmann
Quoted 7 lines · Jens Lehmann

Am 28.07.2010 23:35, schrieb Junio C Hamano:

Quoted 6 lines
Quoted 3 lines · Jens Lehmann

The .gitmodules file is parsed for "submodule.<name>.ignore" entries before looking for them in .git/config. Thus settings found in .git/config will override those from .gitmodules,...

Quoted 7 lines
Quoted 4 lines

Hmph.

The value of "submodule.<name>.path" does not have to be "<name>". There seems to be a bit of confusion here.

Thanks for clearing up my confusion, this is v2 of this patch using the path configured for the submodule instead of its name.

Hmph, I thought that it would make more sense to use <name> so that once the user configures ignore in .git/config, it will persist across moving of the submodule in the superproject tree. I wonder what others would think.

Will queue, anyway.

Thanks.

author
#8 lore raw

Am 06.08.2010 01:27, schrieb Junio C Hamano:

Quoted 6 lines
Quoted 4 lines · Jens Lehmann

Am 28.07.2010 23:35, schrieb Junio C Hamano:

Quoted 2 lines

The value of "submodule.<name>.path" does not have to be "<name>". There seems to be a bit of confusion here.

Quoted 7 lines
Quoted 2 lines

Thanks for clearing up my confusion, this is v2 of this patch using the path configured for the submodule instead of its name.

Hmph, I thought that it would make more sense to use <name> so that once the user configures ignore in .git/config, it will persist across moving of the submodule in the superproject tree. I wonder what others would think.

I think that is just what this patch does, it uses the "config_name_for_path" string_list to get the submodules <name> for the path to then look up what is configured for the "ignore" option under that <name>. So the "ignore" option should survive renames, no? Or did I manage to confuse myself again?

#9 lore raw
Quoted 10 lines · Jens Lehmann

Am 06.08.2010 01:27, schrieb Junio C Hamano:

Quoted 9 lines
Quoted 6 lines · Jens Lehmann

Am 28.07.2010 23:35, schrieb Junio C Hamano:

Quoted 2 lines

The value of "submodule.<name>.path" does not have to be "<name>". There seems to be a bit of confusion here.

Thanks for clearing up my confusion, this is v2 of this patch using the path configured for the submodule instead of its name.

Quoted 6 lines
Quoted 4 lines

Hmph, I thought that it would make more sense to use <name> so that once the user configures ignore in .git/config, it will persist across moving of the submodule in the superproject tree. I wonder what others would think.

I think that is just what this patch does, it uses the ...

Yeah, what you said is all correct. I just misread "... this patch using the path ... instead of its name" to mean that the user-level interface is now keying off of a path, but that is not what you meant. You instead meant that the name is used as the key and it used as if it is a path in the previous round, but now it goes through name-to-path mapping.

Thanks.

#10 lore raw
In reply to Jens Lehmann

Hi,

Quoted 11 lines · Jens Lehmann

These two patches are extremly useful for those users who chose submodules because they wanted to reduce the time it takes for "git status" and "git diff" to recurse through the whole work tree by putting sometimes large amounts of files into submodules, which weren't scanned in the past.

Since 1.7.0 "git status" and "git diff" recurse through submodule work trees, which has a - sometimes drastic - performance impact when large submodules are present. Using the "ignore=dirty" config option restores the former behaviour.

Quoted 7 lines

This option can be set in the .gitmodules file or in .git/config, any settings found in the latter override those from .gitmodules. This enables the distribution of a default setting for this option by simply committing a modified .gitmodules file without each developer having to call "git submodule sync". When using "git status" or "git diff" with "--ignore-submodules=none" option the default behavior of scanning all submodules work trees can be restored temporarily.

This is incredibly useful stuff. Indeed, the time for one "git status" went down from > 50 sec to < 2 sec again in my project. (I have the impression that it still does a little too much, but I will have to have some time for strace()ing to find out for real.) Therefore, I applied it to 4msysgit.git's 'devel' branch.

However, one thing is inconvenient: there are config options per submodule, but no single option to just turn the behavior back to before. I know the reasoning behind showing dirty submodules, so I am fine with that feature being opt-out, but for my real world scenario, it just slows me down, so I _need_ to turn it off.

To put my money where my mouth is (since I need it myself, I also pushed it to 4msysgit.git's 'devel' branch):

-- snipsnap --
From f7b1e98a0d850e5b1bc56a825fcfe5c87076dfa9 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <[email protected]>
Date: Thu, 5 Aug 2010 10:49:55 +0200
Subject: [PATCH] Add the 'diff.ignoreSubmodules' config setting

When you have a lot of submodules checked out, the time penalty to check for dirty submodules can easily imply a multiplication of the total time by the factor 20. This makes the difference between almost instantaneous (< 2 seconds) and unbearably slow (> 50 seconds) here, since the disk caches are constantly overloaded.

To this end, the submodule.*.ignore config option was introduced, but it is per-submodule.

This commit introduces a global config setting to set a default (porcelain) value for the --ignore-submodules option, keeping the default at 'none'. It can be overridden by the submodule.*.ignore setting and by the --ignore-submodules option.

Incidentally, this commit fixes an issue with the overriding logic:
multiple --ignore-submodules options would not clear the previously set flags.

While at it, fix a typo in the documentation for submodule.*.ignore.

Signed-off-by Johannes Schindelin <[email protected]>
---
 Documentation/config.txt |    7 ++++++-
 diff.c                   |    6 +++++-
 submodule.c              |    4 ++++
 3 files changed, 15 insertions(+), 2 deletions(-)
Documentation/config.txt +6 -1
index 5594ce0..8384d12 100644
@@ -832,6 +832,11 @@ diff.renames::
 	will enable basic rename detection.  If set to "copies" or
 	"copy", it will detect copies, as well.
 
+diff.ignoreSubmodules::
+	Sets the default value of --ignore-submodules. Note that this
+	affects only 'git diff' Porcelain, and not lower level 'diff'
+	commands such as 'git diff-files'.
+
 diff.suppressBlankEmpty::
 	A boolean to inhibit the standard behavior of printing a space
 	before each empty output line. Defaults to false.

@@ -1769,7 +1774,7 @@ submodule.<submodulename>.ignore::
 	submodules that have untracked files in their work tree as changed.
 	This setting overrides any setting made in .gitmodules for this submodule,
 	both settings can be overriden on the command line by using the
-	"--ignore-submodule" option.
+	"--ignore-submodules" option.
 
 tar.umask::
 	This variable can be used to restrict the permission bits of
diff.c +5 -1
index 8206047..1ddfdfb 100644
@@ -31,6 +31,7 @@ static const char *external_diff_cmd_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
+static struct diff_options default_diff_options;
 
 static char diff_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_RESET,

@@ -107,6 +108,9 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
 	if (!strcmp(var, "diff.wordregex"))
 		return git_config_string(&diff_word_regex_cfg, var, value);
 
+	if (!strcmp(var, "diff.ignoresubmodules"))
+		handle_ignore_submodules_arg(&default_diff_options, value);
+
 	return git_diff_basic_config(var, value, cb);
 }
 

@@ -2816,7 +2820,7 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
 
 void diff_setup(struct diff_options *options)
 {
-	memset(options, 0, sizeof(*options));
+	memcpy(options, &default_diff_options, sizeof(*options));
 	memset(&diff_queued_diff, 0, sizeof(diff_queued_diff));
 
 	options->file = stdout;
submodule.c +4 -0
index 1bcb0e9..75f3368 100644
@@ -110,6 +110,10 @@ int parse_submodule_config_option(const char *var, const char *value)
 void handle_ignore_submodules_arg(struct diff_options *diffopt,
 				  const char *arg)
 {
+	DIFF_OPT_CLR(diffopt, IGNORE_SUBMODULES);
+	DIFF_OPT_CLR(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
+	DIFF_OPT_CLR(diffopt, IGNORE_DIRTY_SUBMODULES);
+
 	if (!strcmp(arg, "all"))
 		DIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);
 	else if (!strcmp(arg, "untracked"))
Signature · 1 line
1.7.2.1.2240.gde5dd
#11 lore raw
Quoted 11 lines · Johannes Schindelin

From: Johannes Schindelin <[email protected]>
Date: Thu, 5 Aug 2010 10:49:55 +0200
Subject: [PATCH] Add the 'diff.ignoreSubmodules' config setting
...
This commit introduces a global config setting to set a default (porcelain) value for the --ignore-submodules option, keeping the default at 'none'. It can be overridden by the submodule.*.ignore setting and by the --ignore-submodules option.
...

Signed-off-by Johannes Schindelin <[email protected]>

---

Nice, at least from a cursory look.

diff_setup() is shared by everybody both plumbing and Porcelain that use diff and log, but you placed the smudging of the default based on config is in diff_ui_config(), so this patch looks safe.

And the default setting can be overriden with --ignore-submodules from the command line during diff_opt_parse(), which is also safe and sane.

Thanks; will queue.

Quoted 53 lines
diff.c +5 -1
index 8206047..1ddfdfb 100644
@@ -31,6 +31,7 @@ static const char *external_diff_cmd_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
+static struct diff_options default_diff_options;
 
 static char diff_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_RESET,

@@ -107,6 +108,9 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
 	if (!strcmp(var, "diff.wordregex"))
 		return git_config_string(&diff_word_regex_cfg, var, value);
 
+	if (!strcmp(var, "diff.ignoresubmodules"))
+		handle_ignore_submodules_arg(&default_diff_options, value);
+
 	return git_diff_basic_config(var, value, cb);
 }
 

@@ -2816,7 +2820,7 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
 
 void diff_setup(struct diff_options *options)
 {
-	memset(options, 0, sizeof(*options));
+	memcpy(options, &default_diff_options, sizeof(*options));
 	memset(&diff_queued_diff, 0, sizeof(diff_queued_diff));
 
 	options->file = stdout;
submodule.c +4 -0
index 1bcb0e9..75f3368 100644
@@ -110,6 +110,10 @@ int parse_submodule_config_option(const char *var, const char *value)
 void handle_ignore_submodules_arg(struct diff_options *diffopt,
 				  const char *arg)
 {
+	DIFF_OPT_CLR(diffopt, IGNORE_SUBMODULES);
+	DIFF_OPT_CLR(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
+	DIFF_OPT_CLR(diffopt, IGNORE_DIRTY_SUBMODULES);
+
 	if (!strcmp(arg, "all"))
 		DIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);
 	else if (!strcmp(arg, "untracked"))
Signature · 6 lines
1.7.2.1.2240.gde5dd

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author Acked-by
#12 lore raw

Am 05.08.2010 18:49, schrieb Junio C Hamano:

Quoted 14 lines
Quoted 11 lines · Johannes Schindelin

From: Johannes Schindelin <[email protected]>
Date: Thu, 5 Aug 2010 10:49:55 +0200
Subject: [PATCH] Add the 'diff.ignoreSubmodules' config setting
...
This commit introduces a global config setting to set a default (porcelain) value for the --ignore-submodules option, keeping the default at 'none'. It can be overridden by the submodule.*.ignore setting and by the --ignore-submodules option.
...

Signed-off-by Johannes Schindelin <[email protected]>

---

Quoted 1 line

Nice, at least from a cursory look.

Yup, makes sense to me too.

Acked-by Jens Lehmann <[email protected]>
author
#13 lore raw

PATCH 1/1 Add tests for the diff.ignoreSubmodules config option

Signed-off-by Jens Lehmann <[email protected]>

---

Am 05.08.2010 21:08, schrieb Jens Lehmann:

Quoted 10 lines

Am 05.08.2010 18:49, schrieb Junio C Hamano:

Quoted 9 lines
Quoted 7 lines · Johannes Schindelin

This commit introduces a global config setting to set a default (porcelain) value for the --ignore-submodules option, keeping the default at 'none'. It can be overridden by the submodule.*.ignore setting and by the --ignore-submodules option.
...

Signed-off-by Johannes Schindelin <[email protected]>

---

Quoted 5 lines
Quoted 1 line

Nice, at least from a cursory look.

Yup, makes sense to me too.

Acked-by Jens Lehmann <[email protected]>

And here are some test cases for this new option.

 t/t4027-diff-submodule.sh |   10 +++++++++-
 t/t7508-status.sh         |   12 ++++++++++--
 2 files changed, 19 insertions(+), 3 deletions(-)
t/t4027-diff-submodule.sh +9 -1
index 1bc6e77..d99814a 100755
@@ -115,6 +115,9 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)'
 '
 
 test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match) [.git/config]' '
+	git config diff.ignoreSubmodules all &&
+	git diff HEAD >actual &&
+	! test -s actual &&
 	git config submodule.subname.ignore none &&
 	git config submodule.subname.path sub &&
 	git diff HEAD >actual &&

@@ -136,10 +139,14 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)
 	sed -e "1,/^@@/d" actual >actual.body &&
 	expect_from_to >expect.body $subprev $subprev-dirty &&
 	test_cmp expect.body actual.body &&
-	git config --remove-section submodule.subname
+	git config --remove-section submodule.subname &&
+	git config --unset diff.ignoreSubmodules
 '
 
 test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match) [.gitmodules]' '
+	git config diff.ignoreSubmodules dirty &&
+	git diff HEAD >actual &&
+	! test -s actual &&
 	git config --add -f .gitmodules submodule.subname.ignore none &&
 	git config --add -f .gitmodules submodule.subname.path sub &&
 	git diff HEAD >actual &&

@@ -166,6 +173,7 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)
 	test_cmp expect.body actual.body &&
 	git config --remove-section submodule.subname &&
 	git config --remove-section -f .gitmodules submodule.subname &&
+	git config --unset diff.ignoreSubmodules &&
 	rm .gitmodules
 '
t/t7508-status.sh +10 -2
index 1aae762..9c14b85 100755
@@ -849,11 +849,15 @@ test_expect_success '--ignore-submodules=untracked suppresses submodules with un
 '
 
 test_expect_success '.gitmodules ignore=untracked suppresses submodules with untracked content' '
+	git config diff.ignoreSubmodules dirty &&
+	git status >output &&
+	test_cmp expect output &&
 	git config --add -f .gitmodules submodule.subname.ignore untracked &&
 	git config --add -f .gitmodules submodule.subname.path sm &&
 	git status > output &&
 	test_cmp expect output &&
-	git config -f .gitmodules  --remove-section submodule.subname
+	git config -f .gitmodules  --remove-section submodule.subname &&
+	git config --unset diff.ignoreSubmodules
 '
 
 test_expect_success '.git/config ignore=untracked suppresses submodules with untracked content' '

@@ -873,11 +877,15 @@ test_expect_success '--ignore-submodules=dirty suppresses submodules with untrac
 '
 
 test_expect_success '.gitmodules ignore=dirty suppresses submodules with untracked content' '
+	git config diff.ignoreSubmodules dirty &&
+	git status >output &&
+	! test -s actual &&
 	git config --add -f .gitmodules submodule.subname.ignore dirty &&
 	git config --add -f .gitmodules submodule.subname.path sm &&
 	git status > output &&
 	test_cmp expect output &&
-	git config -f .gitmodules  --remove-section submodule.subname
+	git config -f .gitmodules  --remove-section submodule.subname &&
+	git config --unset diff.ignoreSubmodules
 '
 
 test_expect_success '.git/config ignore=dirty suppresses submodules with untracked content' '
Signature · 1 line
1.7.2.1.54.g6bed1
#14 lore raw

Re: [PATCH] Add tests for the diff.ignoreSubmodules config option

Thanks, both. Will queue.