Hi Josh
On 28/01/2025 00:19, Josh Steadmon wrote:
In preparation for implementing a higher-level Rust API for accessing
Git configs, export some of the upstream configset API via libgitpub and
libgit-sys. Since this will be exercised as part of the higher-level API
in the next commit, no tests have been added for libgit-sys.
While we're at it, add git_configset_alloc() and git_configset_free()
functions in libgitpub so that callers can manage config_set structs on
the heap. This also allows non-C external consumers to treat config_sets
as opaque structs.
This interface is looks nice, I've left a couple of comments below
diff --git a/contrib/libgit-sys/public_symbol_export.c b/contrib/libgit-sys/public_symbol_export.c
index cd1602206e..a0297cb1a5 100644
--- a/contrib/libgit-sys/public_symbol_export.c
+++ b/contrib/libgit-sys/public_symbol_export.c
@@ -4,11 +4,40 @@
*/
#include "git-compat-util.h"
+#include "config.h"
#include "contrib/libgit-sys/public_symbol_export.h"
#include "version.h"
#pragma GCC visibility push(default)
Personally I'd prefer it if we actually defined struct libgit_config_set
here
struct libgit_config_set {
struct config_set cs;
}
Then we could avoid all the casts below. For example
struct libgit_config_set *libgit_configset_alloc(void)
{
struct libget_config_set *cs =
xmalloc(sizeof(struct libgit_config_set));
git_configset_init(&cs->cs);
return cs;
}
+struct libgit_config_set *libgit_configset_alloc(void)
+{
+ struct config_set *cs = xmalloc(sizeof(struct config_set));
+ git_configset_init(cs);
+ return (struct libgit_config_set *) cs;
+}
+
+void libgit_configset_free(struct libgit_config_set *cs)
+{
+ git_configset_clear((struct config_set *) cs);
+ free((struct config_set *) cs);
+}
+
+int libgit_configset_add_file(struct libgit_config_set *cs, const char *filename)
+{
+ return git_configset_add_file((struct config_set *) cs, filename);
+}
+
+int libgit_configset_get_int(struct libgit_config_set *cs, const char *key, int *dest)
Style: this and the one below could do with being wrapped at 80 characters
This whole series looks pretty good to me
Best Wishes
Phillip
+{
+ return git_configset_get_int((struct config_set *) cs, key, dest);
+}
+
+int libgit_configset_get_string(struct libgit_config_set *cs, const char *key, char **dest)
+{
+ return git_configset_get_string((struct config_set *) cs, key, dest);
+}
+
const char *libgit_user_agent(void)
{
return git_user_agent();
diff --git a/contrib/libgit-sys/public_symbol_export.h b/contrib/libgit-sys/public_symbol_export.h
index a3372f93fa..701db92d53 100644
--- a/contrib/libgit-sys/public_symbol_export.h
+++ b/contrib/libgit-sys/public_symbol_export.h
@@ -1,6 +1,16 @@
#ifndef PUBLIC_SYMBOL_EXPORT_H
#define PUBLIC_SYMBOL_EXPORT_H
+struct libgit_config_set *libgit_configset_alloc(void);
+
+void libgit_configset_free(struct libgit_config_set *cs);
+
+int libgit_configset_add_file(struct libgit_config_set *cs, const char *filename);
+
+int libgit_configset_get_int(struct libgit_config_set *cs, const char *key, int *dest);
+
+int libgit_configset_get_string(struct libgit_config_set *cs, const char *key, char **dest);
+
const char *libgit_user_agent(void);
const char *libgit_user_agent_sanitized(void);
diff --git a/contrib/libgit-sys/src/lib.rs b/contrib/libgit-sys/src/lib.rs
index d4853f3074..dadb4e5f40 100644
--- a/contrib/libgit-sys/src/lib.rs
+++ b/contrib/libgit-sys/src/lib.rs
@@ -1,15 +1,44 @@
#[cfg(has_std__ffi__c_char)]
-use std::ffi::c_char;
+use std::ffi::{c_char, c_int};
#[cfg(not(has_std__ffi__c_char))]
#[allow(non_camel_case_types)]
pub type c_char = i8;
+#[cfg(not(has_std__ffi__c_char))]
+#[allow(non_camel_case_types)]
+pub type c_int = i32;
+
extern crate libz_sys;
+#[allow(non_camel_case_types)]
+#[repr(C)]
+pub struct libgit_config_set {
+ _data: [u8; 0],
+ _marker: core::marker::PhantomData<(*mut u8, core::marker::PhantomPinned)>,
+}
+
extern "C" {
pub fn libgit_user_agent() -> *const c_char;
pub fn libgit_user_agent_sanitized() -> *const c_char;
+
+ pub fn libgit_configset_alloc() -> *mut libgit_config_set;
+ pub fn libgit_configset_free(cs: *mut libgit_config_set);
+
+ pub fn libgit_configset_add_file(cs: *mut libgit_config_set, filename: *const c_char) -> c_int;
+
+ pub fn libgit_configset_get_int(
+ cs: *mut libgit_config_set,
+ key: *const c_char,
+ int: *mut c_int,
+ ) -> c_int;
+
+ pub fn libgit_configset_get_string(
+ cs: *mut libgit_config_set,
+ key: *const c_char,
+ dest: *mut *mut c_char,
+ ) -> c_int;
+
}
#[cfg(test)]
