Subject: Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)

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

On Mon, Sep 27 2021, Elijah Newren wrote:

> [Did some slight re-ordering of topics]
>
> On Mon, Sep 27, 2021 at 5:53 PM Junio C Hamano <[email-masked]> wrote:
> [...]
>> * en/remerge-diff (2021-08-31) 7 commits
>>  - doc/diff-options: explain the new --remerge-diff option
>>  - show, log: provide a --remerge-diff capability
>>  - tmp-objdir: new API for creating and removing primary object dirs
>>  - merge-ort: capture and print ll-merge warnings in our preferred fashion
>>  - ll-merge: add API for capturing warnings in a strbuf instead of stderr
>>  - merge-ort: add ability to record conflict messages in a file
>>  - merge-ort: mark a few more conflict messages as omittable
>>
>>  A new presentation for two-parent merge "--remerge-diff" can be
>>  used to show the difference between mechanical (and possibly
>>  conflicted) merge results and the recorded resolution.
>>
>>  Will merge to 'next'?
>
> It has been a month that it's been cooking with no issues brought up,
> and it's been in production for nearly a year...
>
> But just this morning I pinged peff and jrnieder if they might have
> time to respectively look at the tmp-objdir stuff (patch 5, plus its
> integration into log-tree.c in patch 7) and the ll-merge.[ch] changes
> (patch 3).  I don't know if either will have time to do it, but
> perhaps wait half a week or so to see if they'll mention they have
> time?  Otherwise, yeah, it's probably time to merge this down.

I haven't poked at it much, but haven't you and Neeraj Singh (CC'd)
independently come up with two slightly different changes in
tmp-objdir.c to do the same thing? See the tmp-objdir.c part of:

http://lore.kernel.org/git/543e[email-masked]

And your:

http://lore.kernel.org/git/67d3[email-masked]

I.e. yours has the object database managed outside, his has it added to
"struct tmp_objdir", but it's the same objdir dance isn't it?

I started reading the combined code in "seen" and found it quite
confusing until I saw what was going on.

For both, if you can agree on some common API: I'd prefer if the
"%s/incoming-XXXXXX" part of the "tmp_objdir_create()" was passed as
some template, perhaps just the string "incoming" as a prefix?

I.e. this was for receive-pack.c originally, now it's for bulk-checkin.c
and log.c, if either of those segfault or is long-running it's quite
confusing to have something called "incoming" if you're manually
inspecting it.  Perhaps "core-fsyncObjectFiles-batch" (or even
"core.fsyncObjectFiles=batch") and "log-remerge-diff" as prefixes for
the two, and "incoming" for the one existing caller in "master"?

>> * en/removing-untracked-fixes (2021-09-27) 12 commits
>>  - Documentation: call out commands that nuke untracked files/directories
>>  - Comment important codepaths regarding nuking untracked files/dirs
>>  - unpack-trees: avoid nuking untracked dir in way of locally deleted file
>>  - unpack-trees: avoid nuking untracked dir in way of unmerged file
>>  - Change unpack_trees' 'reset' flag into an enum
>>  - Remove ignored files by default when they are in the way
>>  - unpack-trees: make dir an internal-only struct
>>  - unpack-trees: introduce preserve_ignored to unpack_trees_options
>>  - read-tree, merge-recursive: overwrite ignored files by default
>>  - checkout, read-tree: fix leak of unpack_trees_options.dir
>>  - t2500: add various tests for nuking untracked files
>>  - Merge branch 'en/am-abort-fix' into en/removing-untracked-fixes
>>
>>  Various fixes in code paths that move untracked files away to make room.
>>
>>  Will merge to 'next'?
>
> I just sent out v3 this morning with five new patches (included in
> your list above).  While I think my patches are good, and I'd like to
> see them merged down to next so I can send my
> current-working-directory-deletion fixes that build on top of it, I'm
> a little surprised you're proposing to merge this series down this
> quickly instead of waiting a little longer for review of the new
> patches.  I'm not complaining...but was that intentional?