Subject: Re: [PATCH 6/7] show, log: provide a --remerge-diff capability

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Tue, 28 Sep 2021 10:01:03 +0200

On Tue, Aug 31 2021, Elijah Newren via GitGitGadget wrote:

>  static int decoration_given;
>  static int use_mailmap_config = 1;
> +static struct tmp_objdir *tmp_objdir;
>  static const char *fmt_patch_subject_prefix = "PATCH";
>  static int fmt_patch_name_max = FORMAT_PATCH_NAME_MAX_DEFAULT;
>  static const char *fmt_pretty;

So here we make this static file-level etc...

> @@ -407,6 +410,17 @@ static int cmd_log_walk(struct rev_info *rev)
>  	int saved_nrl = 0;
>  	int saved_dcctc = 0;
>  
> +	if (rev->remerge_diff) {
> +		tmp_objdir = tmp_objdir_create();
> +		if (!tmp_objdir)
> +			die(_("unable to create temporary object directory"));
> +		tmp_objdir_make_primary(the_repository, tmp_objdir);
> +
> +		strbuf_init(&rev->remerge_objdir_location, 0);
> +		strbuf_addstr(&rev->remerge_objdir_location,
> +			      tmp_objdir_path(tmp_objdir));
> +	}
> +
>  	if (rev->early_output)
>  		setup_early_output();
>  
> @@ -449,6 +463,13 @@ static int cmd_log_walk(struct rev_info *rev)
>  	rev->diffopt.no_free = 0;
>  	diff_free(&rev->diffopt);
>  
> +	if (rev->remerge_diff) {
> +		strbuf_release(&rev->remerge_objdir_location);
> +		tmp_objdir_remove_as_primary(the_repository, tmp_objdir);
> +		tmp_objdir_destroy(tmp_objdir);
> +		tmp_objdir = NULL;

...but all of the "tmp_objdir" usage is in one function, can't the
variable be declared here instead?

We need to hand the "remerge_objdir_location" off to the "rev_info"
struct, but that seems separate from its lifetime.

Re my [1] & [2] I like Neeraj's "atexit cleanup" approach better,
perhaps that makes your cleanup in log-tree.c redundant or easier?

Per [2] it looks like you need to "hand off" the
"remerge_objdir_location", so having the struct live in tmp-objdir.h as
I suggested in [2] might make that work...

1. https://lore.kernel.org/git/[email-masked]/
2. https://lore.kernel.org/git/[email-masked]/