Re: [PATCH v2 05/20] merge-ort: add an err() function similar to one from merge-recursive

[Derrick Stolee <stolee@gmail.com>]

On 11/2/2020 3:43 PM, Elijah Newren wrote:
> Various places in merge-recursive used an err() function when it hit
> some kind of unrecoverable error.  That code was from the reusable bits
> of merge-recursive.c that we liked, such as merge_3way, writing object
> files to the object store, reading blobs from the object store, etc.  So
> create a similar function to allow us to port that code over, and use it
> for when we detect problems returned from collect_merge_info()'s
> traverse_trees() call, which we will be adding next.
> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  merge-ort.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/merge-ort.c b/merge-ort.c
> index df97a54773..537da9f6df 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -61,11 +61,28 @@ struct conflict_info {
>  	unsigned match_mask:3;
>  };
>  
> +static int err(struct merge_options *opt, const char *err, ...)
> +{
> +	va_list params;
> +	struct strbuf sb = STRBUF_INIT;
> +
> +	strbuf_addstr(&sb, "error: ");
> +	va_start(params, err);
> +	strbuf_vaddf(&sb, err, params);
> +	va_end(params);
> +
> +	error("%s", sb.buf);
> +	strbuf_release(&sb);
> +
> +	return -1;
> +}
> +

This seems simple enough to have a duplicate copy lying
around. Do you anticipate that all common code will live
in the same file eventually? Or will these two static err()
method always be duplicated?

Aside: I wonder if these errors could be logged using trace2
primitives, to assist diagnosing problems with merges. CC'ing
Jeff Hostetler if he has an opinion.

>  static int collect_merge_info(struct merge_options *opt,
>  			      struct tree *merge_base,
>  			      struct tree *side1,
>  			      struct tree *side2)
>  {
> +	/* TODO: Implement this using traverse_trees() */
>  	die("Not yet implemented.");
>  }

This comment looks to be applied to the wrong patch.

Thanks,
-Stolee