Re: [PATCH v4] libgit-rs: add get_bool(), get_ulong(), and get_pathname() methods

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Oct 1, 2025, 10:15 UTC
Message-ID: <0cb55558-a48e-42ac-a80e-a24ee2be55ee@gmail.com>
In-Reply-To: <pull.1977.v4.git.1759221968318.gitgitgadget@gmail.com>

[I've cc'd brian to sanity check my suggestion for handling c_ulong in rust 1.63 which lacks std::ffi::c_ulong]

On 30/09/2025 09:46, ions via GitGitGadget wrote:

Show 7 quoted lines

> From: ionnss <zara.leonardo@gmail.com>
> 
> Expand ConfigSet API with three new configuration value parsers:
> 
> - get_bool(): Parse boolean values using git_configset_get_bool()
> - get_ulong(): Parse unsigned long values
> - get_pathname(): Parse file paths, returning PathBuf

I would be nice to explain in the commit message why we require paths to be utf-8 encoded. I've left one comment below, apart from that I think this looks good.

Show 18 quoted lines

> --- a/contrib/libgit-sys/src/lib.rs
> +++ b/contrib/libgit-sys/src/lib.rs
> @@ -1,7 +1,7 @@
>   use std::ffi::c_void;
>   
>   #[cfg(has_std__ffi__c_char)]
> -use std::ffi::{c_char, c_int};
> +use std::ffi::{c_char, c_int, c_ulong};
>   
>   #[cfg(not(has_std__ffi__c_char))]
>   #[allow(non_camel_case_types)]
> @@ -11,6 +11,10 @@ pub type c_char = i8;
>   #[allow(non_camel_case_types)]
>   pub type c_int = i32;
>   
> +#[cfg(not(has_std__ffi__c_char))]
> +#[allow(non_camel_case_types)]
> +pub type c_ulong = u64;

As I said before this wont work because C's ulong type is platform dependent so you cannot assume it 64 bits wide. Looking at the previous discussion[1] the reason we have these fallback definitions is because std::ffi::c_int etc were only added in rust 1.64 and we want to support rust 1.63 as that is the version shipped by Debian oldstable. I think it would be better to have a separate preparatory patch that changes the existing fallbacks to

#[cfg(not(has_std__ff__c_char))] use std::os::raw::{c_char, c_int};

and then this patch can add "c_ulong" to the list.

Thanks

Phillip

[1] https://lore.kernel.org/git/ZtivGeDZ_MZDEDB_@tapette.crustytoothpaste.net/