History

Distinguish unloaded graph frontiers from root commits

The history cache already recorded whether a commit had been loaded, but `parents_of()` returned an empty parent list for an interned frontier. This made an external AutoMerge input look like a root and required an AutoMerge- specific workaround that equated edit scope with cache completeness. Honor the existing loaded bit and move read-through ancestry lookup into `HistoryGraph`. Name traversal-only cached edges `known_parents` and limit descendant rewrites to the explicit edit scope. Looking up missing ancestry neither populates the cache nor expands permission to edit it. Extend the exact-scope regression to distinguish unloaded frontiers, loaded roots, and ancestry reads. It failed with `Some([])` before the fix and now passes, together with the external-input replay regressions.

Codex committed 2 months ago

Commit dd22c8b1 parent 47fd1ed5 files · +81 −46Browse files

Unified · Split

5 files
  1. gix-tix/spec.md +3 −0
  2. gix-tix/src/edit/auto_merge/mod.rs +2 −15
  3. gix-tix/src/edit/rebase.rs +6 −6
  4. gix-tix/src/history.rs +62 −17
  5. gix-tix/src/ref_tree.rs +8 −8
gix-tix/spec.md+3−0
@@ -1176,6 +1176,9 @@ views. · 1175 unmodified lines
11761176 results by immutable commit ID. Idle application state retains detached recipes
11771177 and picker data only. Pin-consuming checkouts may reload the current projection
11781178 to determine which pins must be retained.
1179+ The graph distinguishes an unloaded frontier from a loaded root or shallow
1180+ boundary. Reading an external input's ancestry does not expand the editable
1181+ scope; descendant rewrites remain confined to that scope.
11791182
11801183 ### Reviews
11811184
gix-tix/src/edit/auto_merge/mod.rs+2−15
@@ -309,20 +309,6 @@ pub(crate) struct Preparation { · 308 unmodified lines
309309 pub eager: HashSet<ObjectId>,
310310 }
311311
312−pub(crate) fn parents(
313− repo: &gix::Repository,
314− graph: &crate::history::HistoryGraph,
315− commit_id: ObjectId,
316−) -> Result<Vec<ObjectId>> {
317− match graph
318− .parents_of(commit_id)
319− .filter(|_| graph.is_in_edit_scope(commit_id))
320− {
321− Some(parents) => Ok(parents),
322− None => Ok(repo.find_commit(commit_id)?.parent_ids().map(gix::Id::detach).collect()),
323− }
324−}
325−
326312 /// Only the checkout's ordinary first-parent path requires conflict materialization.
327313 /// Inputs beneath an AutoMerge can instead remain pending and be muted.
328314 pub(crate) fn checkout_path(
@@ -900,7 +886,8 @@ pub(crate) fn expand_plan( · 571 unmodified lines
900886 }
901887 let extra: Vec<_> = affected.into_iter().filter(|id| !original.contains(id)).collect();
902888 for &commit_id in &extra {
903− let parent_commit_id = parents(repo, graph, commit_id)?
889+ let parent_commit_id = graph
890+ .parents_or_load(repo, commit_id)?
904891 .first()
905892 .copied()
906893 .context("a pending input has no parent")?;
gix-tix/src/edit/rebase.rs+6−6
@@ -1267,7 +1267,7 @@ fn perform_inner( · 1266 unmodified lines
12671267 pending.retain(|id| Some(*id) != root);
12681268 }
12691269 for old_id in pending {
1270− let old_parents = auto_merge::parents(&repo, graph, old_id)?;
1270+ let old_parents = graph.parents_or_load(&repo, old_id)?;
12711271 let mut commit = match replacement.as_ref().filter(|_| Some(old_id) == root) {
12721272 Some(commit) => commit.clone(),
12731273 None => repo
@@ -1546,7 +1546,7 @@ pub(super) fn finish_review_with_progress( · 272 unmodified lines
15461546 )?;
15471547 }
15481548 for id in review_ids.iter().chain(&natural_ids) {
1549− if auto_merge::parents(&repo, graph, *id)?.len() > 1
1549+ if graph.parents_or_load(&repo, *id)?.len() > 1
15501550 && !auto_merge::is_auto_merge(&repo.find_commit(*id)?.decode()?.into_owned()?)
15511551 {
15521552 anyhow::bail!("review finish cannot rewrite merge descendants");
@@ -1607,7 +1607,7 @@ pub(super) fn finish_review_with_progress( · 54 unmodified lines
16071607
16081608 rewritten.insert(tip, Some(finished_review));
16091609 for old in natural_ids {
1610− let old_parents = auto_merge::parents(&repo, graph, old)?;
1610+ let old_parents = graph.parents_or_load(&repo, old)?;
16111611 let mut commit = repo.find_commit(old)?.decode()?.into_owned()?;
16121612 if auto_merge::is_auto_merge(&commit) {
16131613 let eager = conflict.is_none() && auto.eager.contains(&old);
@@ -1772,7 +1772,7 @@ pub(crate) fn perform_plan_with_progress( · 158 unmodified lines
17721772 if automatic && step.squash.contains(&id) {
17731773 anyhow::bail!("an AutoMerge cannot be squashed");
17741774 }
1775− if auto_merge::parents(&repo, graph, id)?.len() > 1 && !automatic {
1775+ if graph.parents_or_load(&repo, id)?.len() > 1 && !automatic {
17761776 anyhow::bail!("merge commits cannot be picked by the rebase editor");
17771777 }
17781778 }
@@ -1944,7 +1944,7 @@ pub(crate) fn perform_plan_with_progress( · 165 unmodified lines
19441944 }
19451945 let graph_parents = match step.commit {
19461946 PlanCommit::Pick(id) | PlanCommit::Copy(id) | PlanCommit::Resolved(id) => {
1947− auto_merge::parents(&repo, graph, id)?
1947+ graph.parents_or_load(&repo, id)?
19481948 }
19491949 PlanCommit::Empty(_) => vec![parent],
19501950 };
@@ -2655,7 +2655,7 @@ fn validate( · 704 unmodified lines
26552655 tree: Tree,
26562656 ) -> Result<()> {
26572657 for (position, id) in affected.iter().enumerate() {
2658− let parents = auto_merge::parents(repo, graph, *id)?;
2658+ let parents = graph.parents_or_load(repo, *id)?;
26592659 if parents.len() > 1
26602660 && (position > 0 || removed || tree == Tree::CherryPick)
26612661 && !auto_merge::is_auto_merge(&repo.find_commit(*id)?.decode()?.into_owned()?)
gix-tix/src/history.rs+62−17
@@ -268,7 +268,8 @@ impl HistoryGraph { · 267 unmodified lines
268268 self.commits[index.as_usize()].id
269269 }
270270
271− pub(crate) fn parents(&self, index: CommitIndex) -> &[CommitIndex] {
271+ /// Cached edges for traversal; an unloaded frontier has no known edges.
272+ pub(crate) fn known_parents(&self, index: CommitIndex) -> &[CommitIndex] {
272273 let range = self.commits[index.as_usize()].parents.clone();
273274 &self.parents[range.start as usize..range.end as usize]
274275 }
@@ -326,7 +327,7 @@ impl HistoryGraph { · 51 unmodified lines
326327 if std::mem::replace(&mut seen[index.as_usize()], true) {
327328 continue;
328329 }
329− pending.extend_from_slice(self.parents(index));
330+ pending.extend_from_slice(self.known_parents(index));
330331 }
331332 false
332333 }
@@ -344,9 +345,23 @@ impl HistoryGraph { · 11 unmodified lines
344345 .collect()
345346 }
346347
348+ /// `None` means unloaded or unknown; an empty list means a loaded root or shallow boundary.
347349 pub(crate) fn parents_of(&self, id: ObjectId) -> Option<Vec<ObjectId>> {
348350 let index = self.index(id)?;
349− Some(self.parents(index).iter().map(|parent| self.id(*parent)).collect())
351+ (self.commits[index.as_usize()].state & NODE_LOADED != 0).then(|| {
352+ self.known_parents(index)
353+ .iter()
354+ .map(|parent| self.id(*parent))
355+ .collect()
356+ })
357+ }
358+
359+ /// Read missing ancestry without changing the cached graph or its editable scope.
360+ pub(crate) fn parents_or_load(&self, repo: &gix::Repository, commit_id: ObjectId) -> Result<Vec<ObjectId>> {
361+ match self.parents_of(commit_id) {
362+ Some(parents) => Ok(parents),
363+ None => Ok(repo.find_commit(commit_id)?.parent_ids().map(gix::Id::detach).collect()),
364+ }
350365 }
351366
352367 pub(crate) fn commits_with_merge_descendants(&self) -> HashSet<ObjectId> {
@@ -367,7 +382,7 @@ impl HistoryGraph { · 14 unmodified lines
367382 while let Some(index) = pending.pop() {
368383 if ancestors.insert(index) {
369384 pending.extend(
370− self.parents(index)
385+ self.known_parents(index)
371386 .iter()
372387 .copied()
373388 .filter(|parent| self.commits[parent.as_usize()].state & NODE_IN_VIEW != 0),
@@ -378,18 +393,18 @@ impl HistoryGraph { · 4 unmodified lines
378393 }
379394
380395 pub(crate) fn descendants_in_parent_order(&self, root: ObjectId) -> Option<Vec<ObjectId>> {
381− let root = self.index(root)?;
382− if self.commits[root.as_usize()].state & NODE_IN_VIEW == 0 {
396+ if !self.is_in_edit_scope(root) {
383397 return None;
384398 }
399+ let root = self.index(root)?;
385400 let mut included = HashSet::from([root]);
386401 loop {
387402 let mut changed = false;
388403 for index in 0..self.commits.len() {
389404 let index = CommitIndex::new(index).expect("an existing graph index fits into u32");
390− if self.commits[index.as_usize()].state & NODE_IN_VIEW == 0
405+ if !self.is_in_edit_scope(self.id(index))
391406 || included.contains(&index)
392− || !self.parents(index).iter().any(|parent| included.contains(parent))
407+ || !self.known_parents(index).iter().any(|parent| included.contains(parent))
393408 {
394409 continue;
395410 }
@@ -406,7 +421,7 @@ impl HistoryGraph { · 10 unmodified lines
406421 for index in &included {
407422 if out.contains(index)
408423 || self
409− .parents(*index)
424+ .known_parents(*index)
410425 .iter()
411426 .any(|parent| included.contains(parent) && !out.contains(parent))
412427 {
@@ -431,7 +446,7 @@ impl HistoryGraph { · 18 unmodified lines
431446 continue;
432447 }
433448 self.commits[index.as_usize()].state |= NODE_IN_VIEW;
434− pending.extend_from_slice(self.parents(index));
449+ pending.extend_from_slice(self.known_parents(index));
435450 }
436451 }
437452
@@ -447,7 +462,7 @@ impl HistoryGraph { · 9 unmodified lines
447462 fn set_edit_scope(&mut self, view_tips: &[ObjectId], hidden_tips: &[ObjectId]) {
448463 let (visible, boundary) = view_scope(view_tips, hidden_tips, |id, out| {
449464 if let Some(index) = self.index(id) {
450− out.extend(self.parents(index).iter().map(|parent| self.id(*parent)));
465+ out.extend(self.known_parents(index).iter().map(|parent| self.id(*parent)));
451466 }
452467 });
453468 self.edit_scope = visible;
@@ -455,7 +470,10 @@ impl HistoryGraph { · 1 unmodified line
455470 }
456471
457472 fn parent_ids(&self, index: CommitIndex) -> gix::traverse::commit::ParentIds {
458− self.parents(index).iter().map(|parent| self.id(*parent)).collect()
473+ self.known_parents(index)
474+ .iter()
475+ .map(|parent| self.id(*parent))
476+ .collect()
459477 }
460478
461479 fn ensure_commit(
@@ -748,7 +766,7 @@ impl HistoryGraph { · 286 unmodified lines
748766 propagated |= STALE;
749767 flags[index.as_usize()] = propagated;
750768 }
751− for &parent in self.parents(index) {
769+ for &parent in self.known_parents(index) {
752770 let parent_flags = &mut flags[parent.as_usize()];
753771 let previous = *parent_flags;
754772 if previous & propagated != propagated {
@@ -924,7 +942,7 @@ impl HistoryGraph { · 169 unmodified lines
924942 let stop = !should_store
925943 && self.commits[index.as_usize()].state & NODE_COMPLETE != 0
926944 && (delta & EXPAND == 0 || was_stored && !expand.contains(&id));
927− let parent_indices = self.parents(index).to_vec();
945+ let parent_indices = self.known_parents(index).to_vec();
928946 if should_store {
929947 if let Some(names) = local_refs.get(&id) {
930948 let tracked = resolve_tracking(repo, names)?;
@@ -1180,7 +1198,7 @@ fn hidden_frontier( · 249 unmodified lines
11801198 propagated |= STALE;
11811199 flags[index.as_usize()] = propagated;
11821200 }
1183− let parents = graph.parents(index).to_vec();
1201+ let parents = graph.known_parents(index).to_vec();
11841202 for parent in parents {
11851203 let parent_id = graph.id(parent);
11861204 let parent = graph.ensure_commit(repo, cache, shallow, parent_id, &mut buf)?;
@@ -1381,7 +1399,7 @@ pub(crate) fn load( · 194 unmodified lines
13811399 if hidden.is_empty() {
13821400 graph.commits[index.as_usize()].state |= NODE_COMPLETE;
13831401 }
1384− let parent_indices = graph.parents(index).to_vec();
1402+ let parent_indices = graph.known_parents(index).to_vec();
13851403 let parent_ids = graph.parent_ids(index);
13861404 let generation = graph.commits[index.as_usize()].generation();
13871405 if should_emit && let Some(names) = local_refs.get(&id) {
@@ -3355,6 +3373,33 @@ mod tests { · 1967 unmodified lines
33553373
33563374 assert!(graph.is_in_edit_scope(tip), "the requested commit is in the edit scope");
33573375 assert!(graph.index(parent).is_some(), "the requested commit's parent is known");
3376+ assert_eq!(graph.parents_of(tip), Some(vec![parent]), "loaded edges are available");
3377+ assert_eq!(
3378+ graph.parents_of(parent),
3379+ None,
3380+ "an unloaded frontier is not a root commit"
3381+ );
3382+ let root = repo.rev_parse_single("topic^^")?.detach();
3383+ assert_eq!(
3384+ graph.parents_or_load(&repo, parent)?,
3385+ [root],
3386+ "a frontier can be read on demand"
3387+ );
3388+ assert_eq!(
3389+ graph.parents_of(parent),
3390+ None,
3391+ "an ancestry read does not expand the cache"
3392+ );
3393+ assert_eq!(
3394+ HistoryGraph::for_commits(&repo, &[root])?.parents_of(root),
3395+ Some(Vec::new()),
3396+ "a loaded root has a known empty parent list"
3397+ );
3398+ assert_eq!(
3399+ graph.descendants_in_parent_order(parent),
3400+ None,
3401+ "interning an ancestor does not make it an editable rewrite root"
3402+ );
33583403 assert!(
33593404 !graph.is_in_edit_scope(parent),
33603405 "an interned parent stays outside the explicit edit scope"
@@ -3658,7 +3703,7 @@ mod tests { · 297 unmodified lines
36583703 let repo = crate::test_repository::open(fixture.path())?;
36593704 let index = graph.index(main).expect("the tracking tip was scheduled");
36603705 let fake_parent = graph.intern(id(255)).expect("the small test graph fits in u32");
3661− let mut parents = graph.parents(index).to_vec();
3706+ let mut parents = graph.known_parents(index).to_vec();
36623707 parents.push(fake_parent);
36633708 let start = graph.parents.len() as u32;
36643709 graph.parents.extend(parents);
gix-tix/src/ref_tree.rs+8−8
@@ -998,13 +998,13 @@ impl Overview { · 997 unmodified lines
998998 if !included.insert(index) {
999999 break;
10001000 }
1001− current = graph.parents(index).first().copied();
1001+ current = graph.known_parents(index).first().copied();
10021002 }
10031003 }
10041004 let mut children = vec![Vec::new(); graph.commit_count()];
10051005 for child in included.iter().copied() {
10061006 if let Some(parent) = graph
1007− .parents(child)
1007+ .known_parents(child)
10081008 .first()
10091009 .copied()
10101010 .filter(|parent| included.contains(parent))
@@ -1019,7 +1019,7 @@ impl Overview { · 8 unmodified lines
10191019 anchors.contains(index)
10201020 || children[index.as_usize()].len() != 1
10211021 || graph
1022− .parents(*index)
1022+ .known_parents(*index)
10231023 .first()
10241024 .is_none_or(|parent| !included.contains(parent))
10251025 })
@@ -1058,7 +1058,7 @@ impl Overview { · 32 unmodified lines
10581058 let mut roots = Vec::new();
10591059 for child in 0..nodes.len() {
10601060 let mut hidden = Vec::new();
1061− let mut parent = graph.parents(nodes[child].commit).first().copied();
1061+ let mut parent = graph.known_parents(nodes[child].commit).first().copied();
10621062 while let Some(index) = parent.filter(|index| included.contains(index)) {
10631063 if let Some(parent_node) = by_commit.get(&index).copied() {
10641064 nodes[child].parent = Some(parent_node);
@@ -1071,7 +1071,7 @@ impl Overview { · 6 unmodified lines
10711071 break;
10721072 }
10731073 hidden.push(index);
1074− parent = graph.parents(index).first().copied();
1074+ parent = graph.known_parents(index).first().copied();
10751075 }
10761076 if nodes[child].parent.is_none() {
10771077 roots.push(child);
@@ -1128,13 +1128,13 @@ impl Overlay { · 50 unmodified lines
11281128 continue;
11291129 }
11301130 total += 1;
1131− pending.extend_from_slice(graph.parents(index));
1131+ pending.extend_from_slice(graph.known_parents(index));
11321132 }
11331133 let mut first_parent = vec![false; graph.commit_count()];
11341134 let mut current = Some(selected_commit);
11351135 while let Some(index) = current {
11361136 first_parent[index.as_usize()] = true;
1137− current = graph.parents(index).first().copied();
1137+ current = graph.known_parents(index).first().copied();
11381138 }
11391139 let mut counts = vec![None; overview.nodes.len()];
11401140 counts[selected] = Some(total);
@@ -1187,7 +1187,7 @@ impl Overlay { · 46 unmodified lines
11871187 continue;
11881188 }
11891189 count += 1;
1190− pending.extend_from_slice(graph.parents(index));
1190+ pending.extend_from_slice(graph.known_parents(index));
11911191 }
11921192 self.counts[node] = Some(count);
11931193 }