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

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

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/fetch.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

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

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

[Nguyễn Thái Ngọc Duy]

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);
 
 	return result;
-- 
2.1.0.rc0.78.gc0d8480

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

[Jeff King]

On Thu, Aug 14, 2014 at 06:51:04PM +0700, Nguyễn Thái Ngọc Duy wrote:

> -	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

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

[Jeff King]

On Thu, Aug 14, 2014 at 06:51:05PM +0700, Nguyễn Thái Ngọc Duy wrote:

> 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

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

[Duy Nguyen]

On Fri, Aug 15, 2014 at 2:56 AM, Jeff King <peff@peff.net> wrote:
> 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

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

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/fetch.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

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

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

[Nguyễn Thái Ngọc Duy]

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 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