Re: [PATCH v7 4/4] libgit: add higher-level libgit crate
On 2025.01.27 20:56, Junio C Hamano wrote:
Show 27 quoted lines
> Josh Steadmon <steadmon@google.com> writes:
>
> > -.PHONY: libgit-sys
> > +.PHONY: libgit-sys libgit-rs
> > libgit-sys:
> > $(QUIET)(\
> > cd contrib/libgit-sys && \
> > cargo build \
> > )
> > +libgit-rs:
> > + $(QUIET)(\
> > + cd contrib/libgit-rs && \
> > + cargo build \
> > + )
> > ifdef INCLUDE_LIBGIT_RS
> > -all:: libgit-sys
> > +all:: libgit-sys libgit-rs
> > endif
>
> I somehow would have expected this part of the patch to do
>
> libgit-sys libgit-rs:
> $(QUIET)( \
> cd contrib/$@ && cargo build \
> )
>
> but the above longhand is fine.
Fixed since I'm sending out a V8 for Phillip's feedback anyway. Thanks.
Show 23 quoted lines
> > + /// Load the value for the given key and attempt to parse it as an i32. Dies with a fatal error
> > + /// if the value cannot be parsed. Returns None if the key is not present.
> > + pub fn get_int(&mut self, key: &str) -> Option<i32> {
> > + 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.into())
> > + }
>
> Nice.
>
> I was wondering why libgit_configset_get_int() in [3/4] does not do
> better than just wrapping the raw C interface, which returns the
> error/success status separately from the value we parsed. With this
> higher-layer wrapper around it, the interface becomes a bit more
> like a higher-level language.
>
> Thanks, will queue.Josh Steadmon <steadmon@google.com>