* [PATCH 1/7] run_processes_parallel: treat output of children as byte array
2016-02-25 1:41 [PATCHv16 0/7] Expose submodule parallelism to the user Stefan Beller
@ 2016-02-25 1:41 ` Stefan Beller
2016-02-25 1:42 ` [PATCH 2/7] run-command: expose default_{start_failure, task_finished} Stefan Beller
` (6 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Stefan Beller @ 2016-02-25 1:41 UTC (permalink / raw)
To: git, jrnieder; +Cc: Jens.Lehmann, peff, sunshine, Stefan Beller
We do not want the output to be interrupted by a NUL byte, so we
cannot use raw fputs. Introduce strbuf_write to avoid having long
arguments in run-command.c.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
run-command.c | 8 ++++----
strbuf.c | 6 ++++++
strbuf.h | 6 ++++++
3 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/run-command.c b/run-command.c
index 51fd72c..1f86728 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1011,7 +1011,7 @@ static void pp_cleanup(struct parallel_processes *pp)
* When get_next_task added messages to the buffer in its last
* iteration, the buffered output is non empty.
*/
- fputs(pp->buffered_output.buf, stderr);
+ strbuf_write(&pp->buffered_output, stderr);
strbuf_release(&pp->buffered_output);
sigchain_pop_common();
@@ -1097,7 +1097,7 @@ static void pp_output(struct parallel_processes *pp)
int i = pp->output_owner;
if (pp->children[i].state == GIT_CP_WORKING &&
pp->children[i].err.len) {
- fputs(pp->children[i].err.buf, stderr);
+ strbuf_write(&pp->children[i].err, stderr);
strbuf_reset(&pp->children[i].err);
}
}
@@ -1135,11 +1135,11 @@ static int pp_collect_finished(struct parallel_processes *pp)
strbuf_addbuf(&pp->buffered_output, &pp->children[i].err);
strbuf_reset(&pp->children[i].err);
} else {
- fputs(pp->children[i].err.buf, stderr);
+ strbuf_write(&pp->children[i].err, stderr);
strbuf_reset(&pp->children[i].err);
/* Output all other finished child processes */
- fputs(pp->buffered_output.buf, stderr);
+ strbuf_write(&pp->buffered_output, stderr);
strbuf_reset(&pp->buffered_output);
/*
diff --git a/strbuf.c b/strbuf.c
index 38686ff..71345cd 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -395,6 +395,12 @@ ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint)
return cnt;
}
+ssize_t strbuf_write(struct strbuf *sb, FILE *f)
+{
+ return fwrite(sb->buf, 1, sb->len, f);
+}
+
+
#define STRBUF_MAXLINK (2*PATH_MAX)
int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)
diff --git a/strbuf.h b/strbuf.h
index 2bf90e7..d4f2aa1 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -387,6 +387,12 @@ extern ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t hint
extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);
/**
+ * Write the whole content of the strbuf to the stream not stopping at
+ * NUL bytes.
+ */
+extern ssize_t strbuf_write(struct strbuf *sb, FILE *stream);
+
+/**
* Read a line from a FILE *, overwriting the existing contents
* of the strbuf. The second argument specifies the line
* terminator character, typically `'\n'`.
--
2.7.2.335.g3f96d05
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/7] run-command: expose default_{start_failure, task_finished}
2016-02-25 1:41 [PATCHv16 0/7] Expose submodule parallelism to the user Stefan Beller
2016-02-25 1:41 ` [PATCH 1/7] run_processes_parallel: treat output of children as byte array Stefan Beller
@ 2016-02-25 1:42 ` Stefan Beller
2016-02-25 1:42 ` [PATCH 3/7] run_processes_parallel: rename parameters for the callbacks Stefan Beller
` (5 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Stefan Beller @ 2016-02-25 1:42 UTC (permalink / raw)
To: git, jrnieder; +Cc: Jens.Lehmann, peff, sunshine, Stefan Beller
We want to reuse the error reporting facilities in a later patch.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
run-command.c | 18 +++++++++---------
run-command.h | 19 +++++++++++++++++++
2 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/run-command.c b/run-command.c
index 1f86728..71abafb 100644
--- a/run-command.c
+++ b/run-command.c
@@ -902,10 +902,10 @@ struct parallel_processes {
struct strbuf buffered_output; /* of finished children */
};
-static int default_start_failure(struct child_process *cp,
- struct strbuf *err,
- void *pp_cb,
- void *pp_task_cb)
+int default_start_failure(struct child_process *cp,
+ struct strbuf *err,
+ void *pp_cb,
+ void *pp_task_cb)
{
int i;
@@ -916,11 +916,11 @@ static int default_start_failure(struct child_process *cp,
return 0;
}
-static int default_task_finished(int result,
- struct child_process *cp,
- struct strbuf *err,
- void *pp_cb,
- void *pp_task_cb)
+int default_task_finished(int result,
+ struct child_process *cp,
+ struct strbuf *err,
+ void *pp_cb,
+ void *pp_task_cb)
{
int i;
diff --git a/run-command.h b/run-command.h
index d5a57f9..a054fa6 100644
--- a/run-command.h
+++ b/run-command.h
@@ -164,6 +164,15 @@ typedef int (*start_failure_fn)(struct child_process *cp,
void *pp_task_cb);
/**
+ * If a command fails to start, then print an error message stating the
+ * exact command which failed.
+ */
+int default_start_failure(struct child_process *cp,
+ struct strbuf *err,
+ void *pp_cb,
+ void *pp_task_cb);
+
+/**
* This callback is called on every child process that finished processing.
*
* You must not write to stdout or stderr in this function. Add your
@@ -184,6 +193,16 @@ typedef int (*task_finished_fn)(int result,
void *pp_task_cb);
/**
+ * If the child process returns with a non zero error code, print
+ * an error message of the exact command which failed.
+ */
+int default_task_finished(int result,
+ struct child_process *cp,
+ struct strbuf *err,
+ void *pp_cb,
+ void *pp_task_cb);
+
+/**
* Runs up to n processes at the same time. Whenever a process can be
* started, the callback get_next_task_fn is called to obtain the data
* required to start another child process.
--
2.7.2.335.g3f96d05
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/7] run_processes_parallel: rename parameters for the callbacks
2016-02-25 1:41 [PATCHv16 0/7] Expose submodule parallelism to the user Stefan Beller
2016-02-25 1:41 ` [PATCH 1/7] run_processes_parallel: treat output of children as byte array Stefan Beller
2016-02-25 1:42 ` [PATCH 2/7] run-command: expose default_{start_failure, task_finished} Stefan Beller
@ 2016-02-25 1:42 ` Stefan Beller
2016-02-25 1:47 ` Jonathan Nieder
2016-02-25 1:42 ` [PATCH 4/7] run_processes_parallel: correctly terminate callbacks with an LF Stefan Beller
` (4 subsequent siblings)
7 siblings, 1 reply; 10+ messages in thread
From: Stefan Beller @ 2016-02-25 1:42 UTC (permalink / raw)
To: git, jrnieder; +Cc: Jens.Lehmann, peff, sunshine, Stefan Beller
The refs code has a similar pattern of passing around 'struct strbuf *err',
which is strictly used for error reporting. This is not the case here,
as the strbuf is used to accumulate all the output (whether it is error
or not) for the user. Rename it to 'out'.
Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
run-command.c | 12 ++++++------
run-command.h | 8 ++++----
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/run-command.c b/run-command.c
index 71abafb..20489c8 100644
--- a/run-command.c
+++ b/run-command.c
@@ -903,22 +903,22 @@ struct parallel_processes {
};
int default_start_failure(struct child_process *cp,
- struct strbuf *err,
+ struct strbuf *out,
void *pp_cb,
void *pp_task_cb)
{
int i;
- strbuf_addstr(err, "Starting a child failed:");
+ strbuf_addstr(out, "Starting a child failed:");
for (i = 0; cp->argv[i]; i++)
- strbuf_addf(err, " %s", cp->argv[i]);
+ strbuf_addf(out, " %s", cp->argv[i]);
return 0;
}
int default_task_finished(int result,
struct child_process *cp,
- struct strbuf *err,
+ struct strbuf *out,
void *pp_cb,
void *pp_task_cb)
{
@@ -927,9 +927,9 @@ int default_task_finished(int result,
if (!result)
return 0;
- strbuf_addf(err, "A child failed with return code %d:", result);
+ strbuf_addf(out, "A child failed with return code %d:", result);
for (i = 0; cp->argv[i]; i++)
- strbuf_addf(err, " %s", cp->argv[i]);
+ strbuf_addf(out, " %s", cp->argv[i]);
return 0;
}
diff --git a/run-command.h b/run-command.h
index a054fa6..c8c64bb 100644
--- a/run-command.h
+++ b/run-command.h
@@ -159,7 +159,7 @@ typedef int (*get_next_task_fn)(struct child_process *cp,
* the negative signal number.
*/
typedef int (*start_failure_fn)(struct child_process *cp,
- struct strbuf *err,
+ struct strbuf *out,
void *pp_cb,
void *pp_task_cb);
@@ -168,7 +168,7 @@ typedef int (*start_failure_fn)(struct child_process *cp,
* exact command which failed.
*/
int default_start_failure(struct child_process *cp,
- struct strbuf *err,
+ struct strbuf *out,
void *pp_cb,
void *pp_task_cb);
@@ -188,7 +188,7 @@ int default_start_failure(struct child_process *cp,
*/
typedef int (*task_finished_fn)(int result,
struct child_process *cp,
- struct strbuf *err,
+ struct strbuf *out,
void *pp_cb,
void *pp_task_cb);
@@ -198,7 +198,7 @@ typedef int (*task_finished_fn)(int result,
*/
int default_task_finished(int result,
struct child_process *cp,
- struct strbuf *err,
+ struct strbuf *out,
void *pp_cb,
void *pp_task_cb);
--
2.7.2.335.g3f96d05
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/7] run_processes_parallel: rename parameters for the callbacks
2016-02-25 1:42 ` [PATCH 3/7] run_processes_parallel: rename parameters for the callbacks Stefan Beller
@ 2016-02-25 1:47 ` Jonathan Nieder
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Nieder @ 2016-02-25 1:47 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Jens.Lehmann, peff, sunshine
Stefan Beller wrote:
> +++ b/run-command.h
> @@ -159,7 +159,7 @@ typedef int (*get_next_task_fn)(struct child_process *cp,
> * the negative signal number.
> */
> typedef int (*start_failure_fn)(struct child_process *cp,
> - struct strbuf *err,
> + struct strbuf *out,
> void *pp_cb,
> void *pp_task_cb);
Yay, thank you. The docstrings still refer to err.
Jonathan
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/7] run_processes_parallel: correctly terminate callbacks with an LF
2016-02-25 1:41 [PATCHv16 0/7] Expose submodule parallelism to the user Stefan Beller
` (2 preceding siblings ...)
2016-02-25 1:42 ` [PATCH 3/7] run_processes_parallel: rename parameters for the callbacks Stefan Beller
@ 2016-02-25 1:42 ` Stefan Beller
2016-02-25 1:42 ` [PATCH 5/7] git submodule update: have a dedicated helper for cloning Stefan Beller
` (3 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Stefan Beller @ 2016-02-25 1:42 UTC (permalink / raw)
To: git, jrnieder; +Cc: Jens.Lehmann, peff, sunshine, Stefan Beller
As the strbufs passed around collect all output to the user, and there
is no post processing involved we need to care about the line ending
ourselves.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
run-command.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/run-command.c b/run-command.c
index 20489c8..8e60812 100644
--- a/run-command.c
+++ b/run-command.c
@@ -912,6 +912,7 @@ int default_start_failure(struct child_process *cp,
strbuf_addstr(out, "Starting a child failed:");
for (i = 0; cp->argv[i]; i++)
strbuf_addf(out, " %s", cp->argv[i]);
+ strbuf_addch(out, '\n');
return 0;
}
@@ -930,6 +931,7 @@ int default_task_finished(int result,
strbuf_addf(out, "A child failed with return code %d:", result);
for (i = 0; cp->argv[i]; i++)
strbuf_addf(out, " %s", cp->argv[i]);
+ strbuf_addch(out, '\n');
return 0;
}
--
2.7.2.335.g3f96d05
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 5/7] git submodule update: have a dedicated helper for cloning
2016-02-25 1:41 [PATCHv16 0/7] Expose submodule parallelism to the user Stefan Beller
` (3 preceding siblings ...)
2016-02-25 1:42 ` [PATCH 4/7] run_processes_parallel: correctly terminate callbacks with an LF Stefan Beller
@ 2016-02-25 1:42 ` Stefan Beller
2016-02-25 1:42 ` [PATCH 6/7] submodule update: expose parallelism to the user Stefan Beller
` (2 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Stefan Beller @ 2016-02-25 1:42 UTC (permalink / raw)
To: git, jrnieder; +Cc: Jens.Lehmann, peff, sunshine, Stefan Beller, Junio C Hamano
This introduces a new helper function in git submodule--helper
which takes care of cloning all submodules, which we want to
parallelize eventually.
Some tests (such as empty URL, update_mode=none) are required in the
helper to make the decision for cloning. These checks have been
moved into the C function as well (no need to repeat them in the
shell script).
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/submodule--helper.c | 243 ++++++++++++++++++++++++++++++++++++++++++++
git-submodule.sh | 47 +++------
2 files changed, 256 insertions(+), 34 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f4c3eff..85fb702 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -255,6 +255,248 @@ static int module_clone(int argc, const char **argv, const char *prefix)
return 0;
}
+struct submodule_update_clone {
+ /* index into 'list', the list of submodules to look into for cloning */
+ int current;
+ struct module_list list;
+ int warn_if_uninitialized : 1;
+ /* update parameter passed via commandline*/
+ struct submodule_update_strategy update;
+ /* configuration parameters which are passed on to the children */
+ int quiet;
+ const char *reference;
+ const char *depth;
+ const char *recursive_prefix;
+ const char *prefix;
+ /* lines to be output */
+ struct string_list projectlines;
+ /* If we want to stop as fast as possible and return an error */
+ int quickstop : 1;
+};
+#define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
+ SUBMODULE_UPDATE_STRATEGY_INIT, 0, NULL, NULL, NULL, NULL, \
+ STRING_LIST_INIT_DUP, 0}
+
+/**
+ * Inspect if 'ce' needs to be cloned. If so, prepare the 'child' to be running
+ * the clone and return non zero.
+ */
+static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
+ struct child_process *child,
+ struct submodule_update_clone *suc,
+ struct strbuf *err)
+{
+ const struct submodule *sub = NULL;
+ struct strbuf displaypath_sb = STRBUF_INIT;
+ struct strbuf sb = STRBUF_INIT;
+ const char *displaypath = NULL;
+ char *url = NULL;
+ int needs_cloning = 0;
+
+ if (ce_stage(ce)) {
+ if (suc->recursive_prefix) {
+ strbuf_addf(err, "Skipping unmerged submodule %s/%s\n",
+ suc->recursive_prefix, ce->name);
+ } else {
+ strbuf_addf(err, "Skipping unmerged submodule %s\n",
+ ce->name);
+ }
+ goto cleanup;
+ }
+
+ sub = submodule_from_path(null_sha1, ce->name);
+
+ if (suc->recursive_prefix)
+ displaypath = relative_path(suc->recursive_prefix,
+ ce->name, &displaypath_sb);
+ else
+ displaypath = ce->name;
+
+ if (suc->update.type == SM_UPDATE_NONE
+ || (suc->update.type == SM_UPDATE_UNSPECIFIED
+ && sub->update_strategy.type == SM_UPDATE_NONE)) {
+ strbuf_addf(err, "Skipping submodule '%s'\n",
+ displaypath);
+ goto cleanup;
+ }
+
+ /*
+ * Looking up the url in .git/config.
+ * We must not fall back to .gitmodules as we only want
+ * to process configured submodules.
+ */
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, "submodule.%s.url", sub->name);
+ git_config_get_string(sb.buf, &url);
+ if (!url) {
+ /*
+ * Only mention uninitialized submodules when its
+ * path have been specified
+ */
+ if (suc->warn_if_uninitialized)
+ strbuf_addf(err, _("Submodule path '%s' not initialized\n"
+ "Maybe you want to use 'update --init'?\n"),
+ displaypath);
+ goto cleanup;
+ }
+
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, "%s/.git", ce->name);
+ needs_cloning = !file_exists(sb.buf);
+
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, "%06o %s %d %d\t%s\n", ce->ce_mode,
+ sha1_to_hex(ce->sha1), ce_stage(ce),
+ needs_cloning, ce->name);
+ string_list_append(&suc->projectlines, sb.buf);
+
+ if (!needs_cloning)
+ goto cleanup;
+
+ child->git_cmd = 1;
+ child->no_stdin = 1;
+ child->stdout_to_stderr = 1;
+ child->err = -1;
+ argv_array_push(&child->args, "submodule--helper");
+ argv_array_push(&child->args, "clone");
+ if (suc->quiet)
+ argv_array_push(&child->args, "--quiet");
+ if (suc->prefix)
+ argv_array_pushl(&child->args, "--prefix", suc->prefix, NULL);
+ argv_array_pushl(&child->args, "--path", sub->path, NULL);
+ argv_array_pushl(&child->args, "--name", sub->name, NULL);
+ argv_array_pushl(&child->args, "--url", url, NULL);
+ if (suc->reference)
+ argv_array_push(&child->args, suc->reference);
+ if (suc->depth)
+ argv_array_push(&child->args, suc->depth);
+
+cleanup:
+ free(url);
+ strbuf_reset(&displaypath_sb);
+ strbuf_reset(&sb);
+
+ return needs_cloning;
+}
+
+static int update_clone_get_next_task(struct child_process *child,
+ struct strbuf *err,
+ void *suc_cb,
+ void **void_task_cb)
+{
+ struct submodule_update_clone *suc = suc_cb;
+
+ for (; suc->current < suc->list.nr; suc->current++) {
+ const struct cache_entry *ce = suc->list.entries[suc->current];
+ if (prepare_to_clone_next_submodule(ce, child, suc, err)) {
+ suc->current++;
+ return 1;
+ }
+ }
+ return 0;
+}
+
+static int update_clone_start_failure(struct child_process *child,
+ struct strbuf *err,
+ void *suc_cb,
+ void *void_task_cb)
+{
+ struct submodule_update_clone *suc = suc_cb;
+
+ default_start_failure(child, err, suc_cb, void_task_cb);
+ suc->quickstop = 1;
+
+ return 1;
+}
+
+static int update_clone_task_finished(int result,
+ struct child_process *child,
+ struct strbuf *err,
+ void *suc_cb,
+ void *void_task_cb)
+{
+ struct submodule_update_clone *suc = suc_cb;
+
+ if (!result)
+ return 0;
+
+ default_task_finished(result, child, err, suc_cb, void_task_cb);
+ suc->quickstop = 1;
+
+ return 1;
+}
+
+static int update_clone(int argc, const char **argv, const char *prefix)
+{
+ const char *update = NULL;
+ struct string_list_item *item;
+ struct pathspec pathspec;
+ struct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;
+
+ struct option module_update_clone_options[] = {
+ OPT_STRING(0, "prefix", &prefix,
+ N_("path"),
+ N_("path into the working tree")),
+ OPT_STRING(0, "recursive-prefix", &suc.recursive_prefix,
+ N_("path"),
+ N_("path into the working tree, across nested "
+ "submodule boundaries")),
+ OPT_STRING(0, "update", &update,
+ N_("string"),
+ N_("rebase, merge, checkout or none")),
+ OPT_STRING(0, "reference", &suc.reference, N_("repo"),
+ N_("reference repository")),
+ OPT_STRING(0, "depth", &suc.depth, "<depth>",
+ N_("Create a shallow clone truncated to the "
+ "specified number of revisions")),
+ OPT__QUIET(&suc.quiet, N_("don't print cloning progress")),
+ OPT_END()
+ };
+
+ const char *const git_submodule_helper_usage[] = {
+ N_("git submodule--helper update_clone [--prefix=<path>] [<path>...]"),
+ NULL
+ };
+ suc.prefix = prefix;
+
+ argc = parse_options(argc, argv, prefix, module_update_clone_options,
+ git_submodule_helper_usage, 0);
+
+ if (update)
+ if (parse_submodule_update_strategy(update, &suc.update) < 0)
+ die(_("bad value for update parameter"));
+
+ if (module_list_compute(argc, argv, prefix, &pathspec, &suc.list) < 0)
+ return 1;
+
+ if (pathspec.nr)
+ suc.warn_if_uninitialized = 1;
+
+ gitmodules_config();
+ /* Overlay the parsed .gitmodules file with .git/config */
+ git_config(submodule_config, NULL);
+ run_processes_parallel(1, update_clone_get_next_task,
+ update_clone_start_failure,
+ update_clone_task_finished,
+ &suc);
+
+ /*
+ * We saved the output and put it out all at once now.
+ * That means:
+ * - the listener does not have to interleave their (checkout)
+ * work with our fetching. The writes involved in a
+ * checkout involve more straightforward sequential I/O.
+ * - the listener can avoid doing any work if fetching failed.
+ */
+ if (suc.quickstop)
+ return 1;
+
+ for_each_string_list_item(item, &suc.projectlines)
+ utf8_fprintf(stdout, "%s", item->string);
+
+ return 0;
+}
+
struct cmd_struct {
const char *cmd;
int (*fn)(int, const char **, const char *);
@@ -264,6 +506,7 @@ static struct cmd_struct commands[] = {
{"list", module_list},
{"name", module_name},
{"clone", module_clone},
+ {"update-clone", update_clone}
};
int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/git-submodule.sh b/git-submodule.sh
index 9ee86d4..a6a82d2 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -664,17 +664,20 @@ cmd_update()
cmd_init "--" "$@" || return
fi
- cloned_modules=
- git submodule--helper list --prefix "$wt_prefix" "$@" | {
+ {
+ git submodule--helper update-clone ${GIT_QUIET:+--quiet} \
+ ${wt_prefix:+--prefix "$wt_prefix"} \
+ ${prefix:+--recursive-prefix "$prefix"} \
+ ${update:+--update "$update"} \
+ ${reference:+--reference "$reference"} \
+ ${depth:+--depth "$depth"} \
+ "$@" || echo "#unmatched"
+ } | {
err=
- while read mode sha1 stage sm_path
+ while read mode sha1 stage just_cloned sm_path
do
die_if_unmatched "$mode"
- if test "$stage" = U
- then
- echo >&2 "Skipping unmerged submodule $prefix$sm_path"
- continue
- fi
+
name=$(git submodule--helper name "$sm_path") || exit
url=$(git config submodule."$name".url)
branch=$(get_submodule_config "$name" branch master)
@@ -691,27 +694,10 @@ cmd_update()
displaypath=$(relative_path "$prefix$sm_path")
- if test "$update_module" = "none"
- then
- echo >&2 "Skipping submodule '$displaypath'"
- continue
- fi
-
- if test -z "$url"
- then
- # Only mention uninitialized submodules when its
- # path have been specified
- test "$#" != "0" &&
- say >&2 "$(eval_gettext "Submodule path '\$displaypath' not initialized
-Maybe you want to use 'update --init'?")"
- continue
- fi
-
- if ! test -d "$sm_path"/.git && ! test -f "$sm_path"/.git
+ if test $just_cloned -eq 1
then
- git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$prefix" --path "$sm_path" --name "$name" --url "$url" "$reference" "$depth" || exit
- cloned_modules="$cloned_modules;$name"
subsha1=
+ update_module=checkout
else
subsha1=$(clear_local_git_env; cd "$sm_path" &&
git rev-parse --verify HEAD) ||
@@ -751,13 +737,6 @@ Maybe you want to use 'update --init'?")"
die "$(eval_gettext "Unable to fetch in submodule path '\$displaypath'")"
fi
- # Is this something we just cloned?
- case ";$cloned_modules;" in
- *";$name;"*)
- # then there is no local change to integrate
- update_module=checkout ;;
- esac
-
must_die_on_failure=
case "$update_module" in
checkout)
--
2.7.2.335.g3f96d05
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 6/7] submodule update: expose parallelism to the user
2016-02-25 1:41 [PATCHv16 0/7] Expose submodule parallelism to the user Stefan Beller
` (4 preceding siblings ...)
2016-02-25 1:42 ` [PATCH 5/7] git submodule update: have a dedicated helper for cloning Stefan Beller
@ 2016-02-25 1:42 ` Stefan Beller
2016-02-25 1:42 ` [PATCH 7/7] clone: allow an explicit argument for parallel submodule clones Stefan Beller
2016-02-25 1:54 ` [PATCHv16 0/7] Expose submodule parallelism to the user Jonathan Nieder
7 siblings, 0 replies; 10+ messages in thread
From: Stefan Beller @ 2016-02-25 1:42 UTC (permalink / raw)
To: git, jrnieder; +Cc: Jens.Lehmann, peff, sunshine, Stefan Beller, Junio C Hamano
Expose possible parallelism either via the "--jobs" CLI parameter or
the "submodule.fetchJobs" setting.
By having the variable initialized to -1, we make sure 0 can be passed
into the parallel processing machine, which will then pick as many parallel
workers as there are CPUs.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-submodule.txt | 7 ++++++-
builtin/submodule--helper.c | 16 ++++++++++++----
git-submodule.sh | 9 +++++++++
t/t7406-submodule-update.sh | 12 ++++++++++++
4 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 1572f05..13adebf 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -16,7 +16,7 @@ SYNOPSIS
'git submodule' [--quiet] deinit [-f|--force] [--] <path>...
'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
[-f|--force] [--rebase|--merge] [--reference <repository>]
- [--depth <depth>] [--recursive] [--] [<path>...]
+ [--depth <depth>] [--recursive] [--jobs <n>] [--] [<path>...]
'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
[commit] [--] [<path>...]
'git submodule' [--quiet] foreach [--recursive] <command>
@@ -377,6 +377,11 @@ for linkgit:git-clone[1]'s `--reference` and `--shared` options carefully.
clone with a history truncated to the specified number of revisions.
See linkgit:git-clone[1]
+-j <n>::
+--jobs <n>::
+ This option is only valid for the update command.
+ Clone new submodules in parallel with as many jobs.
+ Defaults to the `submodule.fetchJobs` option.
<path>...::
Paths to submodule(s). When specified this will restrict the command
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 85fb702..32254cd 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -429,6 +429,7 @@ static int update_clone_task_finished(int result,
static int update_clone(int argc, const char **argv, const char *prefix)
{
const char *update = NULL;
+ int max_jobs = -1;
struct string_list_item *item;
struct pathspec pathspec;
struct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;
@@ -449,6 +450,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)
OPT_STRING(0, "depth", &suc.depth, "<depth>",
N_("Create a shallow clone truncated to the "
"specified number of revisions")),
+ OPT_INTEGER('j', "jobs", &max_jobs,
+ N_("parallel jobs")),
OPT__QUIET(&suc.quiet, N_("don't print cloning progress")),
OPT_END()
};
@@ -475,10 +478,15 @@ static int update_clone(int argc, const char **argv, const char *prefix)
gitmodules_config();
/* Overlay the parsed .gitmodules file with .git/config */
git_config(submodule_config, NULL);
- run_processes_parallel(1, update_clone_get_next_task,
- update_clone_start_failure,
- update_clone_task_finished,
- &suc);
+
+ if (max_jobs < 0)
+ max_jobs = parallel_submodules();
+
+ run_processes_parallel(max_jobs,
+ update_clone_get_next_task,
+ update_clone_start_failure,
+ update_clone_task_finished,
+ &suc);
/*
* We saved the output and put it out all at once now.
diff --git a/git-submodule.sh b/git-submodule.sh
index a6a82d2..86018ee 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -645,6 +645,14 @@ cmd_update()
--depth=*)
depth=$1
;;
+ -j|--jobs)
+ case "$2" in '') usage ;; esac
+ jobs="--jobs=$2"
+ shift
+ ;;
+ --jobs=*)
+ jobs=$1
+ ;;
--)
shift
break
@@ -671,6 +679,7 @@ cmd_update()
${update:+--update "$update"} \
${reference:+--reference "$reference"} \
${depth:+--depth "$depth"} \
+ ${jobs:+$jobs} \
"$@" || echo "#unmatched"
} | {
err=
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index dda3929..7fd5142 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -774,4 +774,16 @@ test_expect_success 'submodule update --recursive drops module name before recur
test_i18ngrep "Submodule path .deeper/submodule/subsubmodule.: checked out" actual
)
'
+
+test_expect_success 'submodule update can be run in parallel' '
+ (cd super2 &&
+ GIT_TRACE=$(pwd)/trace.out git submodule update --jobs 7 &&
+ grep "7 tasks" trace.out &&
+ git config submodule.fetchJobs 8 &&
+ GIT_TRACE=$(pwd)/trace.out git submodule update &&
+ grep "8 tasks" trace.out &&
+ GIT_TRACE=$(pwd)/trace.out git submodule update --jobs 9 &&
+ grep "9 tasks" trace.out
+ )
+'
test_done
--
2.7.2.335.g3f96d05
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 7/7] clone: allow an explicit argument for parallel submodule clones
2016-02-25 1:41 [PATCHv16 0/7] Expose submodule parallelism to the user Stefan Beller
` (5 preceding siblings ...)
2016-02-25 1:42 ` [PATCH 6/7] submodule update: expose parallelism to the user Stefan Beller
@ 2016-02-25 1:42 ` Stefan Beller
2016-02-25 1:54 ` [PATCHv16 0/7] Expose submodule parallelism to the user Jonathan Nieder
7 siblings, 0 replies; 10+ messages in thread
From: Stefan Beller @ 2016-02-25 1:42 UTC (permalink / raw)
To: git, jrnieder; +Cc: Jens.Lehmann, peff, sunshine, Stefan Beller, Junio C Hamano
Just pass it along to "git submodule update", which may pick reasonable
defaults if you don't specify an explicit number.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-clone.txt | 6 +++++-
builtin/clone.c | 19 +++++++++++++------
t/t7406-submodule-update.sh | 15 +++++++++++++++
3 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 6bf000d..6db7b6d 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -14,7 +14,7 @@ SYNOPSIS
[-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]
[--dissociate] [--separate-git-dir <git dir>]
[--depth <depth>] [--[no-]single-branch]
- [--recursive | --recurse-submodules] [--] <repository>
+ [--recursive | --recurse-submodules] [--jobs <n>] [--] <repository>
[<directory>]
DESCRIPTION
@@ -221,6 +221,10 @@ objects from the source repository into a pack in the cloned repository.
The result is Git repository can be separated from working
tree.
+-j <n>::
+--jobs <n>::
+ The number of submodules fetched at the same time.
+ Defaults to the `submodule.fetchJobs` option.
<repository>::
The (possibly remote) repository to clone from. See the
diff --git a/builtin/clone.c b/builtin/clone.c
index a0b3cd9..b004fb4 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -50,6 +50,7 @@ static int option_progress = -1;
static struct string_list option_config;
static struct string_list option_reference;
static int option_dissociate;
+static int max_jobs = -1;
static struct option builtin_clone_options[] = {
OPT__VERBOSITY(&option_verbosity),
@@ -72,6 +73,8 @@ static struct option builtin_clone_options[] = {
N_("initialize submodules in the clone")),
OPT_BOOL(0, "recurse-submodules", &option_recursive,
N_("initialize submodules in the clone")),
+ OPT_INTEGER('j', "jobs", &max_jobs,
+ N_("number of submodules cloned in parallel")),
OPT_STRING(0, "template", &option_template, N_("template-directory"),
N_("directory from which templates will be used")),
OPT_STRING_LIST(0, "reference", &option_reference, N_("repo"),
@@ -95,10 +98,6 @@ static struct option builtin_clone_options[] = {
OPT_END()
};
-static const char *argv_submodule[] = {
- "submodule", "update", "--init", "--recursive", NULL
-};
-
static const char *get_repo_path_1(struct strbuf *path, int *is_bundle)
{
static char *suffix[] = { "/.git", "", ".git/.git", ".git" };
@@ -724,8 +723,16 @@ static int checkout(void)
err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
sha1_to_hex(sha1), "1", NULL);
- if (!err && option_recursive)
- err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
+ if (!err && option_recursive) {
+ struct argv_array args = ARGV_ARRAY_INIT;
+ argv_array_pushl(&args, "submodule", "update", "--init", "--recursive", NULL);
+
+ if (max_jobs != -1)
+ argv_array_pushf(&args, "--jobs=%d", max_jobs);
+
+ err = run_command_v_opt(args.argv, RUN_GIT_CMD);
+ argv_array_clear(&args);
+ }
return err;
}
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 7fd5142..090891e 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -786,4 +786,19 @@ test_expect_success 'submodule update can be run in parallel' '
grep "9 tasks" trace.out
)
'
+
+test_expect_success 'git clone passes the parallel jobs config on to submodules' '
+ test_when_finished "rm -rf super4" &&
+ GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 7 . super4 &&
+ grep "7 tasks" trace.out &&
+ rm -rf super4 &&
+ git config --global submodule.fetchJobs 8 &&
+ GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules . super4 &&
+ grep "8 tasks" trace.out &&
+ rm -rf super4 &&
+ GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 9 . super4 &&
+ grep "9 tasks" trace.out &&
+ rm -rf super4
+'
+
test_done
--
2.7.2.335.g3f96d05
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCHv16 0/7] Expose submodule parallelism to the user
2016-02-25 1:41 [PATCHv16 0/7] Expose submodule parallelism to the user Stefan Beller
` (6 preceding siblings ...)
2016-02-25 1:42 ` [PATCH 7/7] clone: allow an explicit argument for parallel submodule clones Stefan Beller
@ 2016-02-25 1:54 ` Jonathan Nieder
7 siblings, 0 replies; 10+ messages in thread
From: Jonathan Nieder @ 2016-02-25 1:54 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Jens.Lehmann, peff, sunshine
Stefan Beller wrote:
> This build on top of 163b9b1f919c762a4bfb693b3aa05ef1aa627fee
> (origin/sb/submodule-parallel-update~5) and replace the series from there on.
Except for the comments in patch 3,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks for writing it and sorry it took so long.
^ permalink raw reply [flat|nested] 10+ messages in thread