On 2024.08.07 21:21, 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. Yeah, needing to free() is the only thing we striclty need from libc right now. Please correct me if I'm wrong, but IIUC then any memory that is allocated on the C side and then passed to Rust needs one of: 1) freed by libc::free() on the Rust side, 2) passed back to the C side to be freed there, or 3) leaked Am I correct in assuming that your opinion is that writing additional *_free() functions on the C side is worth it to avoid libc? If so, then I'm fine with including that in V2. > 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. Even if we can't fully eliminate libc in V2, I'll keep this in mind and avoid adding additional usage of libc as much as possible. > > 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? Yep. > -- > brian m. carlson (they/them or he/him) > Toronto, Ontario, CA
