threads / patch / 23856

patchDon't do status in submodules if status.SubmoduleSummary unset

Subject: [PATCH] Don't do status in submodules if status.SubmoduleSummary unset

## tl;dr

4 messages between May 20, 2010 and May 20, 2010. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Alex Riesen· May 20, 2010, 14:12 UTC · lore
Noticed-by: Andy Parkins
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
--
On Thu, May 20, 2010 at 15:28, Junio C Hamano <gitster@pobox.com> wrote:
> As to the not-working-configuration I don't remember the codepath well, so
> sorry but no answer from me right now.

Maybe because we do a (kind of) gentle status run on submodules whether the status.SubmoduleSummary set or not. Usually a background run of "git status" for every submodules goes unnoticed, just sometimes a submodule is a little too big.

I tried this, but feels like a bit of overkill.
Show changes to wt-status.c +8 −2
diff --git a/wt-status.c b/wt-status.c
index 8ca59a2..d5bcdf9 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -303,7 +303,10 @@ static void
wt_status_collect_changes_worktree(struct wt_status *s)
 	init_revisions(&rev, NULL);
 	setup_revisions(0, NULL, &rev, NULL);
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
-	DIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);
+	if (s->submodule_summary)
+		DIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);
+	else
+		DIFF_OPT_SET(&rev.diffopt, IGNORE_SUBMODULES);
 	if (!s->show_untracked_files)
 		DIFF_OPT_SET(&rev.diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
 	rev.diffopt.format_callback = wt_status_collect_changed_cb;


From 6b7a5e0ac560e1187d8ff359bad8c04927d0b599 Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Thu, 20 May 2010 16:04:11 +0200
Subject: [PATCH] Don't do status in submodules if status.SubmoduleSummary unset

Noticed-by: Andy Parkins
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>

diff --git a/wt-status.c b/wt-status.c
index 8ca59a2..d5bcdf9 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -303,7 +303,10 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
 	init_revisions(&rev, NULL);
 	setup_revisions(0, NULL, &rev, NULL);
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
-	DIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);
+	if (s->submodule_summary)
+		DIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);
+	else
+		DIFF_OPT_SET(&rev.diffopt, IGNORE_SUBMODULES);
 	if (!s->show_untracked_files)
 		DIFF_OPT_SET(&rev.diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
 	rev.diffopt.format_callback = wt_status_collect_changed_cb;
Jens Lehmann· May 20, 2010, 17:45 UTC · re: Alex Riesen · lore

Re: [PATCH] Don't do status in submodules if status.SubmoduleSummary unset

Am 20.05.2010 16:12, schrieb Alex Riesen:
> Maybe because we do a (kind of) gentle status run on submodules
> whether the status.SubmoduleSummary set or not.

Yup, because checking a submodule for its dirtiness has to be done no matter if the summary output is also wanted.

Show 5 quoted lines
> Usually a background
> run of "git status" for every submodules goes unnoticed, just
> sometimes a submodule is a little too big.
> 
> I tried this, but feels like a bit of overkill.

This patch seems to disable submodule output completely for the default case (when status.SubmoduleSummary is false) and breaks 17 test cases.

So no thumbs up from me ;-)
Show 17 quoted lines
> diff --git a/wt-status.c b/wt-status.c
> index 8ca59a2..d5bcdf9 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -303,7 +303,10 @@ static void
> wt_status_collect_changes_worktree(struct wt_status *s)
>  	init_revisions(&rev, NULL);
>  	setup_revisions(0, NULL, &rev, NULL);
>  	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
> -	DIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);
> +	if (s->submodule_summary)
> +		DIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);
> +	else
> +		DIFF_OPT_SET(&rev.diffopt, IGNORE_SUBMODULES);
>  	if (!s->show_untracked_files)
>  		DIFF_OPT_SET(&rev.diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
>  	rev.diffopt.format_callback = wt_status_collect_changed_cb;
Alex Riesen· May 20, 2010, 19:34 UTC · re: Jens Lehmann · lore

Re: [PATCH] Don't do status in submodules if status.SubmoduleSummary unset

On Thu, May 20, 2010 at 19:45, Jens Lehmann <Jens.Lehmann@web.de> wrote:
Show 6 quoted lines
> Am 20.05.2010 16:12, schrieb Alex Riesen:
>> Maybe because we do a (kind of) gentle status run on submodules
>> whether the status.SubmoduleSummary set or not.
>
> Yup, because checking a submodule for its dirtiness has to be done
> no matter if the summary output is also wanted.
Yeah. Why?
Show 8 quoted lines
>> Usually a background
>> run of "git status" for every submodules goes unnoticed, just
>> sometimes a submodule is a little too big.
>>
>> I tried this, but feels like a bit of overkill.
>
> This patch seems to disable submodule output completely for the default
> case (when status.SubmoduleSummary is false) and breaks 17 test cases.
That's why I said it feels like overkill
Jens Lehmann· May 20, 2010, 20:14 UTC · re: Alex Riesen · lore

Re: [PATCH] Don't do status in submodules if status.SubmoduleSummary unset

Am 20.05.2010 21:34, schrieb Alex Riesen:
Show 9 quoted lines
> On Thu, May 20, 2010 at 19:45, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>> Am 20.05.2010 16:12, schrieb Alex Riesen:
>>> Maybe because we do a (kind of) gentle status run on submodules
>>> whether the status.SubmoduleSummary set or not.
>>
>> Yup, because checking a submodule for its dirtiness has to be done
>> no matter if the summary output is also wanted.
> 
> Yeah. Why?

Because summary output only describes what commits happened in the submodule (that operation is rather cheap). The status run is done to tell what changes in the submodules work tree have occurred since the last commit there (and for that we have to scan the whole tree).

Show 10 quoted lines
>>> Usually a background
>>> run of "git status" for every submodules goes unnoticed, just
>>> sometimes a submodule is a little too big.
>>>
>>> I tried this, but feels like a bit of overkill.
>>
>> This patch seems to disable submodule output completely for the default
>> case (when status.SubmoduleSummary is false) and breaks 17 test cases.
> 
> That's why I said it feels like overkill
I just wanted to confirm your feeling ;-)

← back to recent threads