get_next_submodule(): format error string as an error

Commit Message

Jeff King Aug. 14, 2019, 3:59 p.m. UTC

On Wed, Aug 14, 2019 at 09:57:50AM +0000, Paolo Pettinato (ppettina) wrote:

> Could not access submodule 'sm' # fails, plus no newline here :P!

This part seems easy enough to fix.

-- >8 --
Subject: get_next_submodule(): format error string as an error

The run_processes_parallel() interface passes its callback functions an
"err" strbuf in which they can accumulate errors. However, this differs
from our usual "err" strbufs in that the result is not simply passed to
error(), like:

  if (frob_repo(&err) < 0)
	error("frobnication failed: %s", err.buf);

Instead, we append the error buffer as-is to a buffer collecting the
sub-process stderr, adding neither a prefix nor a trailing newline. This
gives callbacks more flexibility (e.g., get_next_submodule() adds its
own "Fetching submodule foo" informational lines), but it means they're
also responsible for formatting any errors themselves.

We forgot to do so in the single error message in get_next_submodule(),
meaning that it was output without a trailing newline. While we're
fixing that, let's also give it the usual "error:" prefix and downcase
the start of the message. We can't use error() here, because it always
outputs directly to stderr.

Looking at other users of run_processes_parallel(), there are a few
similar messages in update_clone_task_finished(). But those sites do
correctly add a newline (they don't use an "error" prefix, but it
doesn't make as much sense there).

Signed-off-by: Jeff King <[e-mail]>
---
 submodule.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Junio C Hamano Aug. 14, 2019, 4:15 p.m. UTC | #1

Jeff King <[e-mail]> writes:

> On Wed, Aug 14, 2019 at 09:57:50AM +0000, Paolo Pettinato (ppettina) wrote:
>
>> Could not access submodule 'sm' # fails, plus no newline here :P!
>
> This part seems easy enough to fix.

Indeed, it is ;-)

> diff --git a/submodule.c b/submodule.c
> index 0f199c5137..a5ba57ac36 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1478,7 +1478,7 @@ static int get_next_submodule(struct child_process *cp,
>  			    !is_empty_dir(ce->name)) {
>  				spf->result = 1;
>  				strbuf_addf(err,
> -					    _("Could not access submodule '%s'"),
> +					    _("error: could not access submodule '%s'\n"),
>  					    ce->name);
>  			}
>  		}

Patch

diff --git a/submodule.c b/submodule.c
index 0f199c5137..a5ba57ac36 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1478,7 +1478,7 @@  static int get_next_submodule(struct child_process *cp,
 			    !is_empty_dir(ce->name)) {
 				spf->result = 1;
 				strbuf_addf(err,
-					    _("Could not access submodule '%s'"),
+					    _("error: could not access submodule '%s'\n"),
 					    ce->name);
 			}
 		}