Merge pull request GitoxideLabs#2433 from GitoxideLabs/codex/nonempty-rewrite

Byron · web-flow · commit 29040a827773 · 2026-02-15T16:17:39.000+01:00
Use nonempty where possible
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/gix-commitgraph/Cargo.toml b/gix-commitgraph/Cargo.toml
@@ -26,6 +26,7 @@ gix-error = { version = "^0.1.0", path = "../gix-error" }
 
 bstr = { version = "1.12.0", default-features = false, features = ["std"] }
 memmap2 = "0.9.7"
+nonempty = "0.12.0"
 serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] }
 
 document-features = { version = "0.2.0", optional = true }
diff --git a/gix-commitgraph/src/access.rs b/gix-commitgraph/src/access.rs
@@ -14,15 +14,8 @@ impl Graph {
     /// The kind of hash used in this `Graph`.
     ///
     /// Note that it is always conforming to the hash used in the owning repository.
-    ///
-    /// # Panics
-    ///
-    /// If the graph does not contain any `File`.
     pub fn object_hash(&self) -> gix_hash::Kind {
-        self.files
-            .first()
-            .map(super::File::object_hash)
-            .expect("graph to have at least one file")
+        self.files.first().object_hash()
     }
 
     /// Returns the commit matching the given `id`.
diff --git a/gix-commitgraph/src/init.rs b/gix-commitgraph/src/init.rs
@@ -54,16 +54,17 @@ impl Graph {
 
     /// Create a new commit graph from a list of `files`.
     pub fn new(files: Vec<File>) -> Result<Self, Message> {
+        let files = nonempty::NonEmpty::from_vec(files)
+            .ok_or_else(|| message!("Commit-graph must contain at least one file"))?;
         let num_commits: u64 = files.iter().map(|f| u64::from(f.num_commits())).sum();
         if num_commits > u64::from(MAX_COMMITS) {
             return Err(message!(
                 "Commit-graph files contain {num_commits} commits altogether, but only {MAX_COMMITS} commits are allowed"
             ));
         }
 
-        for window in files.windows(2) {
-            let f1 = &window[0];
-            let f2 = &window[1];
+        let mut f1 = files.first();
+        for f2 in files.tail() {
             if f1.object_hash() != f2.object_hash() {
                 return Err(message!(
                     "Commit-graph files mismatch: '{path1}' uses hash {hash1:?}, but '{path2}' uses hash {hash2:?}",
@@ -73,6 +74,7 @@ impl Graph {
                     hash2 = f2.object_hash(),
                 ));
             }
+            f1 = f2;
         }
 
         Ok(Self { files })
diff --git a/gix-commitgraph/src/lib.rs b/gix-commitgraph/src/lib.rs
@@ -41,7 +41,7 @@ pub struct File {
 /// may come from one or more `objects/info/commit-graphs/graph-*.graph` files. These files are
 /// generated via `git commit-graph write ...` commands.
 pub struct Graph {
-    files: Vec<File>,
+    files: nonempty::NonEmpty<File>,
 }
 
 /// Instantiate a commit graph from an `.git/objects/info` directory, or one of the various commit-graph files.
diff --git a/gix-commitgraph/src/verify.rs b/gix-commitgraph/src/verify.rs
@@ -69,8 +69,10 @@ impl Graph {
                 .raise());
             }
 
-            for (base_graph_index, (expected, actual)) in self.files[..file_index]
+            for (base_graph_index, (expected, actual)) in self
+                .files
                 .iter()
+                .take(file_index)
                 .map(crate::File::checksum)
                 .zip(file.iter_base_graph_ids())
                 .enumerate()
diff --git a/gix-merge/Cargo.toml b/gix-merge/Cargo.toml
@@ -37,6 +37,7 @@ gix-index = { version = "^0.47.0", path = "../gix-index" }
 thiserror = "2.0.18"
 imara-diff = { version = "0.1.8" }
 bstr = { version = "1.12.0", default-features = false }
+nonempty = "0.12.0"
 serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] }
 
 document-features = { version = "0.2.0", optional = true }
diff --git a/gix-merge/src/commit/function.rs b/gix-merge/src/commit/function.rs
@@ -60,14 +60,15 @@ pub fn commit<'objects>(
         |commit_id: gix_hash::ObjectId| objects.find_commit(&commit_id, &mut state.buf1).map(|c| c.tree());
 
     let (merge_base_tree_id, ancestor_name): (_, Cow<'_, str>) = match merge_bases.clone() {
-        Some(base_commit) if base_commit.len() == 1 => {
-            (commit_to_tree(base_commit[0])?, abbreviate_hash(&base_commit[0]).into())
-        }
-        Some(mut base_commits) => {
+        Some(base_commit) if base_commit.len() == 1 => (
+            commit_to_tree(*base_commit.first())?,
+            abbreviate_hash(base_commit.first()).into(),
+        ),
+        Some(base_commits) => {
             let virtual_base_tree = if options.use_first_merge_base {
-                let first = base_commits.first().expect("if Some() there is at least one.");
-                commit_to_tree(*first)?
+                commit_to_tree(*base_commits.first())?
             } else {
+                let mut base_commits: Vec<_> = base_commits.into();
                 let first = base_commits.pop().expect("at least two");
                 let second = base_commits.pop().expect("at least one left");
                 let out = crate::commit::virtual_merge_base(
@@ -81,7 +82,7 @@ pub fn commit<'objects>(
                     abbreviate_hash,
                     options.tree_merge.clone(),
                 )?;
-                virtual_merge_bases = out.virtual_merge_bases;
+                virtual_merge_bases = Vec::from(out.virtual_merge_bases);
                 out.tree_id
             };
             (virtual_base_tree, "merged common ancestors".into())
diff --git a/gix-merge/src/commit/mod.rs b/gix-merge/src/commit/mod.rs
@@ -44,7 +44,7 @@ pub struct Outcome<'a> {
     /// * an empty tree, if [`allow_missing_merge_base`](Options::allow_missing_merge_base) is enabled.
     pub merge_base_tree_id: gix_hash::ObjectId,
     /// The object ids of all the commits which were found to be merge-bases, or `None` if there was no merge-base.
-    pub merge_bases: Option<Vec<gix_hash::ObjectId>>,
+    pub merge_bases: Option<nonempty::NonEmpty<gix_hash::ObjectId>>,
     /// A list of virtual commits that were created to merge multiple merge-bases into one, the last one being
     /// the one we used as merge-base for the merge.
     /// As they are not reachable by anything they will be garbage collected, but knowing them provides options.
diff --git a/gix-merge/src/commit/virtual_merge_base.rs b/gix-merge/src/commit/virtual_merge_base.rs
@@ -4,7 +4,7 @@ pub struct Outcome {
     /// As they have been written to the object database, they are still available until they are garbage collected.
     /// The last one is the most recently produced and the one returned as `commit_id`.
     /// This is never empty.
-    pub virtual_merge_bases: Vec<gix_hash::ObjectId>,
+    pub virtual_merge_bases: nonempty::NonEmpty<gix_hash::ObjectId>,
     /// The id of the commit that was created to hold the merged tree.
     pub commit_id: gix_hash::ObjectId,
     /// The hash of the merged tree.
@@ -111,7 +111,8 @@ pub(super) mod function {
         }
 
         Ok(super::Outcome {
-            virtual_merge_bases,
+            virtual_merge_bases: nonempty::NonEmpty::from_vec(virtual_merge_bases)
+                .expect("the virtual merge-base process always creates at least one commit"),
             commit_id: merged_commit_id,
             tree_id: tree_id.map_or_else(
                 || {
diff --git a/gix-odb/src/store_impls/dynamic/iter.rs b/gix-odb/src/store_impls/dynamic/iter.rs
@@ -54,9 +54,13 @@ pub enum Ordering {
 ///
 /// Use [`with_ordering()`][AllObjects::with_ordering()] to choose a performance trade-off.
 pub struct AllObjects {
+    /// The current iteration state (pack, loose, or depleted)
     state: State,
+    /// The total number of packed objects (used for size hint)
     num_objects: usize,
+    /// Non-empty collection of loose object databases to iterate through
     loose_dbs: Arc<Vec<loose::Store>>,
+    /// The ordering strategy for returning objects
     order: Ordering,
 }
 
diff --git a/gix-protocol/Cargo.toml b/gix-protocol/Cargo.toml
@@ -87,6 +87,7 @@ gix-refspec = { version = "^0.37.0", path = "../gix-refspec", optional = true }
 gix-lock = { version = "^21.0.0", path = "../gix-lock", optional = true }
 
 thiserror = "2.0.18"
+nonempty = "0.12.0"
 serde = { version = "1.0.114", optional = true, default-features = false, features = [
     "derive",
 ] }
diff --git a/gix-protocol/src/fetch/function.rs b/gix-protocol/src/fetch/function.rs
@@ -208,7 +208,7 @@ fn add_shallow_args(
     args: &mut Arguments,
     shallow: &Shallow,
     shallow_file: &std::path::Path,
-) -> Result<(Option<Vec<gix_hash::ObjectId>>, Option<gix_lock::File>), Error> {
+) -> Result<(Option<nonempty::NonEmpty<gix_hash::ObjectId>>, Option<gix_lock::File>), Error> {
     let expect_change = *shallow != Shallow::NoChange;
     let shallow_lock = expect_change.then(|| acquire_shallow_lock(shallow_file)).transpose()?;
 
diff --git a/gix-revision/Cargo.toml b/gix-revision/Cargo.toml
@@ -38,6 +38,7 @@ gix-trace = { version = "^0.1.18", path = "../gix-trace", optional = true }
 
 bstr = { version = "1.12.0", default-features = false, features = ["std"] }
 bitflags = { version = "2", optional = true }
+nonempty = "0.12.0"
 serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] }
 document-features = { version = "0.2.1", optional = true }
 
diff --git a/gix-revision/src/merge_base/function.rs b/gix-revision/src/merge_base/function.rs
@@ -26,17 +26,17 @@ pub fn merge_base(
     first: ObjectId,
     others: &[ObjectId],
     graph: &mut Graph<'_, '_, graph::Commit<Flags>>,
-) -> Result<Option<Vec<ObjectId>>, Error> {
+) -> Result<Option<nonempty::NonEmpty<ObjectId>>, Error> {
     let _span = gix_trace::coarse!("gix_revision::merge_base()", ?first, ?others);
     if others.is_empty() || others.contains(&first) {
-        return Ok(Some(vec![first]));
+        return Ok(Some(nonempty::NonEmpty::new(first)));
     }
 
     graph.clear_commit_data(|f| *f = Flags::empty());
     let bases = paint_down_to_common(first, others, graph)?;
 
     let bases = remove_redundant(&bases, graph)?;
-    Ok((!bases.is_empty()).then_some(bases))
+    Ok(nonempty::NonEmpty::from_vec(bases))
 }
 
 /// Remove all those commits from `commits` if they are in the history of another commit in `commits`.
diff --git a/gix-revision/src/merge_base/mod.rs b/gix-revision/src/merge_base/mod.rs
@@ -52,7 +52,7 @@ mod octopus {
     ) -> Result<Option<ObjectId>, Error> {
         for other in others {
             if let Some(next) =
-                crate::merge_base(first, std::slice::from_ref(other), graph)?.and_then(|bases| bases.into_iter().next())
+                crate::merge_base(first, std::slice::from_ref(other), graph)?.map(|bases| *bases.first())
             {
                 first = next;
             } else {
diff --git a/gix-revision/tests/revision/merge_base/mod.rs b/gix-revision/tests/revision/merge_base/mod.rs
@@ -112,14 +112,15 @@ mod baseline {
 
     use bstr::ByteSlice;
     use gix_hash::ObjectId;
+    use nonempty::NonEmpty;
 
     /// The expectation as produced by Git itself
     #[derive(Debug)]
     pub struct Expectation {
         pub plain_input: String,
         pub first: ObjectId,
         pub others: Vec<ObjectId>,
-        pub bases: Option<Vec<ObjectId>>,
+        pub bases: Option<NonEmpty<ObjectId>>,
     }
 
     pub fn parse_expectations(baseline: &Path) -> std::io::Result<Vec<Expectation>> {
@@ -143,7 +144,7 @@ mod baseline {
                 plain_input,
                 first,
                 others,
-                bases: if bases.is_empty() { None } else { Some(bases) },
+                bases: NonEmpty::from_vec(bases),
             });
         }
         Ok(out)
diff --git a/gix-shallow/Cargo.toml b/gix-shallow/Cargo.toml
@@ -17,12 +17,13 @@ test = false
 
 [features]
 ## Data structures implement `serde::Serialize` and `serde::Deserialize`.
-serde = ["dep:serde", "gix-hash/serde"]
+serde = ["dep:serde", "gix-hash/serde", "nonempty/serialize"]
 
 [dependencies]
 gix-hash = { version = "^0.22.1", path = "../gix-hash" }
 gix-lock = { version = "^21.0.0", path = "../gix-lock" }
 
 thiserror = "2.0.18"
 bstr = { version = "1.12.0", default-features = false }
+nonempty = "0.12.0"
 serde = { version = "1.0.114", optional = true, default-features = false, features = ["std", "derive"] }
diff --git a/gix-shallow/src/lib.rs b/gix-shallow/src/lib.rs
@@ -17,7 +17,7 @@ pub enum Update {
 /// The list of shallow commits represents the shallow boundary, beyond which we are lacking all (parent) commits.
 /// Note that the list is never empty, as `Ok(None)` is returned in that case indicating the repository
 /// isn't a shallow clone.
-pub fn read(shallow_file: &std::path::Path) -> Result<Option<Vec<gix_hash::ObjectId>>, read::Error> {
+pub fn read(shallow_file: &std::path::Path) -> Result<Option<nonempty::NonEmpty<gix_hash::ObjectId>>, read::Error> {
     use bstr::ByteSlice;
     let buf = match std::fs::read(shallow_file) {
         Ok(buf) => buf,
@@ -31,11 +31,7 @@ pub fn read(shallow_file: &std::path::Path) -> Result<Option<Vec<gix_hash::Objec
         .collect::<Result<Vec<_>, _>>()?;
 
     commits.sort();
-    if commits.is_empty() {
-        Ok(None)
-    } else {
-        Ok(Some(commits))
-    }
+    Ok(nonempty::NonEmpty::from_vec(commits))
 }
 
 ///
@@ -56,10 +52,10 @@ pub mod write {
         /// Git also prunes the set of shallow commits while writing, we don't until we support some sort of pruning.
         pub fn write(
             mut file: gix_lock::File,
-            shallow_commits: Option<Vec<gix_hash::ObjectId>>,
+            shallow_commits: Option<nonempty::NonEmpty<gix_hash::ObjectId>>,
             updates: &[Update],
         ) -> Result<(), Error> {
-            let mut shallow_commits = shallow_commits.unwrap_or_default();
+            let mut shallow_commits = shallow_commits.map(Vec::from).unwrap_or_default();
             for update in updates {
                 match update {
                     Update::Shallow(id) => {
@@ -68,28 +64,23 @@ pub mod write {
                     Update::Unshallow(id) => shallow_commits.retain(|oid| oid != id),
                 }
             }
-            if shallow_commits.is_empty() {
-                std::fs::remove_file(file.resource_path())?;
-                drop(file);
-                return Ok(());
-            }
-
             if shallow_commits.is_empty() {
                 if let Err(err) = std::fs::remove_file(file.resource_path()) {
                     if err.kind() != std::io::ErrorKind::NotFound {
                         return Err(err.into());
                     }
                 }
-            } else {
-                shallow_commits.sort();
-                let mut buf = Vec::<u8>::new();
-                for commit in shallow_commits {
-                    commit.write_hex_to(&mut buf).map_err(Error::Io)?;
-                    buf.push(b'\n');
-                }
-                file.write_all(&buf).map_err(Error::Io)?;
-                file.flush()?;
+                drop(file);
+                return Ok(());
+            }
+            shallow_commits.sort();
+            let mut buf = Vec::<u8>::new();
+            for commit in shallow_commits {
+                commit.write_hex_to(&mut buf).map_err(Error::Io)?;
+                buf.push(b'\n');
             }
+            file.write_all(&buf).map_err(Error::Io)?;
+            file.flush().map_err(Error::Io)?;
             file.commit()?;
             Ok(())
         }
diff --git a/gix/Cargo.toml b/gix/Cargo.toml
@@ -387,6 +387,7 @@ gix-transport = { version = "^0.54.0", path = "../gix-transport", optional = tru
 prodash = { version = "31.0.0", optional = true, features = ["progress-tree"] }
 signal-hook = { version = "0.4.3", default-features = false, optional = true }
 thiserror = "2.0.18"
+nonempty = "0.12.0"
 serde = { version = "1.0.114", optional = true, default-features = false, features = [
     "derive",
 ] }
diff --git a/gix/src/merge.rs b/gix/src/merge.rs
diff --git a/gix/src/repository/revision.rs b/gix/src/repository/revision.rs
diff --git a/gix/src/shallow.rs b/gix/src/shallow.rs
diff --git a/gix/tests/gix/clone.rs b/gix/tests/gix/clone.rs
diff --git a/gix/tests/gix/remote/fetch.rs b/gix/tests/gix/remote/fetch.rs
diff --git a/gix/tests/gix/repository/shallow.rs b/gix/tests/gix/repository/shallow.rs

Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ pub struct File {`
`41`	`41`	/// may come from one or more `objects/info/commit-graphs/graph-*.graph` files. These files are
`42`	`42`	/// generated via `git commit-graph write ...` commands.
`43`	`43`	`pub struct Graph {`
`44`		`- files: Vec<File>,`
	`44`	`+ files: nonempty::NonEmpty<File>,`
`45`	`45`	`}`
`46`	`46`
`47`	`47`	/// Instantiate a commit graph from an `.git/objects/info` directory, or one of the various commit-graph files.