diff mbox series

[v4,4/5] libgit: add higher-level libgit crate

Message ID 29599e9c7be1737bcf0de0541c9635212a1b691d.1728429158.git.steadmon@google.com (mailing list archive)
State Superseded
Headers show
Series Introduce libgit-rs, a Rust wrapper around libgit.a | expand

Commit Message

Josh Steadmon Oct. 8, 2024, 11:19 p.m. UTC
From: Calvin Wan <calvinwan@google.com>

Wrap `struct config_set` and a few of its associated functions in
libgit-sys. Also introduce a higher-level "libgit" crate which provides
a more Rust-friendly interface to config_set structs.

Co-authored-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Josh Steadmon <steadmon@google.com>
---
 .gitignore                              |  1 +
 Makefile                                |  2 +-
 contrib/libgit-rs/Cargo.lock            | 77 ++++++++++++++++++++
 contrib/libgit-rs/Cargo.toml            | 15 ++++
 contrib/libgit-rs/build.rs              |  4 ++
 contrib/libgit-rs/libgit-sys/src/lib.rs |  4 ++
 contrib/libgit-rs/src/lib.rs            | 95 +++++++++++++++++++++++++
 contrib/libgit-rs/testdata/config1      |  2 +
 contrib/libgit-rs/testdata/config2      |  2 +
 contrib/libgit-rs/testdata/config3      |  2 +
 10 files changed, 203 insertions(+), 1 deletion(-)
 create mode 100644 contrib/libgit-rs/Cargo.lock
 create mode 100644 contrib/libgit-rs/Cargo.toml
 create mode 100644 contrib/libgit-rs/build.rs
 create mode 100644 contrib/libgit-rs/src/lib.rs
 create mode 100644 contrib/libgit-rs/testdata/config1
 create mode 100644 contrib/libgit-rs/testdata/config2
 create mode 100644 contrib/libgit-rs/testdata/config3

Comments

Eric Sunshine Oct. 9, 2024, 6:18 a.m. UTC | #1
On Tue, Oct 8, 2024 at 7:19 PM Josh Steadmon <steadmon@google.com> wrote:
> Wrap `struct config_set` and a few of its associated functions in
> libgit-sys. Also introduce a higher-level "libgit" crate which provides
> a more Rust-friendly interface to config_set structs.
>
> Signed-off-by: Calvin Wan <calvinwan@google.com>
> Signed-off-by: Josh Steadmon <steadmon@google.com>
> ---
> diff --git a/contrib/libgit-rs/src/lib.rs b/contrib/libgit-rs/src/lib.rs
> @@ -0,0 +1,95 @@
> +pub struct ConfigSet(*mut libgit_config_set);
> +impl ConfigSet {
> +    pub fn get_int(&mut self, key: &str) -> Option<c_int> {
> +        let key = CString::new(key).expect("Couldn't convert to CString");
> +        let mut val: c_int = 0;
> +        unsafe {
> +            if libgit_configset_get_int(self.0, key.as_ptr(), &mut val as *mut c_int) != 0 {
> +                return None;
> +            }
> +        }
> +        Some(val)
> +    }

Considering that v4 finally gets around to swapping out `CString` for
`String` in order to make this high-level crate more
Rust-programmer-friendly, I was more than a little surprised to see
that this function is still exposing `c_int` rather than, say, `i64`
or such.
Josh Steadmon Oct. 9, 2024, 9:21 p.m. UTC | #2
On 2024.10.09 02:18, Eric Sunshine wrote:
> On Tue, Oct 8, 2024 at 7:19 PM Josh Steadmon <steadmon@google.com> wrote:
> > Wrap `struct config_set` and a few of its associated functions in
> > libgit-sys. Also introduce a higher-level "libgit" crate which provides
> > a more Rust-friendly interface to config_set structs.
> >
> > Signed-off-by: Calvin Wan <calvinwan@google.com>
> > Signed-off-by: Josh Steadmon <steadmon@google.com>
> > ---
> > diff --git a/contrib/libgit-rs/src/lib.rs b/contrib/libgit-rs/src/lib.rs
> > @@ -0,0 +1,95 @@
> > +pub struct ConfigSet(*mut libgit_config_set);
> > +impl ConfigSet {
> > +    pub fn get_int(&mut self, key: &str) -> Option<c_int> {
> > +        let key = CString::new(key).expect("Couldn't convert to CString");
> > +        let mut val: c_int = 0;
> > +        unsafe {
> > +            if libgit_configset_get_int(self.0, key.as_ptr(), &mut val as *mut c_int) != 0 {
> > +                return None;
> > +            }
> > +        }
> > +        Some(val)
> > +    }
> 
> Considering that v4 finally gets around to swapping out `CString` for
> `String` in order to make this high-level crate more
> Rust-programmer-friendly, I was more than a little surprised to see
> that this function is still exposing `c_int` rather than, say, `i64`
> or such.

Yes, definitely an oversight. I'll fix in V5.
Josh Steadmon Oct. 9, 2024, 10:25 p.m. UTC | #3
On 2024.10.08 16:19, Josh Steadmon wrote:
> From: Calvin Wan <calvinwan@google.com>
> 
> Wrap `struct config_set` and a few of its associated functions in
> libgit-sys. Also introduce a higher-level "libgit" crate which provides
> a more Rust-friendly interface to config_set structs.
> 
> Co-authored-by: Josh Steadmon <steadmon@google.com>
> Signed-off-by: Calvin Wan <calvinwan@google.com>
> Signed-off-by: Josh Steadmon <steadmon@google.com>
> ---
>  .gitignore                              |  1 +
>  Makefile                                |  2 +-
>  contrib/libgit-rs/Cargo.lock            | 77 ++++++++++++++++++++
>  contrib/libgit-rs/Cargo.toml            | 15 ++++
>  contrib/libgit-rs/build.rs              |  4 ++
>  contrib/libgit-rs/libgit-sys/src/lib.rs |  4 ++
>  contrib/libgit-rs/src/lib.rs            | 95 +++++++++++++++++++++++++
>  contrib/libgit-rs/testdata/config1      |  2 +
>  contrib/libgit-rs/testdata/config2      |  2 +
>  contrib/libgit-rs/testdata/config3      |  2 +
>  10 files changed, 203 insertions(+), 1 deletion(-)
>  create mode 100644 contrib/libgit-rs/Cargo.lock
>  create mode 100644 contrib/libgit-rs/Cargo.toml
>  create mode 100644 contrib/libgit-rs/build.rs
>  create mode 100644 contrib/libgit-rs/src/lib.rs
>  create mode 100644 contrib/libgit-rs/testdata/config1
>  create mode 100644 contrib/libgit-rs/testdata/config2
>  create mode 100644 contrib/libgit-rs/testdata/config3

Just realized that this commit message is not accurate anymore (and
could provide more useful info anyway). I've reworded it in V5.
diff mbox series

Patch

diff --git a/.gitignore b/.gitignore
index dfd72820fb..0a42f27117 100644
--- a/.gitignore
+++ b/.gitignore
@@ -248,4 +248,5 @@  Release/
 /git.VC.db
 *.dSYM
 /contrib/buildsystems/out
+/contrib/libgit-rs/target
 /contrib/libgit-rs/libgit-sys/target
diff --git a/Makefile b/Makefile
index 0090514e55..abeee01d9e 100644
--- a/Makefile
+++ b/Makefile
@@ -3723,7 +3723,7 @@  clean: profile-clean coverage-clean cocciclean
 	$(RM) $(htmldocs).tar.gz $(manpages).tar.gz
 	$(MAKE) -C Documentation/ clean
 	$(RM) Documentation/GIT-EXCLUDED-PROGRAMS
-	$(RM) -r contrib/libgit-rs/libgit-sys/target
+	$(RM) -r contrib/libgit-rs/target contrib/libgit-rs/libgit-sys/target
 ifndef NO_PERL
 	$(RM) -r perl/build/
 endif
diff --git a/contrib/libgit-rs/Cargo.lock b/contrib/libgit-rs/Cargo.lock
new file mode 100644
index 0000000000..a30c7c8d33
--- /dev/null
+++ b/contrib/libgit-rs/Cargo.lock
@@ -0,0 +1,77 @@ 
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 3
+
+[[package]]
+name = "autocfg"
+version = "1.4.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ace50bade8e6234aa140d9a2f552bbee1db4d353f69b8217bc503490fc1a9f26"
+
+[[package]]
+name = "cc"
+version = "1.1.15"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "57b6a275aa2903740dc87da01c62040406b8812552e97129a63ea8850a17c6e6"
+dependencies = [
+ "shlex",
+]
+
+[[package]]
+name = "libc"
+version = "0.2.158"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d8adc4bb1803a324070e64a98ae98f38934d91957a99cfb3a43dcbc01bc56439"
+
+[[package]]
+name = "libgit"
+version = "0.1.0"
+dependencies = [
+ "autocfg",
+ "libgit-sys",
+]
+
+[[package]]
+name = "libgit-sys"
+version = "0.1.0"
+dependencies = [
+ "autocfg",
+ "libz-sys",
+ "make-cmd",
+]
+
+[[package]]
+name = "libz-sys"
+version = "1.1.20"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d2d16453e800a8cf6dd2fc3eb4bc99b786a9b90c663b8559a5b1a041bf89e472"
+dependencies = [
+ "cc",
+ "libc",
+ "pkg-config",
+ "vcpkg",
+]
+
+[[package]]
+name = "make-cmd"
+version = "0.1.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a8ca8afbe8af1785e09636acb5a41e08a765f5f0340568716c18a8700ba3c0d3"
+
+[[package]]
+name = "pkg-config"
+version = "0.3.30"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d231b230927b5e4ad203db57bbcbee2802f6bce620b1e4a9024a07d94e2907ec"
+
+[[package]]
+name = "shlex"
+version = "1.3.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64"
+
+[[package]]
+name = "vcpkg"
+version = "0.2.15"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426"
diff --git a/contrib/libgit-rs/Cargo.toml b/contrib/libgit-rs/Cargo.toml
new file mode 100644
index 0000000000..6b4387b9e7
--- /dev/null
+++ b/contrib/libgit-rs/Cargo.toml
@@ -0,0 +1,15 @@ 
+[package]
+name = "libgit"
+version = "0.1.0"
+edition = "2021"
+build = "build.rs"
+rust-version = "1.63"
+
+[lib]
+path = "src/lib.rs"
+
+[dependencies]
+libgit-sys = { version = "0.1.0", path = "libgit-sys" }
+
+[build-dependencies]
+autocfg = "1.4.0"
diff --git a/contrib/libgit-rs/build.rs b/contrib/libgit-rs/build.rs
new file mode 100644
index 0000000000..f8bd01a690
--- /dev/null
+++ b/contrib/libgit-rs/build.rs
@@ -0,0 +1,4 @@ 
+pub fn main() {
+    let ac = autocfg::new();
+    ac.emit_has_path("std::ffi::c_char");
+}
diff --git a/contrib/libgit-rs/libgit-sys/src/lib.rs b/contrib/libgit-rs/libgit-sys/src/lib.rs
index dadb4e5f40..4bfc650450 100644
--- a/contrib/libgit-rs/libgit-sys/src/lib.rs
+++ b/contrib/libgit-rs/libgit-sys/src/lib.rs
@@ -1,3 +1,5 @@ 
+use std::ffi::c_void;
+
 #[cfg(has_std__ffi__c_char)]
 use std::ffi::{c_char, c_int};
 
@@ -19,6 +21,8 @@  pub struct libgit_config_set {
 }
 
 extern "C" {
+    pub fn free(ptr: *mut c_void);
+
     pub fn libgit_user_agent() -> *const c_char;
     pub fn libgit_user_agent_sanitized() -> *const c_char;
 
diff --git a/contrib/libgit-rs/src/lib.rs b/contrib/libgit-rs/src/lib.rs
new file mode 100644
index 0000000000..d228091f3d
--- /dev/null
+++ b/contrib/libgit-rs/src/lib.rs
@@ -0,0 +1,95 @@ 
+use std::ffi::{c_void, CStr, CString};
+use std::path::Path;
+
+#[cfg(has_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;
+
+use libgit_sys::*;
+
+pub struct ConfigSet(*mut libgit_config_set);
+impl ConfigSet {
+    pub fn new() -> Self {
+        unsafe { ConfigSet(libgit_configset_alloc()) }
+    }
+
+    pub fn add_files(&mut self, files: &[&Path]) {
+        for file in files {
+            let pstr = file.to_str().expect("Invalid UTF-8");
+            let rs = CString::new(pstr).expect("Couldn't convert to CString");
+            unsafe {
+                libgit_configset_add_file(self.0, rs.as_ptr());
+            }
+        }
+    }
+
+    pub fn get_int(&mut self, key: &str) -> Option<c_int> {
+        let key = CString::new(key).expect("Couldn't convert to CString");
+        let mut val: c_int = 0;
+        unsafe {
+            if libgit_configset_get_int(self.0, key.as_ptr(), &mut val as *mut c_int) != 0 {
+                return None;
+            }
+        }
+
+        Some(val)
+    }
+
+    pub fn get_string(&mut self, key: &str) -> Option<String> {
+        let key = CString::new(key).expect("Couldn't convert key to CString");
+        let mut val: *mut c_char = std::ptr::null_mut();
+        unsafe {
+            if libgit_configset_get_string(self.0, key.as_ptr(), &mut val as *mut *mut c_char) != 0
+            {
+                return None;
+            }
+            let borrowed_str = CStr::from_ptr(val);
+            let owned_str =
+                String::from(borrowed_str.to_str().expect("Couldn't convert val to str"));
+            free(val as *mut c_void); // Free the xstrdup()ed pointer from the C side
+            Some(owned_str)
+        }
+    }
+}
+
+impl Default for ConfigSet {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl Drop for ConfigSet {
+    fn drop(&mut self) {
+        unsafe {
+            libgit_configset_free(self.0);
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn load_configs_via_configset() {
+        let mut cs = ConfigSet::new();
+        cs.add_files(&[
+            Path::new("testdata/config1"),
+            Path::new("testdata/config2"),
+            Path::new("testdata/config3"),
+        ]);
+        // ConfigSet retrieves correct value
+        assert_eq!(cs.get_int("trace2.eventTarget"), Some(1));
+        // ConfigSet respects last config value set
+        assert_eq!(cs.get_int("trace2.eventNesting"), Some(3));
+        // ConfigSet returns None for missing key
+        assert_eq!(cs.get_string("foo.bar"), None);
+    }
+}
diff --git a/contrib/libgit-rs/testdata/config1 b/contrib/libgit-rs/testdata/config1
new file mode 100644
index 0000000000..4e9a9d25d1
--- /dev/null
+++ b/contrib/libgit-rs/testdata/config1
@@ -0,0 +1,2 @@ 
+[trace2]
+	eventNesting = 1
diff --git a/contrib/libgit-rs/testdata/config2 b/contrib/libgit-rs/testdata/config2
new file mode 100644
index 0000000000..b8d1eca423
--- /dev/null
+++ b/contrib/libgit-rs/testdata/config2
@@ -0,0 +1,2 @@ 
+[trace2]
+	eventTarget = 1
diff --git a/contrib/libgit-rs/testdata/config3 b/contrib/libgit-rs/testdata/config3
new file mode 100644
index 0000000000..ca7b9a7c38
--- /dev/null
+++ b/contrib/libgit-rs/testdata/config3
@@ -0,0 +1,2 @@ 
+[trace2]
+	eventNesting = 3