Subject: Re: [PATCH v7 4/4] libgit: add higher-level libgit crate

From: Junio C Hamano <[email-masked]>
Date: Mon, 27 Jan 2025 20:56:52 -0800

Josh Steadmon <[email-masked]> 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.

> +    /// 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.