threads / patch / 24857

patchcheckout: respect diff.ignoreSubmodules setting

Subject: [PATCH] checkout: respect diff.ignoreSubmodules setting

## tl;dr

7 messages between Aug 25, 2010 and Aug 28, 2010. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Johannes Schindelin· Aug 25, 2010, 10:34 UTC · lore

When 'git checkout' reports uncommitted changes, it also does so for submodules.

The default mode is now to look really hard into submodules, not only for different commits, but also for modified files. Since this can be pretty expensive when there are a lot (and large) submodules, there is the diff.ignoreSubmodules option.

Let's respect that setting when 'git checkout' reports the uncommitted changes, since it does nothing else than a 'git diff --name-status'.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/checkout.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)
Show changes to builtin/checkout.c +12 −4
diff --git a/builtin/checkout.c b/builtin/checkout.c
index ff5ac1e..19c23b2 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -18,6 +18,7 @@
 #include "xdiff-interface.h"
 #include "ll-merge.h"
 #include "resolve-undo.h"
+#include "submodule.h"
 
 static const char * const checkout_usage[] = {
 	"git checkout [options] <branch>",
@@ -40,6 +41,7 @@ struct checkout_opts {
 	const char *new_orphan_branch;
 	int new_branch_log;
 	enum branch_track track;
+	struct diff_options diff_options;
 };
 
 static int post_checkout_hook(struct commit *old, struct commit *new,
@@ -282,11 +284,12 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 	return errs;
 }
 
-static void show_local_changes(struct object *head)
+static void show_local_changes(struct object *head, struct diff_options *opts)
 {
 	struct rev_info rev;
 	/* I think we want full paths, even if we're in a subdirectory. */
 	init_revisions(&rev, NULL);
+	rev.diffopt.flags = opts->flags;
 	rev.diffopt.output_format |= DIFF_FORMAT_NAME_STATUS;
 	if (diff_setup_done(&rev.diffopt) < 0)
 		die("diff_setup_done failed");
@@ -471,7 +474,7 @@ static int merge_working_tree(struct checkout_opts *opts,
 		die("unable to write new index file");
 
 	if (!opts->force && !opts->quiet)
-		show_local_changes(&new->commit->object);
+		show_local_changes(&new->commit->object, &opts->diff_options);
 
 	return 0;
 }
@@ -619,7 +622,12 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
 
 static int git_checkout_config(const char *var, const char *value, void *cb)
 {
-	return git_xmerge_config(var, value, cb);
+	if (!strcmp(var, "diff.ignoresubmodules")) {
+		struct checkout_opts *opts = cb;
+		handle_ignore_submodules_arg(&opts->diff_options, value);
+		return 0;
+	}
+	return git_xmerge_config(var, value, NULL);
 }
 
 static int interactive_checkout(const char *revision, const char **pathspec,
@@ -703,7 +711,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	memset(&opts, 0, sizeof(opts));
 	memset(&new, 0, sizeof(new));
 
-	git_config(git_checkout_config, NULL);
+	git_config(git_checkout_config, &opts);
 
 	opts.track = BRANCH_TRACK_UNSPECIFIED;
 
-- 
1.7.2.2.485.gf3638.dirty
Jens Lehmann· Aug 25, 2010, 17:48 UTC · re: Johannes Schindelin · lore

Re: [PATCH] checkout: respect diff.ignoreSubmodules setting

Am 25.08.2010 12:34, schrieb Johannes Schindelin:
Show 13 quoted lines
> 
> When 'git checkout' reports uncommitted changes, it also does so for
> submodules.
> 
> The default mode is now to look really hard into submodules, not only
> for different commits, but also for modified files. Since this can be
> pretty expensive when there are a lot (and large) submodules, there is
> the diff.ignoreSubmodules option.
> 
> Let's respect that setting when 'git checkout' reports the uncommitted
> changes, since it does nothing else than a 'git diff --name-status'.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Thanks for fixing that!
Acked-by: Jens Lehmann <Jens.Lehmann@web.de>
Junio C Hamano· Aug 25, 2010, 20:10 UTC · re: Jens Lehmann · lore

Re: [PATCH] checkout: respect diff.ignoreSubmodules setting

Jens Lehmann <Jens.Lehmann@web.de> writes:
Show 10 quoted lines
> Am 25.08.2010 12:34, schrieb Johannes Schindelin:
>>  ...
>> Let's respect that setting when 'git checkout' reports the uncommitted
>> changes, since it does nothing else than a 'git diff --name-status'.
>> 
>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Thanks for fixing that!
>
> Acked-by: Jens Lehmann <Jens.Lehmann@web.de>
Makes sense.

I wondered if the new parameter to show_local_changes() should just be of type (struct checkout_opts *), so that other options given to checkout, not necessarily related to the diff generation, can influence the output further if we wanted to, but that kind of change can be done if and when it becomes necessary.

Thanks.  Perhaps we would want a new test or two in t2013?
Jens Lehmann· Aug 25, 2010, 21:57 UTC · re: Junio C Hamano · lore

Re: [PATCH] checkout: respect diff.ignoreSubmodules setting

Am 25.08.2010 22:10, schrieb Junio C Hamano:
> Thanks.  Perhaps we would want a new test or two in t2013?
Sure, I will add some as soon as I find some time ...
Jens Lehmann· Aug 28, 2010, 14:48 UTC · re: Jens Lehmann · lore

[PATCH 0/2] checkout: diff.ignoreSubmodules: Add test and honor per submodule settings too

Am 25.08.2010 23:57, schrieb Jens Lehmann:
> Am 25.08.2010 22:10, schrieb Junio C Hamano:
>> Thanks.  Perhaps we would want a new test or two in t2013?
> 
> Sure, I will add some as soon as I find some time ...
Ok, here we go!

While writing the test I noticed it might be nice to use the configuration settings consistently. So I did a second patch teaching checkout to honor the submodule.*.ignore settings the same way diff and status do.

Two other issues surfaced while working on these patches:
1) We might have to teach the --ignore-submodules option to
   checkout too to be able to override the configuration on
   the command line and in scripts.
2) It might be a good idea to let "git status" honor the
   diff.ignoreSubmodules setting too, so that the output is
   consistent with that of diff and checkout.
Opinions?

(While I am willing to write the patches, I am not a user of these config options myself, so feedback from people who do use them is very much appreciated!)

Jens Lehmann (2):
  checkout: Add test for diff.ignoreSubmodules
  checkout: Use submodule.*.ignore settings from .git/config and
    .gitmodules
 Documentation/config.txt      |    3 ++-
 builtin/checkout.c            |    5 +++++
 t/t2013-checkout-submodule.sh |   23 +++++++++++++++++++++++
 3 files changed, 30 insertions(+), 1 deletions(-)
-- 
1.7.2.2.515.g13860
Jens Lehmann· Aug 28, 2010, 14:49 UTC · re: Jens Lehmann · lore

[PATCH 1/2] checkout: Add test for diff.ignoreSubmodules

While at it, document that checkout uses this flag too in the Documentation.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 Documentation/config.txt      |    3 ++-
 t/t2013-checkout-submodule.sh |    7 +++++++
 2 files changed, 9 insertions(+), 1 deletions(-)
Show changes to 2 files +9 −1

Documentation/config.txt, t/t2013-checkout-submodule.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0510ac7..cda6721 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -829,7 +829,8 @@ diff.renames::
 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'.
+	commands such as 'git diff-files'. 'git checkout' also honors
+	this setting when reporting uncommitted changes.

 diff.suppressBlankEmpty::
 	A boolean to inhibit the standard behavior of printing a space
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index fda3f0a..3beb0b3 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -39,4 +39,11 @@ test_expect_success '"checkout <submodule>" updates the index only' '
 	git diff-files --quiet
 '

+test_expect_success '"checkout <submodule>" honors diff.ignoreSubmodules' '
+	git config diff.ignoreSubmodules dirty &&
+	echo x> submodule/untracked &&
+	git checkout HEAD >actual 2>&1 &&
+	! test -s actual
+'
+
 test_done
-- 
1.7.2.2.515.g13860
Jens Lehmann· Aug 28, 2010, 14:50 UTC · re: Jens Lehmann · lore

[PATCH 2/2] checkout: Use submodule.*.ignore settings from .git/config and .gitmodules

For "git status" and the diff family the submodule.*.ignore settings from .git/config and .gitmodules can be used to override the default set via diff.ignoreSubmodules on a per-submodule basis. Let's do this consistently and teach checkout to use these settings too.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 builtin/checkout.c            |    5 +++++
 t/t2013-checkout-submodule.sh |   16 ++++++++++++++++
 2 files changed, 21 insertions(+), 0 deletions(-)
Show changes to 2 files +21 −0

builtin/checkout.c, t/t2013-checkout-submodule.sh

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 19c23b2..f365169 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -627,6 +627,10 @@ static int git_checkout_config(const char *var, const char *value, void *cb)
 		handle_ignore_submodules_arg(&opts->diff_options, value);
 		return 0;
 	}
+
+	if (!prefixcmp(var, "submodule."))
+		return parse_submodule_config_option(var, value);
+
 	return git_xmerge_config(var, value, NULL);
 }

@@ -711,6 +715,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	memset(&opts, 0, sizeof(opts));
 	memset(&new, 0, sizeof(new));

+	gitmodules_config();
 	git_config(git_checkout_config, &opts);

 	opts.track = BRANCH_TRACK_UNSPECIFIED;
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index 3beb0b3..70edbb3 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -46,4 +46,20 @@ test_expect_success '"checkout <submodule>" honors diff.ignoreSubmodules' '
 	! test -s actual
 '

+test_expect_success '"checkout <submodule>" honors submodule.*.ignore from .gitmodules' '
+	git config diff.ignoreSubmodules none &&
+	git config -f .gitmodules submodule.submodule.path submodule &&
+	git config -f .gitmodules submodule.submodule.ignore untracked &&
+	git checkout HEAD >actual 2>&1 &&
+	! test -s actual
+'
+
+test_expect_success '"checkout <submodule>" honors submodule.*.ignore from .git/config' '
+	git config -f .gitmodules submodule.submodule.ignore none &&
+	git config submodule.submodule.path submodule &&
+	git config submodule.submodule.ignore all &&
+	git checkout HEAD >actual 2>&1 &&
+	! test -s actual
+'
+
 test_done
-- 
1.7.2.2.515.g13860

← back to recent threads