{"thread":{"id":"40201","subject":"[PATCH 4/9] thread-utils: add a threaded task queue","startedAt":"2015-08-28T01:14:46Z","lastAt":"2015-08-31T19:05:26Z","messageCount":33,"participants":["Stefan Beller","Johannes Schindelin","Jonathan Nieder","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":9},"messages":[{"id":"268814","messageId":"1440724495-708-1-git-send-email-sbeller@google.com","threadId":"40201","inReplyTo":null,"subject":"[PATCH 0/9] Progress with git submodule","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T01:14:46Z","receivedAt":"2015-08-28T01:14:46Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This series replaces origin/sb/submodule-helper and is based on\n5a1ba6b48a62bf55f9c8305d9850c3a8d22365c5, (Merge 'hv/submodule-config' to\n'sb/submodule-helper', which includes jk/git-path and hv/submodule-config)\n\nWhat changed?\n* The help text of the submodule--helper was adapted to our standard,\n and removed long line, outdated docs, thanks Dscho!\n \n* The run-command API was extended to be able to sync the output from\n various commands running at the same time.\n (In the previous series this was a one-shot only solution cooked for\n git submodule foreach_parallel)\n \n* `git fetch recurse-submodules` will fetch the submodules in parallel!\n\nAny feedback welcome,\nspecially on patch 4..7\n\nThanks,\nStefan\n\nStefan Beller (9):\n submodule: implement `module_list` as a builtin helper\n submodule: implement `module_name` as a builtin helper\n submodule: implement `module_clone` as a builtin helper\n thread-utils: add a threaded task queue\n run-command: add synced output\n submodule: helper to run foreach in parallel\n fetch: fetch submodules in parallel\n index-pack: Use the new worker pool\n pack-objects: Use new worker pool\n\n .gitignore | 1 +\n Documentation/fetch-options.txt | 7 +\n Makefile | 1 +\n builtin.h | 1 +\n builtin/fetch.c | 6 +-\n builtin/index-pack.c | 23 +--\n builtin/pack-objects.c | 175 ++++++-----------\n builtin/pull.c | 6 +\n builtin/submodule--helper.c | 417 ++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh | 177 +++--------------\n git.c | 1 +\n run-command.c | 99 ++++++++--\n run-command.h | 21 ++\n submodule.c | 100 ++++++++--\n submodule.h | 2 +-\n t/t7407-submodule-foreach.sh | 11 ++\n thread-utils.c | 192 ++++++++++++++++++\n thread-utils.h | 35 ++++\n 18 files changed, 960 insertions(+), 315 deletions(-)\n create mode 100644 builtin/submodule--helper.c\n\n-- \n2.5.0.264.g5e52b0d\n"},{"id":"268812","messageId":"1440724495-708-2-git-send-email-sbeller@google.com","threadId":"40201","inReplyTo":"1440724495-708-1-git-send-email-sbeller@google.com","subject":"[PATCH 1/9] submodule: implement `module_list` as a builtin helper","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T01:14:47Z","receivedAt":"2015-08-28T01:14:47Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Most of the submodule operations work on a set of submodules.\nCalculating and using this set is usually done via:\n\n module_list \"$@\" | {\n while read mode sha1 stage sm_path\n do\n # the actual operation\n done\n }\n\nCurrently the function `module_list` is implemented in the\ngit-submodule.sh as a shell script wrapping a perl script.\nThe rewrite is in C, such that it is faster and can later be\neasily adapted when other functions are rewritten in C.\n\ngit-submodule.sh similar to the builtin commands will navigate\nto the top most directory of the repository and keeping the\nsubdirectories as a variable. As the helper is called from\nwithin the git-submodule.sh script, we are already navigated\nto the root level, but the path arguments are stil relative\nto the subdirectory we were in when calling git-submodule.sh.\nThat's why there is a `--prefix` option pointing to an alternative\npath where to anchor relative path arguments.\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n .gitignore | 1 +\n Makefile | 1 +\n builtin.h | 1 +\n builtin/submodule--helper.c | 114 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh | 54 +++------------------\n git.c | 1 +\n 6 files changed, 124 insertions(+), 48 deletions(-)\n create mode 100644 builtin/submodule--helper.c\n\ndiff --git a/.gitignore b/.gitignore\nindex 4fd81ba..1c2f832 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -155,6 +155,7 @@\n /git-status\n /git-stripspace\n /git-submodule\n+/git-submodule--helper\n /git-svn\n /git-symbolic-ref\n /git-tag\ndiff --git a/Makefile b/Makefile\nindex 24b636d..d434e73 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -901,6 +901,7 @@ BUILTIN_OBJS += builtin/shortlog.o\n BUILTIN_OBJS += builtin/show-branch.o\n BUILTIN_OBJS += builtin/show-ref.o\n BUILTIN_OBJS += builtin/stripspace.o\n+BUILTIN_OBJS += builtin/submodule--helper.o\n BUILTIN_OBJS += builtin/symbolic-ref.o\n BUILTIN_OBJS += builtin/tag.o\n BUILTIN_OBJS += builtin/unpack-file.o\ndiff --git a/builtin.h b/builtin.h\nindex 839483d..924e6c4 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -119,6 +119,7 @@ extern int cmd_show(int argc, const char **argv, const char *prefix);\n extern int cmd_show_branch(int argc, const char **argv, const char *prefix);\n extern int cmd_status(int argc, const char **argv, const char *prefix);\n extern int cmd_stripspace(int argc, const char **argv, const char *prefix);\n+extern int cmd_submodule__helper(int argc, const char **argv, const char *prefix);\n extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);\n extern int cmd_tag(int argc, const char **argv, const char *prefix);\n extern int cmd_tar_tree(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nnew file mode 100644\nindex 0000000..beaab7d\n--- /dev/null\n+++ b/builtin/submodule--helper.c\n@@ -0,0 +1,114 @@\n+#include \"builtin.h\"\n+#include \"cache.h\"\n+#include \"parse-options.h\"\n+#include \"quote.h\"\n+#include \"pathspec.h\"\n+#include \"dir.h\"\n+#include \"utf8.h\"\n+\n+static const struct cache_entry **ce_entries;\n+static int ce_alloc, ce_used;\n+static const char *alternative_path;\n+\n+static int module_list_compute(int argc, const char **argv,\n+\t\t\t\tconst char *prefix,\n+\t\t\t\tstruct pathspec *pathspec)\n+{\n+\tint i;\n+\tchar *max_prefix, *ps_matched = NULL;\n+\tint max_prefix_len;\n+\tparse_pathspec(pathspec, 0,\n+\t\t PATHSPEC_PREFER_FULL |\n+\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,\n+\t\t prefix, argv);\n+\n+\t/* Find common prefix for all pathspec's */\n+\tmax_prefix = common_prefix(pathspec);\n+\tmax_prefix_len = max_prefix ? strlen(max_prefix) : 0;\n+\n+\tif (pathspec->nr)\n+\t\tps_matched = xcalloc(pathspec->nr, 1);\n+\n+\tif (read_cache() < 0)\n+\t\tdie(\"index file corrupt\");\n+\n+\tfor (i = 0; i < active_nr; i++) {\n+\t\tconst struct cache_entry *ce = active_cache[i];\n+\n+\t\tif (!match_pathspec(pathspec, ce->name, ce_namelen(ce),\n+\t\t\t\t max_prefix_len, ps_matched,\n+\t\t\t\t S_ISGITLINK(ce->ce_mode) | S_ISDIR(ce->ce_mode)))\n+\t\t\tcontinue;\n+\n+\t\tif (S_ISGITLINK(ce->ce_mode)) {\n+\t\t\tALLOC_GROW(ce_entries, ce_used + 1, ce_alloc);\n+\t\t\tce_entries[ce_used++] = ce;\n+\t\t}\n+\n+\t\twhile (i + 1 < active_nr && !strcmp(ce->name, active_cache[i + 1]->name))\n+\t\t\t/*\n+\t\t\t * Skip entries with the same name in different stages\n+\t\t\t * to make sure an entry is returned only once.\n+\t\t\t */\n+\t\t\ti++;\n+\t}\n+\tfree(max_prefix);\n+\n+\tif (ps_matched && report_path_error(ps_matched, pathspec, prefix))\n+\t\treturn -1;\n+\n+\treturn 0;\n+}\n+\n+static int module_list(int argc, const char **argv, const char *prefix)\n+{\n+\tint i;\n+\tstatic struct pathspec pathspec;\n+\n+\tstruct option module_list_options[] = {\n+\t\tOPT_STRING(0, \"prefix\", &alternative_path,\n+\t\t\t N_(\"path\"),\n+\t\t\t N_(\"alternative anchor for relative paths\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tstatic const char * const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper module_list [--prefix=] [...]\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_list_options,\n+\t\t\t git_submodule_helper_usage, 0);\n+\n+\tif (module_list_compute(argc, argv, alternative_path\n+\t\t\t\t\t ? alternative_path\n+\t\t\t\t\t : prefix, &pathspec) < 0) {\n+\t\tprintf(\"#unmatched\\n\");\n+\t\treturn 1;\n+\t}\n+\n+\tfor (i = 0; i < ce_used; i++) {\n+\t\tconst struct cache_entry *ce = ce_entries[i];\n+\n+\t\tif (ce_stage(ce)) {\n+\t\t\tprintf(\"%06o %s U\\t\", ce->ce_mode, sha1_to_hex(null_sha1));\n+\t\t} else {\n+\t\t\tprintf(\"%06o %s %d\\t\", ce->ce_mode, sha1_to_hex(ce->sha1), ce_stage(ce));\n+\t\t}\n+\n+\t\tutf8_fprintf(stdout, \"%s\\n\", ce->name);\n+\t}\n+\treturn 0;\n+}\n+\n+int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n+{\n+\tif (argc < 2)\n+\t\tgoto usage;\n+\n+\tif (!strcmp(argv[1], \"module_list\"))\n+\t\treturn module_list(argc - 1, argv + 1, prefix);\n+\n+usage:\n+\tusage(\"git submodule--helper module_list\\n\");\n+}\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 36797c3..af9ecef 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -145,48 +145,6 @@ relative_path ()\n \techo \"$result$target\"\n }\n \n-#\n-# Get submodule info for registered submodules\n-# $@ = path to limit submodule list\n-#\n-module_list()\n-{\n-\teval \"set $(git rev-parse --sq --prefix \"$wt_prefix\" -- \"$@\")\"\n-\t(\n-\t\tgit ls-files -z --error-unmatch --stage -- \"$@\" ||\n-\t\techo \"unmatched pathspec exists\"\n-\t) |\n-\t@@PERL@@ -e '\n-\tmy %unmerged = ();\n-\tmy ($null_sha1) = (\"0\" x 40);\n-\tmy @out = ();\n-\tmy $unmatched = 0;\n-\t$/ = \"\\0\";\n-\twhile () {\n-\t\tif (/^unmatched pathspec/) {\n-\t\t\t$unmatched = 1;\n-\t\t\tnext;\n-\t\t}\n-\t\tchomp;\n-\t\tmy ($mode, $sha1, $stage, $path) =\n-\t\t\t/^([0-7]+) ([0-9a-f]{40}) ([0-3])\\t(.*)$/;\n-\t\tnext unless $mode eq \"160000\";\n-\t\tif ($stage ne \"0\") {\n-\t\t\tif (!$unmerged{$path}++) {\n-\t\t\t\tpush @out, \"$mode $null_sha1 U\\t$path\\n\";\n-\t\t\t}\n-\t\t\tnext;\n-\t\t}\n-\t\tpush @out, \"$_\\n\";\n-\t}\n-\tif ($unmatched) {\n-\t\tprint \"#unmatched\\n\";\n-\t} else {\n-\t\tprint for (@out);\n-\t}\n-\t'\n-}\n-\n die_if_unmatched ()\n {\n \tif test \"$1\" = \"#unmatched\"\n@@ -532,7 +490,7 @@ cmd_foreach()\n \t# command in the subshell (and a recursive call to this function)\n \texec 3<&0\n \n-\tmodule_list |\n+\tgit submodule--helper module_list --prefix \"$wt_prefix\"|\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n@@ -592,7 +550,7 @@ cmd_init()\n \t\tshift\n \tdone\n \n-\tmodule_list \"$@\" |\n+\tgit submodule--helper module_list --prefix \"$wt_prefix\" \"$@\" |\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n@@ -674,7 +632,7 @@ cmd_deinit()\n \t\tdie \"$(eval_gettext \"Use '.' if you really want to deinitialize all submodules\")\"\n \tfi\n \n-\tmodule_list \"$@\" |\n+\tgit submodule--helper module_list --prefix \"$wt_prefix\" \"$@\" |\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n@@ -790,7 +748,7 @@ cmd_update()\n \tfi\n \n \tcloned_modules=\n-\tmodule_list \"$@\" | {\n+\tgit submodule--helper module_list --prefix \"$wt_prefix\" \"$@\" | {\n \terr=\n \twhile read mode sha1 stage sm_path\n \tdo\n@@ -1222,7 +1180,7 @@ cmd_status()\n \t\tshift\n \tdone\n \n-\tmodule_list \"$@\" |\n+\tgit submodule--helper module_list --prefix \"$wt_prefix\" \"$@\" |\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n@@ -1299,7 +1257,7 @@ cmd_sync()\n \t\tesac\n \tdone\n \tcd_to_toplevel\n-\tmodule_list \"$@\" |\n+\tgit submodule--helper module_list --prefix \"$wt_prefix\" \"$@\" |\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\ndiff --git a/git.c b/git.c\nindex 55c327c..deecba0 100644\n--- a/git.c\n+++ b/git.c\n@@ -469,6 +469,7 @@ static struct cmd_struct commands[] = {\n \t{ \"stage\", cmd_add, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"status\", cmd_status, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"stripspace\", cmd_stripspace },\n+\t{ \"submodule--helper\", cmd_submodule__helper, RUN_SETUP },\n \t{ \"symbolic-ref\", cmd_symbolic_ref, RUN_SETUP },\n \t{ \"tag\", cmd_tag, RUN_SETUP },\n \t{ \"unpack-file\", cmd_unpack_file, RUN_SETUP },\n-- \n2.5.0.264.g5e52b0d\n"},{"id":"268818","messageId":"1440724495-708-3-git-send-email-sbeller@google.com","threadId":"40201","inReplyTo":"1440724495-708-1-git-send-email-sbeller@google.com","subject":"[PATCH 2/9] submodule: implement `module_name` as a builtin helper","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T01:14:48Z","receivedAt":"2015-08-28T01:14:48Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This implements the helper `module_name` in C instead of shell,\nyielding a nice performance boost.\n\nBefore this patch, I measured a time (best out of three):\n\n $ time ./t7400-submodule-basic.sh >/dev/null\n real\t0m11.066s\n user\t0m3.348s\n sys\t0m8.534s\n\nWith this patch applied I measured (also best out of three)\n\n $ time ./t7400-submodule-basic.sh >/dev/null\n real\t0m10.063s\n user\t0m3.044s\n sys\t0m7.487s\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n builtin/submodule--helper.c | 28 +++++++++++++++++++++++++++-\n git-submodule.sh | 32 +++++++-------------------------\n 2 files changed, 34 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex beaab7d..c8f7e0c 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -5,6 +5,9 @@\n #include \"pathspec.h\"\n #include \"dir.h\"\n #include \"utf8.h\"\n+#include \"submodule.h\"\n+#include \"submodule-config.h\"\n+#include \"string-list.h\"\n \n static const struct cache_entry **ce_entries;\n static int ce_alloc, ce_used;\n@@ -101,6 +104,26 @@ static int module_list(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+static int module_name(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *name;\n+\tconst struct submodule *sub;\n+\n+\tif (argc != 1)\n+\t\tusage(\"git submodule--helper module_name \\n\");\n+\n+\tgitmodules_config();\n+\tsub = submodule_from_path(null_sha1, argv[0]);\n+\n+\tif (!sub)\n+\t\tdie(\"No submodule mapping found in .gitmodules for path '%s'\", argv[0]);\n+\n+\tname = sub->name;\n+\tprintf(\"%s\\n\", name);\n+\n+\treturn 0;\n+}\n+\n int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n {\n \tif (argc < 2)\n@@ -109,6 +132,9 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(argv[1], \"module_list\"))\n \t\treturn module_list(argc - 1, argv + 1, prefix);\n \n+\tif (!strcmp(argv[1], \"module_name\"))\n+\t\treturn module_name(argc - 2, argv + 2, prefix);\n+\n usage:\n-\tusage(\"git submodule--helper module_list\\n\");\n+\tusage(\"git submodule--helper [module_list | module_name]\\n\");\n }\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex af9ecef..e6ff38d 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -178,24 +178,6 @@ get_submodule_config () {\n \tprintf '%s' \"${value:-$default}\"\n }\n \n-\n-#\n-# Map submodule path to submodule name\n-#\n-# $1 = path\n-#\n-module_name()\n-{\n-\t# Do we have \"submodule..path = $1\" defined in .gitmodules file?\n-\tsm_path=\"$1\"\n-\tre=$(printf '%s\\n' \"$1\" | sed -e 's/[].[^$\\\\*]/\\\\&/g')\n-\tname=$( git config -f .gitmodules --get-regexp '^submodule\\..*\\.path$' |\n-\t\tsed -n -e 's|^submodule\\.\\(.*\\)\\.path '\"$re\"'$|\\1|p' )\n-\ttest -z \"$name\" &&\n-\tdie \"$(eval_gettext \"No submodule mapping found in .gitmodules for path '\\$sm_path'\")\"\n-\tprintf '%s\\n' \"$name\"\n-}\n-\n #\n # Clone a submodule\n #\n@@ -498,7 +480,7 @@ cmd_foreach()\n \t\tthen\n \t\t\tdisplaypath=$(relative_path \"$sm_path\")\n \t\t\tsay \"$(eval_gettext \"Entering '\\$prefix\\$displaypath'\")\"\n-\t\t\tname=$(module_name \"$sm_path\")\n+\t\t\tname=$(git submodule--helper module_name \"$sm_path\")\n \t\t\t(\n \t\t\t\tprefix=\"$prefix$sm_path/\"\n \t\t\t\tclear_local_git_env\n@@ -554,7 +536,7 @@ cmd_init()\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n-\t\tname=$(module_name \"$sm_path\") || exit\n+\t\tname=$(git submodule--helper module_name \"$sm_path\") || exit\n \n \t\tdisplaypath=$(relative_path \"$sm_path\")\n \n@@ -636,7 +618,7 @@ cmd_deinit()\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n-\t\tname=$(module_name \"$sm_path\") || exit\n+\t\tname=$(git submodule--helper module_name \"$sm_path\") || exit\n \n \t\tdisplaypath=$(relative_path \"$sm_path\")\n \n@@ -758,7 +740,7 @@ cmd_update()\n \t\t\techo >&2 \"Skipping unmerged submodule $prefix$sm_path\"\n \t\t\tcontinue\n \t\tfi\n-\t\tname=$(module_name \"$sm_path\") || exit\n+\t\tname=$(git submodule--helper module_name \"$sm_path\") || exit\n \t\turl=$(git config submodule.\"$name\".url)\n \t\tbranch=$(get_submodule_config \"$name\" branch master)\n \t\tif ! test -z \"$update\"\n@@ -1022,7 +1004,7 @@ cmd_summary() {\n \t\t\t# Respect the ignore setting for --for-status.\n \t\t\tif test -n \"$for_status\"\n \t\t\tthen\n-\t\t\t\tname=$(module_name \"$sm_path\")\n+\t\t\t\tname=$(git submodule--helper module_name \"$sm_path\")\n \t\t\t\tignore_config=$(get_submodule_config \"$name\" ignore none)\n \t\t\t\ttest $status != A && test $ignore_config = all && continue\n \t\t\tfi\n@@ -1184,7 +1166,7 @@ cmd_status()\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n-\t\tname=$(module_name \"$sm_path\") || exit\n+\t\tname=$(git submodule--helper module_name \"$sm_path\") || exit\n \t\turl=$(git config submodule.\"$name\".url)\n \t\tdisplaypath=$(relative_path \"$prefix$sm_path\")\n \t\tif test \"$stage\" = U\n@@ -1261,7 +1243,7 @@ cmd_sync()\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n-\t\tname=$(module_name \"$sm_path\")\n+\t\tname=$(git submodule--helper module_name \"$sm_path\")\n \t\turl=$(git config -f .gitmodules --get submodule.\"$name\".url)\n \n \t\t# Possibly a url relative to parent\n-- \n2.5.0.264.g5e52b0d\n"},{"id":"268810","messageId":"1440724495-708-4-git-send-email-sbeller@google.com","threadId":"40201","inReplyTo":"1440724495-708-1-git-send-email-sbeller@google.com","subject":"[PATCH 3/9] submodule: implement `module_clone` as a builtin helper","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T01:14:49Z","receivedAt":"2015-08-28T01:14:49Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"`module_clone` is part of the update command,\nwhich I want to convert to C next.\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n builtin/submodule--helper.c | 148 +++++++++++++++++++++++++++++++++++++++++++-\n git-submodule.sh | 80 +-----------------------\n 2 files changed, 150 insertions(+), 78 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex c8f7e0c..d29499c 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -8,6 +8,7 @@\n #include \"submodule.h\"\n #include \"submodule-config.h\"\n #include \"string-list.h\"\n+#include \"run-command.h\"\n \n static const struct cache_entry **ce_entries;\n static int ce_alloc, ce_used;\n@@ -124,6 +125,147 @@ static int module_name(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+static int clone_submodule(const char *path, const char *gitdir, const char *url,\n+\t\t\t const char *depth, const char *reference, int quiet)\n+{\n+\tstruct child_process cp;\n+\tchild_process_init(&cp);\n+\n+\targv_array_push(&cp.args, \"clone\");\n+\targv_array_push(&cp.args, \"--no-checkout\");\n+\tif (quiet)\n+\t\targv_array_push(&cp.args, \"--quiet\");\n+\tif (depth && strcmp(depth, \"\")) {\n+\t\targv_array_push(&cp.args, \"--depth\");\n+\t\targv_array_push(&cp.args, depth);\n+\t}\n+\tif (reference && strcmp(reference, \"\")) {\n+\t\targv_array_push(&cp.args, \"--reference\");\n+\t\targv_array_push(&cp.args, reference);\n+\t}\n+\tif (gitdir) {\n+\t\targv_array_push(&cp.args, \"--separate-git-dir\");\n+\t\targv_array_push(&cp.args, gitdir);\n+\t}\n+\targv_array_push(&cp.args, url);\n+\targv_array_push(&cp.args, path);\n+\n+\tcp.git_cmd = 1;\n+\tcp.env = local_repo_env;\n+\n+\tcp.no_stdin = 1;\n+\tcp.no_stdout = 1;\n+\tcp.no_stderr = 1;\n+\n+\treturn run_command(&cp);\n+}\n+\n+static int module_clone(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *path = NULL, *name = NULL, *url = NULL;\n+\tconst char *reference = NULL, *depth = NULL;\n+\tint quiet = 0;\n+\tFILE *submodule_dot_git;\n+\tconst char *sm_gitdir, *p;\n+\tstruct strbuf rel_path = STRBUF_INIT;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tstruct option module_update_options[] = {\n+\t\tOPT_STRING(0, \"prefix\", &alternative_path,\n+\t\t\t N_(\"path\"),\n+\t\t\t N_(\"alternative anchor for relative paths\")),\n+\t\tOPT_STRING(0, \"path\", &path,\n+\t\t\t N_(\"path\"),\n+\t\t\t N_(\"where the new submodule will be cloned to\")),\n+\t\tOPT_STRING(0, \"name\", &name,\n+\t\t\t N_(\"string\"),\n+\t\t\t N_(\"name of the new submodule\")),\n+\t\tOPT_STRING(0, \"url\", &url,\n+\t\t\t N_(\"string\"),\n+\t\t\t N_(\"url where to clone the submodule from\")),\n+\t\tOPT_STRING(0, \"reference\", &reference,\n+\t\t\t N_(\"string\"),\n+\t\t\t N_(\"reference repository\")),\n+\t\tOPT_STRING(0, \"depth\", &depth,\n+\t\t\t N_(\"string\"),\n+\t\t\t N_(\"depth for shallow clones\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tstatic const char * const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper update [--prefix=] [--quiet] [--remote] [-N|--no-fetch]\"\n+\t\t \"[-f|--force] [--rebase|--merge] [--reference ]\"\n+\t\t \"[--depth ] [--recursive] [--] [...]\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_update_options,\n+\t\t\t git_submodule_helper_usage, 0);\n+\n+\tif (getenv(\"GIT_QUIET\"))\n+\t\tquiet = 1;\n+\n+\tstrbuf_addf(&sb, \"%s/modules/%s\", get_git_dir(), name);\n+\tsm_gitdir = strbuf_detach(&sb, NULL);\n+\n+\tif (!file_exists(sm_gitdir)) {\n+\t\tsafe_create_leading_directories_const(sm_gitdir);\n+\t\tif (clone_submodule(path, sm_gitdir, url, depth, reference, quiet))\n+\t\t\tdie(N_(\"Clone of '%s' into submodule path '%s' failed\"),\n+\t\t\t url, path);\n+\t} else {\n+\t\tsafe_create_leading_directories_const(path);\n+\t\tunlink(sm_gitdir);\n+\t}\n+\n+\t/* Write a .git file in the submodule to redirect to the superproject. */\n+\tif (alternative_path && !strcmp(alternative_path, \"\")) {\n+\t\tp = relative_path(path, alternative_path, &sb);\n+\t\tstrbuf_reset(&sb);\n+\t} else\n+\t\tp = path;\n+\n+\tif (safe_create_leading_directories_const(p) < 0)\n+\t\tdie(\"Could not create directory '%s'\", p);\n+\n+\tstrbuf_addf(&sb, \"%s/.git\", p);\n+\n+\tif (safe_create_leading_directories_const(sb.buf) < 0)\n+\t\tdie(_(\"could not create leading directories of '%s'\"), sb.buf);\n+\tsubmodule_dot_git = fopen(sb.buf, \"w\");\n+\tif (!submodule_dot_git)\n+\t\tdie (\"Cannot open file '%s': %s\", sb.buf, strerror(errno));\n+\n+\tfprintf(submodule_dot_git, \"gitdir: %s\\n\",\n+\t\trelative_path(sm_gitdir, path, &rel_path));\n+\tif (fclose(submodule_dot_git))\n+\t\tdie(\"Could not close file %s\", sb.buf);\n+\tstrbuf_reset(&sb);\n+\n+\t/* Redirect the worktree of the submodule in the superprojects config */\n+\tif (!is_absolute_path(sm_gitdir)) {\n+\t\tchar *s = (char*)sm_gitdir;\n+\t\tif (strbuf_getcwd(&sb))\n+\t\t\tdie_errno(\"unable to get current working directory\");\n+\t\tstrbuf_addf(&sb, \"/%s\", sm_gitdir);\n+\t\tsm_gitdir = strbuf_detach(&sb, NULL);\n+\t\tfree(s);\n+\t}\n+\n+\tif (strbuf_getcwd(&sb))\n+\t\tdie_errno(\"unable to get current working directory\");\n+\tstrbuf_addf(&sb, \"/%s\", path);\n+\n+\tp = git_pathdup_submodule(path, \"config\");\n+\tif (!p)\n+\t\tdie(\"Could not get submodule directory for '%s'\", path);\n+\tgit_config_set_in_file(p, \"core.worktree\",\n+\t\t\t relative_path(sb.buf, sm_gitdir, &rel_path));\n+\tstrbuf_release(&sb);\n+\tfree((char*)sm_gitdir);\n+\treturn 0;\n+}\n+\n int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n {\n \tif (argc < 2)\n@@ -135,6 +277,10 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(argv[1], \"module_name\"))\n \t\treturn module_name(argc - 2, argv + 2, prefix);\n \n+\tif (!strcmp(argv[1], \"module_clone\"))\n+\t\treturn module_clone(argc - 1, argv + 1, prefix);\n+\n usage:\n-\tusage(\"git submodule--helper [module_list | module_name]\\n\");\n+\tusage(\"git submodule--helper [module_list | module_name | \"\n+\t \"module_clone]\\n\");\n }\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex e6ff38d..fb5155e 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -178,80 +178,6 @@ get_submodule_config () {\n \tprintf '%s' \"${value:-$default}\"\n }\n \n-#\n-# Clone a submodule\n-#\n-# $1 = submodule path\n-# $2 = submodule name\n-# $3 = URL to clone\n-# $4 = reference repository to reuse (empty for independent)\n-# $5 = depth argument for shallow clones (empty for deep)\n-#\n-# Prior to calling, cmd_update checks that a possibly existing\n-# path is not a git repository.\n-# Likewise, cmd_add checks that path does not exist at all,\n-# since it is the location of a new submodule.\n-#\n-module_clone()\n-{\n-\tsm_path=$1\n-\tname=$2\n-\turl=$3\n-\treference=\"$4\"\n-\tdepth=\"$5\"\n-\tquiet=\n-\tif test -n \"$GIT_QUIET\"\n-\tthen\n-\t\tquiet=-q\n-\tfi\n-\n-\tgitdir=\n-\tgitdir_base=\n-\tbase_name=$(dirname \"$name\")\n-\n-\tgitdir=$(git rev-parse --git-dir)\n-\tgitdir_base=\"$gitdir/modules/$base_name\"\n-\tgitdir=\"$gitdir/modules/$name\"\n-\n-\tif test -d \"$gitdir\"\n-\tthen\n-\t\tmkdir -p \"$sm_path\"\n-\t\trm -f \"$gitdir/index\"\n-\telse\n-\t\tmkdir -p \"$gitdir_base\"\n-\t\t(\n-\t\t\tclear_local_git_env\n-\t\t\tgit clone $quiet ${depth:+\"$depth\"} -n ${reference:+\"$reference\"} \\\n-\t\t\t\t--separate-git-dir \"$gitdir\" \"$url\" \"$sm_path\"\n-\t\t) ||\n-\t\tdie \"$(eval_gettext \"Clone of '\\$url' into submodule path '\\$sm_path' failed\")\"\n-\tfi\n-\n-\t# We already are at the root of the work tree but cd_to_toplevel will\n-\t# resolve any symlinks that might be present in $PWD\n-\ta=$(cd_to_toplevel && cd \"$gitdir\" && pwd)/\n-\tb=$(cd_to_toplevel && cd \"$sm_path\" && pwd)/\n-\t# Remove all common leading directories after a sanity check\n-\tif test \"${a#$b}\" != \"$a\" || test \"${b#$a}\" != \"$b\"; then\n-\t\tdie \"$(eval_gettext \"Gitdir '\\$a' is part of the submodule path '\\$b' or vice versa\")\"\n-\tfi\n-\twhile test \"${a%%/*}\" = \"${b%%/*}\"\n-\tdo\n-\t\ta=${a#*/}\n-\t\tb=${b#*/}\n-\tdone\n-\t# Now chop off the trailing '/'s that were added in the beginning\n-\ta=${a%/}\n-\tb=${b%/}\n-\n-\t# Turn each leading \"*/\" component into \"../\"\n-\trel=$(printf '%s\\n' \"$b\" | sed -e 's|[^/][^/]*|..|g')\n-\tprintf '%s\\n' \"gitdir: $rel/$a\" >\"$sm_path/.git\"\n-\n-\trel=$(printf '%s\\n' \"$a\" | sed -e 's|[^/][^/]*|..|g')\n-\t(clear_local_git_env; cd \"$sm_path\" && GIT_WORK_TREE=. git config core.worktree \"$rel/$b\")\n-}\n-\n isnumber()\n {\n \tn=$(($1 + 0)) 2>/dev/null && test \"$n\" = \"$1\"\n@@ -301,7 +227,7 @@ cmd_add()\n \t\t\tshift\n \t\t\t;;\n \t\t--depth=*)\n-\t\t\tdepth=$1\n+\t\t\tdepth=\"$1\"\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -412,7 +338,7 @@ Use -f if you really want to add it.\" >&2\n \t\t\t\techo \"$(eval_gettext \"Reactivating local git directory for submodule '\\$sm_name'.\")\"\n \t\t\tfi\n \t\tfi\n-\t\tmodule_clone \"$sm_path\" \"$sm_name\" \"$realrepo\" \"$reference\" \"$depth\" || exit\n+\t\tgit submodule--helper module_clone --prefix \"$wt_prefix\" --path \"$sm_path\" --name \"$sm_name\" --url \"$realrepo\" \"$reference\" \"$depth\" || exit\n \t\t(\n \t\t\tclear_local_git_env\n \t\t\tcd \"$sm_path\" &&\n@@ -774,7 +700,7 @@ Maybe you want to use 'update --init'?\")\"\n \n \t\tif ! test -d \"$sm_path\"/.git && ! test -f \"$sm_path\"/.git\n \t\tthen\n-\t\t\tmodule_clone \"$sm_path\" \"$name\" \"$url\" \"$reference\" \"$depth\" || exit\n+\t\t\tgit submodule--helper module_clone --prefix \"$prefix\" --path \"$sm_path\" --name \"$name\" --url \"$url\" \"$reference\" \"$depth\" || exit\n \t\t\tcloned_modules=\"$cloned_modules;$name\"\n \t\t\tsubsha1=\n \t\telse\n-- \n2.5.0.264.g5e52b0d\n"},{"id":"268809","messageId":"1440724495-708-5-git-send-email-sbeller@google.com","threadId":"40201","inReplyTo":"1440724495-708-1-git-send-email-sbeller@google.com","subject":"[PATCH 4/9] thread-utils: add a threaded task queue","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T01:14:50Z","receivedAt":"2015-08-28T01:14:50Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This adds functionality to do work in a parallel threaded\nfashion while the boiler plate code for setting up threads\nand tearing them down as well as queuing up tasks is hidden\nbehind the new API.\n\nSigned-off-by: Stefan Beller \n---\n run-command.c | 39 +++++++-----\n run-command.h | 3 +\n thread-utils.c | 192 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n thread-utils.h | 35 +++++++++++\n 4 files changed, 253 insertions(+), 16 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 28e1d55..3d37f8c 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -610,10 +610,12 @@ static NORETURN void die_async(const char *err, va_list params)\n \n \tif (!pthread_equal(main_thread, pthread_self())) {\n \t\tstruct async *async = pthread_getspecific(async_key);\n-\t\tif (async->proc_in >= 0)\n-\t\t\tclose(async->proc_in);\n-\t\tif (async->proc_out >= 0)\n-\t\t\tclose(async->proc_out);\n+\t\tif (async) {\n+\t\t\tif (async->proc_in >= 0)\n+\t\t\t\tclose(async->proc_in);\n+\t\t\tif (async->proc_out >= 0)\n+\t\t\t\tclose(async->proc_out);\n+\t\t}\n \t\tpthread_exit((void *)128);\n \t}\n \n@@ -668,6 +670,22 @@ int git_atexit(void (*handler)(void))\n \n #endif\n \n+void setup_main_thread(void)\n+{\n+\tif (!main_thread_set) {\n+\t\t/*\n+\t\t * We assume that the first time that start_async is called\n+\t\t * it is from the main thread.\n+\t\t */\n+\t\tmain_thread_set = 1;\n+\t\tmain_thread = pthread_self();\n+\t\tpthread_key_create(&async_key, NULL);\n+\t\tpthread_key_create(&async_die_counter, NULL);\n+\t\tset_die_routine(die_async);\n+\t\tset_die_is_recursing_routine(async_die_is_recursing);\n+\t}\n+}\n+\n int start_async(struct async *async)\n {\n \tint need_in, need_out;\n@@ -740,18 +758,7 @@ int start_async(struct async *async)\n \telse if (async->out)\n \t\tclose(async->out);\n #else\n-\tif (!main_thread_set) {\n-\t\t/*\n-\t\t * We assume that the first time that start_async is called\n-\t\t * it is from the main thread.\n-\t\t */\n-\t\tmain_thread_set = 1;\n-\t\tmain_thread = pthread_self();\n-\t\tpthread_key_create(&async_key, NULL);\n-\t\tpthread_key_create(&async_die_counter, NULL);\n-\t\tset_die_routine(die_async);\n-\t\tset_die_is_recursing_routine(async_die_is_recursing);\n-\t}\n+\tsetup_main_thread();\n \n \tif (proc_in >= 0)\n \t\tset_cloexec(proc_in);\ndiff --git a/run-command.h b/run-command.h\nindex 5b4425a..176a5b2 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -119,4 +119,7 @@ struct async {\n int start_async(struct async *async);\n int finish_async(struct async *async);\n \n+/* die gracefully from within threads */\n+void setup_main_thread(void);\n+\n #endif\ndiff --git a/thread-utils.c b/thread-utils.c\nindex a2135e0..30ccd79 100644\n--- a/thread-utils.c\n+++ b/thread-utils.c\n@@ -1,5 +1,7 @@\n #include \"cache.h\"\n #include \"thread-utils.h\"\n+#include \"run-command.h\"\n+#include \"git-compat-util.h\"\n \n #if defined(hpux) || defined(__hpux) || defined(_hpux)\n # include \n@@ -75,3 +77,193 @@ int init_recursive_mutex(pthread_mutex_t *m)\n \t}\n \treturn ret;\n }\n+\n+#ifndef NO_PTHREADS\n+struct job_list {\n+\tint (*fct)(struct task_queue *tq, void *task);\n+\tvoid *task;\n+\tstruct job_list *next;\n+};\n+\n+struct task_queue {\n+\tpthread_mutex_t mutex;\n+\tpthread_cond_t cond_non_empty;\n+\n+\tint queued_tasks;\n+\tstruct job_list *first;\n+\tstruct job_list *last;\n+\n+\tpthread_t *threads;\n+\tunsigned max_threads;\n+\tunsigned max_tasks;\n+\n+\tvoid (*finish_function)(struct task_queue *tq);\n+\tint early_return;\n+};\n+\n+static void next_task(struct task_queue *tq,\n+\t\t int (**fct)(struct task_queue *tq, void *task),\n+\t\t void **task,\n+\t\t int *early_return)\n+{\n+\tstruct job_list *job = NULL;\n+\n+\tpthread_mutex_lock(&tq->mutex);\n+\twhile (tq->queued_tasks == 0)\n+\t\tpthread_cond_wait(&tq->cond_non_empty, &tq->mutex);\n+\n+\ttq->early_return |= *early_return;\n+\n+\tif (!tq->early_return) {\n+\t\tjob = tq->first;\n+\t\ttq->first = job->next;\n+\t\tif (!tq->first)\n+\t\t\ttq->last = NULL;\n+\t\ttq->queued_tasks--;\n+\t}\n+\n+\tpthread_mutex_unlock(&tq->mutex);\n+\n+\tif (job) {\n+\t\t*fct = job->fct;\n+\t\t*task = job->task;\n+\t} else {\n+\t\t*fct = NULL;\n+\t\t*task = NULL;\n+\t}\n+\n+\tfree(job);\n+}\n+\n+static void *dispatcher(void *args)\n+{\n+\tvoid *task;\n+\tint (*fct)(struct task_queue *tq, void *task);\n+\tint early_return = 0;\n+\tstruct task_queue *tq = args;\n+\n+\tnext_task(tq, &fct, &task, &early_return);\n+\twhile (fct && !early_return) {\n+\t\tearly_return = fct(tq, task);\n+\t\tnext_task(tq, &fct, &task, &early_return);\n+\t}\n+\n+\tif (tq->finish_function)\n+\t\ttq->finish_function(tq);\n+\n+\tpthread_exit(0);\n+}\n+\n+struct task_queue *create_task_queue(unsigned max_threads)\n+{\n+\tstruct task_queue *tq = xmalloc(sizeof(*tq));\n+\n+\tint i, ret;\n+\tif (!max_threads)\n+\t\ttq->max_threads = online_cpus();\n+\telse\n+\t\ttq->max_threads = max_threads;\n+\n+\tpthread_mutex_init(&tq->mutex, NULL);\n+\tpthread_cond_init(&tq->cond_non_empty, NULL);\n+\n+\ttq->threads = xmalloc(tq->max_threads * sizeof(pthread_t));\n+\n+\ttq->queued_tasks = 0;\n+\ttq->first = NULL;\n+\ttq->last = NULL;\n+\n+\tsetup_main_thread();\n+\n+\tfor (i = 0; i < tq->max_threads; i++) {\n+\t\tret = pthread_create(&tq->threads[i], 0, &dispatcher, tq);\n+\t\tif (ret)\n+\t\t\tdie(\"unable to create thread: %s\", strerror(ret));\n+\t}\n+\n+\ttq->early_return = 0;\n+\n+\treturn tq;\n+}\n+\n+void add_task(struct task_queue *tq,\n+\t int (*fct)(struct task_queue *tq, void *task),\n+\t void *task)\n+{\n+\tstruct job_list *job_list;\n+\n+\tjob_list = xmalloc(sizeof(*job_list));\n+\tjob_list->task = task;\n+\tjob_list->fct = fct;\n+\tjob_list->next = NULL;\n+\n+\tpthread_mutex_lock(&tq->mutex);\n+\n+\tif (!tq->last) {\n+\t\ttq->last = job_list;\n+\t\ttq->first = tq->last;\n+\t} else {\n+\t\ttq->last->next = job_list;\n+\t\ttq->last = tq->last->next;\n+\t}\n+\ttq->queued_tasks++;\n+\n+\tpthread_mutex_unlock(&tq->mutex);\n+\tpthread_cond_signal(&tq->cond_non_empty);\n+}\n+\n+int finish_task_queue(struct task_queue *tq, void (*fct)(struct task_queue *tq))\n+{\n+\tint ret;\n+\tint i;\n+\n+\ttq->finish_function = fct;\n+\n+\tfor (i = 0; i < tq->max_threads; i++)\n+\t\tadd_task(tq, NULL, NULL);\n+\n+\tfor (i = 0; i < tq->max_threads; i++)\n+\t\tpthread_join(tq->threads[i], 0);\n+\n+\tpthread_mutex_destroy(&tq->mutex);\n+\tpthread_cond_destroy(&tq->cond_non_empty);\n+\n+\tif (tq->first)\n+\t\tdie(\"BUG: internal error with queuing jobs for threads\");\n+\n+\tfree(tq->threads);\n+\tret = tq->early_return;\n+\n+\tfree(tq);\n+\treturn ret;\n+}\n+#else /* NO_PTHREADS */\n+\n+struct task_queue {\n+\tint early_return;\n+};\n+\n+struct task_queue *create_task_queue(unsigned max_threads)\n+{\n+\tstruct task_queue *tq = xmalloc(sizeof(*tq));\n+\n+\ttq->early_return = 0;\n+}\n+\n+void add_task(struct task_queue *tq,\n+\t int (*fct)(struct task_queue *tq, void *task),\n+\t void *task)\n+{\n+\tif (tq->early_return)\n+\t\treturn;\n+\n+\ttq->early_return |= fct(tq, task);\n+}\n+\n+int finish_task_queue(struct task_queue *tq, void (*fct)(struct task_queue *tq))\n+{\n+\tint ret = tq->early_return;\n+\tfree(tq);\n+\treturn ret;\n+}\n+#endif\ndiff --git a/thread-utils.h b/thread-utils.h\nindex d9a769d..f41cfb1 100644\n--- a/thread-utils.h\n+++ b/thread-utils.h\n@@ -12,4 +12,39 @@ extern int init_recursive_mutex(pthread_mutex_t*);\n #define online_cpus() 1\n \n #endif\n+\n+/*\n+ * Creates a struct `task_queue`, which holds a list of tasks. Up to\n+ * `max_threads` threads are active to process the enqueued tasks\n+ * processing the tasks in a first in first out order.\n+ *\n+ * If `max_threads` is zero the number of cores available will be used.\n+ *\n+ * Currently this only works in environments with pthreads, in other\n+ * environments, the task will be processed sequentially in `add_task`.\n+ */\n+struct task_queue *create_task_queue(unsigned max_threads);\n+\n+/*\n+ * The function and data are put into the task queue.\n+ *\n+ * The function `fct` must not be NULL, as that's used internally\n+ * in `finish_task_queue` to signal shutdown. If the return code\n+ * of `fct` is unequal to 0, the tasks will stop eventually,\n+ * the current parallel tasks will be flushed out.\n+ */\n+void add_task(struct task_queue *tq,\n+\t int (*fct)(struct task_queue *tq, void *task),\n+\t void *task);\n+\n+/*\n+ * Waits for all tasks to be done and frees the object. The return code\n+ * is zero if all enqueued tasks were processed.\n+ *\n+ * The function `fct` is called once in each thread after the last task\n+ * for that thread was processed. If no thread local cleanup needs to be\n+ * performed, pass NULL.\n+ */\n+int finish_task_queue(struct task_queue *tq, void (*fct)(struct task_queue *tq));\n+\n #endif /* THREAD_COMPAT_H */\n-- \n2.5.0.264.g5e52b0d\n"},{"id":"268815","messageId":"1440724495-708-6-git-send-email-sbeller@google.com","threadId":"40201","inReplyTo":"1440724495-708-1-git-send-email-sbeller@google.com","subject":"[PATCH 5/9] run-command: add synced output","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T01:14:51Z","receivedAt":"2015-08-28T01:14:51Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In the last patch we added an easy way to get a thread pool. Now if we\nwant to run external commands from threads in the thread pool, the output\nwill mix up between the threads.\n\nTo solve this problem we protect the output via a mutex from becoming\ngarbled. Each thread will try to acquire and directly pipe their output\nto the standard output if they are lucky to get the mutex. If they do not\nhave the mutex each thread will buffer their output.\n\nExample:\nLet's assume we have 5 tasks A,B,C,D,E and each task takes a different\namount of time (say `git fetch` for different submodules), then the output\nof tasks in sequential order might look like this:\n\n time -->\n output: |---A---| |-B-| |----C-----------| |-D-| |-E-|\n\nWhen we schedule these tasks into two threads, a schedule\nand sample output over time may look like this:\n\nthread 1: |---A---| |-D-| |-E-|\n\nthread 2: |-B-| |----C-----------|\n\noutput: |---A---|B|------C-------|ED\n\nSo A will be perceived as it would run normally in\nthe single threaded version of foreach. As B has finished\nby the time the mutex becomes available, the whole buffer\nwill just be dumped into the standard output. This will be\nperceived by the user as just a 'very fast' operation of B.\nOnce that is done, C takes the mutex, and flushes the piled\nup buffer to standard output. In case the it is a\ngit command, we have a nice progress display, which will just\nlook like the first half of C happend really quickly.\n\nNotice how E and D are put out in a different order than the\noriginal as the new parallel foreach doesn't care about the\norder.\n\nSo this way of output is really good for human consumption,\nas it only changes the timing, not the actual output.\n\nFor machine consumption the output needs to be prepared in\nthe tasks, by either having a prefix per line or per block\nto indicate whose tasks output is displayed.\n\nSigned-off-by: Stefan Beller \n---\n run-command.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n run-command.h | 18 ++++++++++++++++++\n 2 files changed, 78 insertions(+)\n\ndiff --git a/run-command.c b/run-command.c\nindex 3d37f8c..bac4367 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -556,17 +556,77 @@ int finish_command(struct child_process *cmd)\n }\n \n int run_command(struct child_process *cmd)\n+#ifdef NO_PTHREADS\n {\n \tint code;\n \n \tif (cmd->out < 0 || cmd->err < 0)\n \t\tdie(\"BUG: run_command with a pipe can cause deadlock\");\n \n+\tif (cmd->sync_buf)\n+\t\txwrite(cmd->out, cmd->sync_buf->buf, cmd->sync_buf->len);\n+\n \tcode = start_command(cmd);\n \tif (code)\n \t\treturn code;\n \treturn finish_command(cmd);\n }\n+#else\n+{\n+\tint code, lock_acquired;\n+\n+\tif (!cmd->sync_mutex) {\n+\t\tif (cmd->out < 0 || cmd->err < 0)\n+\t\t\tdie(\"BUG: run_command with a pipe can cause deadlock\");\n+\n+\t\tif (cmd->sync_buf)\n+\t\t\txwrite(cmd->out, cmd->sync_buf->buf, cmd->sync_buf->len);\n+\t} else {\n+\t\tif (cmd->out < 0)\n+\t\t\tdie(\"BUG: run_command with a pipe can cause deadlock\");\n+\n+\t\tif (!cmd->stdout_to_stderr)\n+\t\t\tdie(\"BUG: run_command with sync_mutex not supported \"\n+\t\t\t \"without stdout_to_stderr set\");\n+\n+\t\tif (!cmd->sync_buf)\n+\t\t\tdie(\"BUG: Must pass a buffer when specifying \"\n+\t\t\t \"to sync output\");\n+\t}\n+\n+\tcode = start_command(cmd);\n+\tif (code)\n+\t\treturn code;\n+\n+\tif (cmd->sync_mutex) {\n+\t\twhile (1) {\n+\t\t\tchar buf[1024];\n+\t\t\tssize_t len = xread(cmd->err, buf, sizeof(buf));\n+\t\t\tif (len < 0)\n+\t\t\t\tdie(\"Read from command failed\");\n+\t\t\telse if (len == 0)\n+\t\t\t\tbreak;\n+\t\t\telse\n+\t\t\t\tstrbuf_add(cmd->sync_buf, buf, len);\n+\n+\t\t\tif (!lock_acquired\n+\t\t\t && !pthread_mutex_trylock(cmd->sync_mutex))\n+\t\t\t\tlock_acquired = 1;\n+\t\t\tif (lock_acquired) {\n+\t\t\t\tfputs(cmd->sync_buf->buf, stderr);\n+\t\t\t\tstrbuf_reset(cmd->sync_buf);\n+\t\t\t}\n+\t\t}\n+\t\tif (!lock_acquired)\n+\t\t\tpthread_mutex_lock(cmd->sync_mutex);\n+\n+\t\tfputs(cmd->sync_buf->buf, stderr);\n+\t\tpthread_mutex_unlock(cmd->sync_mutex);\n+\t}\n+\n+\treturn finish_command(cmd);\n+}\n+#endif\n \n int run_command_v_opt(const char **argv, int opt)\n {\ndiff --git a/run-command.h b/run-command.h\nindex 176a5b2..0df83c9 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -43,6 +43,24 @@ struct child_process {\n \tunsigned stdout_to_stderr:1;\n \tunsigned use_shell:1;\n \tunsigned clean_on_exit:1;\n+#ifndef NO_PTHREAD\n+\t/*\n+\t * In a threaded environment running multiple commands from different\n+\t * threads would lead to garbled output as the output of different\n+\t * commands would mix.\n+\t * If this mutex is not NULL, the output of `err` is only done when\n+\t * holding the mutex. If the mutex cannot be acquired, the output\n+\t * will be buffered until the mutex can be acquired.\n+\t */\n+\tpthread_mutex_t *sync_mutex;\n+#endif\n+\t/*\n+\t * The sync_buf will be used to buffer the output while the mutex\n+\t * is not acquired. It can also contain data before being passed into\n+\t * run_command, which will be output together with the output of\n+\t * the child.\n+\t */\n+\tstruct strbuf *sync_buf;\n };\n \n #define CHILD_PROCESS_INIT { NULL, ARGV_ARRAY_INIT, ARGV_ARRAY_INIT }\n-- \n2.5.0.264.g5e52b0d\n"},{"id":"268811","messageId":"1440724495-708-7-git-send-email-sbeller@google.com","threadId":"40201","inReplyTo":"1440724495-708-1-git-send-email-sbeller@google.com","subject":"[PATCH 6/9] submodule: helper to run foreach in parallel","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T01:14:52Z","receivedAt":"2015-08-28T01:14:52Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Similar to `git submodule foreach` the new command `git submodule\nforeach_parallel` will run a command on each submodule.\n\nThe commands are run in parallel up to the number of cores by default,\nor you can specify '-j 4' tun just run with 4 threads for example.\n\nOne major difference to `git submodule foreach` is the handling of input\nand output to the commands. Because of the parallel nature of the execution\nit is not trivial how to schedule the std{in,out,err} channel for submodule\nthe command is run in. So in this patch there is no support for stdin.\nstdout will be piped to stderr. stderr will make use of the synchronized\noutput feature of run_command.\n\nSigned-off-by: Stefan Beller \n---\n builtin/submodule--helper.c | 133 ++++++++++++++++++++++++++++++++++++++++++-\n git-submodule.sh | 11 +++-\n t/t7407-submodule-foreach.sh | 11 ++++\n 3 files changed, 153 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex d29499c..18b67f0 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -8,6 +8,7 @@\n #include \"submodule.h\"\n #include \"submodule-config.h\"\n #include \"string-list.h\"\n+#include \"thread-utils.h\"\n #include \"run-command.h\"\n \n static const struct cache_entry **ce_entries;\n@@ -266,6 +267,127 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+#ifndef NO_PTHREADS\n+struct submodule_args {\n+\tconst char *name;\n+\tconst char *path;\n+\tconst char *sha1;\n+\tconst char *toplevel;\n+\tconst char *prefix;\n+\tconst char **cmd;\n+\tpthread_mutex_t *sync;\n+};\n+\n+int run_cmd_submodule(struct task_queue *aq, void *task)\n+{\n+\tint i;\n+\tstruct submodule_args *args = task;\n+\tstruct strbuf out = STRBUF_INIT;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct child_process *cp = xmalloc(sizeof(*cp));\n+\n+\tstrbuf_addf(&out, N_(\"Entering %s\\n\"), relative_path(args->path, args->prefix, &sb));\n+\n+\tchild_process_init(cp);\n+\targv_array_pushv(&cp->args, args->cmd);\n+\n+\targv_array_pushf(&cp->env_array, \"name=%s\", args->name);\n+\targv_array_pushf(&cp->env_array, \"path=%s\", args->path);\n+\targv_array_pushf(&cp->env_array, \"sha1=%s\", args->sha1);\n+\targv_array_pushf(&cp->env_array, \"toplevel=%s\", args->toplevel);\n+\n+\tfor (i = 0; local_repo_env[i]; i++)\n+\t\targv_array_push(&cp->env_array, local_repo_env[i]);\n+\n+\tcp->no_stdin = 1;\n+\tcp->out = 0;\n+\tcp->err = -1;\n+\tcp->dir = args->path;\n+\tcp->stdout_to_stderr = 1;\n+\tcp->use_shell = 1;\n+\tcp->sync_mutex = args->sync;\n+\tcp->sync_buf = &out;\n+\n+\treturn run_command(cp);\n+}\n+\n+int module_foreach_parallel(int argc, const char **argv, const char *prefix)\n+{\n+\tint i, recursive = 0, number_threads = 0, quiet = 0;\n+\tstatic struct pathspec pathspec;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct task_queue *aq;\n+\tchar **cmd;\n+\tconst char **nullargv = {NULL};\n+\tpthread_mutex_t mutex;\n+\n+\tstruct option module_update_options[] = {\n+\t\tOPT_STRING(0, \"prefix\", &alternative_path,\n+\t\t\t N_(\"path\"),\n+\t\t\t N_(\"alternative anchor for relative paths\")),\n+\t\tOPT_STRING(0, \"cmd\", &cmd,\n+\t\t\t N_(\"string\"),\n+\t\t\t N_(\"command to run\")),\n+\t\tOPT_BOOL('r', \"--recursive\", &recursive,\n+\t\t\t N_(\"Recurse into nexted submodules\")),\n+\t\tOPT_INTEGER('j', \"jobs\", &number_threads,\n+\t\t\t N_(\"Recurse into nexted submodules\")),\n+\t\tOPT__QUIET(&quiet, N_(\"Suppress output\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tstatic const char * const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper foreach [--prefix=] [...]\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_update_options,\n+\t\t\t git_submodule_helper_usage, 0);\n+\n+\tif (module_list_compute(0, nullargv, NULL, &pathspec) < 0)\n+\t\treturn 1;\n+\n+\tgitmodules_config();\n+\n+\tpthread_mutex_init(&mutex, NULL);\n+\taq = create_task_queue(number_threads);\n+\n+\tfor (i = 0; i < ce_used; i++) {\n+\t\tconst struct submodule *sub;\n+\t\tconst struct cache_entry *ce = ce_entries[i];\n+\t\tstruct submodule_args *args = malloc(sizeof(*args));\n+\n+\t\tif (ce_stage(ce))\n+\t\t\targs->sha1 = xstrdup(sha1_to_hex(null_sha1));\n+\t\telse\n+\t\t\targs->sha1 = xstrdup(sha1_to_hex(ce->sha1));\n+\n+\t\tstrbuf_reset(&sb);\n+\t\tstrbuf_addf(&sb, \"%s/.git\", ce->name);\n+\t\tif (!file_exists(sb.buf)) {\n+\t\t\tfree(args);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\targs->path = ce->name;\n+\t\tsub = submodule_from_path(null_sha1, args->path);\n+\t\tif (!sub)\n+\t\t\tdie(\"No submodule mapping found in .gitmodules for path '%s'\", args->path);\n+\n+\t\targs->name = sub->name;\n+\t\targs->toplevel = xgetcwd();\n+\t\targs->cmd = argv;\n+\t\targs->sync = &mutex;\n+\t\targs->prefix = alternative_path;\n+\t\tadd_task(aq, run_cmd_submodule, args);\n+\t}\n+\n+\tfinish_task_queue(aq, NULL);\n+\tpthread_mutex_destroy(&mutex);\n+\treturn 0;\n+}\n+#endif /* NO_PTHREADS */\n+\n int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n {\n \tif (argc < 2)\n@@ -280,7 +402,16 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(argv[1], \"module_clone\"))\n \t\treturn module_clone(argc - 1, argv + 1, prefix);\n \n+#ifndef NO_PTHREADS\n+\tif (!strcmp(argv[1], \"foreach_parallel\"))\n+\t\treturn module_foreach_parallel(argc - 1, argv + 1, prefix);\n+#endif\n+\n usage:\n \tusage(\"git submodule--helper [module_list | module_name | \"\n-\t \"module_clone]\\n\");\n+\t \"module_clone\"\n+#ifndef NO_PTHREADS\n+\t \" | foreach_parallel\"\n+#endif\n+\t \"]\\n\");\n }\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex fb5155e..f06488a 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -431,6 +431,15 @@ cmd_foreach()\n }\n \n #\n+# Execute an arbitrary command sequence in each checked out\n+# submodule in parallel.\n+#\n+cmd_foreach_parallel()\n+{\n+\tgit submodule--helper foreach_parallel --prefix \"$wt_prefix\" $@\n+}\n+\n+#\n # Register submodules in .git/config\n #\n # $@ = requested paths (default to all)\n@@ -1225,7 +1234,7 @@ cmd_sync()\n while test $# != 0 && test -z \"$command\"\n do\n \tcase \"$1\" in\n-\tadd | foreach | init | deinit | update | status | summary | sync)\n+\tadd | foreach | foreach_parallel | init | deinit | update | status | summary | sync)\n \t\tcommand=$1\n \t\t;;\n \t-q|--quiet)\ndiff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\nindex 7ca10b8..16f6138 100755\n--- a/t/t7407-submodule-foreach.sh\n+++ b/t/t7407-submodule-foreach.sh\n@@ -195,6 +195,17 @@ test_expect_success 'test \"foreach --quiet --recursive\"' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'test \"foreach_parallel --quiet\"' '\n+\t(\n+\t\tcd clone2 &&\n+\t\tgit submodule foreach_parallel -q -- \"echo \\$name-\\$path\" > ../actual\n+\t) &&\n+\tgrep nested1-nested1 actual &&\n+\tgrep foo1-sub1 actual &&\n+\tgrep foo2-sub2 actual &&\n+\tgrep foo3-sub3 actual\n+'\n+\n test_expect_success 'use \"update --recursive\" to checkout all submodules' '\n \tgit clone super clone3 &&\n \t(\n-- \n2.5.0.264.g5e52b0d\n"},{"id":"268816","messageId":"1440724495-708-8-git-send-email-sbeller@google.com","threadId":"40201","inReplyTo":"1440724495-708-1-git-send-email-sbeller@google.com","subject":"[PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T01:14:53Z","receivedAt":"2015-08-28T01:14:53Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This makes use of the new task queue and the syncing feature of\nrun-command to fetch a number of submodules at the same time.\n\nThe output will look like it would have been run sequential,\nbut faster.\n\nSigned-off-by: Stefan Beller \n---\n Documentation/fetch-options.txt | 7 +++\n builtin/fetch.c | 6 ++-\n builtin/pull.c | 6 +++\n submodule.c | 100 +++++++++++++++++++++++++++++++++-------\n submodule.h | 2 +-\n 5 files changed, 102 insertions(+), 19 deletions(-)\n\ndiff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\nindex 45583d8..e2a59c3 100644\n--- a/Documentation/fetch-options.txt\n+++ b/Documentation/fetch-options.txt\n@@ -100,6 +100,13 @@ ifndef::git-pull[]\n \treference to a commit that isn't already in the local submodule\n \tclone.\n \n+-j::\n+--jobs=::\n+\tNumber of threads to be used for fetching submodules. Each thread\n+\twill fetch from different submodules, such that fetching many\n+\tsubmodules will be faster. By default the number of cpus will\n+\tbe used .\n+\n --no-recurse-submodules::\n \tDisable recursive fetching of submodules (this has the same effect as\n \tusing the '--recurse-submodules=no' option).\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex ee1f1a9..636707e 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -37,6 +37,7 @@ static int prune = -1; /* unspecified */\n static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;\n static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n static int tags = TAGS_DEFAULT, unshallow, update_shallow;\n+static int max_threads;\n static const char *depth;\n static const char *upload_pack;\n static struct strbuf default_rla = STRBUF_INIT;\n@@ -99,6 +100,8 @@ static struct option builtin_fetch_options[] = {\n \t\t N_(\"fetch all tags and associated objects\"), TAGS_SET),\n \tOPT_SET_INT('n', NULL, &tags,\n \t\t N_(\"do not fetch all tags (--no-tags)\"), TAGS_UNSET),\n+\tOPT_INTEGER('j', \"jobs\", &max_threads,\n+\t\t N_(\"number of threads used for fetching\")),\n \tOPT_BOOL('p', \"prune\", &prune,\n \t\t N_(\"prune remote-tracking branches no longer on remote\")),\n \t{ OPTION_CALLBACK, 0, \"recurse-submodules\", NULL, N_(\"on-demand\"),\n@@ -1217,7 +1220,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t\tresult = fetch_populated_submodules(&options,\n \t\t\t\t\t\t submodule_prefix,\n \t\t\t\t\t\t recurse_submodules,\n-\t\t\t\t\t\t verbosity < 0);\n+\t\t\t\t\t\t verbosity < 0,\n+\t\t\t\t\t\t max_threads);\n \t\targv_array_clear(&options);\n \t}\n \ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 722a83c..fbbda67 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -94,6 +94,7 @@ static int opt_force;\n static char *opt_tags;\n static char *opt_prune;\n static char *opt_recurse_submodules;\n+static char *max_threads;\n static int opt_dry_run;\n static char *opt_keep;\n static char *opt_depth;\n@@ -177,6 +178,9 @@ static struct option pull_options[] = {\n \t\tN_(\"on-demand\"),\n \t\tN_(\"control recursive fetching of submodules\"),\n \t\tPARSE_OPT_OPTARG),\n+\tOPT_PASSTHRU('j', \"jobs\", &max_threads, N_(\"n\"),\n+\t\tN_(\"number of threads used for fetching submodules\"),\n+\t\tPARSE_OPT_OPTARG),\n \tOPT_BOOL(0, \"dry-run\", &opt_dry_run,\n \t\tN_(\"dry run\")),\n \tOPT_PASSTHRU('k', \"keep\", &opt_keep, NULL,\n@@ -524,6 +528,8 @@ static int run_fetch(const char *repo, const char **refspecs)\n \t\targv_array_push(&args, opt_prune);\n \tif (opt_recurse_submodules)\n \t\targv_array_push(&args, opt_recurse_submodules);\n+\tif (max_threads)\n+\t\targv_array_push(&args, max_threads);\n \tif (opt_dry_run)\n \t\targv_array_push(&args, \"--dry-run\");\n \tif (opt_keep)\ndiff --git a/submodule.c b/submodule.c\nindex 9fcc86f..50266a8 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -12,6 +12,7 @@\n #include \"sha1-array.h\"\n #include \"argv-array.h\"\n #include \"blob.h\"\n+#include \"thread-utils.h\"\n \n static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;\n static struct string_list changed_submodule_paths;\n@@ -615,13 +616,79 @@ static void calculate_changed_submodule_paths(void)\n \tinitialized_fetch_ref_tips = 0;\n }\n \n+struct submodule_parallel_fetch {\n+\tstruct child_process cp;\n+\tstruct argv_array argv;\n+\tint *result;\n+};\n+\n+#ifndef NO_PTHREADS\n+static pthread_mutex_t output_mutex = PTHREAD_MUTEX_INITIALIZER;\n+\n+static void set_output_mutex(struct submodule_parallel_fetch *spf)\n+{\n+\tspf->cp.sync_mutex = &output_mutex;\n+}\n+\n+#define lock_output_mutex() pthread_mutex_lock(&output_mutex)\n+\n+#define unlock_output_mutex() pthread_mutex_unlock(&output_mutex)\n+\n+static void destroy_output_mutex()\n+{\n+\tpthread_mutex_destroy(&output_mutex);\n+}\n+\n+#else\n+#define set_output_mutex()\n+#define destroy_output_mutex()\n+#define lock_output_mutex()\n+#define unlock_output_mutex()\n+#endif\n+\n+static struct submodule_parallel_fetch *submodule_parallel_fetch_create()\n+{\n+\tstruct submodule_parallel_fetch *spf = xmalloc(sizeof(*spf));\n+\tchild_process_init(&spf->cp);\n+\tspf->cp.env = local_repo_env;\n+\tspf->cp.git_cmd = 1;\n+\tspf->cp.no_stdin = 1;\n+\tspf->cp.stdout_to_stderr = 1;\n+\tspf->cp.sync_buf = xmalloc(sizeof(spf->cp.sync_buf));\n+\tstrbuf_init(spf->cp.sync_buf, 0);\n+\n+\targv_array_init(&spf->argv);\n+\treturn spf;\n+}\n+\n+static int run_command_and_cleanup(struct task_queue *tq, void *arg)\n+{\n+\tint code;\n+\tstruct submodule_parallel_fetch *spf = arg;\n+\n+\tspf->cp.argv = spf->argv.argv;\n+\n+\tcode = run_command(&spf->cp);\n+\tif (code) {\n+\t\tlock_output_mutex();\n+\t\t*spf->result = code;\n+\t\tunlock_output_mutex();\n+\t}\n+\n+\targv_array_clear(&spf->argv);\n+\tfree((char*)spf->cp.dir);\n+\tfree(spf);\n+\treturn 0;\n+}\n+\n int fetch_populated_submodules(const struct argv_array *options,\n \t\t\t const char *prefix, int command_line_option,\n-\t\t\t int quiet)\n+\t\t\t int quiet, int max_parallel_jobs)\n {\n \tint i, result = 0;\n-\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tstruct task_queue *tq;\n \tstruct argv_array argv = ARGV_ARRAY_INIT;\n+\tstruct submodule_parallel_fetch *spf;\n \tconst char *work_tree = get_git_work_tree();\n \tif (!work_tree)\n \t\tgoto out;\n@@ -635,12 +702,9 @@ int fetch_populated_submodules(const struct argv_array *options,\n \targv_array_push(&argv, \"--recurse-submodules-default\");\n \t/* default value, \"--submodule-prefix\" and its value are added later */\n \n-\tcp.env = local_repo_env;\n-\tcp.git_cmd = 1;\n-\tcp.no_stdin = 1;\n-\n \tcalculate_changed_submodule_paths();\n \n+\ttq = create_task_queue(max_parallel_jobs);\n \tfor (i = 0; i < active_nr; i++) {\n \t\tstruct strbuf submodule_path = STRBUF_INIT;\n \t\tstruct strbuf submodule_git_dir = STRBUF_INIT;\n@@ -693,24 +757,26 @@ int fetch_populated_submodules(const struct argv_array *options,\n \t\tif (!git_dir)\n \t\t\tgit_dir = submodule_git_dir.buf;\n \t\tif (is_directory(git_dir)) {\n+\t\t\tspf = submodule_parallel_fetch_create();\n+\t\t\tspf->result = &result;\n+\t\t\tspf->cp.dir = strbuf_detach(&submodule_path, NULL);\n \t\t\tif (!quiet)\n-\t\t\t\tprintf(\"Fetching submodule %s%s\\n\", prefix, ce->name);\n-\t\t\tcp.dir = submodule_path.buf;\n-\t\t\targv_array_push(&argv, default_argv);\n-\t\t\targv_array_push(&argv, \"--submodule-prefix\");\n-\t\t\targv_array_push(&argv, submodule_prefix.buf);\n-\t\t\tcp.argv = argv.argv;\n-\t\t\tif (run_command(&cp))\n-\t\t\t\tresult = 1;\n-\t\t\targv_array_pop(&argv);\n-\t\t\targv_array_pop(&argv);\n-\t\t\targv_array_pop(&argv);\n+\t\t\t\tstrbuf_addf(spf->cp.sync_buf,\n+\t\t\t\t\t \"Fetching submodule %s%s\",\n+\t\t\t\t\t prefix, ce->name);\n+\t\t\targv_array_pushv(&spf->argv, argv.argv);\n+\t\t\targv_array_push(&spf->argv, default_argv);\n+\t\t\targv_array_push(&spf->argv, \"--submodule-prefix\");\n+\t\t\targv_array_push(&spf->argv, submodule_prefix.buf);\n+\t\t\tset_output_mutex(spf);\n+\t\t\tadd_task(tq, run_command_and_cleanup, spf);\n \t\t}\n \t\tstrbuf_release(&submodule_path);\n \t\tstrbuf_release(&submodule_git_dir);\n \t\tstrbuf_release(&submodule_prefix);\n \t}\n \targv_array_clear(&argv);\n+\tdestroy_output_mutex();\n out:\n \tstring_list_clear(&changed_submodule_paths, 1);\n \treturn result;\ndiff --git a/submodule.h b/submodule.h\nindex 5507c3d..cbc0003 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -31,7 +31,7 @@ void set_config_fetch_recurse_submodules(int value);\n void check_for_new_submodule_commits(unsigned char new_sha1[20]);\n int fetch_populated_submodules(const struct argv_array *options,\n \t\t\t const char *prefix, int command_line_option,\n-\t\t\t int quiet);\n+\t\t\t int quiet, int max_parallel_jobs);\n unsigned is_submodule_modified(const char *path, int ignore_untracked);\n int submodule_uses_gitfile(const char *path);\n int ok_to_remove_submodule(const char *path);\n-- \n2.5.0.264.g5e52b0d\n"},{"id":"268813","messageId":"1440724495-708-9-git-send-email-sbeller@google.com","threadId":"40201","inReplyTo":"1440724495-708-1-git-send-email-sbeller@google.com","subject":"[PATCH 8/9] index-pack: Use the new worker pool","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T01:14:54Z","receivedAt":"2015-08-28T01:14:54Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This demonstrates how the new threading API may be used.\nThere is no change in the workflow, just using the new\nthreading API instead of keeping track of the pthreads\nourselves.\n\nSigned-off-by: Stefan Beller \n---\n builtin/index-pack.c | 23 ++++++++++++-----------\n 1 file changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 3f10840..187b281 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1075,7 +1075,7 @@ static void resolve_base(struct object_entry *obj)\n }\n \n #ifndef NO_PTHREADS\n-static void *threaded_second_pass(void *data)\n+static int threaded_second_pass(struct task_queue *tq, void *data)\n {\n \tset_thread_data(data);\n \tfor (;;) {\n@@ -1096,7 +1096,7 @@ static void *threaded_second_pass(void *data)\n \n \t\tresolve_base(&objects[i]);\n \t}\n-\treturn NULL;\n+\treturn 0;\n }\n #endif\n \n@@ -1195,18 +1195,19 @@ static void resolve_deltas(void)\n \t\t\t\t\t nr_ref_deltas + nr_ofs_deltas);\n \n #ifndef NO_PTHREADS\n-\tnr_dispatched = 0;\n+\n \tif (nr_threads > 1 || getenv(\"GIT_FORCE_THREADS\")) {\n+\t\tstruct task_queue *tq;\n+\t\tnr_dispatched = 0;\n \t\tinit_thread();\n-\t\tfor (i = 0; i < nr_threads; i++) {\n-\t\t\tint ret = pthread_create(&thread_data[i].thread, NULL,\n-\t\t\t\t\t\t threaded_second_pass, thread_data + i);\n-\t\t\tif (ret)\n-\t\t\t\tdie(_(\"unable to create thread: %s\"),\n-\t\t\t\t strerror(ret));\n-\t\t}\n+\n+\t\ttq = create_task_queue(nr_threads);\n \t\tfor (i = 0; i < nr_threads; i++)\n-\t\t\tpthread_join(thread_data[i].thread, NULL);\n+\t\t\tadd_task(tq, threaded_second_pass, thread_data + i);\n+\n+\t\tif (finish_task_queue(tq, NULL))\n+\t\t\tdie(\"Not all threads have finished\");\n+\n \t\tcleanup_thread();\n \t\treturn;\n \t}\n-- \n2.5.0.264.g5e52b0d\n"},{"id":"268817","messageId":"1440724495-708-10-git-send-email-sbeller@google.com","threadId":"40201","inReplyTo":"1440724495-708-1-git-send-email-sbeller@google.com","subject":"[PATCH 9/9] pack-objects: Use new worker pool","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T01:14:55Z","receivedAt":"2015-08-28T01:14:55Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Before we had threads doing the delta finding work, and the main thread\nwas load balancing the threads, i.e. moving work from a thread with a large\namount left to an idle thread whenever such a situation arose.\n\nThis moves the load balancing to the threads themselves. As soon as one\nthread is done working it will look at its peer threads and will pickup\nhalf the work load from the thread with the largest pending load.\n\nBy having the load balancing as part of the threads, the locking and\ncommunication model becomes easier, such that we don't need so many\nmutexes any more.\n\nIt also demonstrates the usage of the new threading pool being easily\napplied in different situations.\n\nSigned-off-by: Stefan Beller \n---\n builtin/pack-objects.c | 175 ++++++++++++++++---------------------------------\n 1 file changed, 57 insertions(+), 118 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 62cc16d..f46d2df 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -17,6 +17,7 @@\n #include \"pack-objects.h\"\n #include \"progress.h\"\n #include \"refs.h\"\n+#include \"run-command.h\"\n #include \"streaming.h\"\n #include \"thread-utils.h\"\n #include \"pack-bitmap.h\"\n@@ -1887,26 +1888,12 @@ static void try_to_free_from_threads(size_t size)\n \n static try_to_free_t old_try_to_free_routine;\n \n-/*\n- * The main thread waits on the condition that (at least) one of the workers\n- * has stopped working (which is indicated in the .working member of\n- * struct thread_params).\n- * When a work thread has completed its work, it sets .working to 0 and\n- * signals the main thread and waits on the condition that .data_ready\n- * becomes 1.\n- */\n-\n struct thread_params {\n-\tpthread_t thread;\n \tstruct object_entry **list;\n \tunsigned list_size;\n \tunsigned remaining;\n \tint window;\n \tint depth;\n-\tint working;\n-\tint data_ready;\n-\tpthread_mutex_t mutex;\n-\tpthread_cond_t cond;\n \tunsigned *processed;\n };\n \n@@ -1933,7 +1920,52 @@ static void cleanup_threaded_search(void)\n \tpthread_mutex_destroy(&progress_mutex);\n }\n \n-static void *threaded_find_deltas(void *arg)\n+static struct thread_params *p;\n+\n+static void threaded_split_largest_workload(struct thread_params *target)\n+{\n+\tint i;\n+\n+\tstruct object_entry **list;\n+\tstruct thread_params *victim = NULL;\n+\tunsigned sub_size = 0;\n+\n+\t/* Find a victim */\n+\tprogress_lock();\n+\tfor (i = 0; i < delta_search_threads; i++)\n+\t\tif (p[i].remaining > 2*window &&\n+\t\t (!victim || victim->remaining < p[i].remaining))\n+\t\t\tvictim = &p[i];\n+\n+\tif (victim) {\n+\t\tsub_size = victim->remaining / 2;\n+\t\tlist = victim->list + victim->list_size - sub_size;\n+\t\twhile (sub_size && list[0]->hash &&\n+\t\t list[0]->hash == list[-1]->hash) {\n+\t\t\tlist++;\n+\t\t\tsub_size--;\n+\t\t}\n+\t\tif (!sub_size) {\n+\t\t\t/*\n+\t\t\t * It is possible for some \"paths\" to have\n+\t\t\t * so many objects that no hash boundary\n+\t\t\t * might be found. Let's just steal the\n+\t\t\t * exact half in that case.\n+\t\t\t */\n+\t\t\tsub_size = victim->remaining / 2;\n+\t\t\tlist -= sub_size;\n+\t\t}\n+\t\tvictim->list_size -= sub_size;\n+\t\tvictim->remaining -= sub_size;\n+\n+\t\ttarget->list = list;\n+\t\ttarget->list_size = sub_size;\n+\t\ttarget->remaining = sub_size;\n+\t}\n+\tprogress_unlock();\n+}\n+\n+static int threaded_find_deltas(struct task_queue *tq, void *arg)\n {\n \tstruct thread_params *me = arg;\n \n@@ -1941,34 +1973,17 @@ static void *threaded_find_deltas(void *arg)\n \t\tfind_deltas(me->list, &me->remaining,\n \t\t\t me->window, me->depth, me->processed);\n \n-\t\tprogress_lock();\n-\t\tme->working = 0;\n-\t\tpthread_cond_signal(&progress_cond);\n-\t\tprogress_unlock();\n-\n-\t\t/*\n-\t\t * We must not set ->data_ready before we wait on the\n-\t\t * condition because the main thread may have set it to 1\n-\t\t * before we get here. In order to be sure that new\n-\t\t * work is available if we see 1 in ->data_ready, it\n-\t\t * was initialized to 0 before this thread was spawned\n-\t\t * and we reset it to 0 right away.\n-\t\t */\n-\t\tpthread_mutex_lock(&me->mutex);\n-\t\twhile (!me->data_ready)\n-\t\t\tpthread_cond_wait(&me->cond, &me->mutex);\n-\t\tme->data_ready = 0;\n-\t\tpthread_mutex_unlock(&me->mutex);\n+\t\tthreaded_split_largest_workload(me);\n \t}\n-\t/* leave ->working 1 so that this doesn't get more work assigned */\n-\treturn NULL;\n+\n+\treturn 0;\n }\n \n static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \t\t\t int window, int depth, unsigned *processed)\n {\n-\tstruct thread_params *p;\n-\tint i, ret, active_threads = 0;\n+\tstruct task_queue *tq;\n+\tint i;\n \n \tinit_threaded_search();\n \n@@ -1980,8 +1995,11 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \tif (progress > pack_to_stdout)\n \t\tfprintf(stderr, \"Delta compression using up to %d threads.\\n\",\n \t\t\t\tdelta_search_threads);\n+\n \tp = xcalloc(delta_search_threads, sizeof(*p));\n \n+\ttq = create_task_queue(delta_search_threads);\n+\n \t/* Partition the work amongst work threads. */\n \tfor (i = 0; i < delta_search_threads; i++) {\n \t\tunsigned sub_size = list_size / (delta_search_threads - i);\n@@ -1993,8 +2011,6 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \t\tp[i].window = window;\n \t\tp[i].depth = depth;\n \t\tp[i].processed = processed;\n-\t\tp[i].working = 1;\n-\t\tp[i].data_ready = 0;\n \n \t\t/* try to split chunks on \"path\" boundaries */\n \t\twhile (sub_size && sub_size < list_size &&\n@@ -2008,87 +2024,10 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \n \t\tlist += sub_size;\n \t\tlist_size -= sub_size;\n+\t\tadd_task(tq, threaded_find_deltas, &p[i]);\n \t}\n \n-\t/* Start work threads. */\n-\tfor (i = 0; i < delta_search_threads; i++) {\n-\t\tif (!p[i].list_size)\n-\t\t\tcontinue;\n-\t\tpthread_mutex_init(&p[i].mutex, NULL);\n-\t\tpthread_cond_init(&p[i].cond, NULL);\n-\t\tret = pthread_create(&p[i].thread, NULL,\n-\t\t\t\t threaded_find_deltas, &p[i]);\n-\t\tif (ret)\n-\t\t\tdie(\"unable to create thread: %s\", strerror(ret));\n-\t\tactive_threads++;\n-\t}\n-\n-\t/*\n-\t * Now let's wait for work completion. Each time a thread is done\n-\t * with its work, we steal half of the remaining work from the\n-\t * thread with the largest number of unprocessed objects and give\n-\t * it to that newly idle thread. This ensure good load balancing\n-\t * until the remaining object list segments are simply too short\n-\t * to be worth splitting anymore.\n-\t */\n-\twhile (active_threads) {\n-\t\tstruct thread_params *target = NULL;\n-\t\tstruct thread_params *victim = NULL;\n-\t\tunsigned sub_size = 0;\n-\n-\t\tprogress_lock();\n-\t\tfor (;;) {\n-\t\t\tfor (i = 0; !target && i < delta_search_threads; i++)\n-\t\t\t\tif (!p[i].working)\n-\t\t\t\t\ttarget = &p[i];\n-\t\t\tif (target)\n-\t\t\t\tbreak;\n-\t\t\tpthread_cond_wait(&progress_cond, &progress_mutex);\n-\t\t}\n-\n-\t\tfor (i = 0; i < delta_search_threads; i++)\n-\t\t\tif (p[i].remaining > 2*window &&\n-\t\t\t (!victim || victim->remaining < p[i].remaining))\n-\t\t\t\tvictim = &p[i];\n-\t\tif (victim) {\n-\t\t\tsub_size = victim->remaining / 2;\n-\t\t\tlist = victim->list + victim->list_size - sub_size;\n-\t\t\twhile (sub_size && list[0]->hash &&\n-\t\t\t list[0]->hash == list[-1]->hash) {\n-\t\t\t\tlist++;\n-\t\t\t\tsub_size--;\n-\t\t\t}\n-\t\t\tif (!sub_size) {\n-\t\t\t\t/*\n-\t\t\t\t * It is possible for some \"paths\" to have\n-\t\t\t\t * so many objects that no hash boundary\n-\t\t\t\t * might be found. Let's just steal the\n-\t\t\t\t * exact half in that case.\n-\t\t\t\t */\n-\t\t\t\tsub_size = victim->remaining / 2;\n-\t\t\t\tlist -= sub_size;\n-\t\t\t}\n-\t\t\ttarget->list = list;\n-\t\t\tvictim->list_size -= sub_size;\n-\t\t\tvictim->remaining -= sub_size;\n-\t\t}\n-\t\ttarget->list_size = sub_size;\n-\t\ttarget->remaining = sub_size;\n-\t\ttarget->working = 1;\n-\t\tprogress_unlock();\n-\n-\t\tpthread_mutex_lock(&target->mutex);\n-\t\ttarget->data_ready = 1;\n-\t\tpthread_cond_signal(&target->cond);\n-\t\tpthread_mutex_unlock(&target->mutex);\n-\n-\t\tif (!sub_size) {\n-\t\t\tpthread_join(target->thread, NULL);\n-\t\t\tpthread_cond_destroy(&target->cond);\n-\t\t\tpthread_mutex_destroy(&target->mutex);\n-\t\t\tactive_threads--;\n-\t\t}\n-\t}\n+\tfinish_task_queue(tq, NULL);\n \tcleanup_threaded_search();\n \tfree(p);\n }\n-- \n2.5.0.264.g5e52b0d\n"},{"id":"268832","messageId":"b5126a0a9438e8737c726d887ebc96e3@www.dscho.org","threadId":"40201","inReplyTo":"1440724495-708-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH 0/9] Progress with git submodule","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-08-28T10:09:02Z","receivedAt":"2015-08-28T10:09:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn 2015-08-28 03:14, Stefan Beller wrote:\n\n> Stefan Beller (9):\n> submodule: implement `module_list` as a builtin helper\n> submodule: implement `module_name` as a builtin helper\n> submodule: implement `module_clone` as a builtin helper\n\nAnother thing that just hit me: is there any specific reason why the underscore? I think we prefer dashes for subcommands (think: cherry-pick) and maybe we want to do the same for subsubcommands...\n\nCiao,\nDscho\n"},{"id":"268843","messageId":"CAGZ79kbBfHUYJN1UBeynSAwfGhSmscarGnzSs9sZJmvN2m0VQg@mail.gmail.com","threadId":"40201","inReplyTo":"b5126a0a9438e8737c726d887ebc96e3@www.dscho.org","subject":"Re: [PATCH 0/9] Progress with git submodule","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T16:35:14Z","receivedAt":"2015-08-28T16:35:14Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Aug 28, 2015 at 3:09 AM, Johannes Schindelin\n wrote:\n> Hi Stefan,\n>\n> On 2015-08-28 03:14, Stefan Beller wrote:\n>\n>> Stefan Beller (9):\n>> submodule: implement `module_list` as a builtin helper\n>> submodule: implement `module_name` as a builtin helper\n>> submodule: implement `module_clone` as a builtin helper\n>\n> Another thing that just hit me: is there any specific reason why the underscore? I think we prefer dashes for subcommands (think: cherry-pick) and maybe we want to do the same for subsubcommands...\n>\n> Ciao,\n> Dscho\n>\n\nJunio wrote on Aug 3rd:\n>>> $ git submodule--helper module_list\n>>\n>> Why would you use an underscore in here as opposed to a dash?\n>\n> Simply because the diff would be easier to read; the callers used to\n> call module_list shell function, now they call the subcommand with the\n> same name of submodule--helper.\n\nI mean no user is expected to use the submodule--helper functions directly and\nwe want to get rid of them eventually anyway (once the whole git-submodule.sh\nis rewritten in C we can just call the C functions instead of calling\nout to helpers)\n"},{"id":"268847","messageId":"CAGZ79kbTAVDVmw+MrXvky6tJWZcG97tT_KAxV7S-pKCiNqRp3g@mail.gmail.com","threadId":"40201","inReplyTo":"1440724495-708-8-git-send-email-sbeller@google.com","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T17:00:00Z","receivedAt":"2015-08-28T17:00:00Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Aug 27, 2015 at 6:14 PM, Stefan Beller wrote:\n> This makes use of the new task queue and the syncing feature of\n> run-command to fetch a number of submodules at the same time.\n>\n> The output will look like it would have been run sequential,\n> but faster.\n\nAnd it breaks the tests t5526-fetch-submodules.sh as the output is done\non stderr only, instead of putting \"Fetching submodule \nto stdout. :(\n\nI guess combining stdout and stderr is not a good strategy after all now.\nAdvantages:\n* The order is preserved if everything is in one stream.\nDisadvantages:\n* It's a change, which may not be expected.\n* It's harder to make it machine parsable, as that probably\n relied on having 2 channels.\n\nSo now I need to come up with a way to either buffer 2 channels at the same\ntime, or we need to redefine parallel submodule fetch output semantics a bit.\n\n>\n> Signed-off-by: Stefan Beller \n> ---\n> Documentation/fetch-options.txt | 7 +++\n> builtin/fetch.c | 6 ++-\n> builtin/pull.c | 6 +++\n> submodule.c | 100 +++++++++++++++++++++++++++++++++-------\n> submodule.h | 2 +-\n> 5 files changed, 102 insertions(+), 19 deletions(-)\n>\n> diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\n> index 45583d8..e2a59c3 100644\n> --- a/Documentation/fetch-options.txt\n> +++ b/Documentation/fetch-options.txt\n> @@ -100,6 +100,13 @@ ifndef::git-pull[]\n> reference to a commit that isn't already in the local submodule\n> clone.\n>\n> +-j::\n> +--jobs=::\n> + Number of threads to be used for fetching submodules. Each thread\n> + will fetch from different submodules, such that fetching many\n> + submodules will be faster. By default the number of cpus will\n> + be used .\n> +\n> --no-recurse-submodules::\n> Disable recursive fetching of submodules (this has the same effect as\n> using the '--recurse-submodules=no' option).\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index ee1f1a9..636707e 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -37,6 +37,7 @@ static int prune = -1; /* unspecified */\n> static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;\n> static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n> static int tags = TAGS_DEFAULT, unshallow, update_shallow;\n> +static int max_threads;\n> static const char *depth;\n> static const char *upload_pack;\n> static struct strbuf default_rla = STRBUF_INIT;\n> @@ -99,6 +100,8 @@ static struct option builtin_fetch_options[] = {\n> N_(\"fetch all tags and associated objects\"), TAGS_SET),\n> OPT_SET_INT('n', NULL, &tags,\n> N_(\"do not fetch all tags (--no-tags)\"), TAGS_UNSET),\n> + OPT_INTEGER('j', \"jobs\", &max_threads,\n> + N_(\"number of threads used for fetching\")),\n> OPT_BOOL('p', \"prune\", &prune,\n> N_(\"prune remote-tracking branches no longer on remote\")),\n> { OPTION_CALLBACK, 0, \"recurse-submodules\", NULL, N_(\"on-demand\"),\n> @@ -1217,7 +1220,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n> result = fetch_populated_submodules(&options,\n> submodule_prefix,\n> recurse_submodules,\n> - verbosity < 0);\n> + verbosity < 0,\n> + max_threads);\n> argv_array_clear(&options);\n> }\n>\n> diff --git a/builtin/pull.c b/builtin/pull.c\n> index 722a83c..fbbda67 100644\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -94,6 +94,7 @@ static int opt_force;\n> static char *opt_tags;\n> static char *opt_prune;\n> static char *opt_recurse_submodules;\n> +static char *max_threads;\n> static int opt_dry_run;\n> static char *opt_keep;\n> static char *opt_depth;\n> @@ -177,6 +178,9 @@ static struct option pull_options[] = {\n> N_(\"on-demand\"),\n> N_(\"control recursive fetching of submodules\"),\n> PARSE_OPT_OPTARG),\n> + OPT_PASSTHRU('j', \"jobs\", &max_threads, N_(\"n\"),\n> + N_(\"number of threads used for fetching submodules\"),\n> + PARSE_OPT_OPTARG),\n> OPT_BOOL(0, \"dry-run\", &opt_dry_run,\n> N_(\"dry run\")),\n> OPT_PASSTHRU('k', \"keep\", &opt_keep, NULL,\n> @@ -524,6 +528,8 @@ static int run_fetch(const char *repo, const char **refspecs)\n> argv_array_push(&args, opt_prune);\n> if (opt_recurse_submodules)\n> argv_array_push(&args, opt_recurse_submodules);\n> + if (max_threads)\n> + argv_array_push(&args, max_threads);\n> if (opt_dry_run)\n> argv_array_push(&args, \"--dry-run\");\n> if (opt_keep)\n> diff --git a/submodule.c b/submodule.c\n> index 9fcc86f..50266a8 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -12,6 +12,7 @@\n> #include \"sha1-array.h\"\n> #include \"argv-array.h\"\n> #include \"blob.h\"\n> +#include \"thread-utils.h\"\n>\n> static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;\n> static struct string_list changed_submodule_paths;\n> @@ -615,13 +616,79 @@ static void calculate_changed_submodule_paths(void)\n> initialized_fetch_ref_tips = 0;\n> }\n>\n> +struct submodule_parallel_fetch {\n> + struct child_process cp;\n> + struct argv_array argv;\n> + int *result;\n> +};\n> +\n> +#ifndef NO_PTHREADS\n> +static pthread_mutex_t output_mutex = PTHREAD_MUTEX_INITIALIZER;\n> +\n> +static void set_output_mutex(struct submodule_parallel_fetch *spf)\n> +{\n> + spf->cp.sync_mutex = &output_mutex;\n> +}\n> +\n> +#define lock_output_mutex() pthread_mutex_lock(&output_mutex)\n> +\n> +#define unlock_output_mutex() pthread_mutex_unlock(&output_mutex)\n> +\n> +static void destroy_output_mutex()\n> +{\n> + pthread_mutex_destroy(&output_mutex);\n> +}\n> +\n> +#else\n> +#define set_output_mutex()\n> +#define destroy_output_mutex()\n> +#define lock_output_mutex()\n> +#define unlock_output_mutex()\n> +#endif\n> +\n> +static struct submodule_parallel_fetch *submodule_parallel_fetch_create()\n> +{\n> + struct submodule_parallel_fetch *spf = xmalloc(sizeof(*spf));\n> + child_process_init(&spf->cp);\n> + spf->cp.env = local_repo_env;\n> + spf->cp.git_cmd = 1;\n> + spf->cp.no_stdin = 1;\n> + spf->cp.stdout_to_stderr = 1;\n> + spf->cp.sync_buf = xmalloc(sizeof(spf->cp.sync_buf));\n> + strbuf_init(spf->cp.sync_buf, 0);\n> +\n> + argv_array_init(&spf->argv);\n> + return spf;\n> +}\n> +\n> +static int run_command_and_cleanup(struct task_queue *tq, void *arg)\n> +{\n> + int code;\n> + struct submodule_parallel_fetch *spf = arg;\n> +\n> + spf->cp.argv = spf->argv.argv;\n> +\n> + code = run_command(&spf->cp);\n> + if (code) {\n> + lock_output_mutex();\n> + *spf->result = code;\n> + unlock_output_mutex();\n> + }\n> +\n> + argv_array_clear(&spf->argv);\n> + free((char*)spf->cp.dir);\n> + free(spf);\n> + return 0;\n> +}\n> +\n> int fetch_populated_submodules(const struct argv_array *options,\n> const char *prefix, int command_line_option,\n> - int quiet)\n> + int quiet, int max_parallel_jobs)\n> {\n> int i, result = 0;\n> - struct child_process cp = CHILD_PROCESS_INIT;\n> + struct task_queue *tq;\n> struct argv_array argv = ARGV_ARRAY_INIT;\n> + struct submodule_parallel_fetch *spf;\n> const char *work_tree = get_git_work_tree();\n> if (!work_tree)\n> goto out;\n> @@ -635,12 +702,9 @@ int fetch_populated_submodules(const struct argv_array *options,\n> argv_array_push(&argv, \"--recurse-submodules-default\");\n> /* default value, \"--submodule-prefix\" and its value are added later */\n>\n> - cp.env = local_repo_env;\n> - cp.git_cmd = 1;\n> - cp.no_stdin = 1;\n> -\n> calculate_changed_submodule_paths();\n>\n> + tq = create_task_queue(max_parallel_jobs);\n> for (i = 0; i < active_nr; i++) {\n> struct strbuf submodule_path = STRBUF_INIT;\n> struct strbuf submodule_git_dir = STRBUF_INIT;\n> @@ -693,24 +757,26 @@ int fetch_populated_submodules(const struct argv_array *options,\n> if (!git_dir)\n> git_dir = submodule_git_dir.buf;\n> if (is_directory(git_dir)) {\n> + spf = submodule_parallel_fetch_create();\n> + spf->result = &result;\n> + spf->cp.dir = strbuf_detach(&submodule_path, NULL);\n> if (!quiet)\n> - printf(\"Fetching submodule %s%s\\n\", prefix, ce->name);\n> - cp.dir = submodule_path.buf;\n> - argv_array_push(&argv, default_argv);\n> - argv_array_push(&argv, \"--submodule-prefix\");\n> - argv_array_push(&argv, submodule_prefix.buf);\n> - cp.argv = argv.argv;\n> - if (run_command(&cp))\n> - result = 1;\n> - argv_array_pop(&argv);\n> - argv_array_pop(&argv);\n> - argv_array_pop(&argv);\n> + strbuf_addf(spf->cp.sync_buf,\n> + \"Fetching submodule %s%s\",\n> + prefix, ce->name);\n> + argv_array_pushv(&spf->argv, argv.argv);\n> + argv_array_push(&spf->argv, default_argv);\n> + argv_array_push(&spf->argv, \"--submodule-prefix\");\n> + argv_array_push(&spf->argv, submodule_prefix.buf);\n> + set_output_mutex(spf);\n> + add_task(tq, run_command_and_cleanup, spf);\n> }\n> strbuf_release(&submodule_path);\n> strbuf_release(&submodule_git_dir);\n> strbuf_release(&submodule_prefix);\n> }\n> argv_array_clear(&argv);\n> + destroy_output_mutex();\n> out:\n> string_list_clear(&changed_submodule_paths, 1);\n> return result;\n> diff --git a/submodule.h b/submodule.h\n> index 5507c3d..cbc0003 100644\n> --- a/submodule.h\n> +++ b/submodule.h\n> @@ -31,7 +31,7 @@ void set_config_fetch_recurse_submodules(int value);\n> void check_for_new_submodule_commits(unsigned char new_sha1[20]);\n> int fetch_populated_submodules(const struct argv_array *options,\n> const char *prefix, int command_line_option,\n> - int quiet);\n> + int quiet, int max_parallel_jobs);\n> unsigned is_submodule_modified(const char *path, int ignore_untracked);\n> int submodule_uses_gitfile(const char *path);\n> int ok_to_remove_submodule(const char *path);\n> --\n> 2.5.0.264.g5e52b0d\n>\n"},{"id":"268848","messageId":"20150828170141.GB8165@google.com","threadId":"40201","inReplyTo":"CAGZ79kbTAVDVmw+MrXvky6tJWZcG97tT_KAxV7S-pKCiNqRp3g@mail.gmail.com","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2015-08-28T17:01:41Z","receivedAt":"2015-08-28T17:01:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Stefan Beller wrote:\n> On Thu, Aug 27, 2015 at 6:14 PM, Stefan Beller wrote:\n\n>> This makes use of the new task queue and the syncing feature of\n>> run-command to fetch a number of submodules at the same time.\n>>\n>> The output will look like it would have been run sequential,\n>> but faster.\n>\n> And it breaks the tests t5526-fetch-submodules.sh as the output is done\n> on stderr only, instead of putting \"Fetching submodule \n> to stdout. :(\n>\n> I guess combining stdout and stderr is not a good strategy after all now.\n\nIMHO the \"Fetching submodule \" output always should have\ngone to stderr. It is not output that scripts would be relying on ---\nit is just progress output.\n\nSo a preliminary patch doing that (and updating tests) would make sense\nto me.\n\nThoughts?\nJonathan\n"},{"id":"268850","messageId":"CAGZ79kY_HmGcgm+sC+9R_cYKQFS-H_gh8g_4Ai8rwFd1NdA6zA@mail.gmail.com","threadId":"40201","inReplyTo":"1440724495-708-7-git-send-email-sbeller@google.com","subject":"Re: [PATCH 6/9] submodule: helper to run foreach in parallel","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T17:08:26Z","receivedAt":"2015-08-28T17:08:26Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Aug 27, 2015 at 6:14 PM, Stefan Beller wrote:\n> Similar to `git submodule foreach` the new command `git submodule\n> foreach_parallel` will run a command on each submodule.\n>\n> The commands are run in parallel up to the number of cores by default,\n> or you can specify '-j 4' tun just run with 4 threads for example.\n>\n> One major difference to `git submodule foreach` is the handling of input\n> and output to the commands. Because of the parallel nature of the execution\n> it is not trivial how to schedule the std{in,out,err} channel for submodule\n> the command is run in. So in this patch there is no support for stdin.\n> stdout will be piped to stderr. stderr will make use of the synchronized\n> output feature of run_command.\n>\n> Signed-off-by: Stefan Beller \n> ---\n> builtin/submodule--helper.c | 133 ++++++++++++++++++++++++++++++++++++++++++-\n> git-submodule.sh | 11 +++-\n> t/t7407-submodule-foreach.sh | 11 ++++\n> 3 files changed, 153 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index d29499c..18b67f0 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -8,6 +8,7 @@\n> #include \"submodule.h\"\n> #include \"submodule-config.h\"\n> #include \"string-list.h\"\n> +#include \"thread-utils.h\"\n> #include \"run-command.h\"\n>\n> static const struct cache_entry **ce_entries;\n> @@ -266,6 +267,127 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n> return 0;\n> }\n>\n> +#ifndef NO_PTHREADS\n> +struct submodule_args {\n> + const char *name;\n> + const char *path;\n> + const char *sha1;\n> + const char *toplevel;\n> + const char *prefix;\n> + const char **cmd;\n> + pthread_mutex_t *sync;\n> +};\n> +\n> +int run_cmd_submodule(struct task_queue *aq, void *task)\n> +{\n> + int i;\n> + struct submodule_args *args = task;\n> + struct strbuf out = STRBUF_INIT;\n> + struct strbuf sb = STRBUF_INIT;\n> + struct child_process *cp = xmalloc(sizeof(*cp));\n> +\n> + strbuf_addf(&out, N_(\"Entering %s\\n\"), relative_path(args->path, args->prefix, &sb));\n> +\n> + child_process_init(cp);\n> + argv_array_pushv(&cp->args, args->cmd);\n> +\n> + argv_array_pushf(&cp->env_array, \"name=%s\", args->name);\n> + argv_array_pushf(&cp->env_array, \"path=%s\", args->path);\n> + argv_array_pushf(&cp->env_array, \"sha1=%s\", args->sha1);\n> + argv_array_pushf(&cp->env_array, \"toplevel=%s\", args->toplevel);\n> +\n> + for (i = 0; local_repo_env[i]; i++)\n> + argv_array_push(&cp->env_array, local_repo_env[i]);\n> +\n> + cp->no_stdin = 1;\n> + cp->out = 0;\n> + cp->err = -1;\n> + cp->dir = args->path;\n> + cp->stdout_to_stderr = 1;\n> + cp->use_shell = 1;\n> + cp->sync_mutex = args->sync;\n> + cp->sync_buf = &out;\n> +\n> + return run_command(cp);\n> +}\n> +\n> +int module_foreach_parallel(int argc, const char **argv, const char *prefix)\n> +{\n> + int i, recursive = 0, number_threads = 0, quiet = 0;\n> + static struct pathspec pathspec;\n> + struct strbuf sb = STRBUF_INIT;\n> + struct task_queue *aq;\n> + char **cmd;\n> + const char **nullargv = {NULL};\n> + pthread_mutex_t mutex;\n> +\n> + struct option module_update_options[] = {\n> + OPT_STRING(0, \"prefix\", &alternative_path,\n> + N_(\"path\"),\n> + N_(\"alternative anchor for relative paths\")),\n> + OPT_STRING(0, \"cmd\", &cmd,\n> + N_(\"string\"),\n> + N_(\"command to run\")),\n> + OPT_BOOL('r', \"--recursive\", &recursive,\n> + N_(\"Recurse into nexted submodules\")),\n> + OPT_INTEGER('j', \"jobs\", &number_threads,\n> + N_(\"Recurse into nexted submodules\")),\n> + OPT__QUIET(&quiet, N_(\"Suppress output\")),\n> + OPT_END()\n> + };\n> +\n> + static const char * const git_submodule_helper_usage[] = {\n> + N_(\"git submodule--helper foreach [--prefix=] [...]\"),\n> + NULL\n> + };\n> +\n> + argc = parse_options(argc, argv, prefix, module_update_options,\n> + git_submodule_helper_usage, 0);\n> +\n> + if (module_list_compute(0, nullargv, NULL, &pathspec) < 0)\n> + return 1;\n> +\n> + gitmodules_config();\n> +\n> + pthread_mutex_init(&mutex, NULL);\n> + aq = create_task_queue(number_threads);\n> +\n> + for (i = 0; i < ce_used; i++) {\n> + const struct submodule *sub;\n> + const struct cache_entry *ce = ce_entries[i];\n> + struct submodule_args *args = malloc(sizeof(*args));\n> +\n> + if (ce_stage(ce))\n> + args->sha1 = xstrdup(sha1_to_hex(null_sha1));\n> + else\n> + args->sha1 = xstrdup(sha1_to_hex(ce->sha1));\n> +\n> + strbuf_reset(&sb);\n> + strbuf_addf(&sb, \"%s/.git\", ce->name);\n> + if (!file_exists(sb.buf)) {\n> + free(args);\n> + continue;\n> + }\n> +\n> + args->path = ce->name;\n> + sub = submodule_from_path(null_sha1, args->path);\n> + if (!sub)\n> + die(\"No submodule mapping found in .gitmodules for path '%s'\", args->path);\n> +\n> + args->name = sub->name;\n> + args->toplevel = xgetcwd();\n> + args->cmd = argv;\n> + args->sync = &mutex;\n> + args->prefix = alternative_path;\n> + add_task(aq, run_cmd_submodule, args);\n> + }\n> +\n> + finish_task_queue(aq, NULL);\n> + pthread_mutex_destroy(&mutex);\n> + return 0;\n> +}\n> +#endif /* NO_PTHREADS */\n> +\n> int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n> {\n> if (argc < 2)\n> @@ -280,7 +402,16 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n> if (!strcmp(argv[1], \"module_clone\"))\n> return module_clone(argc - 1, argv + 1, prefix);\n>\n> +#ifndef NO_PTHREADS\n> + if (!strcmp(argv[1], \"foreach_parallel\"))\n> + return module_foreach_parallel(argc - 1, argv + 1, prefix);\n> +#endif\n> +\n> usage:\n> usage(\"git submodule--helper [module_list | module_name | \"\n> - \"module_clone]\\n\");\n> + \"module_clone\"\n> +#ifndef NO_PTHREADS\n> + \" | foreach_parallel\"\n> +#endif\n> + \"]\\n\");\n> }\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index fb5155e..f06488a 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -431,6 +431,15 @@ cmd_foreach()\n> }\n>\n> #\n> +# Execute an arbitrary command sequence in each checked out\n> +# submodule in parallel.\n> +#\n> +cmd_foreach_parallel()\n> +{\n> + git submodule--helper foreach_parallel --prefix \"$wt_prefix\" $@\n> +}\n> +\n> +#\n> # Register submodules in .git/config\n> #\n> # $@ = requested paths (default to all)\n> @@ -1225,7 +1234,7 @@ cmd_sync()\n> while test $# != 0 && test -z \"$command\"\n> do\n> case \"$1\" in\n> - add | foreach | init | deinit | update | status | summary | sync)\n> + add | foreach | foreach_parallel | init | deinit | update | status | summary | sync)\n> command=$1\n> ;;\n> -q|--quiet)\n> diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\n> index 7ca10b8..16f6138 100755\n> --- a/t/t7407-submodule-foreach.sh\n> +++ b/t/t7407-submodule-foreach.sh\n> @@ -195,6 +195,17 @@ test_expect_success 'test \"foreach --quiet --recursive\"' '\n> test_cmp expect actual\n> '\n>\n> +test_expect_success 'test \"foreach_parallel --quiet\"' '\n> + (\n> + cd clone2 &&\n> + git submodule foreach_parallel -q -- \"echo \\$name-\\$path\" > ../actual\n> + ) &&\n> + grep nested1-nested1 actual &&\n> + grep foo1-sub1 actual &&\n> + grep foo2-sub2 actual &&\n> + grep foo3-sub3 actual\n> +'\n> +\n\nThis test fails, because the variables don't seem to be resolved. actual reads\nliterally \"$name-$path\" instead of the actual values replaced.\n\nUsing the `git submodule--helper foreach_parallel \"echo\n\\$name-\\$path\"` works as expected,\nso I think there is an issue in wrapping that in git-submodule.sh\n\n> test_expect_success 'use \"update --recursive\" to checkout all submodules' '\n> git clone super clone3 &&\n> (\n> --\n> 2.5.0.264.g5e52b0d\n>\n"},{"id":"268852","messageId":"xmqqfv332v84.fsf@gitster.mtv.corp.google.com","threadId":"40201","inReplyTo":"20150828170141.GB8165@google.com","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-28T17:12:11Z","receivedAt":"2015-08-28T17:12:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Stefan Beller wrote:\n>> On Thu, Aug 27, 2015 at 6:14 PM, Stefan Beller wrote:\n>\n>>> This makes use of the new task queue and the syncing feature of\n>>> run-command to fetch a number of submodules at the same time.\n>>>\n>>> The output will look like it would have been run sequential,\n>>> but faster.\n>>\n>> And it breaks the tests t5526-fetch-submodules.sh as the output is done\n>> on stderr only, instead of putting \"Fetching submodule \n>> to stdout. :(\n>>\n>> I guess combining stdout and stderr is not a good strategy after all now.\n>\n> IMHO the \"Fetching submodule \" output always should have\n> gone to stderr. It is not output that scripts would be relying on ---\n> it is just progress output.\n>\n> So a preliminary patch doing that (and updating tests) would make sense\n> to me.\n\nSounds good.\n\nI personally do not think the \"we still do all the output from a\nsingle process while blocking output from others\" buffering\nimplemented in this n-th round (by the way, please use [PATCH v$n\nN/M]) is worth doing, though. It does not make the output machine\nparseable, because the reader does not get any signal in what order\noutput of these subprocesses arrive anyway. The payload does not\neven have \"here is the beginning of output from the process that\nhandled the submodule X\" to delimit them.\n\nMy preference is still (1) leave standard error output all connected\nto the same fd without multiplexing, and (2) line buffer standard\noutput so that the output is at least readable as a text, in a\nsimilar way a log of an irc channel where everybody is talking at\nthe same time.\n"},{"id":"268857","messageId":"CAGZ79kYWJ89MuU4iJH=Mvr_jH72d+EM0hCeDhfpQuapYQH0Nkg@mail.gmail.com","threadId":"40201","inReplyTo":"xmqqfv332v84.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T17:45:11Z","receivedAt":"2015-08-28T17:45:11Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Aug 28, 2015 at 10:12 AM, Junio C Hamano wrote:\n> Jonathan Nieder writes:\n>\n>> Stefan Beller wrote:\n>>> On Thu, Aug 27, 2015 at 6:14 PM, Stefan Beller wrote:\n>>\n>>>> This makes use of the new task queue and the syncing feature of\n>>>> run-command to fetch a number of submodules at the same time.\n>>>>\n>>>> The output will look like it would have been run sequential,\n>>>> but faster.\n>>>\n>>> And it breaks the tests t5526-fetch-submodules.sh as the output is done\n>>> on stderr only, instead of putting \"Fetching submodule \n>>> to stdout. :(\n>>>\n>>> I guess combining stdout and stderr is not a good strategy after all now.\n>>\n>> IMHO the \"Fetching submodule \" output always should have\n>> gone to stderr. It is not output that scripts would be relying on ---\n>> it is just progress output.\n>>\n>> So a preliminary patch doing that (and updating tests) would make sense\n>> to me.\n>\n> Sounds good.\n>\n> I personally do not think the \"we still do all the output from a\n> single process while blocking output from others\" buffering\n> implemented in this n-th round (by the way, please use [PATCH v$n\n> N/M]) is worth doing, though. It does not make the output machine\n> parseable, because the reader does not get any signal in what order\n> output of these subprocesses arrive anyway.\n\n> The payload does not\n> even have \"here is the beginning of output from the process that\n> handled the submodule X\" to delimit them.\n\nOh it does, but it is not handled by the buffering code, but the application\ncode, so for git-fetch we would have \"Fetching submodule \"\nwhile for git submodule foreach we would have \"Entering \"\n\n>\n> My preference is still (1) leave standard error output all connected\n> to the same fd without multiplexing, and (2) line buffer standard\n> output so that the output is at least readable as a text, in a\n> similar way a log of an irc channel where everybody is talking at\n> the same time.\n\nIn case of fetch passing the quiet flag becomes mandatory then,\nbecause the the per-repo progress is put to stderr, which\nalso deletes previous output, to have the nice counters.\n"},{"id":"268861","messageId":"20150828182008.GC8165@google.com","threadId":"40201","inReplyTo":"xmqqfv332v84.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2015-08-28T18:20:08Z","receivedAt":"2015-08-28T18:20:08Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> My preference is still (1) leave standard error output all connected\n> to the same fd without multiplexing, and (2) line buffer standard\n> output so that the output is at least readable as a text, in a\n> similar way a log of an irc channel where everybody is talking at\n> the same time.\n\nThere is something nice about the immediacy of seeing output from all\nthe subprocesses at the same time in that model.\n\nBut for commands that show progress like \"git clone\", \"git checkout\",\nand \"git fetch\", it does not work well at all. They provide output\nthat updates itself by putting a carriage return at the end of each\nchunk of output, like this:\n\n remote: Finding sources: 11% (18/155) \\r\n remote: Finding sources: 12% (19/155) \\r\n\nWith multiple commands producing such output, they will overwrite each\nother's lines, producing a mixture that is confusing and unuseful.\n\nEven with --no-progress, there is a similar sense of confusion in the\nintermixed output. Error messages are hard to find. This is a\ncomment complaint about the current \"repo sync -j\" implementation.\n\nIdeally what I as a user want to see is something like what \"prove\"\nwrites, showing progress on the multiple tasks that are taking place\nat once:\n\n ===( 103;1 0/? 8/? 3/? 11/? 6/? 16/? 1/? 1/? )==============\n\nThat would require more sophisticated inter-process communication than\nseems necessary for the first version of parallel \"git submodule\nupdate\". For the first version that people use in the wild, showing\noutput from one of the tasks at a time, simulating a sped-up\nsequential implementation, seems useful to me.\n\nIn the degenerate case where only one task is running, it reduces to\nthe current output. The output is familiar and easy to use. I quite\nlike the approach.\n\nMy two cents,\nJonathan\n"},{"id":"268862","messageId":"xmqqpp271d6v.fsf@gitster.mtv.corp.google.com","threadId":"40201","inReplyTo":"20150828182008.GC8165@google.com","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-28T18:27:04Z","receivedAt":"2015-08-28T18:27:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Junio C Hamano wrote:\n>\n>> My preference is still (1) leave standard error output all connected\n>> to the same fd without multiplexing, and (2) line buffer standard\n>> output so that the output is at least readable as a text, in a\n>> similar way a log of an irc channel where everybody is talking at\n>> the same time.\n>\n> There is something nice about the immediacy of seeing output from all\n> the subprocesses at the same time in that model.\n>\n> But for commands that show progress like \"git clone\", \"git checkout\",\n> and \"git fetch\", it does not work well at all. They provide output\n> that updates itself by putting a carriage return at the end of each\n> chunk of output, like this:\n>\n> remote: Finding sources: 11% (18/155) \\r\n> remote: Finding sources: 12% (19/155) \\r\n>\n> With multiple commands producing such output, they will overwrite each\n> other's lines, producing a mixture that is confusing and unuseful.\n\nThat example also illustrates why it is not a useful to buffer all\nof these lines and showing them once.\n\n> Ideally what I as a user want to see is something like what \"prove\"\n> writes, showing progress on the multiple tasks that are taking place\n> at once:\n>\n> ===( 103;1 0/? 8/? 3/? 11/? 6/? 16/? 1/? 1/? )==============\n\nTell me how that \"buffer all and show them once\" helps us to get\nnear that ideal.\n\n> That would require more sophisticated inter-process communication than\n> seems necessary for the first version of parallel \"git submodule\n> update\".\n\nExactly. Why waste memory to buffer and stall the entire output\nfrom other processes in the interim solution, then?\n"},{"id":"268863","messageId":"20150828183501.GA17222@sigill.intra.peff.net","threadId":"40201","inReplyTo":"xmqqpp271d6v.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-08-28T18:35:02Z","receivedAt":"2015-08-28T18:35:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 28, 2015 at 11:27:04AM -0700, Junio C Hamano wrote:\n\n> > But for commands that show progress like \"git clone\", \"git checkout\",\n> > and \"git fetch\", it does not work well at all. They provide output\n> > that updates itself by putting a carriage return at the end of each\n> > chunk of output, like this:\n> >\n> > remote: Finding sources: 11% (18/155) \\r\n> > remote: Finding sources: 12% (19/155) \\r\n> >\n> > With multiple commands producing such output, they will overwrite each\n> > other's lines, producing a mixture that is confusing and unuseful.\n> \n> That example also illustrates why it is not a useful to buffer all\n> of these lines and showing them once.\n\nI think Jonathan's point is that you could pick _one_ active child to\nshow without buffering, while simultaneously buffering everybody else's\noutput. When that finishes, pick a new active child, show its buffer,\nand then start showing its output in realtime. And so on.\n\nSo to an observer, it would look like a serial operation, but subsequent\noperations after the first would magically go much faster (because\nthey'd been working and buffering in the background).\n\nAnd that doesn't require any additional IPC magic (though I am not sure\nhow we get progress in the first place if the child stderr is a\npipe...).\n\n-Peff\n"},{"id":"268864","messageId":"CAGZ79kbJEK=xD1=j3ZaiyRHUQNS_ryTT2zqatO5u=BVRH7YHJw@mail.gmail.com","threadId":"40201","inReplyTo":"xmqqpp271d6v.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T18:36:25Z","receivedAt":"2015-08-28T18:36:25Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Aug 28, 2015 at 11:27 AM, Junio C Hamano wrote:\n> Jonathan Nieder writes:\n>\n>> Junio C Hamano wrote:\n>>\n>>> My preference is still (1) leave standard error output all connected\n>>> to the same fd without multiplexing, and (2) line buffer standard\n>>> output so that the output is at least readable as a text, in a\n>>> similar way a log of an irc channel where everybody is talking at\n>>> the same time.\n>>\n>> There is something nice about the immediacy of seeing output from all\n>> the subprocesses at the same time in that model.\n>>\n>> But for commands that show progress like \"git clone\", \"git checkout\",\n>> and \"git fetch\", it does not work well at all. They provide output\n>> that updates itself by putting a carriage return at the end of each\n>> chunk of output, like this:\n>>\n>> remote: Finding sources: 11% (18/155) \\r\n>> remote: Finding sources: 12% (19/155) \\r\n>>\n>> With multiple commands producing such output, they will overwrite each\n>> other's lines, producing a mixture that is confusing and unuseful.\n>\n> That example also illustrates why it is not a useful to buffer all\n> of these lines and showing them once.\n>\n>> Ideally what I as a user want to see is something like what \"prove\"\n>> writes, showing progress on the multiple tasks that are taking place\n>> at once:\n>>\n>> ===( 103;1 0/? 8/? 3/? 11/? 6/? 16/? 1/? 1/? )==============\n>\n> Tell me how that \"buffer all and show them once\" helps us to get\n> near that ideal.\n\nIt doesn't, but it looks better than the irc like everybody speaks at\nthe same time\nsolution IMHO.\n\nAt the very the user may not even be interested in the details similar\nto what prove\nprovides. The client can estimate its own total progress by weighting\nthe progress\nfrom all tasks.\n\n>\n>> That would require more sophisticated inter-process communication than\n>> seems necessary for the first version of parallel \"git submodule\n>> update\".\n>\n> Exactly. Why waste memory to buffer and stall the entire output\n> from other processes in the interim solution, then?\n>\n>\n"},{"id":"268866","messageId":"xmqqlhcv1cjk.fsf@gitster.mtv.corp.google.com","threadId":"40201","inReplyTo":"20150828183501.GA17222@sigill.intra.peff.net","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-28T18:41:03Z","receivedAt":"2015-08-28T18:41:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I think Jonathan's point is that you could pick _one_ active child to\n> show without buffering, while simultaneously buffering everybody else's\n> output. When that finishes, pick a new active child, show its buffer,\n> and then start showing its output in realtime. And so on.\n>\n> So to an observer, it would look like a serial operation, but subsequent\n> operations after the first would magically go much faster (because\n> they'd been working and buffering in the background).\n\nOK, I can buy that explanation.\n\n> And that doesn't require any additional IPC magic (though I am not sure\n> how we get progress in the first place if the child stderr is a\n> pipe...).\n\nThat is a good point ;-)\n"},{"id":"268867","messageId":"CAGZ79kZALSJ9oY+6ZwyQ9CPFiV-AAq7c-r9mxw3N+gvsDw8m4A@mail.gmail.com","threadId":"40201","inReplyTo":"20150828183501.GA17222@sigill.intra.peff.net","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T18:41:17Z","receivedAt":"2015-08-28T18:41:17Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Aug 28, 2015 at 11:35 AM, Jeff King wrote:\n> On Fri, Aug 28, 2015 at 11:27:04AM -0700, Junio C Hamano wrote:\n>\n>> > But for commands that show progress like \"git clone\", \"git checkout\",\n>> > and \"git fetch\", it does not work well at all. They provide output\n>> > that updates itself by putting a carriage return at the end of each\n>> > chunk of output, like this:\n>> >\n>> > remote: Finding sources: 11% (18/155) \\r\n>> > remote: Finding sources: 12% (19/155) \\r\n>> >\n>> > With multiple commands producing such output, they will overwrite each\n>> > other's lines, producing a mixture that is confusing and unuseful.\n>>\n>> That example also illustrates why it is not a useful to buffer all\n>> of these lines and showing them once.\n>\n> I think Jonathan's point is that you could pick _one_ active child to\n> show without buffering, while simultaneously buffering everybody else's\n> output. When that finishes, pick a new active child, show its buffer,\n> and then start showing its output in realtime. And so on.\n\nor better yet, pick that child with the most progress (i.e. flush all finished\nchildren and then pick the next active child), that would approximate\nthe progress in the output best, as it would reduce the hidden\nbuffered progress.\n\n>\n> So to an observer, it would look like a serial operation, but subsequent\n> operations after the first would magically go much faster (because\n> they'd been working and buffering in the background).\n>\n> And that doesn't require any additional IPC magic (though I am not sure\n> how we get progress in the first place if the child stderr is a\n> pipe...).\n\nMoving the contents from the pipe to a strbuf buffer which we can grow\nindefinitely\n(way larger than pipe limits, but the output of a git fetch should be\nsmall enough for that).\n\n\n\n>\n> -Peff\n"},{"id":"268868","messageId":"20150828184250.GD8165@google.com","threadId":"40201","inReplyTo":"xmqqpp271d6v.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2015-08-28T18:42:50Z","receivedAt":"2015-08-28T18:42:50Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder writes:\n\n>> remote: Finding sources: 11% (18/155) \\r\n>> remote: Finding sources: 12% (19/155) \\r\n>>\n>> With multiple commands producing such output, they will overwrite each\n>> other's lines, producing a mixture that is confusing and unuseful.\n>\n> That example also illustrates why it is not a useful to buffer all\n> of these lines and showing them once.\n\nI don't completely follow. Are you referring to the wasted memory to\nstore the line that is going to be written and rewritten or some other\naspect?\n\nToday (without parallelism), if I clone a repository with multiple\nsubmodules and walk away from the terminal, then when I get back I see\n\n\tCloning into 'plugins/cookbook-plugin'...\n\tremote: Counting objects: 36, done\n\tremote: Finding sources: 100% (36/36)\n\tremote: Total 1192 (delta 196), reused 1192 (delta 196)\n\tReceiving objects: 100% (1192/1192), 239.46 KiB | 0 bytes/s, done.\n\tResolving deltas: 100% (196/196), done.\n\tChecking connectivity... done.\n\tSubmodule path 'plugins/cookbook-plugin': checked out 'b9d3ca8a65030071e28be19296ba867ab424fbbf'\n\tCloning into 'plugins/download-commands'...\n\tremote: Counting objects: 37, done\n\tremote: Finding sources: 100% (37/37)\n\tremote: Total 448 (delta 46), reused 448 (delta 46)\n\tReceiving objects: 100% (448/448), 96.13 KiB | 0 bytes/s, done.\n\tResolving deltas: 100% (46/46), done.\n\tChecking connectivity... done.\n\tSubmodule path 'plugins/download-commands': checked out '99e61fb06a4505a9558c23a56213cb32ceaa9cca'\n\t...\n\nThe output for each submodule is in one chunk and I can understand what\nhappened in each.\n\nBy contrast, with inter-mixing the speed of output tells you something\nabout whether the process is stalled while you stare at the screen and\nwait for it to finish (good) but the result on the screen is\nunintelligible when the process finishes (bad).\n\nShowing interactive output for one task still provides the real-time\nfeedback about whether it is completely stuck and needs to be cancelled.\nIt is easier to make sense of even from the point of view of real-time\nprogress than the intermixed output.\n\nWhat problem is the intermixed output meant to solve?\n\nIn other words, what is your objection about? Perhaps there is a way to\nboth satisfy that objection and to have output clumped per task, but it\nis hard to know without knowing what the objection is.\n\nHope that helps,\nJonathan\n"},{"id":"268869","messageId":"20150828184405.GA17721@sigill.intra.peff.net","threadId":"40201","inReplyTo":"CAGZ79kZALSJ9oY+6ZwyQ9CPFiV-AAq7c-r9mxw3N+gvsDw8m4A@mail.gmail.com","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-08-28T18:44:05Z","receivedAt":"2015-08-28T18:44:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 28, 2015 at 11:41:17AM -0700, Stefan Beller wrote:\n\n> > So to an observer, it would look like a serial operation, but subsequent\n> > operations after the first would magically go much faster (because\n> > they'd been working and buffering in the background).\n> >\n> > And that doesn't require any additional IPC magic (though I am not sure\n> > how we get progress in the first place if the child stderr is a\n> > pipe...).\n> \n> Moving the contents from the pipe to a strbuf buffer which we can grow\n> indefinitely\n> (way larger than pipe limits, but the output of a git fetch should be\n> small enough for that).\n\nRight, clearly we can't rely on pipe buffers to be large enough here\n(though we _may_ want to rely on tempfiles if we aren't sure that the\nstdout is bounded in a reasonable way).\n\nBut what I meant was: the child will only show progress if stderr is a\ntty, but here it is not.\n\nI wonder if we need to set GIT_STDERR_IS_TTY=1 in the parent process,\nand then respect it in the children (this is similar to what\nGIT_PAGER_IN_USE does for stdout).\n\n-Peff\n"},{"id":"268870","messageId":"20150828184442.GE8165@google.com","threadId":"40201","inReplyTo":"20150828183501.GA17222@sigill.intra.peff.net","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2015-08-28T18:44:42Z","receivedAt":"2015-08-28T18:44:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> I think Jonathan's point is that you could pick _one_ active child to\n> show without buffering, while simultaneously buffering everybody else's\n> output.\n\nYep. Thanks for interpreting.\n\n[...]\n> So to an observer, it would look like a serial operation, but subsequent\n> operations after the first would magically go much faster (because\n> they'd been working and buffering in the background).\n\nYes.\n\nThanks,\nJonathan\n"},{"id":"268871","messageId":"20150828185050.GF8165@google.com","threadId":"40201","inReplyTo":"20150828184405.GA17721@sigill.intra.peff.net","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2015-08-28T18:50:50Z","receivedAt":"2015-08-28T18:50:50Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> Right, clearly we can't rely on pipe buffers to be large enough here\n> (though we _may_ want to rely on tempfiles if we aren't sure that the\n> stdout is bounded in a reasonable way).\n>\n> But what I meant was: the child will only show progress if stderr is a\n> tty, but here it is not.\n\nFor clone / fetch, we can pass --progress explicitly.\n\nFor some reason 'git checkout' doesn't support a --progress option. I\nsuppose it should. ;-)\n\nThanks,\nJonathan\n"},{"id":"268872","messageId":"20150828185330.GA18041@sigill.intra.peff.net","threadId":"40201","inReplyTo":"20150828185050.GF8165@google.com","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-08-28T18:53:31Z","receivedAt":"2015-08-28T18:53:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 28, 2015 at 11:50:50AM -0700, Jonathan Nieder wrote:\n\n> > But what I meant was: the child will only show progress if stderr is a\n> > tty, but here it is not.\n> \n> For clone / fetch, we can pass --progress explicitly.\n> \n> For some reason 'git checkout' doesn't support a --progress option. I\n> suppose it should. ;-)\n\nYeah, that will work for those tools, but I thought you could pass\narbitrary shell commands. It would be nice if git sub-commands run\nthrough those just magically worked, even though we don't have an\nopportunity to change their command-line parameters.\n\n-Peff\n"},{"id":"268873","messageId":"CAGZ79kboFpySoTTwLQ5N2pOZSvw9HjEBhB-vmQ4-nP8RD7b63Q@mail.gmail.com","threadId":"40201","inReplyTo":"20150828184405.GA17721@sigill.intra.peff.net","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T18:59:43Z","receivedAt":"2015-08-28T18:59:43Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Aug 28, 2015 at 11:44 AM, Jeff King wrote:\n> On Fri, Aug 28, 2015 at 11:41:17AM -0700, Stefan Beller wrote:\n>\n>> > So to an observer, it would look like a serial operation, but subsequent\n>> > operations after the first would magically go much faster (because\n>> > they'd been working and buffering in the background).\n>> >\n>> > And that doesn't require any additional IPC magic (though I am not sure\n>> > how we get progress in the first place if the child stderr is a\n>> > pipe...).\n>>\n>> Moving the contents from the pipe to a strbuf buffer which we can grow\n>> indefinitely\n>> (way larger than pipe limits, but the output of a git fetch should be\n>> small enough for that).\n>\n> Right, clearly we can't rely on pipe buffers to be large enough here\n> (though we _may_ want to rely on tempfiles if we aren't sure that the\n> stdout is bounded in a reasonable way).\n>\n> But what I meant was: the child will only show progress if stderr is a\n> tty, but here it is not.\n\nOh, I forgot about that.\n\n>\n> I wonder if we need to set GIT_STDERR_IS_TTY=1 in the parent process,\n> and then respect it in the children (this is similar to what\n> GIT_PAGER_IN_USE does for stdout).\n\nThe use of GIT_PAGER_IN_USE looks straightforward to me. I'll try to add\nGIT_STDERR_IS_TTY then.\n\n\n>\n> -Peff\n"},{"id":"268874","messageId":"CAGZ79kYkJno-VCLbt_L6neMpJqdYRiXtMF7VUjh5OJxTi4Jiqg@mail.gmail.com","threadId":"40201","inReplyTo":"20150828185330.GA18041@sigill.intra.peff.net","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-28T19:02:25Z","receivedAt":"2015-08-28T19:02:25Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Aug 28, 2015 at 11:53 AM, Jeff King wrote:\n> On Fri, Aug 28, 2015 at 11:50:50AM -0700, Jonathan Nieder wrote:\n>\n>> > But what I meant was: the child will only show progress if stderr is a\n>> > tty, but here it is not.\n>>\n>> For clone / fetch, we can pass --progress explicitly.\n>>\n>> For some reason 'git checkout' doesn't support a --progress option. I\n>> suppose it should. ;-)\n>\n> Yeah, that will work for those tools, but I thought you could pass\n> arbitrary shell commands. It would be nice if git sub-commands run\n> through those just magically worked, even though we don't have an\n> opportunity to change their command-line parameters.\n\nTechnically speaking this discusses the patch for fetch only and we'd\nwant to have that discussion at the previous patch. ;)\nBut as both of them use the same code, the sync feature of run-command,\nwhich is in patch 5/9, we want to have git commands just work anyway.\n\n>\n> -Peff\n"},{"id":"269021","messageId":"xmqqy4gruw5h.fsf@gitster.mtv.corp.google.com","threadId":"40201","inReplyTo":"1440724495-708-4-git-send-email-sbeller@google.com","subject":"Re: [PATCH 3/9] submodule: implement `module_clone` as a builtin helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-31T18:53:46Z","receivedAt":"2015-08-31T18:53:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> +\t\tchar *s = (char*)sm_gitdir;\n\n\tchar *s = (char *)sm_gitdir;\n\nMany other places have similar, also in 7/9.\n"},{"id":"269023","messageId":"xmqqtwrfuw1n.fsf@gitster.mtv.corp.google.com","threadId":"40201","inReplyTo":"1440724495-708-8-git-send-email-sbeller@google.com","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-31T18:56:04Z","receivedAt":"2015-08-31T18:56:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> +static void destroy_output_mutex()\n\nstatic void destroy_output_mutex(void)\n\n> +{\n> +\tpthread_mutex_destroy(&output_mutex);\n> +}\n> +\n> +#else\n> +#define set_output_mutex()\n> +#define destroy_output_mutex()\n> +#define lock_output_mutex()\n> +#define unlock_output_mutex()\n> +#endif\n> +\n> +static struct submodule_parallel_fetch *submodule_parallel_fetch_create()\n\nstatic struct submodule_parallel_fetch *submodule_parallel_fetch_create(void)\n"},{"id":"269028","messageId":"20150831190526.GD20555@sigill.intra.peff.net","threadId":"40201","inReplyTo":"xmqqtwrfuw1n.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/9] fetch: fetch submodules in parallel","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-08-31T19:05:26Z","receivedAt":"2015-08-31T19:05:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 31, 2015 at 11:56:04AM -0700, Junio C Hamano wrote:\n\n> Stefan Beller writes:\n> \n> > +static void destroy_output_mutex()\n> \n> static void destroy_output_mutex(void)\n\nYep. Stefan, you may want to beef up the warning flags in your\nconfig.mak. For reference, I use:\n\n CFLAGS += -Wall -Werror\n CFLAGS += -Wdeclaration-after-statement\n CFLAGS += -Wpointer-arith\n CFLAGS += -Wstrict-prototypes\n CFLAGS += -Wvla\n CFLAGS += -Wold-style-declaration\n CFLAGS += -Wold-style-definition\n\nthough note that you will need to relax some of those if compiling older\nversions of git. My complete config.mak is at:\n\n https://github.com/peff/git/blob/meta/config/config.mak\n\nwhich handles this semi-automatically (it's wildly undocumented, but I'd\nbe happy to explain any of it if anybody is interested).\n\n-Peff\n"}]}