Re: [PATCH v2 04/18] make: merge reftable lib into libgit.a

[Junio C Hamano <gitster@pobox.com>]

Ezekiel Newren <ezekielnewren@gmail.com> writes:

> On Fri, Sep 19, 2025 at 1:02 PM Junio C Hamano <gitster@pobox.com> wrote:
>> Aside from the comment already given about the fact that the
>> proposed log message does not explain any reason why these change
>> are necessary, this step and the previous step are fairly hostile to
>> merging the topic to play well with other topics, especially given
>> that there would be topics in flight that may want to add, remove,
>> or reorder these two existing lists.
>>
>> I wonder if these could have been arranged like the following instead?
>>
>>  * Drop "REFTABLE_LIB = reftable/libreftable.a" and the target that
>>    runs "ar" to mantain that archive.
>>
>>  * Leave "REFTABLE_OBJS += $objects.o" lines alone.
>>
>>  * Add them into LIB_OBJS so that they are included in libgit.a,
>>    perhaps a single line like this:
>>
>>         LIB_OBJS += $(REFTABLE_OBJS)
>>
>> Wouldn't that have worked equally well for the (unstated) purpose of
>> these two patches without incurring unnecessary risk of mismerges?
>>
>> Similar arrangement for xdiff.
>
> Like the previous two commits; This one continues the effort to get
> ...
> The reason why ...

Neither answers my main question, though.

Instead of rolling everything into LIB_OBJS directly, wouldn't it
have been much easier to work with if reftable-related ones are left
in REFTABLE_OBJS and then RERFTABLE_OBJS gets added to LIB_OBJS?
Wouldn't it have been less prone to mismerges to do it that way?

> However I think I'll drop these 3 commits since 'cargo test' doesn't
> need to be part of the introduction of Rust. It would be nice for make
> to be able to run Rust unit tests at some point though.

As we can always extend things more, getting something close to the
minimally viable set with some tests for sanity checking would be a
good first goal.  If you pare down way too much, however, we may end
up to be pretty close to what Patrick sent out originally with the
varint conversion, so let's make sure we do not drop below the
minimum that still demonstrates that we have Rust integration that
is viable going forward.

Thanks.