Re: [PATCH v7 2/4] libgit-sys: introduce Rust wrapper for libgit.a

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

Josh Steadmon <steadmon@google.com> writes:

> On 2025.01.28 15:08, Phillip Wood wrote:
>> 
>> 
>> On 28/01/2025 00:19, Josh Steadmon wrote:
>> > Introduce libgit-sys, a Rust wrapper crate that allows Rust code to call
>> > functions in libgit.a. This initial patch defines build rules and an
>> > interface that exposes user agent string getter functions as a proof of
>> > concept. This library can be tested with `cargo test`.
>> 
>> It's great to see some tests. This is looking good, I've left a couple of
>> small comments below.
>> 
>> > +contrib/libgit-sys/partial_symbol_export.o: contrib/libgit-sys/public_symbol_export.o libgit.a reftable/libreftable.a xdiff/lib.a
>> > +	$(LD) -r $^ -o $@
>> 
>> This is a very long line and the ones below are pretty long - perhaps we
>> could put the list of sources in a variable?
>
> Done for V8.
>
>
>> > +contrib/libgit-sys/hidden_symbol_export.o: contrib/libgit-sys/partial_symbol_export.o
>> > +	$(OBJCOPY) --localize-hidden $^ $@
>> > +
>> > +contrib/libgit-sys/libgitpub.a: contrib/libgit-sys/hidden_symbol_export.o
>> > +	$(AR) $(ARFLAGS) $@ $^
>> > [...]
>> > diff --git a/contrib/libgit-sys/public_symbol_export.c b/contrib/libgit-sys/public_symbol_export.c
>> > new file mode 100644
>> > index 0000000000..cd1602206e
>> > --- /dev/null
>> > +++ b/contrib/libgit-sys/public_symbol_export.c
>> > @@ -0,0 +1,22 @@
>> > +/* Shim to publicly export Git symbols. These must be renamed so that the
>> 
>> Style: Multiline comments start with an empty "/*" so this should be
>> 
>> /*
>>  * Shim ...
>
> Whoops, fixed, thanks.

The series is looking good and it seems that we'll be expecting a
hopefully small and final reroll?  I'll mark the topic in the
"What's cooking" draft as such.

Thanks, all.