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

["brian m. carlson" <sandals@crustytoothpaste.net>]

[-- Attachment #1: Type: text/plain, Size: 2646 bytes --]

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.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]