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

On Sun, Aug 11, 2024 at 10:17 PM Junio C Hamano [off-list ref] wrote:

Junio C Hamano [off-list ref] writes:

quoted

After running a diff between two things, or a series of diffs while
walking the history, the diff computation is concluded by a call to
diff_result_code() to extract the exit status of the diff machinery.

The function can work on "struct diffopt", but all the callers
historically and currently pass "struct diffopt" that is embedded in
the "struct rev_info" that is used to hold the remerge_diff bit and
the remerge_objdir variable that points at the temporary object
directory in use.

Redefine diff_result_code() to take the whole "struct rev_info" to
give it an access to these members related to remerge-diff, so that
it can get rid of the temporary object directory for any and all
callers that used the feature.  We can lose the equivalent code to
do so from the code paths for individual commands, diff-tree, diff,
and log.

Signed-off-by: Junio C Hamano <redacted>
---

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.