From ff2242fc2cee6b8f2003f850108a551cfe2052b7 Mon Sep 17 00:00:00 2001 From: Paul Horn Date: Fri, 21 Jan 2022 15:50:50 +0100 Subject: [PATCH 1/4] Add support for custom git extensions This would allow one to use git worktrees together with sparse-checkout. Reference: https://github.com/libgit2/libgit2/pull/6031 --- libgit2-sys/lib.rs | 4 +++ src/opts.rs | 84 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/libgit2-sys/lib.rs b/libgit2-sys/lib.rs index 951397cc2c..bade631b20 100644 --- a/libgit2-sys/lib.rs +++ b/libgit2-sys/lib.rs @@ -1870,6 +1870,10 @@ git_enum! { GIT_OPT_ENABLE_HTTP_EXPECT_CONTINUE, GIT_OPT_GET_MWINDOW_FILE_LIMIT, GIT_OPT_SET_MWINDOW_FILE_LIMIT, + GIT_OPT_SET_ODB_PACKED_PRIORITY, + GIT_OPT_SET_ODB_LOOSE_PRIORITY, + GIT_OPT_GET_EXTENSIONS, + GIT_OPT_SET_EXTENSIONS, } } diff --git a/src/opts.rs b/src/opts.rs index 007b272c1b..d1ca23d1a8 100644 --- a/src/opts.rs +++ b/src/opts.rs @@ -1,7 +1,9 @@ //! Bindings to libgit2's git_libgit2_opts function. use std::ffi::CString; +use std::ptr; +use crate::string_array::StringArray; use crate::util::Binding; use crate::{raw, Buf, ConfigLevel, Error, IntoCString}; @@ -119,6 +121,56 @@ pub fn strict_hash_verification(enabled: bool) { debug_assert!(error >= 0); } +/// Returns the list of git extensions that are supported. This is the list of +/// built-in extensions supported by libgit2 and custom extensions that have +/// been added with [`set_extensions`]. Extensions that have been negated will +/// not be returned. +pub fn get_extensions() -> Result { + crate::init(); + + let mut extensions = raw::git_strarray { + strings: ptr::null_mut(), + count: 0, + }; + + unsafe { + try_call!(raw::git_libgit2_opts( + raw::GIT_OPT_GET_EXTENSIONS as libc::c_int, + &mut extensions + )); + Ok(StringArray::from_raw(extensions)) + } +} + +/// Set that the given git extensions are supported by the caller. Extensions +/// supported by libgit2 may be negated by prefixing them with a `!`. +/// For example: setting extensions to `[ "!noop", "newext" ]` indicates that +/// the caller does not want to support repositories with the `noop` extension +/// but does want to support repositories with the `newext` extension. +pub fn set_extensions(extensions: &[E]) -> Result<(), Error> +where + for<'x> &'x E: IntoCString, +{ + crate::init(); + + let extensions = extensions + .iter() + .map(|e| e.into_c_string()) + .collect::, _>>()?; + + let extension_ptrs = extensions.iter().map(|e| e.as_ptr()).collect::>(); + + unsafe { + try_call!(raw::git_libgit2_opts( + raw::GIT_OPT_SET_EXTENSIONS as libc::c_int, + extension_ptrs.as_ptr(), + extension_ptrs.len() as libc::size_t + )); + } + + Ok(()) +} + #[cfg(test)] mod test { use super::*; @@ -127,4 +179,36 @@ mod test { fn smoke() { strict_hash_verification(false); } + + #[test] + fn test_get_extensions() -> Result<(), Error> { + let extensions = get_extensions()?; + + assert_eq!(extensions.len(), 1); + assert_eq!(extensions.get(0), Some("noop")); + + Ok(()) + } + + #[test] + fn test_add_extensions() -> Result<(), Error> { + set_extensions(&["custom"])?; + let extensions = get_extensions()?; + + assert_eq!(extensions.len(), 2); + assert_eq!(extensions.get(0), Some("noop")); + assert_eq!(extensions.get(1), Some("custom")); + Ok(()) + } + + #[test] + fn test_remove_extensions() -> Result<(), Error> { + set_extensions(&["custom", "!ignore", "!noop", "other"])?; + let extensions = get_extensions()?; + + assert_eq!(extensions.len(), 2); + assert_eq!(extensions.get(0), Some("custom")); + assert_eq!(extensions.get(1), Some("other")); + Ok(()) + } } From f0606d8a8a5a7183a129ba1edfe2792e37c83a74 Mon Sep 17 00:00:00 2001 From: Paul Horn Date: Fri, 21 Jan 2022 16:37:42 +0100 Subject: [PATCH 2/4] Mark extension methods unsafe --- src/opts.rs | 112 ++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 83 insertions(+), 29 deletions(-) diff --git a/src/opts.rs b/src/opts.rs index d1ca23d1a8..2484ece972 100644 --- a/src/opts.rs +++ b/src/opts.rs @@ -125,7 +125,12 @@ pub fn strict_hash_verification(enabled: bool) { /// built-in extensions supported by libgit2 and custom extensions that have /// been added with [`set_extensions`]. Extensions that have been negated will /// not be returned. -pub fn get_extensions() -> Result { +/// +/// # Safety +/// +/// libgit2 stores user extensions in a static variable. +/// This function is effectively reading a `static mut` and should be treated as such +pub unsafe fn get_extensions() -> Result { crate::init(); let mut extensions = raw::git_strarray { @@ -133,13 +138,12 @@ pub fn get_extensions() -> Result { count: 0, }; - unsafe { - try_call!(raw::git_libgit2_opts( - raw::GIT_OPT_GET_EXTENSIONS as libc::c_int, - &mut extensions - )); - Ok(StringArray::from_raw(extensions)) - } + try_call!(raw::git_libgit2_opts( + raw::GIT_OPT_GET_EXTENSIONS as libc::c_int, + &mut extensions + )); + + Ok(StringArray::from_raw(extensions)) } /// Set that the given git extensions are supported by the caller. Extensions @@ -147,7 +151,12 @@ pub fn get_extensions() -> Result { /// For example: setting extensions to `[ "!noop", "newext" ]` indicates that /// the caller does not want to support repositories with the `noop` extension /// but does want to support repositories with the `newext` extension. -pub fn set_extensions(extensions: &[E]) -> Result<(), Error> +/// +/// # Safety +/// +/// libgit2 stores user extensions in a static variable. +/// This function is effectively modifying a `static mut` and should be treated as such +pub unsafe fn set_extensions(extensions: &[E]) -> Result<(), Error> where for<'x> &'x E: IntoCString, { @@ -160,13 +169,11 @@ where let extension_ptrs = extensions.iter().map(|e| e.as_ptr()).collect::>(); - unsafe { - try_call!(raw::git_libgit2_opts( - raw::GIT_OPT_SET_EXTENSIONS as libc::c_int, - extension_ptrs.as_ptr(), - extension_ptrs.len() as libc::size_t - )); - } + try_call!(raw::git_libgit2_opts( + raw::GIT_OPT_SET_EXTENSIONS as libc::c_int, + extension_ptrs.as_ptr(), + extension_ptrs.len() as libc::size_t + )); Ok(()) } @@ -174,41 +181,88 @@ where #[cfg(test)] mod test { use super::*; + use std::sync::atomic::{AtomicBool, Ordering}; #[test] fn smoke() { strict_hash_verification(false); } + // since tests are run in parallel, we must force the extensions test to run + // serialized to avoid UB + + static LOCKED: AtomicBool = AtomicBool::new(false); + + struct LockGuard {} + impl Drop for LockGuard { + fn drop(&mut self) { + LOCKED.store(false, Ordering::SeqCst); + } + } + + fn lock() -> LockGuard { + while LOCKED + .compare_exchange_weak(false, true, Ordering::SeqCst, Ordering::SeqCst) + .is_err() + {} + + LockGuard {} + } + #[test] fn test_get_extensions() -> Result<(), Error> { - let extensions = get_extensions()?; + let _lock = lock(); - assert_eq!(extensions.len(), 1); - assert_eq!(extensions.get(0), Some("noop")); + let extensions = unsafe { get_extensions() }?; + + let mut extensions = extensions.iter().flatten().collect::>(); + extensions.sort_unstable(); + assert_eq!(extensions, vec!["noop"]); Ok(()) } #[test] fn test_add_extensions() -> Result<(), Error> { - set_extensions(&["custom"])?; - let extensions = get_extensions()?; + let _lock = lock(); + + unsafe { + set_extensions(&["custom"])?; + } + + let extensions = unsafe { get_extensions() }?; + + let mut extensions = extensions.iter().flatten().collect::>(); + extensions.sort_unstable(); + assert_eq!(extensions, vec!["custom", "noop"]); + + // reset extensions since they are a shared state + unsafe { + set_extensions(&["!custom"])?; + } - assert_eq!(extensions.len(), 2); - assert_eq!(extensions.get(0), Some("noop")); - assert_eq!(extensions.get(1), Some("custom")); Ok(()) } #[test] fn test_remove_extensions() -> Result<(), Error> { - set_extensions(&["custom", "!ignore", "!noop", "other"])?; - let extensions = get_extensions()?; + let _lock = lock(); + + unsafe { + set_extensions(&["custom", "!ignore", "!noop", "other"])?; + } + + let extensions = unsafe { get_extensions() }?; + + let mut extensions = extensions.iter().flatten().collect::>(); + extensions.sort_unstable(); + assert_eq!(extensions, vec!["custom", "other"]); + + // reset extensions since they are a shared state + unsafe { + set_extensions(&["!custom", "!other"])?; + } - assert_eq!(extensions.len(), 2); - assert_eq!(extensions.get(0), Some("custom")); - assert_eq!(extensions.get(1), Some("other")); Ok(()) } } From 8e10836cc31ccddf616adc5f26e0d74ef865f47b Mon Sep 17 00:00:00 2001 From: Paul Horn Date: Mon, 24 Jan 2022 13:21:17 +0100 Subject: [PATCH 3/4] Replace custom spin-lock in tests with serial-test crate --- Cargo.toml | 1 + src/opts.rs | 29 ++++------------------------- 2 files changed, 5 insertions(+), 25 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2f5a415a86..24ff095035 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,7 @@ openssl-sys = { version = "0.9.0", optional = true } openssl-probe = { version = "0.1", optional = true } [dev-dependencies] +serial_test = "0.5.1" structopt = "0.3" time = "0.1.39" tempfile = "3.1.0" diff --git a/src/opts.rs b/src/opts.rs index 2484ece972..e6fb8eb17e 100644 --- a/src/opts.rs +++ b/src/opts.rs @@ -181,7 +181,7 @@ where #[cfg(test)] mod test { use super::*; - use std::sync::atomic::{AtomicBool, Ordering}; + use serial_test::serial; #[test] fn smoke() { @@ -191,28 +191,9 @@ mod test { // since tests are run in parallel, we must force the extensions test to run // serialized to avoid UB - static LOCKED: AtomicBool = AtomicBool::new(false); - - struct LockGuard {} - impl Drop for LockGuard { - fn drop(&mut self) { - LOCKED.store(false, Ordering::SeqCst); - } - } - - fn lock() -> LockGuard { - while LOCKED - .compare_exchange_weak(false, true, Ordering::SeqCst, Ordering::SeqCst) - .is_err() - {} - - LockGuard {} - } - #[test] + #[serial] fn test_get_extensions() -> Result<(), Error> { - let _lock = lock(); - let extensions = unsafe { get_extensions() }?; let mut extensions = extensions.iter().flatten().collect::>(); @@ -223,9 +204,8 @@ mod test { } #[test] + #[serial] fn test_add_extensions() -> Result<(), Error> { - let _lock = lock(); - unsafe { set_extensions(&["custom"])?; } @@ -245,9 +225,8 @@ mod test { } #[test] + #[serial] fn test_remove_extensions() -> Result<(), Error> { - let _lock = lock(); - unsafe { set_extensions(&["custom", "!ignore", "!noop", "other"])?; } From 4e3fa2196c47d0a5135f50dc6edeba70ac7d694a Mon Sep 17 00:00:00 2001 From: Paul Horn Date: Mon, 24 Jan 2022 19:30:33 +0100 Subject: [PATCH 4/4] Rewrite tests as integration tests --- Cargo.toml | 1 - src/opts.rs | 58 -------------------------------------- tests/add_extensions.rs | 19 +++++++++++++ tests/get_extensions.rs | 14 +++++++++ tests/remove_extensions.rs | 19 +++++++++++++ 5 files changed, 52 insertions(+), 59 deletions(-) create mode 100644 tests/add_extensions.rs create mode 100644 tests/get_extensions.rs create mode 100644 tests/remove_extensions.rs diff --git a/Cargo.toml b/Cargo.toml index 24ff095035..2f5a415a86 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,7 +27,6 @@ openssl-sys = { version = "0.9.0", optional = true } openssl-probe = { version = "0.1", optional = true } [dev-dependencies] -serial_test = "0.5.1" structopt = "0.3" time = "0.1.39" tempfile = "3.1.0" diff --git a/src/opts.rs b/src/opts.rs index e6fb8eb17e..a89df4e1c9 100644 --- a/src/opts.rs +++ b/src/opts.rs @@ -181,67 +181,9 @@ where #[cfg(test)] mod test { use super::*; - use serial_test::serial; #[test] fn smoke() { strict_hash_verification(false); } - - // since tests are run in parallel, we must force the extensions test to run - // serialized to avoid UB - - #[test] - #[serial] - fn test_get_extensions() -> Result<(), Error> { - let extensions = unsafe { get_extensions() }?; - - let mut extensions = extensions.iter().flatten().collect::>(); - extensions.sort_unstable(); - assert_eq!(extensions, vec!["noop"]); - - Ok(()) - } - - #[test] - #[serial] - fn test_add_extensions() -> Result<(), Error> { - unsafe { - set_extensions(&["custom"])?; - } - - let extensions = unsafe { get_extensions() }?; - - let mut extensions = extensions.iter().flatten().collect::>(); - extensions.sort_unstable(); - assert_eq!(extensions, vec!["custom", "noop"]); - - // reset extensions since they are a shared state - unsafe { - set_extensions(&["!custom"])?; - } - - Ok(()) - } - - #[test] - #[serial] - fn test_remove_extensions() -> Result<(), Error> { - unsafe { - set_extensions(&["custom", "!ignore", "!noop", "other"])?; - } - - let extensions = unsafe { get_extensions() }?; - - let mut extensions = extensions.iter().flatten().collect::>(); - extensions.sort_unstable(); - assert_eq!(extensions, vec!["custom", "other"]); - - // reset extensions since they are a shared state - unsafe { - set_extensions(&["!custom", "!other"])?; - } - - Ok(()) - } } diff --git a/tests/add_extensions.rs b/tests/add_extensions.rs new file mode 100644 index 0000000000..fe37e1eeb7 --- /dev/null +++ b/tests/add_extensions.rs @@ -0,0 +1,19 @@ +//! Test for `set_extensions`, which writes a global state maintained by libgit2 + +use git2::opts::{get_extensions, set_extensions}; +use git2::Error; + +#[test] +fn test_add_extensions() -> Result<(), Error> { + unsafe { + set_extensions(&["custom"])?; + } + + let extensions = unsafe { get_extensions() }?; + + assert_eq!(extensions.len(), 2); + assert_eq!(extensions.get(0), Some("noop")); + assert_eq!(extensions.get(1), Some("custom")); + + Ok(()) +} diff --git a/tests/get_extensions.rs b/tests/get_extensions.rs new file mode 100644 index 0000000000..ac049c6816 --- /dev/null +++ b/tests/get_extensions.rs @@ -0,0 +1,14 @@ +//! Test for `get_extensions`, which reads a global state maintained by libgit2 + +use git2::opts::get_extensions; +use git2::Error; + +#[test] +fn test_get_extensions() -> Result<(), Error> { + let extensions = unsafe { get_extensions() }?; + + assert_eq!(extensions.len(), 1); + assert_eq!(extensions.get(0), Some("noop")); + + Ok(()) +} diff --git a/tests/remove_extensions.rs b/tests/remove_extensions.rs new file mode 100644 index 0000000000..366da7392b --- /dev/null +++ b/tests/remove_extensions.rs @@ -0,0 +1,19 @@ +//! Test for `set_extensions`, which writes a global state maintained by libgit2 + +use git2::opts::{get_extensions, set_extensions}; +use git2::Error; + +#[test] +fn test_remove_extensions() -> Result<(), Error> { + unsafe { + set_extensions(&["custom", "!ignore", "!noop", "other"])?; + } + + let extensions = unsafe { get_extensions() }?; + + assert_eq!(extensions.len(), 2); + assert_eq!(extensions.get(0), Some("custom")); + assert_eq!(extensions.get(1), Some("other")); + + Ok(()) +}