Merge pull request #10468 from Byron/other-fix

entrypoint-consistency

Author: Byron

Cargo.lock

@@ -11,15 +11,24 @@ use std::ops::{Deref, Index, IndexMut};
 /// Mutation
 impl Graph {
     /// Insert `segment` to the graph so that it's not connected to any other segment, and return its index.
-    pub fn insert_root(&mut self, segment: Segment) -> SegmentIndex {
-        let index = self.inner.add_node(segment);
-        self.inner[index].id = index;
+    ///
+    /// Note that as a side effect, the [entrypoint](Self::lookup_entrypoint()) will also be set if it's not
+    /// set yet.
+    pub fn insert_segment_set_entrypoint(&mut self, segment: Segment) -> SegmentIndex {
+        let index = self.insert_segment(segment);
         if self.entrypoint.is_none() {
             self.entrypoint = Some((index, None))
         }
         index
     }
 
+    /// Insert `segment` to the graph so that it's not connected to any other segment, and return its index.
+    pub fn insert_segment(&mut self, segment: Segment) -> SegmentIndex {
+        let index = self.inner.add_node(segment);
+        self.inner[index].id = index;
+        index
+    }
+
     /// Put `dst` on top of `src`, connecting it from the `src_commit` specifically,
     /// an index valid for [`Segment::commits_unique_from_tip`] in `src` to the commit at `dst_commit` in `dst`.
     ///

crates/but-graph/src/api.rs

@@ -145,7 +145,7 @@ impl Graph {
                 // It's OK to default-initialise this here as overlays are only used when redoing
                 // the traversal.
                 let (_repo, meta, _entrypoint) = Overlay::default().into_parts(repo, meta);
-                graph.insert_root(branch_segment_from_name_and_meta(
+                graph.insert_segment_set_entrypoint(branch_segment_from_name_and_meta(
                     Some((ref_name, None)),
                     &meta,
                     None,
@@ -242,8 +242,10 @@ impl Graph {
         meta: &OverlayMetadata<'_, T>,
         options: Options,
     ) -> anyhow::Result<Self> {
+        let ref_name = ref_name.into();
         let mut graph = Graph {
             options: options.clone(),
+            entrypoint_ref: ref_name.clone(),
             ..Graph::default()
         };
         let Options {
@@ -256,7 +258,6 @@ impl Graph {
         } = options;
 
         let max_limit = Limit::new(limit);
-        let ref_name = ref_name.into();
         if ref_name
             .as_ref()
             .is_some_and(|name| name.category() == Some(Category::RemoteBranch))
@@ -271,6 +272,8 @@ impl Graph {
 
         let configured_remote_tracking_branches =
             remotes::configured_remote_tracking_branches(repo)?;
+        let (workspaces, target_refs) =
+            obtain_workspace_infos(repo, ref_name.as_ref().map(|rn| rn.as_ref()), meta)?;
         let refs_by_id = repo.collect_ref_mapping_by_prefix(
             [
                 "refs/heads/",
@@ -287,9 +290,11 @@ impl Graph {
             } else {
                 None
             }),
+            &workspaces
+                .iter()
+                .map(|(_, ref_name, _)| ref_name.as_ref())
+                .collect::<Vec<_>>(),
         )?;
-        let (workspaces, target_refs) =
-            obtain_workspace_infos(repo, ref_name.as_ref().map(|rn| rn.as_ref()), meta)?;
         let mut seen = gix::revwalk::graph::IdMap::<SegmentIndex>::default();
         let mut goals = Goals::default();
         // The tip transports itself.
@@ -332,8 +337,8 @@ impl Graph {
             dangerously_skip_postprocessing_for_debugging,
         };
         if tip_is_not_workspace_commit {
-            let current = graph.insert_root(branch_segment_from_name_and_meta(
-                ref_name.clone().map(|rn| (rn, None)),
+            let current = graph.insert_segment_set_entrypoint(branch_segment_from_name_and_meta(
+                None,
                 meta,
                 Some((&ctx.refs_by_id, tip)),
             )?);
@@ -352,10 +357,7 @@ impl Graph {
             let target = ws_meta.target_ref.as_ref().and_then(|trn| {
                 let tid = try_refname_to_id(repo, trn.as_ref())
                     .map_err(|err| {
-                        tracing::warn!(
-                            "Ignoring non-existing target branch {trn}: {err}",
-                            trn = trn.as_bstr()
-                        );
+                        tracing::warn!("Ignoring non-existing target branch {trn}: {err}");
                         err
                     })
                     .ok()??;
@@ -386,7 +388,16 @@ impl Graph {
             // The limits for the target ref and the worktree ref are synced so they can always find each other,
             // while being able to stop when the entrypoint is included.
             ws_segment.metadata = Some(SegmentMetadata::Workspace(ws_meta));
-            let ws_segment = graph.insert_root(ws_segment);
+            let ws_segment = graph.insert_segment(ws_segment);
+            if graph.entrypoint.is_none()
+                && graph
+                    .entrypoint_ref
+                    .as_ref()
+                    .zip(ref_name.as_ref())
+                    .is_some_and(|(a, b)| a == b)
+            {
+                graph.entrypoint = Some((ws_segment, None));
+            }
             // As workspaces typically have integration branches which can help us to stop the traversal,
             // pick these up first.
             _ = next.push_front_exhausted((
@@ -401,45 +412,45 @@ impl Graph {
             ));
 
             if let Some((target_ref, target_ref_id, local_tip_info)) = target {
-                let target_segment = graph.insert_root(branch_segment_from_name_and_meta(
+                let target_segment = graph.insert_segment(branch_segment_from_name_and_meta(
                     Some((target_ref, None)),
                     meta,
                     None,
                 )?);
-                let (local_sidx, local_goal) = if let Some((local_ref_name, target_local_tip)) =
-                    local_tip_info
-                {
-                    let local_sidx = graph.insert_root(branch_segment_from_name_and_meta_sibling(
-                        None,
-                        Some(target_segment),
-                        meta,
-                        Some((&ctx.refs_by_id, target_local_tip)),
-                    )?);
-                    // We use auto-naming based on ambiguity - if the name ends up something else,
-                    // remove the nodes sibling link.
-                    let has_sibling_link = {
-                        let s = &mut graph[local_sidx];
-                        if s.ref_name.as_ref().is_none_or(|rn| rn != &local_ref_name) {
-                            s.sibling_segment_id = None;
-                            false
-                        } else {
-                            true
-                        }
+                let (local_sidx, local_goal) =
+                    if let Some((local_ref_name, target_local_tip)) = local_tip_info {
+                        let local_sidx =
+                            graph.insert_segment(branch_segment_from_name_and_meta_sibling(
+                                None,
+                                Some(target_segment),
+                                meta,
+                                Some((&ctx.refs_by_id, target_local_tip)),
+                            )?);
+                        // We use auto-naming based on ambiguity - if the name ends up something else,
+                        // remove the nodes sibling link.
+                        let has_sibling_link = {
+                            let s = &mut graph[local_sidx];
+                            if s.ref_name.as_ref().is_none_or(|rn| rn != &local_ref_name) {
+                                s.sibling_segment_id = None;
+                                false
+                            } else {
+                                true
+                            }
+                        };
+                        let goal = goals.flag_for(target_local_tip).unwrap_or_default();
+                        _ = next.push_front_exhausted((
+                            target_local_tip,
+                            CommitFlags::NotInRemote | goal,
+                            Instruction::CollectCommit { into: local_sidx },
+                            max_limit
+                                .with_indirect_goal(tip, &mut goals)
+                                .without_allowance(),
+                        ));
+                        next.add_goal_to(tip, goal);
+                        (has_sibling_link.then_some(local_sidx), goal)
+                    } else {
+                        (None, CommitFlags::empty())
                     };
-                    let goal = goals.flag_for(target_local_tip).unwrap_or_default();
-                    _ = next.push_front_exhausted((
-                        target_local_tip,
-                        CommitFlags::NotInRemote | goal,
-                        Instruction::CollectCommit { into: local_sidx },
-                        max_limit
-                            .with_indirect_goal(tip, &mut goals)
-                            .without_allowance(),
-                    ));
-                    next.add_goal_to(tip, goal);
-                    (has_sibling_link.then_some(local_sidx), goal)
-                } else {
-                    (None, CommitFlags::empty())
-                };
                 _ = next.push_front_exhausted((
                     target_ref_id,
                     CommitFlags::Integrated,
@@ -466,7 +477,7 @@ impl Graph {
                 // have to adjust the existing queue item.
                 existing_segment
             } else {
-                let extra_target_sidx = graph.insert_root(branch_segment_from_name_and_meta(
+                let extra_target_sidx = graph.insert_segment(branch_segment_from_name_and_meta(
                     None,
                     meta,
                     Some((&ctx.refs_by_id, extra_target)),
@@ -515,7 +526,7 @@ impl Graph {
                     meta,
                     Some((&ctx.refs_by_id, segment_tip.detach())),
                 )?;
-                let segment = graph.insert_root(segment);
+                let segment = graph.insert_segment(segment);
                 _ = next.push_back_exhausted((
                     segment_tip.detach(),
                     CommitFlags::NotInRemote,

crates/but-graph/src/init/mod.rs

@@ -1,6 +1,5 @@
 use crate::init::walk::RefsById;
 use crate::init::{Entrypoint, Overlay};
-use crate::is_workspace_ref_name;
 use but_core::{RefMetadata, ref_metadata};
 use gix::prelude::ReferenceExt;
 use std::borrow::Cow;
@@ -173,15 +172,17 @@ impl<'repo> OverlayRepo<'repo> {
             .upstream_branch_and_remote_for_tracking_branch(name)?)
     }
 
-    // Create a mapping of all heads to the object ids they point to.
+    /// Create a mapping of all heads to the object ids they point to.
+    /// `workspace_ref_names` is the names of all known workspace references.
     pub fn collect_ref_mapping_by_prefix<'a>(
         &self,
         prefixes: impl Iterator<Item = &'a str>,
+        workspace_ref_names: &[&gix::refs::FullNameRef],
     ) -> anyhow::Result<RefsById> {
         let mut seen = (!self.nonoverriding_references.is_empty()).then(BTreeSet::new);
         let mut ref_filter =
             |r: gix::Reference<'_>| -> Option<(gix::ObjectId, gix::refs::FullName)> {
-                if is_workspace_ref_name(r.name()) {
+                if workspace_ref_names.contains(&r.name()) {
                     return None;
                 }
                 let id = r.try_id()?;