From eeff9c21bda1c94856698539391fc1b36486a54b Mon Sep 17 00:00:00 2001 From: Fin Christensen Date: Tue, 30 Nov 2021 12:48:47 +0100 Subject: [PATCH 1/9] Add wrappers for git_index_conflict_{add,remove,get,cleanup} --- libgit2-sys/lib.rs | 1 + src/index.rs | 176 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+) diff --git a/libgit2-sys/lib.rs b/libgit2-sys/lib.rs index a113a29526..7d7e2febe7 100644 --- a/libgit2-sys/lib.rs +++ b/libgit2-sys/lib.rs @@ -2923,6 +2923,7 @@ extern "C" { our_entry: *const git_index_entry, their_entry: *const git_index_entry, ) -> c_int; + pub fn git_index_conflict_cleanup(index: *mut git_index) -> c_int; pub fn git_index_conflict_remove(index: *mut git_index, path: *const c_char) -> c_int; pub fn git_index_conflict_get( ancestor_out: *mut *const git_index_entry, diff --git a/src/index.rs b/src/index.rs index b2e8dfe5c1..5a844251a4 100644 --- a/src/index.rs +++ b/src/index.rs @@ -412,6 +412,182 @@ impl Index { unsafe { raw::git_index_has_conflicts(self.raw) == 1 } } + /// Add or update index entries to represent a conflict. Any staged entries + /// that exist at the given paths will be removed. + /// + /// The entries are the entries from the tree included in the merge. Any entry + /// may be `None` to indicate that that file was not present in the trees during + /// the merge. For example, ancestor_entry may be `None` to indicate that a file + /// was added in both branches and must be resolved. + pub fn conflict_add( + &self, + ancestor_entry: Option<&IndexEntry>, + our_entry: Option<&IndexEntry>, + their_entry: Option<&IndexEntry>, + ) -> Result<(), Error> { + let mut ancestor_raw: Option = None; + let mut our_raw: Option = None; + let mut their_raw: Option = None; + + if let Some(ancestor_entry) = ancestor_entry { + let ancestor_path = CString::new(&ancestor_entry.path[..])?; + let mut ancestor_flags = ancestor_entry.flags & !raw::GIT_INDEX_ENTRY_NAMEMASK; + + if ancestor_entry.path.len() < raw::GIT_INDEX_ENTRY_NAMEMASK as usize { + ancestor_flags |= ancestor_entry.path.len() as u16; + } else { + ancestor_flags |= raw::GIT_INDEX_ENTRY_NAMEMASK; + } + + unsafe { + ancestor_raw = Some(raw::git_index_entry { + dev: ancestor_entry.dev, + ino: ancestor_entry.ino, + mode: ancestor_entry.mode, + uid: ancestor_entry.uid, + gid: ancestor_entry.gid, + file_size: ancestor_entry.file_size, + id: *ancestor_entry.id.raw(), + flags: ancestor_flags, + flags_extended: ancestor_entry.flags_extended, + path: ancestor_path.as_ptr(), + mtime: raw::git_index_time { + seconds: ancestor_entry.mtime.seconds(), + nanoseconds: ancestor_entry.mtime.nanoseconds(), + }, + ctime: raw::git_index_time { + seconds: ancestor_entry.ctime.seconds(), + nanoseconds: ancestor_entry.ctime.nanoseconds(), + }, + }); + } + } + + if let Some(our_entry) = our_entry { + let our_path = CString::new(&our_entry.path[..])?; + let mut our_flags = our_entry.flags & !raw::GIT_INDEX_ENTRY_NAMEMASK; + + if our_entry.path.len() < raw::GIT_INDEX_ENTRY_NAMEMASK as usize { + our_flags |= our_entry.path.len() as u16; + } else { + our_flags |= raw::GIT_INDEX_ENTRY_NAMEMASK; + } + + unsafe { + our_raw = Some(raw::git_index_entry { + dev: our_entry.dev, + ino: our_entry.ino, + mode: our_entry.mode, + uid: our_entry.uid, + gid: our_entry.gid, + file_size: our_entry.file_size, + id: *our_entry.id.raw(), + flags: our_flags, + flags_extended: our_entry.flags_extended, + path: our_path.as_ptr(), + mtime: raw::git_index_time { + seconds: our_entry.mtime.seconds(), + nanoseconds: our_entry.mtime.nanoseconds(), + }, + ctime: raw::git_index_time { + seconds: our_entry.ctime.seconds(), + nanoseconds: our_entry.ctime.nanoseconds(), + }, + }); + } + } + + if let Some(their_entry) = their_entry { + let their_path = CString::new(&their_entry.path[..])?; + let mut their_flags = their_entry.flags & !raw::GIT_INDEX_ENTRY_NAMEMASK; + + if their_entry.path.len() < raw::GIT_INDEX_ENTRY_NAMEMASK as usize { + their_flags |= their_entry.path.len() as u16; + } else { + their_flags |= raw::GIT_INDEX_ENTRY_NAMEMASK; + } + + unsafe { + their_raw = Some(raw::git_index_entry { + dev: their_entry.dev, + ino: their_entry.ino, + mode: their_entry.mode, + uid: their_entry.uid, + gid: their_entry.gid, + file_size: their_entry.file_size, + id: *their_entry.id.raw(), + flags: their_flags, + flags_extended: their_entry.flags_extended, + path: their_path.as_ptr(), + mtime: raw::git_index_time { + seconds: their_entry.mtime.seconds(), + nanoseconds: their_entry.mtime.nanoseconds(), + }, + ctime: raw::git_index_time { + seconds: their_entry.ctime.seconds(), + nanoseconds: their_entry.ctime.nanoseconds(), + }, + }); + } + } + + let ancestor_raw_ptr = ancestor_raw.as_ref().map_or_else(std::ptr::null, |ptr| ptr); + let our_raw_ptr = our_raw.as_ref().map_or_else(std::ptr::null, |ptr| ptr); + let their_raw_ptr = their_raw.as_ref().map_or_else(std::ptr::null, |ptr| ptr); + unsafe { + try_call!(raw::git_index_conflict_add(self.raw, ancestor_raw_ptr, our_raw_ptr, their_raw_ptr)); + Ok(()) + } + } + + /// Remove all conflicts in the index (entries with a stage greater than 0). + pub fn conflict_cleanup(&self) -> Result<(), Error> { + unsafe { + try_call!(raw::git_index_conflict_cleanup(self.raw)); + Ok(()) + } + } + + /// Get the index entries that represent a conflict of a single file. + /// + /// The entries are not modifiable. + pub fn conflict_get(&self, path: &Path) -> Result { + let path = path_to_repo_path(path)?; + let mut ancestor = ptr::null(); + let mut our = ptr::null(); + let mut their = ptr::null(); + + unsafe { + try_call!( + raw::git_index_conflict_get(&mut ancestor, &mut our, &mut their, self.raw, path) + ); + Ok(IndexConflict { + ancestor: match ancestor.is_null() { + false => Some(IndexEntry::from_raw(*ancestor)), + true => None, + }, + our: match our.is_null() { + false => Some(IndexEntry::from_raw(*our)), + true => None, + }, + their: match their.is_null() { + false => Some(IndexEntry::from_raw(*their)), + true => None, + }, + }) + } + } + + /// Removes the index entries that represent a conflict of a single file. + pub fn conflict_remove(&self, path: &Path) -> Result<(), Error> { + let path = path_to_repo_path(path)?; + + unsafe { + try_call!(raw::git_index_conflict_remove(self.raw, path)); + Ok(()) + } + } + /// Get the full path to the index file on disk. /// /// Returns `None` if this is an in-memory index. From da622f778d15c6970cb216dda301603194a76657 Mon Sep 17 00:00:00 2001 From: Fin Christensen Date: Tue, 30 Nov 2021 12:57:41 +0100 Subject: [PATCH 2/9] Run `cargo fmt` --- src/index.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/index.rs b/src/index.rs index 5a844251a4..6a89843067 100644 --- a/src/index.rs +++ b/src/index.rs @@ -535,7 +535,12 @@ impl Index { let our_raw_ptr = our_raw.as_ref().map_or_else(std::ptr::null, |ptr| ptr); let their_raw_ptr = their_raw.as_ref().map_or_else(std::ptr::null, |ptr| ptr); unsafe { - try_call!(raw::git_index_conflict_add(self.raw, ancestor_raw_ptr, our_raw_ptr, their_raw_ptr)); + try_call!(raw::git_index_conflict_add( + self.raw, + ancestor_raw_ptr, + our_raw_ptr, + their_raw_ptr + )); Ok(()) } } @@ -558,9 +563,13 @@ impl Index { let mut their = ptr::null(); unsafe { - try_call!( - raw::git_index_conflict_get(&mut ancestor, &mut our, &mut their, self.raw, path) - ); + try_call!(raw::git_index_conflict_get( + &mut ancestor, + &mut our, + &mut their, + self.raw, + path + )); Ok(IndexConflict { ancestor: match ancestor.is_null() { false => Some(IndexEntry::from_raw(*ancestor)), From aa6d3fb2b168bf6d1bae67e01ad1763defad7606 Mon Sep 17 00:00:00 2001 From: Fin Christensen Date: Thu, 2 Dec 2021 16:09:06 +0100 Subject: [PATCH 3/9] Add helper function to construct raw::git_index_entry's --- src/index.rs | 276 ++++++++++++++++----------------------------------- 1 file changed, 85 insertions(+), 191 deletions(-) diff --git a/src/index.rs b/src/index.rs index 6a89843067..7296ad4728 100644 --- a/src/index.rs +++ b/src/index.rs @@ -85,6 +85,70 @@ pub struct IndexEntry { pub path: Vec, } +// We cannot return raw::git_index_entry instances directly, as they rely on +// a CString which is owned by the function. To make the pointer to the CString +// valid during usage of raw::git_index_entry, we supply the index entry in a +// callback where pointers to the CString are valid. +fn try_raw_entries( + entries: &[Option<&IndexEntry>], + cb: impl FnOnce(&[*const raw::git_index_entry]) -> Result<(), Error>, +) -> Result<(), Error> { + let paths = entries.iter() + .map(|entry| { + if let Some(entry) = entry { + CString::new(&entry.path[..]).map(|ok| Some(ok)) + } else { + Ok(None) + } + }) + .collect::>, std::ffi::NulError>>()?; + + let raw_entries = entries.iter().zip(&paths).map(|(entry, path)| { + if let Some(entry) = entry { + // libgit2 encodes the length of the path in the lower bits of the + // `flags` entry, so mask those out and recalculate here to ensure we + // don't corrupt anything. + let mut flags = entry.flags & !raw::GIT_INDEX_ENTRY_NAMEMASK; + + if entry.path.len() < raw::GIT_INDEX_ENTRY_NAMEMASK as usize { + flags |= entry.path.len() as u16; + } else { + flags |= raw::GIT_INDEX_ENTRY_NAMEMASK; + } + + unsafe { + Some(raw::git_index_entry { + dev: entry.dev, + ino: entry.ino, + mode: entry.mode, + uid: entry.uid, + gid: entry.gid, + file_size: entry.file_size, + id: *entry.id.raw(), + flags, + flags_extended: entry.flags_extended, + path: path.as_ref().unwrap().as_ptr(), + mtime: raw::git_index_time { + seconds: entry.mtime.seconds(), + nanoseconds: entry.mtime.nanoseconds(), + }, + ctime: raw::git_index_time { + seconds: entry.ctime.seconds(), + nanoseconds: entry.ctime.nanoseconds(), + }, + }) + } + } else { + None + } + }).collect::>(); + let raw_entry_ptrs = raw_entries.iter() + .map(|opt| opt.as_ref().map_or_else(std::ptr::null, |ptr| ptr)) + .collect::>(); + + cb(&raw_entry_ptrs) +} + impl Index { /// Creates a new in-memory index. /// @@ -145,43 +209,12 @@ impl Index { /// given 'source_entry', it will be replaced. Otherwise, the 'source_entry' /// will be added. pub fn add(&mut self, entry: &IndexEntry) -> Result<(), Error> { - let path = CString::new(&entry.path[..])?; - - // libgit2 encodes the length of the path in the lower bits of the - // `flags` entry, so mask those out and recalculate here to ensure we - // don't corrupt anything. - let mut flags = entry.flags & !raw::GIT_INDEX_ENTRY_NAMEMASK; - - if entry.path.len() < raw::GIT_INDEX_ENTRY_NAMEMASK as usize { - flags |= entry.path.len() as u16; - } else { - flags |= raw::GIT_INDEX_ENTRY_NAMEMASK; - } - - unsafe { - let raw = raw::git_index_entry { - dev: entry.dev, - ino: entry.ino, - mode: entry.mode, - uid: entry.uid, - gid: entry.gid, - file_size: entry.file_size, - id: *entry.id.raw(), - flags, - flags_extended: entry.flags_extended, - path: path.as_ptr(), - mtime: raw::git_index_time { - seconds: entry.mtime.seconds(), - nanoseconds: entry.mtime.nanoseconds(), - }, - ctime: raw::git_index_time { - seconds: entry.ctime.seconds(), - nanoseconds: entry.ctime.nanoseconds(), - }, - }; - try_call!(raw::git_index_add(self.raw, &raw)); + try_raw_entries(&[Some(entry)], |raws| { + unsafe { + try_call!(raw::git_index_add(self.raw, raws[0])); + } Ok(()) - } + }) } /// Add or update an index entry from a buffer in memory @@ -202,46 +235,14 @@ impl Index { /// no longer be marked as conflicting. The data about the conflict will be /// moved to the "resolve undo" (REUC) section. pub fn add_frombuffer(&mut self, entry: &IndexEntry, data: &[u8]) -> Result<(), Error> { - let path = CString::new(&entry.path[..])?; - - // libgit2 encodes the length of the path in the lower bits of the - // `flags` entry, so mask those out and recalculate here to ensure we - // don't corrupt anything. - let mut flags = entry.flags & !raw::GIT_INDEX_ENTRY_NAMEMASK; - - if entry.path.len() < raw::GIT_INDEX_ENTRY_NAMEMASK as usize { - flags |= entry.path.len() as u16; - } else { - flags |= raw::GIT_INDEX_ENTRY_NAMEMASK; - } - - unsafe { - let raw = raw::git_index_entry { - dev: entry.dev, - ino: entry.ino, - mode: entry.mode, - uid: entry.uid, - gid: entry.gid, - file_size: entry.file_size, - id: *entry.id.raw(), - flags, - flags_extended: entry.flags_extended, - path: path.as_ptr(), - mtime: raw::git_index_time { - seconds: entry.mtime.seconds(), - nanoseconds: entry.mtime.nanoseconds(), - }, - ctime: raw::git_index_time { - seconds: entry.ctime.seconds(), - nanoseconds: entry.ctime.nanoseconds(), - }, - }; - - let ptr = data.as_ptr() as *const c_void; - let len = data.len() as size_t; - try_call!(raw::git_index_add_frombuffer(self.raw, &raw, ptr, len)); + try_raw_entries(&[Some(entry)], |raws| { + unsafe { + let ptr = data.as_ptr() as *const c_void; + let len = data.len() as size_t; + try_call!(raw::git_index_add_frombuffer(self.raw, raws[0], ptr, len)); + } Ok(()) - } + }) } /// Add or update an index entry from a file on disk @@ -425,124 +426,17 @@ impl Index { our_entry: Option<&IndexEntry>, their_entry: Option<&IndexEntry>, ) -> Result<(), Error> { - let mut ancestor_raw: Option = None; - let mut our_raw: Option = None; - let mut their_raw: Option = None; - - if let Some(ancestor_entry) = ancestor_entry { - let ancestor_path = CString::new(&ancestor_entry.path[..])?; - let mut ancestor_flags = ancestor_entry.flags & !raw::GIT_INDEX_ENTRY_NAMEMASK; - - if ancestor_entry.path.len() < raw::GIT_INDEX_ENTRY_NAMEMASK as usize { - ancestor_flags |= ancestor_entry.path.len() as u16; - } else { - ancestor_flags |= raw::GIT_INDEX_ENTRY_NAMEMASK; - } - - unsafe { - ancestor_raw = Some(raw::git_index_entry { - dev: ancestor_entry.dev, - ino: ancestor_entry.ino, - mode: ancestor_entry.mode, - uid: ancestor_entry.uid, - gid: ancestor_entry.gid, - file_size: ancestor_entry.file_size, - id: *ancestor_entry.id.raw(), - flags: ancestor_flags, - flags_extended: ancestor_entry.flags_extended, - path: ancestor_path.as_ptr(), - mtime: raw::git_index_time { - seconds: ancestor_entry.mtime.seconds(), - nanoseconds: ancestor_entry.mtime.nanoseconds(), - }, - ctime: raw::git_index_time { - seconds: ancestor_entry.ctime.seconds(), - nanoseconds: ancestor_entry.ctime.nanoseconds(), - }, - }); - } - } - - if let Some(our_entry) = our_entry { - let our_path = CString::new(&our_entry.path[..])?; - let mut our_flags = our_entry.flags & !raw::GIT_INDEX_ENTRY_NAMEMASK; - - if our_entry.path.len() < raw::GIT_INDEX_ENTRY_NAMEMASK as usize { - our_flags |= our_entry.path.len() as u16; - } else { - our_flags |= raw::GIT_INDEX_ENTRY_NAMEMASK; - } - - unsafe { - our_raw = Some(raw::git_index_entry { - dev: our_entry.dev, - ino: our_entry.ino, - mode: our_entry.mode, - uid: our_entry.uid, - gid: our_entry.gid, - file_size: our_entry.file_size, - id: *our_entry.id.raw(), - flags: our_flags, - flags_extended: our_entry.flags_extended, - path: our_path.as_ptr(), - mtime: raw::git_index_time { - seconds: our_entry.mtime.seconds(), - nanoseconds: our_entry.mtime.nanoseconds(), - }, - ctime: raw::git_index_time { - seconds: our_entry.ctime.seconds(), - nanoseconds: our_entry.ctime.nanoseconds(), - }, - }); - } - } - - if let Some(their_entry) = their_entry { - let their_path = CString::new(&their_entry.path[..])?; - let mut their_flags = their_entry.flags & !raw::GIT_INDEX_ENTRY_NAMEMASK; - - if their_entry.path.len() < raw::GIT_INDEX_ENTRY_NAMEMASK as usize { - their_flags |= their_entry.path.len() as u16; - } else { - their_flags |= raw::GIT_INDEX_ENTRY_NAMEMASK; - } - + try_raw_entries(&[ancestor_entry, our_entry, their_entry], |raw_entries| { unsafe { - their_raw = Some(raw::git_index_entry { - dev: their_entry.dev, - ino: their_entry.ino, - mode: their_entry.mode, - uid: their_entry.uid, - gid: their_entry.gid, - file_size: their_entry.file_size, - id: *their_entry.id.raw(), - flags: their_flags, - flags_extended: their_entry.flags_extended, - path: their_path.as_ptr(), - mtime: raw::git_index_time { - seconds: their_entry.mtime.seconds(), - nanoseconds: their_entry.mtime.nanoseconds(), - }, - ctime: raw::git_index_time { - seconds: their_entry.ctime.seconds(), - nanoseconds: their_entry.ctime.nanoseconds(), - }, - }); + try_call!(raw::git_index_conflict_add( + self.raw, + raw_entries[0], + raw_entries[1], + raw_entries[2] + )); + Ok(()) } - } - - let ancestor_raw_ptr = ancestor_raw.as_ref().map_or_else(std::ptr::null, |ptr| ptr); - let our_raw_ptr = our_raw.as_ref().map_or_else(std::ptr::null, |ptr| ptr); - let their_raw_ptr = their_raw.as_ref().map_or_else(std::ptr::null, |ptr| ptr); - unsafe { - try_call!(raw::git_index_conflict_add( - self.raw, - ancestor_raw_ptr, - our_raw_ptr, - their_raw_ptr - )); - Ok(()) - } + }) } /// Remove all conflicts in the index (entries with a stage greater than 0). From 3a85808f49e55fe295d993dae006988020933588 Mon Sep 17 00:00:00 2001 From: Fin Christensen Date: Thu, 2 Dec 2021 16:13:23 +0100 Subject: [PATCH 4/9] Format code, ci was failing --- src/index.rs | 93 ++++++++++++++++++++++++++++------------------------ 1 file changed, 50 insertions(+), 43 deletions(-) diff --git a/src/index.rs b/src/index.rs index 7296ad4728..513ae9f47e 100644 --- a/src/index.rs +++ b/src/index.rs @@ -93,7 +93,8 @@ fn try_raw_entries( entries: &[Option<&IndexEntry>], cb: impl FnOnce(&[*const raw::git_index_entry]) -> Result<(), Error>, ) -> Result<(), Error> { - let paths = entries.iter() + let paths = entries + .iter() .map(|entry| { if let Some(entry) = entry { CString::new(&entry.path[..]).map(|ok| Some(ok)) @@ -103,46 +104,51 @@ fn try_raw_entries( }) .collect::>, std::ffi::NulError>>()?; - let raw_entries = entries.iter().zip(&paths).map(|(entry, path)| { - if let Some(entry) = entry { - // libgit2 encodes the length of the path in the lower bits of the - // `flags` entry, so mask those out and recalculate here to ensure we - // don't corrupt anything. - let mut flags = entry.flags & !raw::GIT_INDEX_ENTRY_NAMEMASK; - - if entry.path.len() < raw::GIT_INDEX_ENTRY_NAMEMASK as usize { - flags |= entry.path.len() as u16; + let raw_entries = entries + .iter() + .zip(&paths) + .map(|(entry, path)| { + if let Some(entry) = entry { + // libgit2 encodes the length of the path in the lower bits of the + // `flags` entry, so mask those out and recalculate here to ensure we + // don't corrupt anything. + let mut flags = entry.flags & !raw::GIT_INDEX_ENTRY_NAMEMASK; + + if entry.path.len() < raw::GIT_INDEX_ENTRY_NAMEMASK as usize { + flags |= entry.path.len() as u16; + } else { + flags |= raw::GIT_INDEX_ENTRY_NAMEMASK; + } + + unsafe { + Some(raw::git_index_entry { + dev: entry.dev, + ino: entry.ino, + mode: entry.mode, + uid: entry.uid, + gid: entry.gid, + file_size: entry.file_size, + id: *entry.id.raw(), + flags, + flags_extended: entry.flags_extended, + path: path.as_ref().unwrap().as_ptr(), + mtime: raw::git_index_time { + seconds: entry.mtime.seconds(), + nanoseconds: entry.mtime.nanoseconds(), + }, + ctime: raw::git_index_time { + seconds: entry.ctime.seconds(), + nanoseconds: entry.ctime.nanoseconds(), + }, + }) + } } else { - flags |= raw::GIT_INDEX_ENTRY_NAMEMASK; - } - - unsafe { - Some(raw::git_index_entry { - dev: entry.dev, - ino: entry.ino, - mode: entry.mode, - uid: entry.uid, - gid: entry.gid, - file_size: entry.file_size, - id: *entry.id.raw(), - flags, - flags_extended: entry.flags_extended, - path: path.as_ref().unwrap().as_ptr(), - mtime: raw::git_index_time { - seconds: entry.mtime.seconds(), - nanoseconds: entry.mtime.nanoseconds(), - }, - ctime: raw::git_index_time { - seconds: entry.ctime.seconds(), - nanoseconds: entry.ctime.nanoseconds(), - }, - }) + None } - } else { - None - } - }).collect::>(); - let raw_entry_ptrs = raw_entries.iter() + }) + .collect::>(); + let raw_entry_ptrs = raw_entries + .iter() .map(|opt| opt.as_ref().map_or_else(std::ptr::null, |ptr| ptr)) .collect::>(); @@ -426,8 +432,9 @@ impl Index { our_entry: Option<&IndexEntry>, their_entry: Option<&IndexEntry>, ) -> Result<(), Error> { - try_raw_entries(&[ancestor_entry, our_entry, their_entry], |raw_entries| { - unsafe { + try_raw_entries( + &[ancestor_entry, our_entry, their_entry], + |raw_entries| unsafe { try_call!(raw::git_index_conflict_add( self.raw, raw_entries[0], @@ -435,8 +442,8 @@ impl Index { raw_entries[2] )); Ok(()) - } - }) + }, + ) } /// Remove all conflicts in the index (entries with a stage greater than 0). From 68e7cfc1c546c39f4f33ab83cf9e1f89f3f9fd59 Mon Sep 17 00:00:00 2001 From: Fin Christensen Date: Thu, 2 Dec 2021 20:31:34 +0100 Subject: [PATCH 5/9] Make try_raw_entries stack allocated --- src/index.rs | 118 ++++++++++++++++++++++++++------------------------- 1 file changed, 60 insertions(+), 58 deletions(-) diff --git a/src/index.rs b/src/index.rs index 513ae9f47e..16479918e6 100644 --- a/src/index.rs +++ b/src/index.rs @@ -89,68 +89,70 @@ pub struct IndexEntry { // a CString which is owned by the function. To make the pointer to the CString // valid during usage of raw::git_index_entry, we supply the index entry in a // callback where pointers to the CString are valid. -fn try_raw_entries( - entries: &[Option<&IndexEntry>], - cb: impl FnOnce(&[*const raw::git_index_entry]) -> Result<(), Error>, +fn try_raw_entries( + entries: &[Option<&IndexEntry>; N], + cb: impl FnOnce(&[*const raw::git_index_entry; N]) -> Result<(), Error>, ) -> Result<(), Error> { - let paths = entries - .iter() - .map(|entry| { - if let Some(entry) = entry { - CString::new(&entry.path[..]).map(|ok| Some(ok)) + let mut paths: [Option; N] = unsafe { + std::mem::MaybeUninit::uninit().assume_init() + }; + for (idx, entry) in entries.iter().enumerate() { + paths[idx] = if let Some(entry) = entry { + Some(CString::new(&entry.path[..])?) + } else { + None + } + } + + let mut raw_entries: [Option; N] = unsafe { + std::mem::MaybeUninit::uninit().assume_init() + }; + for (idx, (entry, path)) in entries.iter().zip(&paths).enumerate() { + raw_entries[idx] = if let Some(entry) = entry { + // libgit2 encodes the length of the path in the lower bits of the + // `flags` entry, so mask those out and recalculate here to ensure we + // don't corrupt anything. + let mut flags = entry.flags & !raw::GIT_INDEX_ENTRY_NAMEMASK; + + if entry.path.len() < raw::GIT_INDEX_ENTRY_NAMEMASK as usize { + flags |= entry.path.len() as u16; } else { - Ok(None) + flags |= raw::GIT_INDEX_ENTRY_NAMEMASK; } - }) - .collect::>, std::ffi::NulError>>()?; - - let raw_entries = entries - .iter() - .zip(&paths) - .map(|(entry, path)| { - if let Some(entry) = entry { - // libgit2 encodes the length of the path in the lower bits of the - // `flags` entry, so mask those out and recalculate here to ensure we - // don't corrupt anything. - let mut flags = entry.flags & !raw::GIT_INDEX_ENTRY_NAMEMASK; - - if entry.path.len() < raw::GIT_INDEX_ENTRY_NAMEMASK as usize { - flags |= entry.path.len() as u16; - } else { - flags |= raw::GIT_INDEX_ENTRY_NAMEMASK; - } - - unsafe { - Some(raw::git_index_entry { - dev: entry.dev, - ino: entry.ino, - mode: entry.mode, - uid: entry.uid, - gid: entry.gid, - file_size: entry.file_size, - id: *entry.id.raw(), - flags, - flags_extended: entry.flags_extended, - path: path.as_ref().unwrap().as_ptr(), - mtime: raw::git_index_time { - seconds: entry.mtime.seconds(), - nanoseconds: entry.mtime.nanoseconds(), - }, - ctime: raw::git_index_time { - seconds: entry.ctime.seconds(), - nanoseconds: entry.ctime.nanoseconds(), - }, - }) - } - } else { - None + + unsafe { + Some(raw::git_index_entry { + dev: entry.dev, + ino: entry.ino, + mode: entry.mode, + uid: entry.uid, + gid: entry.gid, + file_size: entry.file_size, + id: *entry.id.raw(), + flags, + flags_extended: entry.flags_extended, + path: path.as_ref().unwrap().as_ptr(), + mtime: raw::git_index_time { + seconds: entry.mtime.seconds(), + nanoseconds: entry.mtime.nanoseconds(), + }, + ctime: raw::git_index_time { + seconds: entry.ctime.seconds(), + nanoseconds: entry.ctime.nanoseconds(), + }, + }) } - }) - .collect::>(); - let raw_entry_ptrs = raw_entries - .iter() - .map(|opt| opt.as_ref().map_or_else(std::ptr::null, |ptr| ptr)) - .collect::>(); + } else { + None + } + } + + let mut raw_entry_ptrs: [*const raw::git_index_entry; N] = unsafe { + std::mem::MaybeUninit::uninit().assume_init() + }; + for (idx, entry) in raw_entries.iter().enumerate() { + raw_entry_ptrs[idx] = entry.as_ref().map_or_else(std::ptr::null, |ptr| ptr); + } cb(&raw_entry_ptrs) } From 35b8804da2f408ab80b1be77c0305aaa4c26616f Mon Sep 17 00:00:00 2001 From: Fin Christensen Date: Thu, 2 Dec 2021 21:57:44 +0100 Subject: [PATCH 6/9] Initialize array without dropping uninitialized contents, fixes CI --- src/index.rs | 67 +++++++++++++++++++++++++++++----------------------- 1 file changed, 37 insertions(+), 30 deletions(-) diff --git a/src/index.rs b/src/index.rs index 16479918e6..9af4ff712f 100644 --- a/src/index.rs +++ b/src/index.rs @@ -96,19 +96,22 @@ fn try_raw_entries( let mut paths: [Option; N] = unsafe { std::mem::MaybeUninit::uninit().assume_init() }; - for (idx, entry) in entries.iter().enumerate() { - paths[idx] = if let Some(entry) = entry { + for (path, entry) in paths.iter_mut().zip(entries.iter()) { + let c_path = if let Some(entry) = entry { Some(CString::new(&entry.path[..])?) } else { None - } + }; + + let path_ptr: *mut Option = path; + unsafe { path_ptr.write(c_path); } } let mut raw_entries: [Option; N] = unsafe { std::mem::MaybeUninit::uninit().assume_init() }; - for (idx, (entry, path)) in entries.iter().zip(&paths).enumerate() { - raw_entries[idx] = if let Some(entry) = entry { + for (raw_entry, (entry, path)) in raw_entries.iter_mut().zip(entries.iter().zip(&paths)) { + let c_raw_entry = if let Some(entry) = entry { // libgit2 encodes the length of the path in the lower bits of the // `flags` entry, so mask those out and recalculate here to ensure we // don't corrupt anything. @@ -120,38 +123,42 @@ fn try_raw_entries( flags |= raw::GIT_INDEX_ENTRY_NAMEMASK; } - unsafe { - Some(raw::git_index_entry { - dev: entry.dev, - ino: entry.ino, - mode: entry.mode, - uid: entry.uid, - gid: entry.gid, - file_size: entry.file_size, - id: *entry.id.raw(), - flags, - flags_extended: entry.flags_extended, - path: path.as_ref().unwrap().as_ptr(), - mtime: raw::git_index_time { - seconds: entry.mtime.seconds(), - nanoseconds: entry.mtime.nanoseconds(), - }, - ctime: raw::git_index_time { - seconds: entry.ctime.seconds(), - nanoseconds: entry.ctime.nanoseconds(), - }, - }) - } + Some(raw::git_index_entry { + dev: entry.dev, + ino: entry.ino, + mode: entry.mode, + uid: entry.uid, + gid: entry.gid, + file_size: entry.file_size, + id: unsafe { *entry.id.raw() }, + flags, + flags_extended: entry.flags_extended, + path: path.as_ref().unwrap().as_ptr(), + mtime: raw::git_index_time { + seconds: entry.mtime.seconds(), + nanoseconds: entry.mtime.nanoseconds(), + }, + ctime: raw::git_index_time { + seconds: entry.ctime.seconds(), + nanoseconds: entry.ctime.nanoseconds(), + }, + }) } else { None - } + }; + + let raw_entry_ptr: *mut Option = raw_entry; + unsafe { raw_entry_ptr.write(c_raw_entry); } } let mut raw_entry_ptrs: [*const raw::git_index_entry; N] = unsafe { std::mem::MaybeUninit::uninit().assume_init() }; - for (idx, entry) in raw_entries.iter().enumerate() { - raw_entry_ptrs[idx] = entry.as_ref().map_or_else(std::ptr::null, |ptr| ptr); + for (raw_entry_ptr, raw_entry) in raw_entry_ptrs.iter_mut().zip(raw_entries.iter()) { + let c_raw_entry_ptr = raw_entry.as_ref().map_or_else(std::ptr::null, |ptr| ptr); + + let raw_entry_ptr_ptr: *mut *const raw::git_index_entry = raw_entry_ptr; + unsafe { raw_entry_ptr_ptr.write(c_raw_entry_ptr); } } cb(&raw_entry_ptrs) From b0ab4ee7b29565017c3954ccaaea50e688642aec Mon Sep 17 00:00:00 2001 From: Fin Christensen Date: Thu, 2 Dec 2021 21:58:22 +0100 Subject: [PATCH 7/9] Format code, forgot again... --- src/index.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/index.rs b/src/index.rs index 9af4ff712f..61dd4261a8 100644 --- a/src/index.rs +++ b/src/index.rs @@ -93,9 +93,7 @@ fn try_raw_entries( entries: &[Option<&IndexEntry>; N], cb: impl FnOnce(&[*const raw::git_index_entry; N]) -> Result<(), Error>, ) -> Result<(), Error> { - let mut paths: [Option; N] = unsafe { - std::mem::MaybeUninit::uninit().assume_init() - }; + let mut paths: [Option; N] = unsafe { std::mem::MaybeUninit::uninit().assume_init() }; for (path, entry) in paths.iter_mut().zip(entries.iter()) { let c_path = if let Some(entry) = entry { Some(CString::new(&entry.path[..])?) @@ -104,12 +102,13 @@ fn try_raw_entries( }; let path_ptr: *mut Option = path; - unsafe { path_ptr.write(c_path); } + unsafe { + path_ptr.write(c_path); + } } - let mut raw_entries: [Option; N] = unsafe { - std::mem::MaybeUninit::uninit().assume_init() - }; + let mut raw_entries: [Option; N] = + unsafe { std::mem::MaybeUninit::uninit().assume_init() }; for (raw_entry, (entry, path)) in raw_entries.iter_mut().zip(entries.iter().zip(&paths)) { let c_raw_entry = if let Some(entry) = entry { // libgit2 encodes the length of the path in the lower bits of the @@ -148,17 +147,20 @@ fn try_raw_entries( }; let raw_entry_ptr: *mut Option = raw_entry; - unsafe { raw_entry_ptr.write(c_raw_entry); } + unsafe { + raw_entry_ptr.write(c_raw_entry); + } } - let mut raw_entry_ptrs: [*const raw::git_index_entry; N] = unsafe { - std::mem::MaybeUninit::uninit().assume_init() - }; + let mut raw_entry_ptrs: [*const raw::git_index_entry; N] = + unsafe { std::mem::MaybeUninit::uninit().assume_init() }; for (raw_entry_ptr, raw_entry) in raw_entry_ptrs.iter_mut().zip(raw_entries.iter()) { let c_raw_entry_ptr = raw_entry.as_ref().map_or_else(std::ptr::null, |ptr| ptr); let raw_entry_ptr_ptr: *mut *const raw::git_index_entry = raw_entry_ptr; - unsafe { raw_entry_ptr_ptr.write(c_raw_entry_ptr); } + unsafe { + raw_entry_ptr_ptr.write(c_raw_entry_ptr); + } } cb(&raw_entry_ptrs) From b2eb90eb22a20378ff046441a6c2f4b1d64f9cd3 Mon Sep 17 00:00:00 2001 From: Fin Christensen Date: Fri, 3 Dec 2021 09:44:35 +0100 Subject: [PATCH 8/9] Implement callback-less helper function --- src/index.rs | 200 +++++++++++++++++++++++++-------------------------- 1 file changed, 100 insertions(+), 100 deletions(-) diff --git a/src/index.rs b/src/index.rs index 61dd4261a8..bfaf65ce5d 100644 --- a/src/index.rs +++ b/src/index.rs @@ -85,85 +85,41 @@ pub struct IndexEntry { pub path: Vec, } -// We cannot return raw::git_index_entry instances directly, as they rely on -// a CString which is owned by the function. To make the pointer to the CString -// valid during usage of raw::git_index_entry, we supply the index entry in a -// callback where pointers to the CString are valid. -fn try_raw_entries( - entries: &[Option<&IndexEntry>; N], - cb: impl FnOnce(&[*const raw::git_index_entry; N]) -> Result<(), Error>, -) -> Result<(), Error> { - let mut paths: [Option; N] = unsafe { std::mem::MaybeUninit::uninit().assume_init() }; - for (path, entry) in paths.iter_mut().zip(entries.iter()) { - let c_path = if let Some(entry) = entry { - Some(CString::new(&entry.path[..])?) +impl IndexEntry { + // Expecting c_path to contain a CString with the contents of self.path + fn to_raw(&self, c_path: &CString) -> raw::git_index_entry { + // libgit2 encodes the length of the path in the lower bits of the + // `flags` entry, so mask those out and recalculate here to ensure we + // don't corrupt anything. + let mut flags = self.flags & !raw::GIT_INDEX_ENTRY_NAMEMASK; + + if self.path.len() < raw::GIT_INDEX_ENTRY_NAMEMASK as usize { + flags |= self.path.len() as u16; } else { - None - }; - - let path_ptr: *mut Option = path; - unsafe { - path_ptr.write(c_path); + flags |= raw::GIT_INDEX_ENTRY_NAMEMASK; } - } - - let mut raw_entries: [Option; N] = - unsafe { std::mem::MaybeUninit::uninit().assume_init() }; - for (raw_entry, (entry, path)) in raw_entries.iter_mut().zip(entries.iter().zip(&paths)) { - let c_raw_entry = if let Some(entry) = entry { - // libgit2 encodes the length of the path in the lower bits of the - // `flags` entry, so mask those out and recalculate here to ensure we - // don't corrupt anything. - let mut flags = entry.flags & !raw::GIT_INDEX_ENTRY_NAMEMASK; - - if entry.path.len() < raw::GIT_INDEX_ENTRY_NAMEMASK as usize { - flags |= entry.path.len() as u16; - } else { - flags |= raw::GIT_INDEX_ENTRY_NAMEMASK; - } - Some(raw::git_index_entry { - dev: entry.dev, - ino: entry.ino, - mode: entry.mode, - uid: entry.uid, - gid: entry.gid, - file_size: entry.file_size, - id: unsafe { *entry.id.raw() }, - flags, - flags_extended: entry.flags_extended, - path: path.as_ref().unwrap().as_ptr(), - mtime: raw::git_index_time { - seconds: entry.mtime.seconds(), - nanoseconds: entry.mtime.nanoseconds(), - }, - ctime: raw::git_index_time { - seconds: entry.ctime.seconds(), - nanoseconds: entry.ctime.nanoseconds(), - }, - }) - } else { - None - }; - - let raw_entry_ptr: *mut Option = raw_entry; - unsafe { - raw_entry_ptr.write(c_raw_entry); - } - } - - let mut raw_entry_ptrs: [*const raw::git_index_entry; N] = - unsafe { std::mem::MaybeUninit::uninit().assume_init() }; - for (raw_entry_ptr, raw_entry) in raw_entry_ptrs.iter_mut().zip(raw_entries.iter()) { - let c_raw_entry_ptr = raw_entry.as_ref().map_or_else(std::ptr::null, |ptr| ptr); - - let raw_entry_ptr_ptr: *mut *const raw::git_index_entry = raw_entry_ptr; - unsafe { - raw_entry_ptr_ptr.write(c_raw_entry_ptr); + raw::git_index_entry { + dev: self.dev, + ino: self.ino, + mode: self.mode, + uid: self.uid, + gid: self.gid, + file_size: self.file_size, + id: unsafe { *self.id.raw() }, + flags, + flags_extended: self.flags_extended, + path: c_path.as_ptr(), + mtime: raw::git_index_time { + seconds: self.mtime.seconds(), + nanoseconds: self.mtime.nanoseconds(), + }, + ctime: raw::git_index_time { + seconds: self.ctime.seconds(), + nanoseconds: self.ctime.nanoseconds(), + }, } } - - cb(&raw_entry_ptrs) } impl Index { @@ -226,12 +182,13 @@ impl Index { /// given 'source_entry', it will be replaced. Otherwise, the 'source_entry' /// will be added. pub fn add(&mut self, entry: &IndexEntry) -> Result<(), Error> { - try_raw_entries(&[Some(entry)], |raws| { - unsafe { - try_call!(raw::git_index_add(self.raw, raws[0])); - } - Ok(()) - }) + let c_path = CString::new(&entry.path[..])?; + let raw_entry = entry.to_raw(&c_path); + + unsafe { + try_call!(raw::git_index_add(self.raw, &raw_entry)); + } + Ok(()) } /// Add or update an index entry from a buffer in memory @@ -252,14 +209,17 @@ impl Index { /// no longer be marked as conflicting. The data about the conflict will be /// moved to the "resolve undo" (REUC) section. pub fn add_frombuffer(&mut self, entry: &IndexEntry, data: &[u8]) -> Result<(), Error> { - try_raw_entries(&[Some(entry)], |raws| { - unsafe { - let ptr = data.as_ptr() as *const c_void; - let len = data.len() as size_t; - try_call!(raw::git_index_add_frombuffer(self.raw, raws[0], ptr, len)); - } - Ok(()) - }) + let c_path = CString::new(&entry.path[..])?; + let raw_entry = entry.to_raw(&c_path); + + unsafe { + let ptr = data.as_ptr() as *const c_void; + let len = data.len() as size_t; + try_call!(raw::git_index_add_frombuffer( + self.raw, &raw_entry, ptr, len + )); + } + Ok(()) } /// Add or update an index entry from a file on disk @@ -443,18 +403,58 @@ impl Index { our_entry: Option<&IndexEntry>, their_entry: Option<&IndexEntry>, ) -> Result<(), Error> { - try_raw_entries( - &[ancestor_entry, our_entry, their_entry], - |raw_entries| unsafe { - try_call!(raw::git_index_conflict_add( - self.raw, - raw_entries[0], - raw_entries[1], - raw_entries[2] - )); - Ok(()) - }, - ) + let ancestor_c_path = if let Some(entry) = ancestor_entry { + Some(CString::new(&entry.path[..])?) + } else { + None + }; + let our_c_path = if let Some(entry) = our_entry { + Some(CString::new(&entry.path[..])?) + } else { + None + }; + let their_c_path = if let Some(entry) = their_entry { + Some(CString::new(&entry.path[..])?) + } else { + None + }; + + let mut raw_ancestor_entry = None; + let mut raw_our_entry = None; + let mut raw_their_entry = None; + + if let Some(entry) = ancestor_entry { + let c_path = ancestor_c_path.as_ref().unwrap(); + raw_ancestor_entry = Some(entry.to_raw(&c_path)); + } + if let Some(entry) = our_entry { + let c_path = our_c_path.as_ref().unwrap(); + raw_our_entry = Some(entry.to_raw(&c_path)); + } + if let Some(entry) = their_entry { + let c_path = their_c_path.as_ref().unwrap(); + raw_their_entry = Some(entry.to_raw(&c_path)); + } + + let raw_ancestor_entry_ptr = raw_ancestor_entry + .as_ref() + .map_or_else(std::ptr::null, |ptr| ptr); + let raw_our_entry_ptr = raw_our_entry + .as_ref() + .map_or_else(std::ptr::null, |ptr| ptr); + let raw_their_entry_ptr = raw_their_entry + .as_ref() + .map_or_else(std::ptr::null, |ptr| ptr); + + unsafe { + try_call!(raw::git_index_conflict_add( + self.raw, + raw_ancestor_entry_ptr, + raw_our_entry_ptr, + raw_their_entry_ptr + )); + } + Ok(()) } /// Remove all conflicts in the index (entries with a stage greater than 0). From b548535af418b231091a5f89014dd4483888cf9d Mon Sep 17 00:00:00 2001 From: Fin Christensen Date: Sat, 19 Mar 2022 18:10:28 +0100 Subject: [PATCH 9/9] Add libgit2-sys to workspace members --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 27d9a92638..a089110add 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,4 +43,4 @@ ssh_key_from_memory = ["libgit2-sys/ssh_key_from_memory"] zlib-ng-compat = ["libgit2-sys/zlib-ng-compat"] [workspace] -members = ["systest", "git2-curl"] +members = ["systest", "git2-curl", "libgit2-sys"]