"blanet via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> Elijah Newren, the author of the remerge diff feature, notes that other
> callers of `log-tree.c:log_tree_commit` (the only caller of
> `log-tree.c:do_remerge_diff`) also exist, but:
>
> `builtin/am.c`: manually sets all flags; remerge_diff is not among them
> `sequencer.c`: manually sets all flags; remerge_diff is not among them
>
> so `builtin/diff-tree.c` really is the only caller that was overlooked
> when remerge-diff functionality was added.
That is more than OK as a band-aid, and I'll take the patch as-is,
but I have to wonder if we do even better in a future follow-up
patch.
Any time do_remerge_diff() is entered, we know that either the end
user (from the command line) or the hard-coded caller (like
am/sequencer cited above) wants us to do the remerge-diff, which in
turn requires us to have the temporary object directory rotated into
the status of the primary object store. And there is nothing in
that object directory rotation code that requires caller-specific
customization---it is the same "create remerge-diff directory as
tmp-objdir, rotate it into the alt object store chain as the
primary" regardless of the actual caller).
So wouldn't it work well if we
(1) at the beginning of do_remerge_diff(), only once for a rev_info
structure:
(1-a) lazily do the "object directory rotation"
(1-b) set up an atexit handler to clear the temporary object
store
(2) remove all the "ah, we need to prepare and tear down the
temporary object store for _this_ operation" we have sprinkled
in different code paths (including the one added by the fix we
are looking at).
That way, we won't have to worry about adding future remerge_diff
users, including existing hard-coded callers.
ANyway, thanks for the fix. It is very pleasing to see contributors
working well together.
