threads / patch / 37347

patch, 2 partsfetch: silence git-gc if --quiet is given

Subject: [PATCH 2/2] fetch: silence git-gc if --quiet is given

## tl;dr

7 messages between Aug 14, 2014 and Aug 16, 2014. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

Nguyễn Thái Ngọc Duy· Aug 14, 2014, 11:51 UTC · lore

[PATCH 1/2] fetch: convert argv_gc_auto to struct argv_array

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/fetch.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
Show changes to builtin/fetch.c +3 −4
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e8d0cca..9394194 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1110,9 +1110,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	struct string_list list = STRING_LIST_INIT_NODUP;
 	struct remote *remote;
 	int result = 0;
-	static const char *argv_gc_auto[] = {
-		"gc", "--auto", NULL,
-	};
+	struct argv_array argv_gc_auto = ARGV_ARRAY_INIT;
 
 	packet_trace_identity("fetch");
 
@@ -1198,7 +1196,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	list.strdup_strings = 1;
 	string_list_clear(&list, 0);
 
-	run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+	argv_array_pushl(&argv_gc_auto, "gc", "--auto", NULL);
+	run_command_v_opt(argv_gc_auto.argv, RUN_GIT_CMD);
 
 	return result;
 }

-- 
2.1.0.rc0.78.gc0d8480
Nguyễn Thái Ngọc Duy· Aug 14, 2014, 11:51 UTC · re: Nguyễn Thái Ngọc Duy · lore
Noticed-by: Matthew Flaschen <mflaschen@wikimedia.org>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/fetch.c | 2 ++
 1 file changed, 2 insertions(+)
Show changes to builtin/fetch.c +2 −0
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 9394194..4ff4080 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1197,6 +1197,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	string_list_clear(&list, 0);
 
 	argv_array_pushl(&argv_gc_auto, "gc", "--auto", NULL);
+	if (verbosity < 0)
+		argv_array_push(&argv_gc_auto,"--quiet");
 	run_command_v_opt(argv_gc_auto.argv, RUN_GIT_CMD);
 
 	return result;

-- 
2.1.0.rc0.78.gc0d8480
Jeff King· Aug 14, 2014, 19:56 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 2/2] fetch: silence git-gc if --quiet is given

On Thu, Aug 14, 2014 at 06:51:05PM +0700, Nguyễn Thái Ngọc Duy wrote:
Show 17 quoted lines
> Noticed-by: Matthew Flaschen <mflaschen@wikimedia.org>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/fetch.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 9394194..4ff4080 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1197,6 +1197,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>  	string_list_clear(&list, 0);
>  
>  	argv_array_pushl(&argv_gc_auto, "gc", "--auto", NULL);
> +	if (verbosity < 0)
> +		argv_array_push(&argv_gc_auto,"--quiet");
>  	run_command_v_opt(argv_gc_auto.argv, RUN_GIT_CMD);

I think this is a fine fix for this specific problem, and we should apply it. But I do wonder if it would be simpler in the long run to treat verbosity as a global option, and pass it around via a GIT_QUIET (or GIT_VERBOSITY) environment variable.

I would not be surprised at all to find that there are other cases where sub-programs do not respect the parent verbosity (I know we have had problems with progress reporting flags carried over the transport interface in the past, but I think we fixed all of those).

-Peff
Duy Nguyen· Aug 15, 2014, 12:09 UTC · re: Jeff King · lore

Re: [PATCH 2/2] fetch: silence git-gc if --quiet is given

On Fri, Aug 15, 2014 at 2:56 AM, Jeff King <peff@peff.net> wrote:
Show 9 quoted lines
> I think this is a fine fix for this specific problem, and we should
> apply it. But I do wonder if it would be simpler in the long run to
> treat verbosity as a global option, and pass it around via a GIT_QUIET
> (or GIT_VERBOSITY) environment variable.
>
> I would not be surprised at all to find that there are other cases where
> sub-programs do not respect the parent verbosity (I know we have had
> problems with progress reporting flags carried over the transport
> interface in the past, but I think we fixed all of those).

I don't see any easy way to make everybody aware of $GIT_QUIET. But the idea is nice.

-- 
Duy
Jeff King· Aug 14, 2014, 19:53 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 1/2] fetch: convert argv_gc_auto to struct argv_array

On Thu, Aug 14, 2014 at 06:51:04PM +0700, Nguyễn Thái Ngọc Duy wrote:
Show 14 quoted lines
> -	static const char *argv_gc_auto[] = {
> -		"gc", "--auto", NULL,
> -	};
> +	struct argv_array argv_gc_auto = ARGV_ARRAY_INIT;
>  
>  	packet_trace_identity("fetch");
>  
> @@ -1198,7 +1196,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>  	list.strdup_strings = 1;
>  	string_list_clear(&list, 0);
>  
> -	run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
> +	argv_array_pushl(&argv_gc_auto, "gc", "--auto", NULL);
> +	run_command_v_opt(argv_gc_auto.argv, RUN_GIT_CMD);
argv_array_clear() here afterwards?

Not a huge deal since we are about to exit, but in the name of good hygiene.

-Peff
Nguyễn Thái Ngọc Duy· Aug 16, 2014, 01:19 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v2 1/2] fetch: convert argv_gc_auto to struct argv_array

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/fetch.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
Show changes to builtin/fetch.c +4 −4
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e8d0cca..5f06114 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1110,9 +1110,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	struct string_list list = STRING_LIST_INIT_NODUP;
 	struct remote *remote;
 	int result = 0;
-	static const char *argv_gc_auto[] = {
-		"gc", "--auto", NULL,
-	};
+	struct argv_array argv_gc_auto = ARGV_ARRAY_INIT;
 
 	packet_trace_identity("fetch");
 
@@ -1198,7 +1196,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	list.strdup_strings = 1;
 	string_list_clear(&list, 0);
 
-	run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+	argv_array_pushl(&argv_gc_auto, "gc", "--auto", NULL);
+	run_command_v_opt(argv_gc_auto.argv, RUN_GIT_CMD);
+	argv_array_clear(&argv_gc_auto);
 
 	return result;
 }

-- 
2.1.0.rc0.78.gc0d8480
Nguyễn Thái Ngọc Duy· Aug 16, 2014, 01:19 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v2 2/2] fetch: silence git-gc if --quiet is given

Noticed-by: Matthew Flaschen <mflaschen@wikimedia.org>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/fetch.c | 2 ++
 1 file changed, 2 insertions(+)
Show changes to builtin/fetch.c +2 −0
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 5f06114..159fb7e 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1197,6 +1197,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	string_list_clear(&list, 0);
 
 	argv_array_pushl(&argv_gc_auto, "gc", "--auto", NULL);
+	if (verbosity < 0)
+		argv_array_push(&argv_gc_auto, "--quiet");
 	run_command_v_opt(argv_gc_auto.argv, RUN_GIT_CMD);
 	argv_array_clear(&argv_gc_auto);
 

-- 
2.1.0.rc0.78.gc0d8480

← back to recent threads