Subject: Re: [PATCH 2/2] remerge-diff: clean up temporary objdir at a central place

From: Junio C Hamano <[email-masked]>
Date: Sun, 08 Sep 2024 14:16:20 -0700

Elijah Newren <[email-masked]> writes:

>> I forgot to add that I am not happy with this "centralized tear
>> down" step, even though I am reasonably happy with the "lazy set-up"
>> step.  I wonder why the remerge-diff related members have to exist
>> in the rev_info structure in the first place, instead of being in
>> the diffopt structure?  Moving them to diffopt may make the end
>> result much more pleasant to read.
>
> I'm not sure they need to exist in the rev_info structure.  I was
> probably thinking that e.g. log --remerge-diff would "need to do lots
> of diffs" but I only needed the temporary store setup once, and once
> in my mind matched better with rev_info.  I wasn't aware of
> diff_options.no_free or anything surrounding it.  If we can do the
> temporary store setup in diffopt and only do it once for all N diffs
> in a `git log --remerge-diff` run, then we could move this stuff from
> rev_info to diffopt.

After staring at the data structure around this area for a while, I
think these two patches are OK, especially within the current
structure.  rev_info structure has a set of bits that are marked as
"Diff flags" (some of which are not about diff at all, though; for
example, verbose_header and no_commit_id belong more to the "Format
info" group), and those related to merges do belong there, as they
need more than "here are two tree-like things I want to compare".

So, I'll mark these two patches for 'next' now.

Thanks.