[PATCHv21 00/10] Expose submodule parallelism to the user

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



* evicted patches:
  run-command: expose default_{start_failure, task_finished}
  run_processes_parallel: correctly terminate callbacks with an LF
* rebased on top of origin/sb/no-child-process-access-in-run-parallel-callbacks
* followed Jonathans advice on improving the situation for translators.

Junio wrote:
> Yup, let's stop rerolling this during pre-release period, and
> instead concentrate on fixing what's already in 'master' ;-)

Ok, feel free to ignore this series, I just want to put out
my latest version. Once you pick that up I'll likely put patches on top,
rather than rerolling as both Jonathan and me have the impression this
series has wedged out the rough errors by now and now we do subtle fixes
only.

Thanks,
Stefan
  

git diff origin/sb/submodule-parallel-update which is v20 of the series,
but as v21 has a different anchor point, the commit
origin/sb/no-child-process-access-in-run-parallel-callbacks is included
in this diff.

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 0272c98..a484945 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -298,13 +298,12 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
 	int needs_cloning = 0;
 
 	if (ce_stage(ce)) {
-		if (suc->recursive_prefix) {
-			strbuf_addf(out, "Skipping unmerged submodule %s/%s\n",
-				    suc->recursive_prefix, ce->name);
-		} else {
-			strbuf_addf(out, "Skipping unmerged submodule %s\n",
-				    ce->name);
-		}
+		if (suc->recursive_prefix)
+			strbuf_addf(&sb, "%s/%s", suc->recursive_prefix, ce->name);
+		else
+			strbuf_addf(&sb, "%s", ce->name);
+		strbuf_addf(out, _("Skipping unmerged submodule %s"), sb.buf);
+		strbuf_addch(out, '\n');
 		goto cleanup;
 	}
 
@@ -319,8 +318,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
 	if (suc->update.type == SM_UPDATE_NONE
 	    || (suc->update.type == SM_UPDATE_UNSPECIFIED
 		&& sub->update_strategy.type == SM_UPDATE_NONE)) {
-		strbuf_addf(out, "Skipping submodule '%s'\n",
-			    displaypath);
+		strbuf_addf(out, _("Skipping submodule '%s'"), displaypath);
+		strbuf_addch(out, '\n');
 		goto cleanup;
 	}
 
@@ -337,10 +336,15 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
 		 * Only mention uninitialized submodules when their
 		 * path have been specified
 		 */
-		if (suc->warn_if_uninitialized)
-			strbuf_addf(out, _("Submodule path '%s' not initialized\n"
-				    "Maybe you want to use 'update --init'?\n"),
-				    displaypath);
+		if (suc->warn_if_uninitialized) {
+			strbuf_addf(out,
+				_("Submodule path '%s' not initialized"),
+				displaypath);
+			strbuf_addch(out, '\n');
+			strbuf_addstr(out,
+				_("Maybe you want to use 'update --init'?"));
+			strbuf_addch(out, '\n');
+		}
 		goto cleanup;
 	}
 
@@ -400,21 +404,16 @@ static int update_clone_get_next_task(struct child_process *child,
 	return 0;
 }
 
-static int update_clone_start_failure(struct child_process *child,
-				      struct strbuf *err,
+static int update_clone_start_failure(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)
@@ -424,9 +423,7 @@ static int update_clone_task_finished(int result,
 	if (!result)
 		return 0;
 
-	default_task_finished(result, child, err, suc_cb, void_task_cb);
 	suc->quickstop = 1;
-
 	return 1;
 }
 
@@ -525,13 +522,13 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
 {
 	int i;
 	if (argc < 2)
-		die(_("fatal: submodule--helper subcommand must be "
+		die(_("submodule--helper subcommand must be "
 		      "called with a subcommand"));
 
 	for (i = 0; i < ARRAY_SIZE(commands); i++)
 		if (!strcmp(argv[1], commands[i].cmd))
 			return commands[i].fn(argc - 1, argv + 1, prefix);
 
-	die(_("fatal: '%s' is not a valid submodule--helper "
+	die(_("'%s' is not a valid submodule--helper "
 	      "subcommand"), argv[1]);
 }
diff --git a/run-command.c b/run-command.c
index 6fad42f..62c6721 100644
--- a/run-command.c
+++ b/run-command.c
@@ -902,37 +902,18 @@ struct parallel_processes {
 	struct strbuf buffered_output; /* of finished children */
 };
 
-int default_start_failure(struct child_process *cp,
-			  struct strbuf *out,
-			  void *pp_cb,
-			  void *pp_task_cb)
+static int default_start_failure(struct strbuf *out,
+				 void *pp_cb,
+				 void *pp_task_cb)
 {
-	int i;
-
-	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;
 }
 
-int default_task_finished(int result,
-			  struct child_process *cp,
-			  struct strbuf *out,
-			  void *pp_cb,
-			  void *pp_task_cb)
+static int default_task_finished(int result,
+				 struct strbuf *out,
+				 void *pp_cb,
+				 void *pp_task_cb)
 {
-	int i;
-
-	if (!result)
-		return 0;
-
-	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;
 }
 
@@ -1050,8 +1031,7 @@ static int pp_start_one(struct parallel_processes *pp)
 	pp->children[i].process.no_stdin = 1;
 
 	if (start_command(&pp->children[i].process)) {
-		code = pp->start_failure(&pp->children[i].process,
-					 &pp->children[i].err,
+		code = pp->start_failure(&pp->children[i].err,
 					 pp->data,
 					 &pp->children[i].data);
 		strbuf_addbuf(&pp->buffered_output, &pp->children[i].err);
@@ -1119,7 +1099,7 @@ static int pp_collect_finished(struct parallel_processes *pp)
 
 		code = finish_command(&pp->children[i].process);
 
-		code = pp->task_finished(code, &pp->children[i].process,
+		code = pp->task_finished(code,
 					 &pp->children[i].err, pp->data,
 					 &pp->children[i].data);
 
diff --git a/run-command.h b/run-command.h
index 2bd8fee..05cde5f 100644
--- a/run-command.h
+++ b/run-command.h
@@ -158,21 +158,11 @@ typedef int (*get_next_task_fn)(struct child_process *cp,
  * To send a signal to other child processes for abortion, return
  * the negative signal number.
  */
-typedef int (*start_failure_fn)(struct child_process *cp,
-				struct strbuf *out,
+typedef int (*start_failure_fn)(struct strbuf *out,
 				void *pp_cb,
 				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 *out,
-			  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
@@ -187,22 +177,11 @@ int default_start_failure(struct child_process *cp,
  * the negative signal number.
  */
 typedef int (*task_finished_fn)(int result,
-				struct child_process *cp,
 				struct strbuf *out,
 				void *pp_cb,
 				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 *out,
-			  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.
diff --git a/submodule.c b/submodule.c
index 051f722..4bd14de 100644
--- a/submodule.c
+++ b/submodule.c
@@ -732,8 +732,7 @@ static int get_next_submodule(struct child_process *cp,
 	return 0;
 }
 
-static int fetch_start_failure(struct child_process *cp,
-			       struct strbuf *err,
+static int fetch_start_failure(struct strbuf *err,
 			       void *cb, void *task_cb)
 {
 	struct submodule_parallel_fetch *spf = cb;
@@ -743,8 +742,8 @@ static int fetch_start_failure(struct child_process *cp,
 	return 0;
 }
 
-static int fetch_finish(int retvalue, struct child_process *cp,
-			struct strbuf *err, void *cb, void *task_cb)
+static int fetch_finish(int retvalue, struct strbuf *err,
+			void *cb, void *task_cb)
 {
 	struct submodule_parallel_fetch *spf = cb;
 
diff --git a/test-run-command.c b/test-run-command.c
index fbe0a27..30a64a9 100644
--- a/test-run-command.c
+++ b/test-run-command.c
@@ -41,7 +41,6 @@ static int no_job(struct child_process *cp,
 }
 
 static int task_finished(int result,
-			 struct child_process *cp,
 			 struct strbuf *err,
 			 void *pp_cb,
 			 void *pp_task_cb)


Stefan Beller (10):
  submodule-config: keep update strategy around
  submodule-config: drop check against NULL
  fetching submodules: respect `submodule.fetchJobs` config option
  submodule update: direct error message to stderr
  run_processes_parallel: treat output of children as byte array
  run_processes_parallel: rename parameters for the callbacks
  git submodule update: have a dedicated helper for cloning
  submodule helper: remove double 'fatal: ' prefix
  submodule update: expose parallelism to the user
  clone: allow an explicit argument for parallel submodule clones

 Documentation/config.txt        |   6 +
 Documentation/git-clone.txt     |   6 +-
 Documentation/git-submodule.txt |   7 +-
 builtin/clone.c                 |  19 ++-
 builtin/fetch.c                 |   2 +-
 builtin/submodule--helper.c     | 256 +++++++++++++++++++++++++++++++++++++++-
 git-submodule.sh                |  56 ++++-----
 run-command.c                   |  12 +-
 run-command.h                   |  10 +-
 strbuf.c                        |   6 +
 strbuf.h                        |   6 +
 submodule-config.c              |  19 ++-
 submodule-config.h              |   2 +
 submodule.c                     |  37 +++++-
 submodule.h                     |  18 +++
 t/t5526-fetch-submodules.sh     |  14 +++
 t/t7400-submodule-basic.sh      |   4 +-
 t/t7406-submodule-update.sh     |  27 +++++
 18 files changed, 445 insertions(+), 62 deletions(-)

-- 
2.8.0.rc0.1.g68b4e3f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]