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

On Wednesday, August 7, 2024 5:21 PM, brian m. carlson wrote:

On 2024-08-07 at 18:21:28, Josh Steadmon wrote:

quoted

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.

quoted

+[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.

quoted

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