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

Commit Message

Elijah Newren Nov. 29, 2020, 7:43 a.m. UTC

From: Elijah Newren <[e-mail]>

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 <[e-mail]>
---
 merge-ort.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

Comments

Ævar Arnfjörð Bjarmason Nov. 29, 2020, 10:23 a.m. UTC | #1

On Sun, Nov 29 2020, Elijah Newren via GitGitGadget wrote:

>  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.");
>  }
>  

Looks like this doesn't belong in this patch & should instead be
squashed into "[PATCH 02/20] merge-ort: add some high-level algorithm
structure".

Ævar Arnfjörð Bjarmason Nov. 29, 2020, 10:26 a.m. UTC | #2

On Sun, Nov 29 2020, Elijah Newren via GitGitGadget wrote:

> +		err(opt, _("collecting merge info failed for trees %s, %s, %s"),
> +		    oid_to_hex(&merge_base->object.oid),
> +		    oid_to_hex(&side1->object.oid),
> +		    oid_to_hex(&side2->object.oid));

(Sorry about the two E-Mails, didn't spot this at first). This error
message without context for translators is going to give them no idea
what %s/%s/%s are. Maybe this before it:

 /*
  * TRANSLATORS: The %s arguments are: 1) tree SHA-1 of a merge base 2-3) the
  * trees for the two trees we're merging.
  */

Elijah Newren Nov. 30, 2020, 4:56 p.m. UTC | #3

On Sun, Nov 29, 2020 at 2:23 AM Ævar Arnfjörð Bjarmason
<[e-mail]> wrote:
>
>
> On Sun, Nov 29 2020, Elijah Newren via GitGitGadget wrote:
>
> >  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.");
> >  }
> >
>
> Looks like this doesn't belong in this patch & should instead be
> squashed into "[PATCH 02/20] merge-ort: add some high-level algorithm
> structure".

Indeed, and Derrick pointed out the same thing but when I went back
through all the emails to try to make sure I covered everything, I
somehow missed that particular piece of his comments.  Anyway, I've
fixed it up locally along with your two other suggestions.  I'll wait
a bit more for other feedback before sending the next re-roll.

Thanks for taking a look!

Patch

diff --git a/merge-ort.c b/merge-ort.c
index d737762700..baf31bcc28 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -158,11 +158,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;
+}
+
 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.");
 }
 
@@ -265,7 +282,15 @@  static void merge_ort_nonrecursive_internal(struct merge_options *opt,
 {
 	struct object_id working_tree_oid;
 
-	collect_merge_info(opt, merge_base, side1, side2);
+	if (collect_merge_info(opt, merge_base, side1, side2) != 0) {
+		err(opt, _("collecting merge info failed for trees %s, %s, %s"),
+		    oid_to_hex(&merge_base->object.oid),
+		    oid_to_hex(&side1->object.oid),
+		    oid_to_hex(&side2->object.oid));
+		result->clean = -1;
+		return;
+	}
+
 	result->clean = detect_and_process_renames(opt, merge_base,
 						   side1, side2);
 	process_entries(opt, &working_tree_oid);