RE: [RFC PATCH 3/6] contrib/cgit-rs: introduce Rust wrapper for libgit.a

[<rsbecker@nexbridge.com>]

On Wednesday, August 7, 2024 5:21 PM, brian m. carlson wrote:
>On 2024-08-07 at 18:21:28, Josh Steadmon wrote:
>> Introduce cgit-rs, 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. A proof-of-concept library consumer is provided in
>> contrib/cgit-rs/src/main.rs. This executable can be run with `cargo
>> run`
>>
>> Symbols in cgit can collide with symbols from other libraries such as
>> libgit2. We avoid this by first exposing library symbols in
>> public_symbol_export.[ch]. These symbols are prepended with "libgit_"
>> to avoid collisions and set to visible using a visibility pragma. In
>> build.rs, Rust builds contrib/cgit-rs/libcgit.a, which also contains
>> libgit.a and other dependent libraries, with -fvisibility=hidden to
>> hide all symbols within those libraries that haven't been exposed with
>> a visibility pragma.
>
>I think this is a good idea.  It's optional and it allows us to add functionality as we go
>along.  Platforms that don't have Rust can just omit building it.
>
>> +[dependencies]
>> +libc = "0.2.155"
>
>I don't love that we're using libc here.  It would be better to use rustix because that
>provides safe APIs that are compatible with POSIX, but I think for now we need this
>because rustix doesn't offer memory management like free(3).  I'd really prefer that
>we didn't have to do memory management in Rust, but maybe that can come in
>with a future series.

This is a good point. Libc is not portable, but because I can't build with RUST anyway,
I hope that libc is restricted to this facility if used. It should not be included in the
git C build. It is probably moot for me anyway for this series, but I have to mention it
in case anyone else gets the idea to include it as a dependency for git C.

>libc also comes with the downside that it calls the actual libc functions, so you have
>to write things like stat64 on Linux if you want a 64-bit stat, but that doesn't exist
>on some of the BSDs, so you have to write something else and compile it
>conditionally, and all of that makes the portability of it worse than with C.
>
>In any event, I have the intention to send a patch to replace libc with rustix in the
>future if this series lands.
>
>> diff --git a/contrib/cgit-rs/public_symbol_export.c
>> b/contrib/cgit-rs/public_symbol_export.c
>> new file mode 100644
>> index 0000000000..3d1cd6cc4f
>> --- /dev/null
>> +++ b/contrib/cgit-rs/public_symbol_export.c
>> @@ -0,0 +1,20 @@
>> +// Shim to publicly export Git symbols. These must be renamed so that
>> +the // original symbols can be hidden. Renaming these with a
>> +"libgit_" prefix also // avoid conflicts with other libraries such as libgit2.
>> +
>> +#include "contrib/cgit-rs/public_symbol_export.h"
>> +#include "version.h"
>> +
>> +#pragma GCC visibility push(default)
>
>I assume this also works in clang?
>--
>brian m. carlson (they/them or he/him)
>Toronto, Ontario, CA