diff --git a/gix-revision/src/merge_base/function.rs b/gix-revision/src/merge_base/function.rs index ba7b0cfd200..f05d30f441b 100644 --- a/gix-revision/src/merge_base/function.rs +++ b/gix-revision/src/merge_base/function.rs @@ -22,6 +22,8 @@ use crate::{Graph, PriorityQueue, merge_base::Flags}; /// /// For repeated calls, be sure to re-use `graph` as its content will be kept and reused for a great speed-up. The contained flags /// will automatically be cleared. +/// With a commit-graph providing nonzero, unsaturated generations, the walk stops once either side's non-stale +/// frontier is exhausted and no merge-base candidates remain queued. pub fn merge_base( first: ObjectId, others: &[ObjectId], @@ -148,34 +150,84 @@ fn remove_redundant( .collect()) } +struct PaintQueue { + queue: PriorityQueue, + /// Non-stale queued commits carrying each color. Candidates count toward both sides, keeping the walk alive + /// until they have been recorded, even if no exclusively colored commits remain on one side. + non_stale: [usize; 2], +} + +impl PaintQueue { + fn update_counts(&mut self, flags: Flags, add: bool) { + if flags.contains(Flags::STALE) { + return; + } + for (side, count) in [Flags::COMMIT1, Flags::COMMIT2].into_iter().zip(&mut self.non_stale) { + if flags.contains(side) { + if add { + *count += 1; + } else { + *count -= 1; + } + } + } + } + + fn insert(&mut self, commit_id: ObjectId, commit: &mut graph::Commit, flags: Flags) { + if commit.data.contains(Flags::ENQUEUED) { + self.update_counts(commit.data, false); + } else { + self.queue.insert(GenThenTime::from(&*commit), commit_id); + commit.data |= Flags::ENQUEUED; + } + commit.data |= flags; + self.update_counts(commit.data, true); + } + + fn pop(&mut self, graph: &mut Graph<'_, '_, graph::Commit>) -> Option<(GenThenTime, ObjectId)> { + let (info, commit_id) = self.queue.pop()?; + // Keep this commit counted until after the exit check so the last pending candidate is processed. + // Side exhaustion is only final when children are visited before parents. Missing, zero, or saturated + // generations fall back to date ordering, which can propagate a color to an already visited commit. + if self.non_stale == [0, 0] + || (self.non_stale.contains(&0) + && info.generation > 0 + && info.generation < gix_commitgraph::GENERATION_NUMBER_MAX) + { + return None; + } + let commit = graph.get_mut(&commit_id).expect("everything queued is in graph"); + commit.data.remove(Flags::ENQUEUED); + self.update_counts(commit.data, false); + Some((info, commit_id)) + } +} + fn paint_down_to_common( first: ObjectId, others: &[ObjectId], graph: &mut Graph<'_, '_, graph::Commit>, ) -> Result, Error> { - let mut queue = PriorityQueue::::new(); + let mut queue = PaintQueue { + queue: PriorityQueue::new(), + non_stale: [0; 2], + }; graph .get_or_insert_full_commit(first, |commit| { - commit.data |= Flags::COMMIT1; - queue.insert(GenThenTime::from(&*commit), first); + queue.insert(first, commit, Flags::COMMIT1); }) .map_err(|_| Simple("could not insert commit into graph"))?; for other in others { graph .get_or_insert_full_commit(*other, |commit| { - commit.data |= Flags::COMMIT2; - queue.insert(GenThenTime::from(&*commit), *other); + queue.insert(*other, commit, Flags::COMMIT2); }) .map_err(|_| Simple("could not insert commit into graph"))?; } let mut out = Vec::new(); - while queue - .iter_unordered() - .any(|id| graph.get(id).is_some_and(|commit| !commit.data.contains(Flags::STALE))) - { - let (info, commit_id) = queue.pop().expect("we have non-stale"); + while let Some((info, commit_id)) = queue.pop(graph) { let commit = graph.get_mut(&commit_id).expect("everything queued is in graph"); let mut flags_without_result = commit.data & (Flags::COMMIT1 | Flags::COMMIT2 | Flags::STALE); if flags_without_result == (Flags::COMMIT1 | Flags::COMMIT2) { @@ -190,8 +242,7 @@ fn paint_down_to_common( graph .get_or_insert_full_commit(parent_id, |parent| { if (parent.data & flags_without_result) != flags_without_result { - parent.data |= flags_without_result; - queue.insert(GenThenTime::from(&*parent), parent_id); + queue.insert(parent_id, parent, flags_without_result); } }) .map_err(|_| Simple("could not insert parent commit into graph"))?; diff --git a/gix-revision/src/merge_base/mod.rs b/gix-revision/src/merge_base/mod.rs index a42c0a30214..3599d27c66e 100644 --- a/gix-revision/src/merge_base/mod.rs +++ b/gix-revision/src/merge_base/mod.rs @@ -11,6 +11,8 @@ bitflags::bitflags! { const STALE = 1 << 2; /// The commit was already put ontto the results list. const RESULT = 1 << 3; + /// The commit is currently in the paint queue. + const ENQUEUED = 1 << 4; } } diff --git a/gix-revision/tests/revision/merge_base.rs b/gix-revision/tests/revision/merge_base.rs index 1cb6eafb3f8..61881e1dd8e 100644 --- a/gix-revision/tests/revision/merge_base.rs +++ b/gix-revision/tests/revision/merge_base.rs @@ -40,6 +40,81 @@ fn validate() -> crate::Result { Ok(()) } +#[test] +fn exhausted_side_skips_unrelated_history() -> crate::Result { + let root = gix_testtools::scripted_fixture_read_only("make_merge_base_repos.sh")?; + let odb = odb_at(root.join(".git/objects"))?; + let tip_commit_id = tag_commit_id(&root, "PL")?; + let base_commit_id = tag_commit_id(&root, "C2")?; + let unrelated_commit_id = tag_commit_id(&root, "L0")?; + + // PL merges the C and L chains. Once C2 is found, the rest of L cannot + // provide another merge base, even though its queued commits are not stale. + for use_commitgraph in [false, true] { + let cache = use_commitgraph + .then(|| gix_commitgraph::Graph::from_info_dir(&odb.store_ref().path().join("info"))) + .transpose()?; + for (first_commit_id, other_commit_id) in [(tip_commit_id, base_commit_id), (base_commit_id, tip_commit_id)] { + let mut graph = gix_revision::Graph::new(&odb, cache.as_ref()); + for others in [ + &[other_commit_id][..], + &[other_commit_id, other_commit_id][..], + &[other_commit_id][..], + ] { + assert_eq!( + merge_base(first_commit_id, others, &mut graph)?, + Some(nonempty::NonEmpty::new(base_commit_id)), + "the pending common ancestor survives side exhaustion, duplicates, and graph reuse" + ); + assert_eq!( + graph.contains(&unrelated_commit_id), + !use_commitgraph, + "only reliable generation ordering lets the walk skip the unrelated L0 ancestor" + ); + } + } + } + Ok(()) +} + +#[test] +fn unreliable_generations_do_not_allow_side_exhaustion() -> crate::Result { + let root = gix_testtools::scripted_fixture_read_only("make_merge_base_repos.sh")?; + let odb = odb_at(root.join(".git/objects"))?; + // G and H share B, but clock skew visits B's ancestor E first. Missing, + // zero, or saturated generations cannot prevent an exhausted color from returning. + // Zero represents a legacy commit-graph without computed generations. + let first_commit_id = tag_commit_id(&root, "G")?; + let other_commit_id = tag_commit_id(&root, "H")?; + let base_commit_id = tag_commit_id(&root, "B")?; + for generation in [None, Some(0), Some(gix_commitgraph::GENERATION_NUMBER_MAX)] { + let mut graph = gix_revision::Graph::new(&odb, None); + for name in ["A", "B", "C", "D", "E", "F", "G", "H"] { + graph.get_or_insert_full_commit(tag_commit_id(&root, name)?, |commit| { + commit.generation = generation; + })?; + } + for (first_commit_id, other_commit_id) in + [(first_commit_id, other_commit_id), (other_commit_id, first_commit_id)] + { + assert_eq!( + merge_base(first_commit_id, &[other_commit_id, other_commit_id], &mut graph)?, + Some(nonempty::NonEmpty::new(base_commit_id)), + "generation {generation:?} requires finishing the date-ordered walk despite temporary side exhaustion" + ); + } + } + Ok(()) +} + +fn tag_commit_id(root: &std::path::Path, name: &str) -> crate::Result { + Ok(gix_hash::ObjectId::from_hex( + std::fs::read_to_string(root.join(".git/refs/tags").join(name))? + .trim() + .as_bytes(), + )?) +} + mod octopus { use crate::{hex_to_id, odb_at};