* [PATCH 0/2] libgit-rs: add get_bool() method to ConfigSet
@ 2025-09-25 11:44 ions via GitGitGadget
2025-09-25 11:44 ` [PATCH 1/2] po: fix escaped underscores in README.md ionnss via GitGitGadget
` (2 more replies)
0 siblings, 3 replies; 22+ messages in thread
From: ions via GitGitGadget @ 2025-09-25 11:44 UTC (permalink / raw)
To: git; +Cc: ions
Purpose
This pull request introduces a get_bool() method to the ConfigSet module in
the libgit-rs library. The goal is to enhance the functionality of ConfigSet
by providing a way to fetch and handle boolean configuration values more
easily and consistently.
Implementation Details
• Added a get_bool() method to the ConfigSet module.
• The method retrieves configuration values as boolean values, ensuring
proper parsing and error handling.
• This addition simplifies the process of working with boolean
configurations for developers using the ConfigSet module.
Testing
• Added unit tests to verify the correctness of the get_bool() method.
• Tested edge cases to ensure robustness.
ionnss (2):
po: fix escaped underscores in README.md
libgit-rs: add get_bool() method to ConfigSet
contrib/libgit-rs/src/config.rs | 24 ++++++++++++++++++++++++
contrib/libgit-rs/testdata/config3 | 2 ++
po/README.md | 6 +++---
3 files changed, 29 insertions(+), 3 deletions(-)
base-commit: bb69721404348ea2db0a081c41ab6ebfe75bdec8
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1977%2Fionnss%2Fadd-rust-configset-get-bool-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1977/ionnss/add-rust-configset-get-bool-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1977
--
gitgitgadget
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 1/2] po: fix escaped underscores in README.md 2025-09-25 11:44 [PATCH 0/2] libgit-rs: add get_bool() method to ConfigSet ions via GitGitGadget @ 2025-09-25 11:44 ` ionnss via GitGitGadget 2025-09-25 11:44 ` [PATCH 2/2] libgit-rs: add get_bool() method to ConfigSet ionnss via GitGitGadget 2025-09-27 0:07 ` [PATCH v2 0/3] " ions via GitGitGadget 2 siblings, 0 replies; 22+ messages in thread From: ionnss via GitGitGadget @ 2025-09-25 11:44 UTC (permalink / raw) To: git; +Cc: ions, ionnss From: ionnss <zara.leonardo@gmail.com> Remove unnecessary backslashes from language code examples. The underscores in "ll\_CC" and "zh\_CN" don't need escaping in Markdown. Signed-off-by: ionnss <zara.leonardo@gmail.com> --- po/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/po/README.md b/po/README.md index ec08aa24ad..d7757bed4e 100644 --- a/po/README.md +++ b/po/README.md @@ -13,9 +13,9 @@ We will use XX as an alias to refer to the language translation code in the following paragraphs, for example we use "po/XX.po" to refer to the translation file for a specific language. But this doesn't mean that the language code has only two letters. The language code can be in one -of two forms: "ll" or "ll\_CC". Here "ll" is the ISO 639 two-letter +of two forms: "ll" or "ll_CC". Here "ll" is the ISO 639 two-letter language code and "CC" is the ISO 3166 two-letter code for country names -and subdivisions. For example: "de" for German language code, "zh\_CN" +and subdivisions. For example: "de" for German language code, "zh_CN" for Simplified Chinese language code. @@ -126,7 +126,7 @@ you add a translation for the first time by running: make po-init PO_FILE=po/XX.po ``` -where XX is the locale, e.g. "de", "is", "pt\_BR", "zh\_CN", etc. +where XX is the locale, e.g. "de", "is", "pt_BR", "zh_CN", etc. The newly generated message file "po/XX.po" is based on the core pot file "po/git-core.pot", so it contains only a minimal set of messages -- gitgitgadget ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/2] libgit-rs: add get_bool() method to ConfigSet 2025-09-25 11:44 [PATCH 0/2] libgit-rs: add get_bool() method to ConfigSet ions via GitGitGadget 2025-09-25 11:44 ` [PATCH 1/2] po: fix escaped underscores in README.md ionnss via GitGitGadget @ 2025-09-25 11:44 ` ionnss via GitGitGadget 2025-09-26 6:43 ` Chris Torek 2025-09-26 9:58 ` Phillip Wood 2025-09-27 0:07 ` [PATCH v2 0/3] " ions via GitGitGadget 2 siblings, 2 replies; 22+ messages in thread From: ionnss via GitGitGadget @ 2025-09-25 11:44 UTC (permalink / raw) To: git; +Cc: ions, ionnss From: ionnss <zara.leonardo@gmail.com> Add support for parsing boolean configuration values in the Rust ConfigSet API. The method follows Git's standard boolean parsing rules, accepting true/yes/on/1 as true and false/no/off/0 as false. The implementation reuses the existing get_string() infrastructure and adds case-insensitive boolean parsing logic. Signed-off-by: ionnss <zara.leonardo@gmail.com> --- contrib/libgit-rs/src/config.rs | 24 ++++++++++++++++++++++++ contrib/libgit-rs/testdata/config3 | 2 ++ 2 files changed, 26 insertions(+) diff --git a/contrib/libgit-rs/src/config.rs b/contrib/libgit-rs/src/config.rs index 6bf04845c8..3f4a32c72d 100644 --- a/contrib/libgit-rs/src/config.rs +++ b/contrib/libgit-rs/src/config.rs @@ -68,6 +68,26 @@ impl ConfigSet { Some(owned_str) } } + + pub fn get_bool(&mut self, key: &str) -> Option<bool> { + 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 + match owned_str.to_lowercase().as_str() { + "true" | "yes" | "on" | "1" => Some(true), + "false" | "no" | "off" | "0" => Some(false), + _ => None, + } + } + } } impl Default for ConfigSet { @@ -102,5 +122,9 @@ mod tests { assert_eq!(cs.get_int("trace2.eventNesting"), Some(3)); // ConfigSet returns None for missing key assert_eq!(cs.get_string("foo.bar"), None); + // Test boolean parsing + assert_eq!(cs.get_bool("test.booleanValue"), Some(true)); + // Test missing boolean key + assert_eq!(cs.get_bool("missing.boolean"), None); } } diff --git a/contrib/libgit-rs/testdata/config3 b/contrib/libgit-rs/testdata/config3 index ca7b9a7c38..83a474ccef 100644 --- a/contrib/libgit-rs/testdata/config3 +++ b/contrib/libgit-rs/testdata/config3 @@ -1,2 +1,4 @@ [trace2] eventNesting = 3 +[test] + booleanValue = true -- gitgitgadget ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] libgit-rs: add get_bool() method to ConfigSet 2025-09-25 11:44 ` [PATCH 2/2] libgit-rs: add get_bool() method to ConfigSet ionnss via GitGitGadget @ 2025-09-26 6:43 ` Chris Torek 2025-09-26 9:58 ` Phillip Wood 1 sibling, 0 replies; 22+ messages in thread From: Chris Torek @ 2025-09-26 6:43 UTC (permalink / raw) To: ionnss via GitGitGadget; +Cc: git, ionnss A bit minor, and I'm not a real Rust programmer, but: On Thu, Sep 25, 2025 at 4:44 AM ionnss via GitGitGadget <gitgitgadget@gmail.com> wrote: > > From: ionnss <zara.leonardo@gmail.com> > > Add support for parsing boolean configuration values in the Rust > ConfigSet API. The method follows Git's standard boolean parsing > rules, accepting true/yes/on/1 as true and false/no/off/0 as false. > > The implementation reuses the existing get_string() infrastructure > and adds case-insensitive boolean parsing logic. > > Signed-off-by: ionnss <zara.leonardo@gmail.com> > --- > contrib/libgit-rs/src/config.rs | 24 ++++++++++++++++++++++++ > contrib/libgit-rs/testdata/config3 | 2 ++ > 2 files changed, 26 insertions(+) > > diff --git a/contrib/libgit-rs/src/config.rs b/contrib/libgit-rs/src/config.rs > index 6bf04845c8..3f4a32c72d 100644 > --- a/contrib/libgit-rs/src/config.rs > +++ b/contrib/libgit-rs/src/config.rs > @@ -68,6 +68,26 @@ impl ConfigSet { > Some(owned_str) > } > } > + > + pub fn get_bool(&mut self, key: &str) -> Option<bool> { > + let key = CString::new(key).expect("Couldn't convert key to CString"); The string argument for `.expect` should be phrased in a more positive manner in terms of what is expected, since failure will cause a panic. So, something like: let key = CString::new(key).expect("boolean key should be valid CString"); which would produce, e.g., panic: boolean key should be valid CString: ... details of key ... A similar rule applies to the later `.expect`. Chris ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] libgit-rs: add get_bool() method to ConfigSet 2025-09-25 11:44 ` [PATCH 2/2] libgit-rs: add get_bool() method to ConfigSet ionnss via GitGitGadget 2025-09-26 6:43 ` Chris Torek @ 2025-09-26 9:58 ` Phillip Wood 2025-09-26 17:15 ` Junio C Hamano 1 sibling, 1 reply; 22+ messages in thread From: Phillip Wood @ 2025-09-26 9:58 UTC (permalink / raw) To: ionnss via GitGitGadget, git; +Cc: ions, Josh Steadmon On 25/09/2025 12:44, ionnss via GitGitGadget wrote: > From: ionnss <zara.leonardo@gmail.com> > > Add support for parsing boolean configuration values in the Rust > ConfigSet API. The method follows Git's standard boolean parsing > rules, accepting true/yes/on/1 as true and false/no/off/0 as false. > > The implementation reuses the existing get_string() infrastructure > and adds case-insensitive boolean parsing logic. It's nice to know that someone is using the rust bindings. The code in contrib/libgit-rs is intended to be safe wrappers around the unsafe functions in contrib/libgit-sys which wrap git's C code. I think what we need to do here is add a binding for git_configset_get_bool() to libgit-sys and then wrap that in libgit-rs. We don't want to start implementing the parsing separately as they'll inevitably end up behaving differently to git. For example what you have here parses "00" or "100" differently to git. Thanks Phillip > Signed-off-by: ionnss <zara.leonardo@gmail.com> > --- > contrib/libgit-rs/src/config.rs | 24 ++++++++++++++++++++++++ > contrib/libgit-rs/testdata/config3 | 2 ++ > 2 files changed, 26 insertions(+) > > diff --git a/contrib/libgit-rs/src/config.rs b/contrib/libgit-rs/src/config.rs > index 6bf04845c8..3f4a32c72d 100644 > --- a/contrib/libgit-rs/src/config.rs > +++ b/contrib/libgit-rs/src/config.rs > @@ -68,6 +68,26 @@ impl ConfigSet { > Some(owned_str) > } > } > + > + pub fn get_bool(&mut self, key: &str) -> Option<bool> { > + 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 > + match owned_str.to_lowercase().as_str() { > + "true" | "yes" | "on" | "1" => Some(true), > + "false" | "no" | "off" | "0" => Some(false), > + _ => None, > + } > + } > + } > } > > impl Default for ConfigSet { > @@ -102,5 +122,9 @@ mod tests { > assert_eq!(cs.get_int("trace2.eventNesting"), Some(3)); > // ConfigSet returns None for missing key > assert_eq!(cs.get_string("foo.bar"), None); > + // Test boolean parsing > + assert_eq!(cs.get_bool("test.booleanValue"), Some(true)); > + // Test missing boolean key > + assert_eq!(cs.get_bool("missing.boolean"), None); > } > } > diff --git a/contrib/libgit-rs/testdata/config3 b/contrib/libgit-rs/testdata/config3 > index ca7b9a7c38..83a474ccef 100644 > --- a/contrib/libgit-rs/testdata/config3 > +++ b/contrib/libgit-rs/testdata/config3 > @@ -1,2 +1,4 @@ > [trace2] > eventNesting = 3 > +[test] > + booleanValue = true ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] libgit-rs: add get_bool() method to ConfigSet 2025-09-26 9:58 ` Phillip Wood @ 2025-09-26 17:15 ` Junio C Hamano 0 siblings, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2025-09-26 17:15 UTC (permalink / raw) To: Phillip Wood; +Cc: ionnss via GitGitGadget, git, ions, Josh Steadmon Phillip Wood <phillip.wood123@gmail.com> writes: > On 25/09/2025 12:44, ionnss via GitGitGadget wrote: >> From: ionnss <zara.leonardo@gmail.com> >> Add support for parsing boolean configuration values in the Rust >> ConfigSet API. The method follows Git's standard boolean parsing >> rules, accepting true/yes/on/1 as true and false/no/off/0 as false. >> The implementation reuses the existing get_string() infrastructure >> and adds case-insensitive boolean parsing logic. > > It's nice to know that someone is using the rust bindings. The code in > contrib/libgit-rs is intended to be safe wrappers around the unsafe > functions in contrib/libgit-sys which wrap git's C code. I think what > we need to do here is add a binding for git_configset_get_bool() to > libgit-sys and then wrap that in libgit-rs. We don't want to start > implementing the parsing separately as they'll inevitably end up > behaving differently to git. For example what you have here parses > "00" or "100" differently to git. > > Thanks Thanks for paying attention to an important detail on the design criteria. As this is a binding to allow Rust programs to access the feature implemented and offered by Git, I fully agree that reimplementing what Git does in an incompatible way in Rust would not help the targetted intended audiences at all (that is better off done in a project that aims to reimplement what Git does, like gitoxide, targetting native Rust solution). If I were not paying attention, I would very likely have missed the misparses of number-as-bool, distracted and blinded by the prettier and nicer syntax the language offers over the original in C X-<. Thanks. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 0/3] libgit-rs: add get_bool() method to ConfigSet 2025-09-25 11:44 [PATCH 0/2] libgit-rs: add get_bool() method to ConfigSet ions via GitGitGadget 2025-09-25 11:44 ` [PATCH 1/2] po: fix escaped underscores in README.md ionnss via GitGitGadget 2025-09-25 11:44 ` [PATCH 2/2] libgit-rs: add get_bool() method to ConfigSet ionnss via GitGitGadget @ 2025-09-27 0:07 ` ions via GitGitGadget 2025-09-27 0:07 ` [PATCH v2 1/3] po: fix escaped underscores in README.md ionnss via GitGitGadget ` (4 more replies) 2 siblings, 5 replies; 22+ messages in thread From: ions via GitGitGadget @ 2025-09-27 0:07 UTC (permalink / raw) To: git; +Cc: Chris Torek, Phillip Wood, ions Purpose This pull request introduces a get_bool() method to the ConfigSet module in the libgit-rs library. The goal is to enhance the functionality of ConfigSet by providing a way to fetch and handle boolean configuration values more easily and consistently. Implementation Details • Added a get_bool() method to the ConfigSet module. • The method retrieves configuration values as boolean values, ensuring proper parsing and error handling. • This addition simplifies the process of working with boolean configurations for developers using the ConfigSet module. Testing • Added unit tests to verify the correctness of the get_bool() method. • Tested edge cases to ensure robustness. ionnss (3): po: fix escaped underscores in README.md libgit-rs: add get_bool() method to ConfigSet libgit-rs: address review feedback for get_bool() contrib/libgit-rs/src/config.rs | 27 +++++++++++++++++++++++++++ contrib/libgit-rs/testdata/config3 | 2 +- contrib/libgit-rs/testdata/config4 | 10 ++++++++++ contrib/libgit-sys/src/lib.rs | 6 ++++++ po/README.md | 6 +++--- 5 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 contrib/libgit-rs/testdata/config4 base-commit: bb69721404348ea2db0a081c41ab6ebfe75bdec8 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1977%2Fionnss%2Fadd-rust-configset-get-bool-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1977/ionnss/add-rust-configset-get-bool-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1977 Range-diff vs v1: 1: d7810781fc = 1: d7810781fc po: fix escaped underscores in README.md 2: a5904a2ac0 = 2: a5904a2ac0 libgit-rs: add get_bool() method to ConfigSet -: ---------- > 3: 43784e3ff9 libgit-rs: address review feedback for get_bool() -- gitgitgadget ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/3] po: fix escaped underscores in README.md 2025-09-27 0:07 ` [PATCH v2 0/3] " ions via GitGitGadget @ 2025-09-27 0:07 ` ionnss via GitGitGadget 2025-09-27 0:07 ` [PATCH v2 2/3] libgit-rs: add get_bool() method to ConfigSet ionnss via GitGitGadget ` (3 subsequent siblings) 4 siblings, 0 replies; 22+ messages in thread From: ionnss via GitGitGadget @ 2025-09-27 0:07 UTC (permalink / raw) To: git; +Cc: Chris Torek, Phillip Wood, ions, ionnss From: ionnss <zara.leonardo@gmail.com> Remove unnecessary backslashes from language code examples. The underscores in "ll\_CC" and "zh\_CN" don't need escaping in Markdown. Signed-off-by: ionnss <zara.leonardo@gmail.com> --- po/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/po/README.md b/po/README.md index ec08aa24ad..d7757bed4e 100644 --- a/po/README.md +++ b/po/README.md @@ -13,9 +13,9 @@ We will use XX as an alias to refer to the language translation code in the following paragraphs, for example we use "po/XX.po" to refer to the translation file for a specific language. But this doesn't mean that the language code has only two letters. The language code can be in one -of two forms: "ll" or "ll\_CC". Here "ll" is the ISO 639 two-letter +of two forms: "ll" or "ll_CC". Here "ll" is the ISO 639 two-letter language code and "CC" is the ISO 3166 two-letter code for country names -and subdivisions. For example: "de" for German language code, "zh\_CN" +and subdivisions. For example: "de" for German language code, "zh_CN" for Simplified Chinese language code. @@ -126,7 +126,7 @@ you add a translation for the first time by running: make po-init PO_FILE=po/XX.po ``` -where XX is the locale, e.g. "de", "is", "pt\_BR", "zh\_CN", etc. +where XX is the locale, e.g. "de", "is", "pt_BR", "zh_CN", etc. The newly generated message file "po/XX.po" is based on the core pot file "po/git-core.pot", so it contains only a minimal set of messages -- gitgitgadget ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 2/3] libgit-rs: add get_bool() method to ConfigSet 2025-09-27 0:07 ` [PATCH v2 0/3] " ions via GitGitGadget 2025-09-27 0:07 ` [PATCH v2 1/3] po: fix escaped underscores in README.md ionnss via GitGitGadget @ 2025-09-27 0:07 ` ionnss via GitGitGadget 2025-09-27 0:07 ` [PATCH v2 3/3] libgit-rs: address review feedback for get_bool() ionnss via GitGitGadget ` (2 subsequent siblings) 4 siblings, 0 replies; 22+ messages in thread From: ionnss via GitGitGadget @ 2025-09-27 0:07 UTC (permalink / raw) To: git; +Cc: Chris Torek, Phillip Wood, ions, ionnss From: ionnss <zara.leonardo@gmail.com> Add support for parsing boolean configuration values in the Rust ConfigSet API. The method follows Git's standard boolean parsing rules, accepting true/yes/on/1 as true and false/no/off/0 as false. The implementation reuses the existing get_string() infrastructure and adds case-insensitive boolean parsing logic. Signed-off-by: ionnss <zara.leonardo@gmail.com> --- contrib/libgit-rs/src/config.rs | 24 ++++++++++++++++++++++++ contrib/libgit-rs/testdata/config3 | 2 ++ 2 files changed, 26 insertions(+) diff --git a/contrib/libgit-rs/src/config.rs b/contrib/libgit-rs/src/config.rs index 6bf04845c8..3f4a32c72d 100644 --- a/contrib/libgit-rs/src/config.rs +++ b/contrib/libgit-rs/src/config.rs @@ -68,6 +68,26 @@ impl ConfigSet { Some(owned_str) } } + + pub fn get_bool(&mut self, key: &str) -> Option<bool> { + 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 + match owned_str.to_lowercase().as_str() { + "true" | "yes" | "on" | "1" => Some(true), + "false" | "no" | "off" | "0" => Some(false), + _ => None, + } + } + } } impl Default for ConfigSet { @@ -102,5 +122,9 @@ mod tests { assert_eq!(cs.get_int("trace2.eventNesting"), Some(3)); // ConfigSet returns None for missing key assert_eq!(cs.get_string("foo.bar"), None); + // Test boolean parsing + assert_eq!(cs.get_bool("test.booleanValue"), Some(true)); + // Test missing boolean key + assert_eq!(cs.get_bool("missing.boolean"), None); } } diff --git a/contrib/libgit-rs/testdata/config3 b/contrib/libgit-rs/testdata/config3 index ca7b9a7c38..83a474ccef 100644 --- a/contrib/libgit-rs/testdata/config3 +++ b/contrib/libgit-rs/testdata/config3 @@ -1,2 +1,4 @@ [trace2] eventNesting = 3 +[test] + booleanValue = true -- gitgitgadget ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 3/3] libgit-rs: address review feedback for get_bool() 2025-09-27 0:07 ` [PATCH v2 0/3] " ions via GitGitGadget 2025-09-27 0:07 ` [PATCH v2 1/3] po: fix escaped underscores in README.md ionnss via GitGitGadget 2025-09-27 0:07 ` [PATCH v2 2/3] libgit-rs: add get_bool() method to ConfigSet ionnss via GitGitGadget @ 2025-09-27 0:07 ` ionnss via GitGitGadget 2025-09-27 2:01 ` [PATCH v2 0/3] libgit-rs: add get_bool() method to ConfigSet Junio C Hamano 2025-09-27 3:51 ` [PATCH v3 " ions via GitGitGadget 4 siblings, 0 replies; 22+ messages in thread From: ionnss via GitGitGadget @ 2025-09-27 0:07 UTC (permalink / raw) To: git; +Cc: Chris Torek, Phillip Wood, ions, ionnss From: ionnss <zara.leonardo@gmail.com> - Use git_configset_get_bool() C function instead of reimplementing parsing - Fix libgit_configset_get_bool() function signature in bindings - Improve .expect() error messages to be more descriptive - Add comprehensive boolean tests including edge cases (00, 100, 007) This addresses feedback from Phillip Wood and Chris Torek about using Git's actual boolean parsing logic rather than duplicating it in Rust. Signed-off-by: ionnss <zara.leonardo@gmail.com> --- contrib/libgit-rs/src/config.rs | 33 ++++++++++++++++-------------- contrib/libgit-rs/testdata/config3 | 4 +--- contrib/libgit-rs/testdata/config4 | 10 +++++++++ contrib/libgit-sys/src/lib.rs | 6 ++++++ 4 files changed, 35 insertions(+), 18 deletions(-) create mode 100644 contrib/libgit-rs/testdata/config4 diff --git a/contrib/libgit-rs/src/config.rs b/contrib/libgit-rs/src/config.rs index 3f4a32c72d..b280b952b2 100644 --- a/contrib/libgit-rs/src/config.rs +++ b/contrib/libgit-rs/src/config.rs @@ -69,24 +69,18 @@ impl ConfigSet { } } + /// Load the value for the given key and attempt to parse it as a boolean. Dies with a fatal error + /// if the value cannot be parsed. Returns None if the key is not present. pub fn get_bool(&mut self, key: &str) -> Option<bool> { - let key = CString::new(key).expect("Couldn't convert key to CString"); - let mut val: *mut c_char = std::ptr::null_mut(); + let key = CString::new(key).expect("config key should be valid CString"); + let mut val: c_int = 0; unsafe { - if libgit_configset_get_string(self.0, key.as_ptr(), &mut val as *mut *mut c_char) != 0 - { + if libgit_configset_get_bool(self.0, key.as_ptr(), &mut val as *mut c_int) != 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 - match owned_str.to_lowercase().as_str() { - "true" | "yes" | "on" | "1" => Some(true), - "false" | "no" | "off" | "0" => Some(false), - _ => None, - } } + + Some(val != 0) } } @@ -115,6 +109,7 @@ mod tests { Path::new("testdata/config1"), Path::new("testdata/config2"), Path::new("testdata/config3"), + Path::new("testdata/config4"), ]); // ConfigSet retrieves correct value assert_eq!(cs.get_int("trace2.eventTarget"), Some(1)); @@ -122,8 +117,16 @@ mod tests { assert_eq!(cs.get_int("trace2.eventNesting"), Some(3)); // ConfigSet returns None for missing key assert_eq!(cs.get_string("foo.bar"), None); - // Test boolean parsing - assert_eq!(cs.get_bool("test.booleanValue"), Some(true)); + // Test boolean parsing - comprehensive tests + assert_eq!(cs.get_bool("test.boolTrue"), Some(true)); + assert_eq!(cs.get_bool("test.boolFalse"), Some(false)); + assert_eq!(cs.get_bool("test.boolYes"), Some(true)); + assert_eq!(cs.get_bool("test.boolNo"), Some(false)); + assert_eq!(cs.get_bool("test.boolOne"), Some(true)); + assert_eq!(cs.get_bool("test.boolZero"), Some(false)); + assert_eq!(cs.get_bool("test.boolZeroZero"), Some(false)); // "00" → false + assert_eq!(cs.get_bool("test.boolHundred"), Some(true)); // "100" → true + assert_eq!(cs.get_bool("test.boolSeven"), Some(true)); // "007" → true // Test missing boolean key assert_eq!(cs.get_bool("missing.boolean"), None); } diff --git a/contrib/libgit-rs/testdata/config3 b/contrib/libgit-rs/testdata/config3 index 83a474ccef..3ea5b96f12 100644 --- a/contrib/libgit-rs/testdata/config3 +++ b/contrib/libgit-rs/testdata/config3 @@ -1,4 +1,2 @@ [trace2] - eventNesting = 3 -[test] - booleanValue = true + eventNesting = 3 \ No newline at end of file diff --git a/contrib/libgit-rs/testdata/config4 b/contrib/libgit-rs/testdata/config4 new file mode 100644 index 0000000000..5b75385c38 --- /dev/null +++ b/contrib/libgit-rs/testdata/config4 @@ -0,0 +1,10 @@ +[test] + boolTrue = true + boolFalse = false + boolYes = yes + boolNo = no + boolOne = 1 + boolZero = 0 + boolZeroZero = 00 + boolHundred = 100 + boolSeven = 007 diff --git a/contrib/libgit-sys/src/lib.rs b/contrib/libgit-sys/src/lib.rs index 4bfc650450..b104fda8f6 100644 --- a/contrib/libgit-sys/src/lib.rs +++ b/contrib/libgit-sys/src/lib.rs @@ -43,6 +43,12 @@ extern "C" { dest: *mut *mut c_char, ) -> c_int; + pub fn libgit_configset_get_bool( + cs: *mut libgit_config_set, + key: *const c_char, + dest: *mut c_int, + ) -> c_int; + } #[cfg(test)] -- gitgitgadget ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2 0/3] libgit-rs: add get_bool() method to ConfigSet 2025-09-27 0:07 ` [PATCH v2 0/3] " ions via GitGitGadget ` (2 preceding siblings ...) 2025-09-27 0:07 ` [PATCH v2 3/3] libgit-rs: address review feedback for get_bool() ionnss via GitGitGadget @ 2025-09-27 2:01 ` Junio C Hamano 2025-09-27 3:51 ` [PATCH v3 " ions via GitGitGadget 4 siblings, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2025-09-27 2:01 UTC (permalink / raw) To: ions via GitGitGadget; +Cc: git, Chris Torek, Phillip Wood, ions "ions via GitGitGadget" <gitgitgadget@gmail.com> writes: > Purpose > > This pull request introduces a get_bool() method to the ConfigSet module in > the libgit-rs library. The goal is to enhance the functionality of ConfigSet > by providing a way to fetch and handle boolean configuration values more > easily and consistently. > > Implementation Details > > • Added a get_bool() method to the ConfigSet module. > > • The method retrieves configuration values as boolean values, ensuring > proper parsing and error handling. > > • This addition simplifies the process of working with boolean > configurations for developers using the ConfigSet module. > > Testing > > • Added unit tests to verify the correctness of the get_bool() method. > > • Tested edge cases to ensure robustness. Somebody may suggest a better way to organize your cover letter for second and subsequent iterations, but it is helpful to returning readers to list what changed since the previous iteration. > ionnss (3): > po: fix escaped underscores in README.md > libgit-rs: add get_bool() method to ConfigSet > libgit-rs: address review feedback for get_bool() I am not sure if you really wanted to make this a thre-patch series. The README.md patch looks pretty much independent and irrelevant to the Rust topic to me. I am not a GGG user, but there must be a way to tell it to send out a two-patch series from this branch. > base-commit: bb69721404348ea2db0a081c41ab6ebfe75bdec8 Perhaps you'd need to rebase the top two commit on top of this bb697214 (The twelfth batch, 2025-09-23). Currently your two commits that belong to this libgit-rs series are not built on top of that commit in the mainline. They are built on top of d7810781fc, as can be seen below. > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1977%2Fionnss%2Fadd-rust-configset-get-bool-v2 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1977/ionnss/add-rust-configset-get-bool-v2 > Pull-Request: https://github.com/gitgitgadget/git/pull/1977 > > Range-diff vs v1: > > 1: d7810781fc = 1: d7810781fc po: fix escaped underscores in README.md > 2: a5904a2ac0 = 2: a5904a2ac0 libgit-rs: add get_bool() method to ConfigSet > -: ---------- > 3: 43784e3ff9 libgit-rs: address review feedback for get_bool() Actually, I do not think you want a two-patch series, either. Review comments (sorry, I do not recall the details) must have pointed out what was wrong in the initial iteration, but this iteration is carrying the same badness without correcting anyting in a5904a2ac0 and resending it with "oops that was wrong in the previous step, and here is a band-aid on top" commit that is the new 43784e3ff9. Until there is a concensus that your topic is ready and gets merged to 'next', you have the luxury to pretend as if you are a perfect developer who never made such mistakes that can easily be pointed out on the mailing list by others. In short, anything in 43784e3ff9 that removes lines that were added in a5904a2ac0 and replaces them with something else are unwanted "I went there, it was a bad idea, here I am correcting the earlier mistake" changes. And the way to pretend that you are a perfect developer is to rewrite (i.e. "commit --amend") your "add get_bool()" with the new strategy to wrap the C version used by real Git, instead of reimplementing it manually in Rust. This is a chance you have to pretend that you weren't even tempted to reimplement the thing in Rust and went straight to the right solution. So grab that chance. Once your topic gets merged to 'next', you no longer have that opportunity to wholesale replace your patches to pretend that you never made mistakes. This is because nobody reading "git log -p" later is interested in your (or anybody's) mistakes that have already been corrected. It might record your cherished memory to celebrate your growth as a developer, but that is not something the history of a public project is used for. After your topic is merged to 'next', we'd go incremental. This is because being in 'next' means that enough reviewers agreed that a particular iteration is good enough. If later it was found that they are not good enough and need corrections, such a mistake that nobody managed to spot during the review becomes worth recording and its correction worth describing as a separate patch on top. But we are not there yet with this series. Thanks. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 0/3] libgit-rs: add get_bool() method to ConfigSet 2025-09-27 0:07 ` [PATCH v2 0/3] " ions via GitGitGadget ` (3 preceding siblings ...) 2025-09-27 2:01 ` [PATCH v2 0/3] libgit-rs: add get_bool() method to ConfigSet Junio C Hamano @ 2025-09-27 3:51 ` ions via GitGitGadget 2025-09-27 3:51 ` [PATCH v3 1/3] po: fix escaped underscores in README.md ionnss via GitGitGadget ` (3 more replies) 4 siblings, 4 replies; 22+ messages in thread From: ions via GitGitGadget @ 2025-09-27 3:51 UTC (permalink / raw) To: git; +Cc: Chris Torek, Phillip Wood, ions Purpose This pull request introduces a get_bool() method to the ConfigSet module in the libgit-rs library. The goal is to enhance the functionality of ConfigSet by providing a way to fetch and handle boolean configuration values more easily and consistently. Implementation Details • Added a get_bool() method to the ConfigSet module. • The method retrieves configuration values as boolean values, ensuring proper parsing and error handling. • This addition simplifies the process of working with boolean configurations for developers using the ConfigSet module. Testing • Added unit tests to verify the correctness of the get_bool() method. • Tested edge cases to ensure robustness. ionnss (3): po: fix escaped underscores in README.md libgit-rs: add get_bool() method to ConfigSet libgit-rs: add get_ulong() and get_pathname() methods contrib/libgit-rs/src/config.rs | 83 +++++++++++++++++++++++++++++- contrib/libgit-rs/testdata/config3 | 2 +- contrib/libgit-rs/testdata/config4 | 13 +++++ contrib/libgit-sys/src/lib.rs | 24 ++++++++- po/README.md | 6 +-- 5 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 contrib/libgit-rs/testdata/config4 base-commit: bb69721404348ea2db0a081c41ab6ebfe75bdec8 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1977%2Fionnss%2Fadd-rust-configset-get-bool-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1977/ionnss/add-rust-configset-get-bool-v3 Pull-Request: https://github.com/gitgitgadget/git/pull/1977 Range-diff vs v2: 1: d7810781fc = 1: d7810781fc po: fix escaped underscores in README.md 3: 43784e3ff9 ! 2: 479c263bc1 libgit-rs: address review feedback for get_bool() @@ Metadata Author: ionnss <zara.leonardo@gmail.com> ## Commit message ## - libgit-rs: address review feedback for get_bool() + libgit-rs: add get_bool() method to ConfigSet - - Use git_configset_get_bool() C function instead of reimplementing parsing - - Fix libgit_configset_get_bool() function signature in bindings - - Improve .expect() error messages to be more descriptive - - Add comprehensive boolean tests including edge cases (00, 100, 007) + Add support for parsing boolean configuration values using Git's + git_configset_get_bool() C function. This ensures consistent behavior + with Git's native boolean parsing logic. - This addresses feedback from Phillip Wood and Chris Torek about using - Git's actual boolean parsing logic rather than duplicating it in Rust. + The method handles all Git boolean formats (true/false, yes/no, on/off, + 1/0) and edge cases like "00" and "100" correctly. + + Includes comprehensive tests for various boolean formats and edge cases. Signed-off-by: ionnss <zara.leonardo@gmail.com> ## contrib/libgit-rs/src/config.rs ## @@ contrib/libgit-rs/src/config.rs: impl ConfigSet { + Some(owned_str) } } - ++ + /// Load the value for the given key and attempt to parse it as a boolean. Dies with a fatal error + /// if the value cannot be parsed. Returns None if the key is not present. - pub fn get_bool(&mut self, key: &str) -> Option<bool> { -- let key = CString::new(key).expect("Couldn't convert key to CString"); -- let mut val: *mut c_char = std::ptr::null_mut(); ++ pub fn get_bool(&mut self, key: &str) -> Option<bool> { + let key = CString::new(key).expect("config key should be valid CString"); + let mut val: c_int = 0; - unsafe { -- if libgit_configset_get_string(self.0, key.as_ptr(), &mut val as *mut *mut c_char) != 0 -- { ++ unsafe { + if libgit_configset_get_bool(self.0, key.as_ptr(), &mut val as *mut c_int) != 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 -- match owned_str.to_lowercase().as_str() { -- "true" | "yes" | "on" | "1" => Some(true), -- "false" | "no" | "off" | "0" => Some(false), -- _ => None, -- } - } ++ return None; ++ } ++ } + + Some(val != 0) - } ++ } } + impl Default for ConfigSet { @@ contrib/libgit-rs/src/config.rs: mod tests { Path::new("testdata/config1"), Path::new("testdata/config2"), @@ contrib/libgit-rs/src/config.rs: mod tests { assert_eq!(cs.get_int("trace2.eventNesting"), Some(3)); // ConfigSet returns None for missing key assert_eq!(cs.get_string("foo.bar"), None); -- // Test boolean parsing -- assert_eq!(cs.get_bool("test.booleanValue"), Some(true)); + // Test boolean parsing - comprehensive tests + assert_eq!(cs.get_bool("test.boolTrue"), Some(true)); + assert_eq!(cs.get_bool("test.boolFalse"), Some(false)); @@ contrib/libgit-rs/src/config.rs: mod tests { + assert_eq!(cs.get_bool("test.boolZero"), Some(false)); + assert_eq!(cs.get_bool("test.boolZeroZero"), Some(false)); // "00" → false + assert_eq!(cs.get_bool("test.boolHundred"), Some(true)); // "100" → true -+ assert_eq!(cs.get_bool("test.boolSeven"), Some(true)); // "007" → true - // Test missing boolean key - assert_eq!(cs.get_bool("missing.boolean"), None); ++ // Test missing boolean key ++ assert_eq!(cs.get_bool("missing.boolean"), None); } + } ## contrib/libgit-rs/testdata/config3 ## @@ [trace2] - eventNesting = 3 --[test] -- booleanValue = true + eventNesting = 3 \ No newline at end of file @@ contrib/libgit-rs/testdata/config4 (new) + boolZero = 0 + boolZeroZero = 00 + boolHundred = 100 -+ boolSeven = 007 ## contrib/libgit-sys/src/lib.rs ## @@ contrib/libgit-sys/src/lib.rs: extern "C" { 2: a5904a2ac0 ! 3: 1ac8d76819 libgit-rs: add get_bool() method to ConfigSet @@ Metadata Author: ionnss <zara.leonardo@gmail.com> ## Commit message ## - libgit-rs: add get_bool() method to ConfigSet + libgit-rs: add get_ulong() and get_pathname() methods - Add support for parsing boolean configuration values in the Rust - ConfigSet API. The method follows Git's standard boolean parsing - rules, accepting true/yes/on/1 as true and false/no/off/0 as false. + Expand the ConfigSet API with additional configuration value types: - The implementation reuses the existing get_string() infrastructure - and adds case-insensitive boolean parsing logic. + - get_ulong(): Parse unsigned long integers for large numeric values + - get_pathname(): Parse file paths, returning PathBuf for type safety + + Both functions follow the same pattern as existing get_* methods, + using Git's C functions for consistent parsing behavior. + + Add comprehensive tests covering normal cases, edge cases, and + error handling for all new functionality. Signed-off-by: ionnss <zara.leonardo@gmail.com> ## contrib/libgit-rs/src/config.rs ## +@@ + use std::ffi::{c_void, CStr, CString}; +-use std::path::Path; ++use std::path::{Path, PathBuf}; + + #[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)] +@@ contrib/libgit-rs/src/config.rs: type c_char = i8; + #[allow(non_camel_case_types)] + type c_int = i32; + ++#[cfg(not(has_std__ffi__c_char))] ++#[allow(non_camel_case_types)] ++type c_ulong = u64; ++ + use libgit_sys::*; + + /// A ConfigSet is an in-memory cache for config-like files such as `.gitmodules` or `.gitconfig`. @@ contrib/libgit-rs/src/config.rs: impl ConfigSet { - Some(owned_str) - } + + Some(val != 0) } + -+ pub fn get_bool(&mut self, key: &str) -> Option<bool> { -+ let key = CString::new(key).expect("Couldn't convert key to CString"); ++ /// Load the value for the given key and attempt to parse it as an unsigned long. Dies with a fatal error ++ /// if the value cannot be parsed. Returns None if the key is not present. ++ pub fn get_ulong(&mut self, key: &str) -> Option<u64> { ++ let key = CString::new(key).expect("config key should be valid CString"); ++ let mut val: c_ulong = 0; ++ unsafe { ++ if libgit_configset_get_ulong(self.0, key.as_ptr(), &mut val as *mut c_ulong) != 0 { ++ return None; ++ } ++ } ++ Some(val as u64) ++ } ++ ++ /// Load the value for the given key and attempt to parse it as a file path. Dies with a fatal error ++ /// if the value cannot be converted to a PathBuf. Returns None if the key is not present. ++ pub fn get_pathname(&mut self, key: &str) -> Option<PathBuf> { ++ let key = CString::new(key).expect("config key should be valid 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 ++ if libgit_configset_get_pathname(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")); ++ let owned_str = String::from( ++ borrowed_str ++ .to_str() ++ .expect("config path should be valid UTF-8"), ++ ); + free(val as *mut c_void); // Free the xstrdup()ed pointer from the C side -+ match owned_str.to_lowercase().as_str() { -+ "true" | "yes" | "on" | "1" => Some(true), -+ "false" | "no" | "off" | "0" => Some(false), -+ _ => None, -+ } ++ Some(PathBuf::from(owned_str)) + } + } } impl Default for ConfigSet { @@ contrib/libgit-rs/src/config.rs: mod tests { - assert_eq!(cs.get_int("trace2.eventNesting"), Some(3)); - // ConfigSet returns None for missing key - assert_eq!(cs.get_string("foo.bar"), None); -+ // Test boolean parsing -+ assert_eq!(cs.get_bool("test.booleanValue"), Some(true)); -+ // Test missing boolean key -+ assert_eq!(cs.get_bool("missing.boolean"), None); + assert_eq!(cs.get_bool("test.boolHundred"), Some(true)); // "100" → true + // Test missing boolean key + assert_eq!(cs.get_bool("missing.boolean"), None); ++ // Test ulong parsing ++ assert_eq!(cs.get_ulong("test.ulongSmall"), Some(42)); ++ assert_eq!(cs.get_ulong("test.ulongBig"), Some(4294967296)); // > 32-bit int ++ assert_eq!(cs.get_ulong("missing.ulong"), None); ++ // Test pathname parsing ++ assert_eq!( ++ cs.get_pathname("test.pathRelative"), ++ Some(PathBuf::from("./some/path")) ++ ); ++ assert_eq!( ++ cs.get_pathname("test.pathAbsolute"), ++ Some(PathBuf::from("/usr/bin/git")) ++ ); ++ assert_eq!(cs.get_pathname("missing.path"), None); } } - ## contrib/libgit-rs/testdata/config3 ## + ## contrib/libgit-rs/testdata/config4 ## +@@ + boolZero = 0 + boolZeroZero = 00 + boolHundred = 100 ++ ulongSmall = 42 ++ ulongBig = 4294967296 ++ pathRelative = ./some/path ++ pathAbsolute = /usr/bin/git + + ## contrib/libgit-sys/src/lib.rs ## @@ - [trace2] - eventNesting = 3 -+[test] -+ booleanValue = true + 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)] +@@ contrib/libgit-sys/src/lib.rs: 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; ++ + extern crate libz_sys; + + #[allow(non_camel_case_types)] +@@ contrib/libgit-sys/src/lib.rs: extern "C" { + dest: *mut c_int, + ) -> c_int; + ++ pub fn libgit_configset_get_ulong( ++ cs: *mut libgit_config_set, ++ key: *const c_char, ++ dest: *mut c_ulong, ++ ) -> c_int; ++ ++ pub fn libgit_configset_get_pathname( ++ cs: *mut libgit_config_set, ++ key: *const c_char, ++ dest: *mut *mut c_char, ++ ) -> c_int; ++ + } + + #[cfg(test)] -- gitgitgadget ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 1/3] po: fix escaped underscores in README.md 2025-09-27 3:51 ` [PATCH v3 " ions via GitGitGadget @ 2025-09-27 3:51 ` ionnss via GitGitGadget 2025-09-29 13:26 ` Phillip Wood 2025-09-27 3:51 ` [PATCH v3 2/3] libgit-rs: add get_bool() method to ConfigSet ionnss via GitGitGadget ` (2 subsequent siblings) 3 siblings, 1 reply; 22+ messages in thread From: ionnss via GitGitGadget @ 2025-09-27 3:51 UTC (permalink / raw) To: git; +Cc: Chris Torek, Phillip Wood, ions, ionnss From: ionnss <zara.leonardo@gmail.com> Remove unnecessary backslashes from language code examples. The underscores in "ll\_CC" and "zh\_CN" don't need escaping in Markdown. Signed-off-by: ionnss <zara.leonardo@gmail.com> --- po/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/po/README.md b/po/README.md index ec08aa24ad..d7757bed4e 100644 --- a/po/README.md +++ b/po/README.md @@ -13,9 +13,9 @@ We will use XX as an alias to refer to the language translation code in the following paragraphs, for example we use "po/XX.po" to refer to the translation file for a specific language. But this doesn't mean that the language code has only two letters. The language code can be in one -of two forms: "ll" or "ll\_CC". Here "ll" is the ISO 639 two-letter +of two forms: "ll" or "ll_CC". Here "ll" is the ISO 639 two-letter language code and "CC" is the ISO 3166 two-letter code for country names -and subdivisions. For example: "de" for German language code, "zh\_CN" +and subdivisions. For example: "de" for German language code, "zh_CN" for Simplified Chinese language code. @@ -126,7 +126,7 @@ you add a translation for the first time by running: make po-init PO_FILE=po/XX.po ``` -where XX is the locale, e.g. "de", "is", "pt\_BR", "zh\_CN", etc. +where XX is the locale, e.g. "de", "is", "pt_BR", "zh_CN", etc. The newly generated message file "po/XX.po" is based on the core pot file "po/git-core.pot", so it contains only a minimal set of messages -- gitgitgadget ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v3 1/3] po: fix escaped underscores in README.md 2025-09-27 3:51 ` [PATCH v3 1/3] po: fix escaped underscores in README.md ionnss via GitGitGadget @ 2025-09-29 13:26 ` Phillip Wood 0 siblings, 0 replies; 22+ messages in thread From: Phillip Wood @ 2025-09-29 13:26 UTC (permalink / raw) To: ionnss via GitGitGadget, git; +Cc: Chris Torek, ions As Junio has already said, you should drop this patch from this series as it is unrelated to the others and submit it separately (which I think you may have done already) Running git rebase --onto HEAD~3 HEAD~2 should do the trick Thanks Phillip On 27/09/2025 04:51, ionnss via GitGitGadget wrote: > From: ionnss <zara.leonardo@gmail.com> > > Remove unnecessary backslashes from language code examples. > The underscores in "ll\_CC" and "zh\_CN" don't need escaping > in Markdown. > > Signed-off-by: ionnss <zara.leonardo@gmail.com> > --- > po/README.md | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/po/README.md b/po/README.md > index ec08aa24ad..d7757bed4e 100644 > --- a/po/README.md > +++ b/po/README.md > @@ -13,9 +13,9 @@ We will use XX as an alias to refer to the language translation code in > the following paragraphs, for example we use "po/XX.po" to refer to the > translation file for a specific language. But this doesn't mean that > the language code has only two letters. The language code can be in one > -of two forms: "ll" or "ll\_CC". Here "ll" is the ISO 639 two-letter > +of two forms: "ll" or "ll_CC". Here "ll" is the ISO 639 two-letter > language code and "CC" is the ISO 3166 two-letter code for country names > -and subdivisions. For example: "de" for German language code, "zh\_CN" > +and subdivisions. For example: "de" for German language code, "zh_CN" > for Simplified Chinese language code. > > > @@ -126,7 +126,7 @@ you add a translation for the first time by running: > make po-init PO_FILE=po/XX.po > ``` > > -where XX is the locale, e.g. "de", "is", "pt\_BR", "zh\_CN", etc. > +where XX is the locale, e.g. "de", "is", "pt_BR", "zh_CN", etc. > > The newly generated message file "po/XX.po" is based on the core pot > file "po/git-core.pot", so it contains only a minimal set of messages ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 2/3] libgit-rs: add get_bool() method to ConfigSet 2025-09-27 3:51 ` [PATCH v3 " ions via GitGitGadget 2025-09-27 3:51 ` [PATCH v3 1/3] po: fix escaped underscores in README.md ionnss via GitGitGadget @ 2025-09-27 3:51 ` ionnss via GitGitGadget 2025-09-29 13:23 ` Phillip Wood 2025-09-27 3:51 ` [PATCH v3 3/3] libgit-rs: add get_ulong() and get_pathname() methods ionnss via GitGitGadget 2025-09-30 8:46 ` [PATCH v4] libgit-rs: add get_bool(), get_ulong(), " ions via GitGitGadget 3 siblings, 1 reply; 22+ messages in thread From: ionnss via GitGitGadget @ 2025-09-27 3:51 UTC (permalink / raw) To: git; +Cc: Chris Torek, Phillip Wood, ions, ionnss From: ionnss <zara.leonardo@gmail.com> Add support for parsing boolean configuration values using Git's git_configset_get_bool() C function. This ensures consistent behavior with Git's native boolean parsing logic. The method handles all Git boolean formats (true/false, yes/no, on/off, 1/0) and edge cases like "00" and "100" correctly. Includes comprehensive tests for various boolean formats and edge cases. Signed-off-by: ionnss <zara.leonardo@gmail.com> --- contrib/libgit-rs/src/config.rs | 26 ++++++++++++++++++++++++++ contrib/libgit-rs/testdata/config3 | 2 +- contrib/libgit-rs/testdata/config4 | 9 +++++++++ contrib/libgit-sys/src/lib.rs | 6 ++++++ 4 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 contrib/libgit-rs/testdata/config4 diff --git a/contrib/libgit-rs/src/config.rs b/contrib/libgit-rs/src/config.rs index 6bf04845c8..72ee88801b 100644 --- a/contrib/libgit-rs/src/config.rs +++ b/contrib/libgit-rs/src/config.rs @@ -68,6 +68,20 @@ impl ConfigSet { Some(owned_str) } } + + /// Load the value for the given key and attempt to parse it as a boolean. Dies with a fatal error + /// if the value cannot be parsed. Returns None if the key is not present. + pub fn get_bool(&mut self, key: &str) -> Option<bool> { + let key = CString::new(key).expect("config key should be valid CString"); + let mut val: c_int = 0; + unsafe { + if libgit_configset_get_bool(self.0, key.as_ptr(), &mut val as *mut c_int) != 0 { + return None; + } + } + + Some(val != 0) + } } impl Default for ConfigSet { @@ -95,6 +109,7 @@ mod tests { Path::new("testdata/config1"), Path::new("testdata/config2"), Path::new("testdata/config3"), + Path::new("testdata/config4"), ]); // ConfigSet retrieves correct value assert_eq!(cs.get_int("trace2.eventTarget"), Some(1)); @@ -102,5 +117,16 @@ mod tests { assert_eq!(cs.get_int("trace2.eventNesting"), Some(3)); // ConfigSet returns None for missing key assert_eq!(cs.get_string("foo.bar"), None); + // Test boolean parsing - comprehensive tests + assert_eq!(cs.get_bool("test.boolTrue"), Some(true)); + assert_eq!(cs.get_bool("test.boolFalse"), Some(false)); + assert_eq!(cs.get_bool("test.boolYes"), Some(true)); + assert_eq!(cs.get_bool("test.boolNo"), Some(false)); + assert_eq!(cs.get_bool("test.boolOne"), Some(true)); + assert_eq!(cs.get_bool("test.boolZero"), Some(false)); + assert_eq!(cs.get_bool("test.boolZeroZero"), Some(false)); // "00" → false + assert_eq!(cs.get_bool("test.boolHundred"), Some(true)); // "100" → true + // Test missing boolean key + assert_eq!(cs.get_bool("missing.boolean"), None); } } diff --git a/contrib/libgit-rs/testdata/config3 b/contrib/libgit-rs/testdata/config3 index ca7b9a7c38..3ea5b96f12 100644 --- a/contrib/libgit-rs/testdata/config3 +++ b/contrib/libgit-rs/testdata/config3 @@ -1,2 +1,2 @@ [trace2] - eventNesting = 3 + eventNesting = 3 \ No newline at end of file diff --git a/contrib/libgit-rs/testdata/config4 b/contrib/libgit-rs/testdata/config4 new file mode 100644 index 0000000000..c0755a32be --- /dev/null +++ b/contrib/libgit-rs/testdata/config4 @@ -0,0 +1,9 @@ +[test] + boolTrue = true + boolFalse = false + boolYes = yes + boolNo = no + boolOne = 1 + boolZero = 0 + boolZeroZero = 00 + boolHundred = 100 diff --git a/contrib/libgit-sys/src/lib.rs b/contrib/libgit-sys/src/lib.rs index 4bfc650450..b104fda8f6 100644 --- a/contrib/libgit-sys/src/lib.rs +++ b/contrib/libgit-sys/src/lib.rs @@ -43,6 +43,12 @@ extern "C" { dest: *mut *mut c_char, ) -> c_int; + pub fn libgit_configset_get_bool( + cs: *mut libgit_config_set, + key: *const c_char, + dest: *mut c_int, + ) -> c_int; + } #[cfg(test)] -- gitgitgadget ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v3 2/3] libgit-rs: add get_bool() method to ConfigSet 2025-09-27 3:51 ` [PATCH v3 2/3] libgit-rs: add get_bool() method to ConfigSet ionnss via GitGitGadget @ 2025-09-29 13:23 ` Phillip Wood 0 siblings, 0 replies; 22+ messages in thread From: Phillip Wood @ 2025-09-29 13:23 UTC (permalink / raw) To: ionnss via GitGitGadget, git; +Cc: Chris Torek, ions On 27/09/2025 04:51, ionnss via GitGitGadget wrote: > From: ionnss <zara.leonardo@gmail.com> > > Add support for parsing boolean configuration values using Git's > git_configset_get_bool() C function. This ensures consistent behavior > with Git's native boolean parsing logic. > > The method handles all Git boolean formats (true/false, yes/no, on/off, > 1/0) and edge cases like "00" and "100" correctly. > > Includes comprehensive tests for various boolean formats and edge cases. Unfortunately when I try to run those tests with make INCLUDE_LIBGIT_RS=1 && cd contrib/libgit-rs && cargo test I see = note: /usr/bin/ld: /home/phil/src/git/update-ref-date/contrib/libgit-rs/target/debug/deps/libgit-8c021564b9d1ec26.9j3smxv8a5f6u8czhaxq401mn.rcgu.o: in function `libgit::config::ConfigSet::get_bool': /home/phil/src/git/update-ref-date/contrib/libgit-rs/src/config.rs:78: undefined reference to `libgit_configset_get_bool' collect2: error: ld returned 1 exit status This happens because libgit_configset_get_bool() is not defined in contrib/libgit-sys/public_symbol_export.[ch]. You need to wrap configset_get_bool() in the some way that configset_get_int() is. Thanks Phillip > Signed-off-by: ionnss <zara.leonardo@gmail.com> > --- > contrib/libgit-rs/src/config.rs | 26 ++++++++++++++++++++++++++ > contrib/libgit-rs/testdata/config3 | 2 +- > contrib/libgit-rs/testdata/config4 | 9 +++++++++ > contrib/libgit-sys/src/lib.rs | 6 ++++++ > 4 files changed, 42 insertions(+), 1 deletion(-) > create mode 100644 contrib/libgit-rs/testdata/config4 > > diff --git a/contrib/libgit-rs/src/config.rs b/contrib/libgit-rs/src/config.rs > index 6bf04845c8..72ee88801b 100644 > --- a/contrib/libgit-rs/src/config.rs > +++ b/contrib/libgit-rs/src/config.rs > @@ -68,6 +68,20 @@ impl ConfigSet { > Some(owned_str) > } > } > + > + /// Load the value for the given key and attempt to parse it as a boolean. Dies with a fatal error > + /// if the value cannot be parsed. Returns None if the key is not present. > + pub fn get_bool(&mut self, key: &str) -> Option<bool> { > + let key = CString::new(key).expect("config key should be valid CString"); > + let mut val: c_int = 0; > + unsafe { > + if libgit_configset_get_bool(self.0, key.as_ptr(), &mut val as *mut c_int) != 0 { > + return None; > + } > + } > + > + Some(val != 0) > + } > } > > impl Default for ConfigSet { > @@ -95,6 +109,7 @@ mod tests { > Path::new("testdata/config1"), > Path::new("testdata/config2"), > Path::new("testdata/config3"), > + Path::new("testdata/config4"), > ]); > // ConfigSet retrieves correct value > assert_eq!(cs.get_int("trace2.eventTarget"), Some(1)); > @@ -102,5 +117,16 @@ mod tests { > assert_eq!(cs.get_int("trace2.eventNesting"), Some(3)); > // ConfigSet returns None for missing key > assert_eq!(cs.get_string("foo.bar"), None); > + // Test boolean parsing - comprehensive tests > + assert_eq!(cs.get_bool("test.boolTrue"), Some(true)); > + assert_eq!(cs.get_bool("test.boolFalse"), Some(false)); > + assert_eq!(cs.get_bool("test.boolYes"), Some(true)); > + assert_eq!(cs.get_bool("test.boolNo"), Some(false)); > + assert_eq!(cs.get_bool("test.boolOne"), Some(true)); > + assert_eq!(cs.get_bool("test.boolZero"), Some(false)); > + assert_eq!(cs.get_bool("test.boolZeroZero"), Some(false)); // "00" → false > + assert_eq!(cs.get_bool("test.boolHundred"), Some(true)); // "100" → true > + // Test missing boolean key > + assert_eq!(cs.get_bool("missing.boolean"), None); > } > } > diff --git a/contrib/libgit-rs/testdata/config3 b/contrib/libgit-rs/testdata/config3 > index ca7b9a7c38..3ea5b96f12 100644 > --- a/contrib/libgit-rs/testdata/config3 > +++ b/contrib/libgit-rs/testdata/config3 > @@ -1,2 +1,2 @@ > [trace2] > - eventNesting = 3 > + eventNesting = 3 > \ No newline at end of file > diff --git a/contrib/libgit-rs/testdata/config4 b/contrib/libgit-rs/testdata/config4 > new file mode 100644 > index 0000000000..c0755a32be > --- /dev/null > +++ b/contrib/libgit-rs/testdata/config4 > @@ -0,0 +1,9 @@ > +[test] > + boolTrue = true > + boolFalse = false > + boolYes = yes > + boolNo = no > + boolOne = 1 > + boolZero = 0 > + boolZeroZero = 00 > + boolHundred = 100 > diff --git a/contrib/libgit-sys/src/lib.rs b/contrib/libgit-sys/src/lib.rs > index 4bfc650450..b104fda8f6 100644 > --- a/contrib/libgit-sys/src/lib.rs > +++ b/contrib/libgit-sys/src/lib.rs > @@ -43,6 +43,12 @@ extern "C" { > dest: *mut *mut c_char, > ) -> c_int; > > + pub fn libgit_configset_get_bool( > + cs: *mut libgit_config_set, > + key: *const c_char, > + dest: *mut c_int, > + ) -> c_int; > + > } > > #[cfg(test)] ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 3/3] libgit-rs: add get_ulong() and get_pathname() methods 2025-09-27 3:51 ` [PATCH v3 " ions via GitGitGadget 2025-09-27 3:51 ` [PATCH v3 1/3] po: fix escaped underscores in README.md ionnss via GitGitGadget 2025-09-27 3:51 ` [PATCH v3 2/3] libgit-rs: add get_bool() method to ConfigSet ionnss via GitGitGadget @ 2025-09-27 3:51 ` ionnss via GitGitGadget 2025-09-29 13:23 ` Phillip Wood 2025-09-30 8:46 ` [PATCH v4] libgit-rs: add get_bool(), get_ulong(), " ions via GitGitGadget 3 siblings, 1 reply; 22+ messages in thread From: ionnss via GitGitGadget @ 2025-09-27 3:51 UTC (permalink / raw) To: git; +Cc: Chris Torek, Phillip Wood, ions, ionnss From: ionnss <zara.leonardo@gmail.com> Expand the ConfigSet API with additional configuration value types: - get_ulong(): Parse unsigned long integers for large numeric values - get_pathname(): Parse file paths, returning PathBuf for type safety Both functions follow the same pattern as existing get_* methods, using Git's C functions for consistent parsing behavior. Add comprehensive tests covering normal cases, edge cases, and error handling for all new functionality. Signed-off-by: ionnss <zara.leonardo@gmail.com> --- contrib/libgit-rs/src/config.rs | 57 ++++++++++++++++++++++++++++-- contrib/libgit-rs/testdata/config4 | 4 +++ contrib/libgit-sys/src/lib.rs | 18 +++++++++- 3 files changed, 76 insertions(+), 3 deletions(-) diff --git a/contrib/libgit-rs/src/config.rs b/contrib/libgit-rs/src/config.rs index 72ee88801b..ffd9f311b6 100644 --- a/contrib/libgit-rs/src/config.rs +++ b/contrib/libgit-rs/src/config.rs @@ -1,8 +1,8 @@ use std::ffi::{c_void, CStr, CString}; -use std::path::Path; +use std::path::{Path, PathBuf}; #[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)] @@ -12,6 +12,10 @@ type c_char = i8; #[allow(non_camel_case_types)] type c_int = i32; +#[cfg(not(has_std__ffi__c_char))] +#[allow(non_camel_case_types)] +type c_ulong = u64; + use libgit_sys::*; /// A ConfigSet is an in-memory cache for config-like files such as `.gitmodules` or `.gitconfig`. @@ -82,6 +86,41 @@ impl ConfigSet { Some(val != 0) } + + /// Load the value for the given key and attempt to parse it as an unsigned long. Dies with a fatal error + /// if the value cannot be parsed. Returns None if the key is not present. + pub fn get_ulong(&mut self, key: &str) -> Option<u64> { + let key = CString::new(key).expect("config key should be valid CString"); + let mut val: c_ulong = 0; + unsafe { + if libgit_configset_get_ulong(self.0, key.as_ptr(), &mut val as *mut c_ulong) != 0 { + return None; + } + } + Some(val as u64) + } + + /// Load the value for the given key and attempt to parse it as a file path. Dies with a fatal error + /// if the value cannot be converted to a PathBuf. Returns None if the key is not present. + pub fn get_pathname(&mut self, key: &str) -> Option<PathBuf> { + let key = CString::new(key).expect("config key should be valid CString"); + let mut val: *mut c_char = std::ptr::null_mut(); + unsafe { + if libgit_configset_get_pathname(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("config path should be valid UTF-8"), + ); + free(val as *mut c_void); // Free the xstrdup()ed pointer from the C side + Some(PathBuf::from(owned_str)) + } + } } impl Default for ConfigSet { @@ -128,5 +167,19 @@ mod tests { assert_eq!(cs.get_bool("test.boolHundred"), Some(true)); // "100" → true // Test missing boolean key assert_eq!(cs.get_bool("missing.boolean"), None); + // Test ulong parsing + assert_eq!(cs.get_ulong("test.ulongSmall"), Some(42)); + assert_eq!(cs.get_ulong("test.ulongBig"), Some(4294967296)); // > 32-bit int + assert_eq!(cs.get_ulong("missing.ulong"), None); + // Test pathname parsing + assert_eq!( + cs.get_pathname("test.pathRelative"), + Some(PathBuf::from("./some/path")) + ); + assert_eq!( + cs.get_pathname("test.pathAbsolute"), + Some(PathBuf::from("/usr/bin/git")) + ); + assert_eq!(cs.get_pathname("missing.path"), None); } } diff --git a/contrib/libgit-rs/testdata/config4 b/contrib/libgit-rs/testdata/config4 index c0755a32be..bd621ab480 100644 --- a/contrib/libgit-rs/testdata/config4 +++ b/contrib/libgit-rs/testdata/config4 @@ -7,3 +7,7 @@ boolZero = 0 boolZeroZero = 00 boolHundred = 100 + ulongSmall = 42 + ulongBig = 4294967296 + pathRelative = ./some/path + pathAbsolute = /usr/bin/git diff --git a/contrib/libgit-sys/src/lib.rs b/contrib/libgit-sys/src/lib.rs index b104fda8f6..07386572ec 100644 --- 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; + extern crate libz_sys; #[allow(non_camel_case_types)] @@ -49,6 +53,18 @@ extern "C" { dest: *mut c_int, ) -> c_int; + pub fn libgit_configset_get_ulong( + cs: *mut libgit_config_set, + key: *const c_char, + dest: *mut c_ulong, + ) -> c_int; + + pub fn libgit_configset_get_pathname( + cs: *mut libgit_config_set, + key: *const c_char, + dest: *mut *mut c_char, + ) -> c_int; + } #[cfg(test)] -- gitgitgadget ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v3 3/3] libgit-rs: add get_ulong() and get_pathname() methods 2025-09-27 3:51 ` [PATCH v3 3/3] libgit-rs: add get_ulong() and get_pathname() methods ionnss via GitGitGadget @ 2025-09-29 13:23 ` Phillip Wood 0 siblings, 0 replies; 22+ messages in thread From: Phillip Wood @ 2025-09-29 13:23 UTC (permalink / raw) To: ionnss via GitGitGadget, git; +Cc: Chris Torek, ions On 27/09/2025 04:51, ionnss via GitGitGadget wrote: > From: ionnss <zara.leonardo@gmail.com> > > Expand the ConfigSet API with additional configuration value types: > > - get_ulong(): Parse unsigned long integers for large numeric values I'm torn as to whether we should keep the same name as the C function as you have done, or call it get_u64 to avoid the name depending on a platform dependent type. It is very welcome that this function returns u64. > - get_pathname(): Parse file paths, returning PathBuf for type safety I'm not sure why these two functions are in the same commit, it would make more sense to separate them out I think like you have done for get_bool(). > Both functions follow the same pattern as existing get_* methods, > using Git's C functions for consistent parsing behavior. > > Add comprehensive tests covering normal cases, edge cases, and > error handling for all new functionality. We can debate what constituets "comprehensive" but it is good to see that the new functions are tested. > diff --git a/contrib/libgit-rs/src/config.rs b/contrib/libgit-rs/src/config.rs > index 72ee88801b..ffd9f311b6 100644 > --- a/contrib/libgit-rs/src/config.rs > +++ b/contrib/libgit-rs/src/config.rs > @@ -1,8 +1,8 @@ > use std::ffi::{c_void, CStr, CString}; > -use std::path::Path; > +use std::path::{Path, PathBuf}; > > #[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)] > @@ -12,6 +12,10 @@ type c_char = i8; > #[allow(non_camel_case_types)] > type c_int = i32; > > +#[cfg(not(has_std__ffi__c_char))] > +#[allow(non_camel_case_types)] > +type c_ulong = u64; This is a bit problematic as the type depends on the platform. I'm not entirely clear why the current code doesn't just rely on having std::ffi define these types. > + > use libgit_sys::*; > > /// A ConfigSet is an in-memory cache for config-like files such as `.gitmodules` or `.gitconfig`. > @@ -82,6 +86,41 @@ impl ConfigSet { > > Some(val != 0) > } > + > + /// Load the value for the given key and attempt to parse it as an unsigned long. Dies with a fatal error > + /// if the value cannot be parsed. Returns None if the key is not present. Please wrap the comments to 80 columns > + pub fn get_ulong(&mut self, key: &str) -> Option<u64> { > + let key = CString::new(key).expect("config key should be valid CString"); > + let mut val: c_ulong = 0; > + unsafe { > + if libgit_configset_get_ulong(self.0, key.as_ptr(), &mut val as *mut c_ulong) != 0 { > + return None; > + } > + } > + Some(val as u64) > + } This looks good > + /// Load the value for the given key and attempt to parse it as a file path. Dies with a fatal error > + /// if the value cannot be converted to a PathBuf. Returns None if the key is not present. > + pub fn get_pathname(&mut self, key: &str) -> Option<PathBuf> { > + let key = CString::new(key).expect("config key should be valid CString"); > + let mut val: *mut c_char = std::ptr::null_mut(); > + unsafe { > + if libgit_configset_get_pathname(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("config path should be valid UTF-8"), As we're returning a PathBuf it is a shame that we're restricted to UTF-8 encoded paths. Unfortunately rust's standard library does not seem to provide an easy way to convert a CStr to an OsString on windows as one needs to convert it to a UTF-16 encoded string first. > + pub fn libgit_configset_get_ulong( > + cs: *mut libgit_config_set, > + key: *const c_char, > + dest: *mut c_ulong, > + ) -> c_int; > + > + pub fn libgit_configset_get_pathname( > + cs: *mut libgit_config_set, > + key: *const c_char, > + dest: *mut *mut c_char, > + ) -> c_int; As with the previous patch you need to define these functions in public_symbol_export.[ch] Thanks Phillip ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4] libgit-rs: add get_bool(), get_ulong(), and get_pathname() methods 2025-09-27 3:51 ` [PATCH v3 " ions via GitGitGadget ` (2 preceding siblings ...) 2025-09-27 3:51 ` [PATCH v3 3/3] libgit-rs: add get_ulong() and get_pathname() methods ionnss via GitGitGadget @ 2025-09-30 8:46 ` ions via GitGitGadget 2025-10-01 10:15 ` Phillip Wood 3 siblings, 1 reply; 22+ messages in thread From: ions via GitGitGadget @ 2025-09-30 8:46 UTC (permalink / raw) To: git; +Cc: ions, ionnss 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 All methods use Git's C functions for consistent behavior and include wrapper functions in public_symbol_export.[ch] for FFI. Includes comprehensive tests for all new functionality. Signed-off-by: ionnss <zara.leonardo@gmail.com> --- libgit-rs: add get_bool(), get_ulong(), and get_pathname() methods to ConfigSet libgit-rs: Enhanced ConfigSet API ================================= This series expands the ConfigSet API with support for additional configuration value types, following Git's established patterns by wrapping native C functions. Implementation ============== Adds three new ConfigSet methods: * get_bool(): Parse boolean values using git_configset_get_bool() * get_ulong(): Parse unsigned long integers using git_configset_get_ulong() * get_pathname(): Parse file paths using git_configset_get_pathname(), returning PathBuf All methods follow the existing pattern of get_int() and get_string(), ensuring consistent behavior with Git's native parsing logic. Changes in v4 ============= * Fixed missing wrapper functions in public_symbol_export.[ch] (Philip's feedback) * Rebased to correct base commit bb69721404 (Junio's feedback) * Squashed into single clean commit as requested * Removed unrelated README.md commit * Fixed commit message formatting per GitGitGadget requirements The ConfigSet API now supports 5 configuration value types: int, string, bool, ulong, and pathname. Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1977%2Fionnss%2Fadd-rust-configset-get-bool-v4 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1977/ionnss/add-rust-configset-get-bool-v4 Pull-Request: https://github.com/gitgitgadget/git/pull/1977 Range-diff vs v3: 1: d7810781fc < -: ---------- po: fix escaped underscores in README.md 2: 479c263bc1 < -: ---------- libgit-rs: add get_bool() method to ConfigSet 3: 1ac8d76819 ! 1: 618deca808 libgit-rs: add get_ulong() and get_pathname() methods @@ Metadata Author: ionnss <zara.leonardo@gmail.com> ## Commit message ## - libgit-rs: add get_ulong() and get_pathname() methods + libgit-rs: add get_bool(), get_ulong(), and get_pathname() methods - Expand the ConfigSet API with additional configuration value types: + Expand ConfigSet API with three new configuration value parsers: - - get_ulong(): Parse unsigned long integers for large numeric values - - get_pathname(): Parse file paths, returning PathBuf for type safety + - get_bool(): Parse boolean values using git_configset_get_bool() + - get_ulong(): Parse unsigned long values + - get_pathname(): Parse file paths, returning PathBuf - Both functions follow the same pattern as existing get_* methods, - using Git's C functions for consistent parsing behavior. + All methods use Git's C functions for consistent behavior and + include wrapper functions in public_symbol_export.[ch] for FFI. - Add comprehensive tests covering normal cases, edge cases, and - error handling for all new functionality. + Includes comprehensive tests for all new functionality. Signed-off-by: ionnss <zara.leonardo@gmail.com> @@ contrib/libgit-rs/src/config.rs: type c_char = i8; /// A ConfigSet is an in-memory cache for config-like files such as `.gitmodules` or `.gitconfig`. @@ contrib/libgit-rs/src/config.rs: impl ConfigSet { - - Some(val != 0) + Some(owned_str) + } } + ++ /// Load the value for the given key and attempt to parse it as a boolean. Dies with a fatal error ++ /// if the value cannot be parsed. Returns None if the key is not present. ++ pub fn get_bool(&mut self, key: &str) -> Option<bool> { ++ let key = CString::new(key).expect("config key should be valid CString"); ++ let mut val: c_int = 0; ++ unsafe { ++ if libgit_configset_get_bool(self.0, key.as_ptr(), &mut val as *mut c_int) != 0 { ++ return None; ++ } ++ } ++ ++ Some(val != 0) ++ } ++ + /// Load the value for the given key and attempt to parse it as an unsigned long. Dies with a fatal error + /// if the value cannot be parsed. Returns None if the key is not present. + pub fn get_ulong(&mut self, key: &str) -> Option<u64> { @@ contrib/libgit-rs/src/config.rs: impl ConfigSet { impl Default for ConfigSet { @@ contrib/libgit-rs/src/config.rs: mod tests { - assert_eq!(cs.get_bool("test.boolHundred"), Some(true)); // "100" → true - // Test missing boolean key - assert_eq!(cs.get_bool("missing.boolean"), None); + Path::new("testdata/config1"), + Path::new("testdata/config2"), + Path::new("testdata/config3"), ++ Path::new("testdata/config4"), + ]); + // ConfigSet retrieves correct value + assert_eq!(cs.get_int("trace2.eventTarget"), Some(1)); +@@ contrib/libgit-rs/src/config.rs: mod tests { + assert_eq!(cs.get_int("trace2.eventNesting"), Some(3)); + // ConfigSet returns None for missing key + assert_eq!(cs.get_string("foo.bar"), None); ++ // Test boolean parsing - comprehensive tests ++ assert_eq!(cs.get_bool("test.boolTrue"), Some(true)); ++ assert_eq!(cs.get_bool("test.boolFalse"), Some(false)); ++ assert_eq!(cs.get_bool("test.boolYes"), Some(true)); ++ assert_eq!(cs.get_bool("test.boolNo"), Some(false)); ++ assert_eq!(cs.get_bool("test.boolOne"), Some(true)); ++ assert_eq!(cs.get_bool("test.boolZero"), Some(false)); ++ assert_eq!(cs.get_bool("test.boolZeroZero"), Some(false)); // "00" → false ++ assert_eq!(cs.get_bool("test.boolHundred"), Some(true)); // "100" → true ++ // Test missing boolean key ++ assert_eq!(cs.get_bool("missing.boolean"), None); + // Test ulong parsing + assert_eq!(cs.get_ulong("test.ulongSmall"), Some(42)); + assert_eq!(cs.get_ulong("test.ulongBig"), Some(4294967296)); // > 32-bit int @@ contrib/libgit-rs/src/config.rs: mod tests { } } - ## contrib/libgit-rs/testdata/config4 ## + ## contrib/libgit-rs/testdata/config3 ## @@ - boolZero = 0 - boolZeroZero = 00 - boolHundred = 100 + [trace2] +- eventNesting = 3 ++ eventNesting = 3 + \ No newline at end of file + + ## contrib/libgit-rs/testdata/config4 (new) ## +@@ ++[test] ++ boolTrue = true ++ boolFalse = false ++ boolYes = yes ++ boolNo = no ++ boolOne = 1 ++ boolZero = 0 ++ boolZeroZero = 00 ++ boolHundred = 100 + ulongSmall = 42 + ulongBig = 4294967296 + pathRelative = ./some/path + pathAbsolute = /usr/bin/git + ## contrib/libgit-sys/public_symbol_export.c ## +@@ contrib/libgit-sys/public_symbol_export.c: int libgit_configset_get_string(struct libgit_config_set *cs, const char *key, + return git_configset_get_string(&cs->cs, key, dest); + } + ++int libgit_configset_get_bool(struct libgit_config_set *cs, const char *key, ++ int *dest) ++{ ++ return git_configset_get_bool(&cs->cs, key, dest); ++} ++ ++int libgit_configset_get_ulong(struct libgit_config_set *cs, const char *key, ++ unsigned long *dest) ++{ ++ return git_configset_get_ulong(&cs->cs, key, dest); ++} ++ ++int libgit_configset_get_pathname(struct libgit_config_set *cs, const char *key, ++ char **dest) ++{ ++ return git_configset_get_pathname(&cs->cs, key, dest); ++} ++ + const char *libgit_user_agent(void) + { + return git_user_agent(); + + ## contrib/libgit-sys/public_symbol_export.h ## +@@ contrib/libgit-sys/public_symbol_export.h: int libgit_configset_get_int(struct libgit_config_set *cs, const char *key, int + + int libgit_configset_get_string(struct libgit_config_set *cs, const char *key, char **dest); + ++int libgit_configset_get_bool(struct libgit_config_set *cs, const char *key, int *dest); ++ ++int libgit_configset_get_ulong(struct libgit_config_set *cs, const char *key, unsigned long *dest); ++ ++int libgit_configset_get_pathname(struct libgit_config_set *cs, const char *key, char **dest); ++ + const char *libgit_user_agent(void); + + const char *libgit_user_agent_sanitized(void); + ## contrib/libgit-sys/src/lib.rs ## @@ use std::ffi::c_void; @@ contrib/libgit-sys/src/lib.rs: pub type c_char = i8; #[allow(non_camel_case_types)] @@ contrib/libgit-sys/src/lib.rs: extern "C" { - dest: *mut c_int, + dest: *mut *mut c_char, ) -> c_int; ++ pub fn libgit_configset_get_bool( ++ cs: *mut libgit_config_set, ++ key: *const c_char, ++ dest: *mut c_int, ++ ) -> c_int; ++ + pub fn libgit_configset_get_ulong( + cs: *mut libgit_config_set, + key: *const c_char, contrib/libgit-rs/src/config.rs | 83 ++++++++++++++++++++++- contrib/libgit-rs/testdata/config3 | 2 +- contrib/libgit-rs/testdata/config4 | 13 ++++ contrib/libgit-sys/public_symbol_export.c | 18 +++++ contrib/libgit-sys/public_symbol_export.h | 6 ++ contrib/libgit-sys/src/lib.rs | 24 ++++++- 6 files changed, 142 insertions(+), 4 deletions(-) create mode 100644 contrib/libgit-rs/testdata/config4 diff --git a/contrib/libgit-rs/src/config.rs b/contrib/libgit-rs/src/config.rs index 6bf04845c8..ffd9f311b6 100644 --- a/contrib/libgit-rs/src/config.rs +++ b/contrib/libgit-rs/src/config.rs @@ -1,8 +1,8 @@ use std::ffi::{c_void, CStr, CString}; -use std::path::Path; +use std::path::{Path, PathBuf}; #[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)] @@ -12,6 +12,10 @@ type c_char = i8; #[allow(non_camel_case_types)] type c_int = i32; +#[cfg(not(has_std__ffi__c_char))] +#[allow(non_camel_case_types)] +type c_ulong = u64; + use libgit_sys::*; /// A ConfigSet is an in-memory cache for config-like files such as `.gitmodules` or `.gitconfig`. @@ -68,6 +72,55 @@ impl ConfigSet { Some(owned_str) } } + + /// Load the value for the given key and attempt to parse it as a boolean. Dies with a fatal error + /// if the value cannot be parsed. Returns None if the key is not present. + pub fn get_bool(&mut self, key: &str) -> Option<bool> { + let key = CString::new(key).expect("config key should be valid CString"); + let mut val: c_int = 0; + unsafe { + if libgit_configset_get_bool(self.0, key.as_ptr(), &mut val as *mut c_int) != 0 { + return None; + } + } + + Some(val != 0) + } + + /// Load the value for the given key and attempt to parse it as an unsigned long. Dies with a fatal error + /// if the value cannot be parsed. Returns None if the key is not present. + pub fn get_ulong(&mut self, key: &str) -> Option<u64> { + let key = CString::new(key).expect("config key should be valid CString"); + let mut val: c_ulong = 0; + unsafe { + if libgit_configset_get_ulong(self.0, key.as_ptr(), &mut val as *mut c_ulong) != 0 { + return None; + } + } + Some(val as u64) + } + + /// Load the value for the given key and attempt to parse it as a file path. Dies with a fatal error + /// if the value cannot be converted to a PathBuf. Returns None if the key is not present. + pub fn get_pathname(&mut self, key: &str) -> Option<PathBuf> { + let key = CString::new(key).expect("config key should be valid CString"); + let mut val: *mut c_char = std::ptr::null_mut(); + unsafe { + if libgit_configset_get_pathname(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("config path should be valid UTF-8"), + ); + free(val as *mut c_void); // Free the xstrdup()ed pointer from the C side + Some(PathBuf::from(owned_str)) + } + } } impl Default for ConfigSet { @@ -95,6 +148,7 @@ mod tests { Path::new("testdata/config1"), Path::new("testdata/config2"), Path::new("testdata/config3"), + Path::new("testdata/config4"), ]); // ConfigSet retrieves correct value assert_eq!(cs.get_int("trace2.eventTarget"), Some(1)); @@ -102,5 +156,30 @@ mod tests { assert_eq!(cs.get_int("trace2.eventNesting"), Some(3)); // ConfigSet returns None for missing key assert_eq!(cs.get_string("foo.bar"), None); + // Test boolean parsing - comprehensive tests + assert_eq!(cs.get_bool("test.boolTrue"), Some(true)); + assert_eq!(cs.get_bool("test.boolFalse"), Some(false)); + assert_eq!(cs.get_bool("test.boolYes"), Some(true)); + assert_eq!(cs.get_bool("test.boolNo"), Some(false)); + assert_eq!(cs.get_bool("test.boolOne"), Some(true)); + assert_eq!(cs.get_bool("test.boolZero"), Some(false)); + assert_eq!(cs.get_bool("test.boolZeroZero"), Some(false)); // "00" → false + assert_eq!(cs.get_bool("test.boolHundred"), Some(true)); // "100" → true + // Test missing boolean key + assert_eq!(cs.get_bool("missing.boolean"), None); + // Test ulong parsing + assert_eq!(cs.get_ulong("test.ulongSmall"), Some(42)); + assert_eq!(cs.get_ulong("test.ulongBig"), Some(4294967296)); // > 32-bit int + assert_eq!(cs.get_ulong("missing.ulong"), None); + // Test pathname parsing + assert_eq!( + cs.get_pathname("test.pathRelative"), + Some(PathBuf::from("./some/path")) + ); + assert_eq!( + cs.get_pathname("test.pathAbsolute"), + Some(PathBuf::from("/usr/bin/git")) + ); + assert_eq!(cs.get_pathname("missing.path"), None); } } diff --git a/contrib/libgit-rs/testdata/config3 b/contrib/libgit-rs/testdata/config3 index ca7b9a7c38..3ea5b96f12 100644 --- a/contrib/libgit-rs/testdata/config3 +++ b/contrib/libgit-rs/testdata/config3 @@ -1,2 +1,2 @@ [trace2] - eventNesting = 3 + eventNesting = 3 \ No newline at end of file diff --git a/contrib/libgit-rs/testdata/config4 b/contrib/libgit-rs/testdata/config4 new file mode 100644 index 0000000000..bd621ab480 --- /dev/null +++ b/contrib/libgit-rs/testdata/config4 @@ -0,0 +1,13 @@ +[test] + boolTrue = true + boolFalse = false + boolYes = yes + boolNo = no + boolOne = 1 + boolZero = 0 + boolZeroZero = 00 + boolHundred = 100 + ulongSmall = 42 + ulongBig = 4294967296 + pathRelative = ./some/path + pathAbsolute = /usr/bin/git diff --git a/contrib/libgit-sys/public_symbol_export.c b/contrib/libgit-sys/public_symbol_export.c index dfbb257115..8d6a0e1ba5 100644 --- a/contrib/libgit-sys/public_symbol_export.c +++ b/contrib/libgit-sys/public_symbol_export.c @@ -46,6 +46,24 @@ int libgit_configset_get_string(struct libgit_config_set *cs, const char *key, return git_configset_get_string(&cs->cs, key, dest); } +int libgit_configset_get_bool(struct libgit_config_set *cs, const char *key, + int *dest) +{ + return git_configset_get_bool(&cs->cs, key, dest); +} + +int libgit_configset_get_ulong(struct libgit_config_set *cs, const char *key, + unsigned long *dest) +{ + return git_configset_get_ulong(&cs->cs, key, dest); +} + +int libgit_configset_get_pathname(struct libgit_config_set *cs, const char *key, + char **dest) +{ + return git_configset_get_pathname(&cs->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 701db92d53..f9adda6561 100644 --- a/contrib/libgit-sys/public_symbol_export.h +++ b/contrib/libgit-sys/public_symbol_export.h @@ -11,6 +11,12 @@ int libgit_configset_get_int(struct libgit_config_set *cs, const char *key, int int libgit_configset_get_string(struct libgit_config_set *cs, const char *key, char **dest); +int libgit_configset_get_bool(struct libgit_config_set *cs, const char *key, int *dest); + +int libgit_configset_get_ulong(struct libgit_config_set *cs, const char *key, unsigned long *dest); + +int libgit_configset_get_pathname(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 4bfc650450..07386572ec 100644 --- 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; + extern crate libz_sys; #[allow(non_camel_case_types)] @@ -43,6 +47,24 @@ extern "C" { dest: *mut *mut c_char, ) -> c_int; + pub fn libgit_configset_get_bool( + cs: *mut libgit_config_set, + key: *const c_char, + dest: *mut c_int, + ) -> c_int; + + pub fn libgit_configset_get_ulong( + cs: *mut libgit_config_set, + key: *const c_char, + dest: *mut c_ulong, + ) -> c_int; + + pub fn libgit_configset_get_pathname( + cs: *mut libgit_config_set, + key: *const c_char, + dest: *mut *mut c_char, + ) -> c_int; + } #[cfg(test)] base-commit: bb69721404348ea2db0a081c41ab6ebfe75bdec8 -- gitgitgadget ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v4] libgit-rs: add get_bool(), get_ulong(), and get_pathname() methods 2025-09-30 8:46 ` [PATCH v4] libgit-rs: add get_bool(), get_ulong(), " ions via GitGitGadget @ 2025-10-01 10:15 ` Phillip Wood 2025-10-06 21:20 ` brian m. carlson 0 siblings, 1 reply; 22+ messages in thread From: Phillip Wood @ 2025-10-01 10:15 UTC (permalink / raw) To: ions via GitGitGadget, git; +Cc: ions, brian m . carlson [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: > 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. > --- 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/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4] libgit-rs: add get_bool(), get_ulong(), and get_pathname() methods 2025-10-01 10:15 ` Phillip Wood @ 2025-10-06 21:20 ` brian m. carlson 2025-10-08 13:36 ` Phillip Wood 0 siblings, 1 reply; 22+ messages in thread From: brian m. carlson @ 2025-10-06 21:20 UTC (permalink / raw) To: phillip.wood; +Cc: ions via GitGitGadget, git, ions [-- Attachment #1: Type: text/plain, Size: 1017 bytes --] On 2025-10-01 at 10:15:28, Phillip Wood wrote: > [I've cc'd brian to sanity check my suggestion for handling c_ulong in rust > 1.63 which lacks std::ffi::c_ulong] > > 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. It's just fine to use `std::os::raw` in general without needing to use `std::ffi` conditionally. I'd just default to that until we move away from Rust 1.63. That's what I do myself. -- brian m. carlson (they/them) Toronto, Ontario, CA [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 262 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4] libgit-rs: add get_bool(), get_ulong(), and get_pathname() methods 2025-10-06 21:20 ` brian m. carlson @ 2025-10-08 13:36 ` Phillip Wood 0 siblings, 0 replies; 22+ messages in thread From: Phillip Wood @ 2025-10-08 13:36 UTC (permalink / raw) To: brian m. carlson, phillip.wood, ions via GitGitGadget, git, ions On 06/10/2025 22:20, brian m. carlson wrote: > On 2025-10-01 at 10:15:28, Phillip Wood wrote: >> [I've cc'd brian to sanity check my suggestion for handling c_ulong in rust >> 1.63 which lacks std::ffi::c_ulong] >> >> 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. > > It's just fine to use `std::os::raw` in general without needing to use > `std::ffi` conditionally. I'd just default to that until we move away > from Rust 1.63. That's what I do myself. Thanks brian, that sounds much simpler. Lets add a preparatory commit to use std::os::raw unconditionally. Thanks Phillip ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2025-10-08 13:36 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-25 11:44 [PATCH 0/2] libgit-rs: add get_bool() method to ConfigSet ions via GitGitGadget 2025-09-25 11:44 ` [PATCH 1/2] po: fix escaped underscores in README.md ionnss via GitGitGadget 2025-09-25 11:44 ` [PATCH 2/2] libgit-rs: add get_bool() method to ConfigSet ionnss via GitGitGadget 2025-09-26 6:43 ` Chris Torek 2025-09-26 9:58 ` Phillip Wood 2025-09-26 17:15 ` Junio C Hamano 2025-09-27 0:07 ` [PATCH v2 0/3] " ions via GitGitGadget 2025-09-27 0:07 ` [PATCH v2 1/3] po: fix escaped underscores in README.md ionnss via GitGitGadget 2025-09-27 0:07 ` [PATCH v2 2/3] libgit-rs: add get_bool() method to ConfigSet ionnss via GitGitGadget 2025-09-27 0:07 ` [PATCH v2 3/3] libgit-rs: address review feedback for get_bool() ionnss via GitGitGadget 2025-09-27 2:01 ` [PATCH v2 0/3] libgit-rs: add get_bool() method to ConfigSet Junio C Hamano 2025-09-27 3:51 ` [PATCH v3 " ions via GitGitGadget 2025-09-27 3:51 ` [PATCH v3 1/3] po: fix escaped underscores in README.md ionnss via GitGitGadget 2025-09-29 13:26 ` Phillip Wood 2025-09-27 3:51 ` [PATCH v3 2/3] libgit-rs: add get_bool() method to ConfigSet ionnss via GitGitGadget 2025-09-29 13:23 ` Phillip Wood 2025-09-27 3:51 ` [PATCH v3 3/3] libgit-rs: add get_ulong() and get_pathname() methods ionnss via GitGitGadget 2025-09-29 13:23 ` Phillip Wood 2025-09-30 8:46 ` [PATCH v4] libgit-rs: add get_bool(), get_ulong(), " ions via GitGitGadget 2025-10-01 10:15 ` Phillip Wood 2025-10-06 21:20 ` brian m. carlson 2025-10-08 13:36 ` Phillip Wood
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).