[1/2] remerge-diff: lazily prpare temporary objdir on demand

Commit Message

Junio C Hamano Aug. 9, 2024, 10:30 p.m. UTC

It is error prone for each caller that sets revs.remerge_diff bit
to be responsible for preparing a temporary object directory and
rotate it into the list of alternate object stores, making it the
primary object store.

Instead, remove the code to set up and arrange the temporary object
directory from the current callers and implement it in the code that
runs remerge-diff logic.  The code to undo the futzing of the list
of alternate object store is still spread across the callers, but we
will deal with it in future steps.

Signed-off-by: Junio C Hamano <[e-mail]>
---

 * So here are two patches to illustate how the code to do the lazy
   set-up and centralized tear down looks like.  The first part is
   the lazy set-up.

 builtin/diff-tree.c |  7 -------
 builtin/log.c       |  7 -------
 log-tree.c          | 16 ++++++++++++----
 3 files changed, 12 insertions(+), 18 deletions(-)

Comments

Eric Sunshine Aug. 9, 2024, 10:37 p.m. UTC | #1

On Fri, Aug 9, 2024 at 6:31 PM Junio C Hamano <[e-mail]> wrote:
> remerge-diff: lazily prpare temporary objdir on demand

s/prpare/prepare/

> It is error prone for each caller that sets revs.remerge_diff bit
> to be responsible for preparing a temporary object directory and
> rotate it into the list of alternate object stores, making it the
> primary object store.
>
> Instead, remove the code to set up and arrange the temporary object
> directory from the current callers and implement it in the code that
> runs remerge-diff logic.  The code to undo the futzing of the list
> of alternate object store is still spread across the callers, but we
> will deal with it in future steps.
>
> Signed-off-by: Junio C Hamano <[e-mail]>

Patch

diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 12012ea093..af51cd29c6 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -168,13 +168,6 @@  int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 
 	opt->diffopt.rotate_to_strict = 1;
 
-	if (opt->remerge_diff) {
-		opt->remerge_objdir = tmp_objdir_create("remerge-diff");
-		if (!opt->remerge_objdir)
-			die(_("unable to create temporary object directory"));
-		tmp_objdir_replace_primary_odb(opt->remerge_objdir, 1);
-	}
-
 	/*
 	 * NOTE!  We expect "a..b" to expand to "^a b" but it is
 	 * perfectly valid for revision range parser to yield "b ^a",
diff --git a/builtin/log.c b/builtin/log.c
index af6403cb71..227823a16e 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -494,13 +494,6 @@  static int cmd_log_walk_no_free(struct rev_info *rev)
 	int saved_nrl = 0;
 	int saved_dcctc = 0;
 
-	if (rev->remerge_diff) {
-		rev->remerge_objdir = tmp_objdir_create("remerge-diff");
-		if (!rev->remerge_objdir)
-			die(_("unable to create temporary object directory"));
-		tmp_objdir_replace_primary_odb(rev->remerge_objdir, 1);
-	}
-
 	if (rev->early_output)
 		setup_early_output();
 
diff --git a/log-tree.c b/log-tree.c
index 337b9334cd..83e648b7cd 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1019,6 +1019,17 @@  static int do_remerge_diff(struct rev_info *opt,
 	struct strbuf parent1_desc = STRBUF_INIT;
 	struct strbuf parent2_desc = STRBUF_INIT;
 
+	/*
+	 * Lazily prepare a temporary object directory and rotate it
+	 * into the alternative object store list as the primary.
+	 */
+	if (opt->remerge_diff && !opt->remerge_objdir) {
+		opt->remerge_objdir = tmp_objdir_create("remerge-diff");
+		if (!opt->remerge_objdir)
+			return error(_("unable to create temporary object directory"));
+		tmp_objdir_replace_primary_odb(opt->remerge_objdir, 1);
+	}
+
 	/* Setup merge options */
 	init_merge_options(&o, the_repository);
 	o.show_rename_progress = 0;
@@ -1053,10 +1064,7 @@  static int do_remerge_diff(struct rev_info *opt,
 	merge_finalize(&o, &res);
 
 	/* Clean up the contents of the temporary object directory */
-	if (opt->remerge_objdir)
-		tmp_objdir_discard_objects(opt->remerge_objdir);
-	else
-		BUG("did a remerge diff without remerge_objdir?!?");
+	tmp_objdir_discard_objects(opt->remerge_objdir);
 
 	return !opt->loginfo;
 }