From 85556d4e37db4c8ed88c68a602f6f85054996bea Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 1 Sep 2012 07:27:35 -0400 Subject: fetch: use argv_array instead of hand-building arrays Fetch invokes itself recursively when recursing into submodules or handling "fetch --multiple". In both cases, it builds the child's command line by pushing options onto a statically-sized array. In both cases, the array is currently just big enough to handle the largest possible case. However, this technique is brittle and error-prone, so let's replace it with a dynamic argv_array. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/fetch.c | 47 +++++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 22 deletions(-) (limited to 'builtin/fetch.c') diff --git a/builtin/fetch.c b/builtin/fetch.c index 1c8cb62445..d5442e1eb7 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -14,6 +14,7 @@ #include "transport.h" #include "submodule.h" #include "connected.h" +#include "argv-array.h" static const char * const builtin_fetch_usage[] = { "git fetch [] [ [...]]", @@ -841,38 +842,35 @@ static int add_remote_or_group(const char *name, struct string_list *list) return 1; } -static void add_options_to_argv(int *argc, const char **argv) +static void add_options_to_argv(struct argv_array *argv) { if (dry_run) - argv[(*argc)++] = "--dry-run"; + argv_array_push(argv, "--dry-run"); if (prune) - argv[(*argc)++] = "--prune"; + argv_array_push(argv, "--prune"); if (update_head_ok) - argv[(*argc)++] = "--update-head-ok"; + argv_array_push(argv, "--update-head-ok"); if (force) - argv[(*argc)++] = "--force"; + argv_array_push(argv, "--force"); if (keep) - argv[(*argc)++] = "--keep"; + argv_array_push(argv, "--keep"); if (recurse_submodules == RECURSE_SUBMODULES_ON) - argv[(*argc)++] = "--recurse-submodules"; + argv_array_push(argv, "--recurse-submodules"); else if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND) - argv[(*argc)++] = "--recurse-submodules=on-demand"; + argv_array_push(argv, "--recurse-submodules=on-demand"); if (verbosity >= 2) - argv[(*argc)++] = "-v"; + argv_array_push(argv, "-v"); if (verbosity >= 1) - argv[(*argc)++] = "-v"; + argv_array_push(argv, "-v"); else if (verbosity < 0) - argv[(*argc)++] = "-q"; + argv_array_push(argv, "-q"); } static int fetch_multiple(struct string_list *list) { int i, result = 0; - const char *argv[12] = { "fetch", "--append" }; - int argc = 2; - - add_options_to_argv(&argc, argv); + struct argv_array argv = ARGV_ARRAY_INIT; if (!append && !dry_run) { int errcode = truncate_fetch_head(); @@ -880,18 +878,22 @@ static int fetch_multiple(struct string_list *list) return errcode; } + argv_array_pushl(&argv, "fetch", "--append", NULL); + add_options_to_argv(&argv); + for (i = 0; i < list->nr; i++) { const char *name = list->items[i].string; - argv[argc] = name; - argv[argc + 1] = NULL; + argv_array_push(&argv, name); if (verbosity >= 0) printf(_("Fetching %s\n"), name); - if (run_command_v_opt(argv, RUN_GIT_CMD)) { + if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) { error(_("Could not fetch %s"), name); result = 1; } + argv_array_pop(&argv); } + argv_array_clear(&argv); return result; } @@ -1007,13 +1009,14 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) } if (!result && (recurse_submodules != RECURSE_SUBMODULES_OFF)) { - const char *options[10]; - int num_options = 0; - add_options_to_argv(&num_options, options); - result = fetch_populated_submodules(num_options, options, + struct argv_array options = ARGV_ARRAY_INIT; + + add_options_to_argv(&options); + result = fetch_populated_submodules(options.argc, options.argv, submodule_prefix, recurse_submodules, verbosity < 0); + argv_array_clear(&options); } /* All names were strdup()ed or strndup()ed */ -- cgit v1.2.1 From 50d89ad6542c8acafefa6d42f8b42dfa9b8fafe1 Mon Sep 17 00:00:00 2001 From: Jens Lehmann Date: Sat, 1 Sep 2012 17:27:06 +0200 Subject: submodule: use argv_array instead of hand-building arrays fetch_populated_submodules() allocates the full argv array it uses to recurse into the submodules from the number of given options plus the six argv values it is going to add. It then initializes it with those values which won't change during the iteration and copies the given options into it. Inside the loop the two argv values different for each submodule get replaced with those currently valid. However, this technique is brittle and error-prone (as the comment to explain the magic number 6 indicates), so let's replace it with an argv_array. Instead of replacing the argv values, push them to the argv_array just before the run_command() call (including the option separating them) and pop them from the argv_array right after that. Signed-off-by: Jens Lehmann Signed-off-by: Junio C Hamano --- builtin/fetch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'builtin/fetch.c') diff --git a/builtin/fetch.c b/builtin/fetch.c index d5442e1eb7..6196e91798 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1012,7 +1012,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) struct argv_array options = ARGV_ARRAY_INIT; add_options_to_argv(&options); - result = fetch_populated_submodules(options.argc, options.argv, + result = fetch_populated_submodules(&options, submodule_prefix, recurse_submodules, verbosity < 0); -- cgit v1.2.1 From 85566460897ee76555a7c90a951fe2d50272eb5e Mon Sep 17 00:00:00 2001 From: Dan Johnson Date: Wed, 5 Sep 2012 17:22:19 -0400 Subject: fetch --all: pass --tags/--no-tags through to each remote When fetch is invoked with --all, we need to pass the tag-following preference to each individual fetch; without this, we will always auto-follow tags, preventing us from fetching the remote tags into a remote-specific namespace, for example. Reported-by: Oswald Buddenhagen Signed-off-by: Dan Johnson Signed-off-by: Junio C Hamano --- builtin/fetch.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'builtin/fetch.c') diff --git a/builtin/fetch.c b/builtin/fetch.c index 6196e91798..4494aed0c7 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -858,6 +858,10 @@ static void add_options_to_argv(struct argv_array *argv) argv_array_push(argv, "--recurse-submodules"); else if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND) argv_array_push(argv, "--recurse-submodules=on-demand"); + if (tags == TAGS_SET) + argv_array_push(argv, "--tags"); + else if (tags == TAGS_UNSET) + argv_array_push(argv, "--no-tags"); if (verbosity >= 2) argv_array_push(argv, "-v"); if (verbosity >= 1) -- cgit v1.2.1