Make sure the graph will look the same even if entrypoints are used.

Author: Byron

crates/but-graph/src/api.rs

@@ -11,6 +11,8 @@ 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.
+    /// Note that as a side effect, the [entrypoint](Self::lookup_entrypoint()) will also be set if it's not
+    /// set yet.
     pub fn insert_root(&mut self, segment: Segment) -> SegmentIndex {
         let index = self.inner.add_node(segment);
         self.inner[index].id = index;

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

@@ -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.
@@ -333,7 +338,7 @@ impl Graph {
         };
         if tip_is_not_workspace_commit {
             let current = graph.insert_root(branch_segment_from_name_and_meta(
-                ref_name.clone().map(|rn| (rn, None)),
+                None,
                 meta,
                 Some((&ctx.refs_by_id, tip)),
             )?);

crates/but-graph/src/init/overlay.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()?;

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

@@ -77,6 +77,9 @@ impl Graph {
         // the foundation for figuring out our workspaces more easily.
         self.workspace_upgrades(meta, repo)?;
 
+        // Point entrypoint to the right spot after all the virtual branches were added.
+        self.set_entrypoint_to_ref_name(meta)?;
+
         // However, when it comes to using remotes to disambiguate, it's better to
         // *not* do that before workspaces are sorted as it might incorrectly place
         // a segment on top of another one, setting a first-second relationship that isn't
@@ -95,6 +98,108 @@ impl Graph {
         Ok(self)
     }
 
+    /// After everything, assure the entrypoint still points to a segment with the correct ref-name,
+    /// if one was given when starting the traversal.
+    /// If not, try to find a segment with the right ref-name.
+    ///
+    /// *This is the brute-force way of doing it, instead of ensuring that the workspace upgrade functions
+    /// that create independent and dependent branches keep everything up-to-date at all times.
+    fn set_entrypoint_to_ref_name<T: RefMetadata>(
+        &mut self,
+        meta: &OverlayMetadata<'_, T>,
+    ) -> anyhow::Result<()> {
+        let Some(((ep_sidx, _commit_idx), desired_ref_name)) =
+            self.entrypoint.zip(self.entrypoint_ref.clone())
+        else {
+            return Ok(());
+        };
+
+        let ep_segment_is_correctly_named = self[ep_sidx]
+            .ref_name
+            .as_ref()
+            .is_some_and(|rn| rn == &desired_ref_name);
+        if ep_segment_is_correctly_named {
+            return Ok(());
+        }
+
+        let (sidx_with_desired_name, sidx_with_first_commit_with_desired_name) = self
+            .node_weights()
+            .find_map(|s| {
+                s.ref_name
+                    .as_ref()
+                    .is_some_and(|rn| rn == &desired_ref_name)
+                    .then_some((Some(s.id), None))
+                    .or_else(|| {
+                        s.commits.first().and_then(|c| {
+                            c.refs
+                                .iter()
+                                .position(|rn| rn == &desired_ref_name)
+                                .map(|pos| (None, Some((s.id, pos))))
+                        })
+                    })
+            })
+            .unwrap_or_default();
+        if let Some(new_ep_sidx) = sidx_with_desired_name {
+            let assume_tip_is_not_available_in_segment_anymore = None;
+            self.entrypoint = Some((new_ep_sidx, assume_tip_is_not_available_in_segment_anymore));
+        } else if let Some((new_ep_sidx, ref_idx)) = sidx_with_first_commit_with_desired_name {
+            let s = &mut self.inner[new_ep_sidx];
+            let desired_ref_name = s
+                .commits
+                .first_mut()
+                .context("BUG: we have ref_idx because the first commit was checked")?
+                .refs
+                .remove(ref_idx);
+            if s.ref_name.is_some() {
+                // ref-name is known to not be the desired one, and the first commit of this segment has the name
+                // we seek. For that, we now create a new
+                let new_ep_sidx_first_commit_idx = s.commits.first().map(|_| 0);
+                let incoming_edges = collect_edges_at_commit_reverse_order(
+                    &self.inner,
+                    (new_ep_sidx, new_ep_sidx_first_commit_idx),
+                    Direction::Incoming,
+                );
+                let entrypoint_segment =
+                    branch_segment_from_name_and_meta(Some((desired_ref_name, None)), meta, None)?;
+                let entrypoint_sidx = self.insert_root(entrypoint_segment);
+                self.connect_segments(
+                    entrypoint_sidx,
+                    None,
+                    new_ep_sidx,
+                    new_ep_sidx_first_commit_idx,
+                );
+                for edge in incoming_edges {
+                    self.inner.add_edge(
+                        edge.source,
+                        entrypoint_sidx,
+                        Edge {
+                            src: edge.weight.src,
+                            src_id: edge.weight.src_id,
+                            dst: None,
+                            dst_id: None,
+                        },
+                    );
+                    self.inner.remove_edge(edge.id);
+                }
+                self.entrypoint = Some((entrypoint_sidx, None));
+            } else {
+                self.entrypoint = Some((new_ep_sidx, Some(0)));
+                // It's really important to get the name, as the HEAD is pointing to this segment.
+                // So find the segment with the ambiguous ref we desire, and rewrite it to be non-ambiguous.
+                // The reason we wait till now is to not disturb the workspace upgrades, which act differently
+                // if they already have a named segment.
+                // Note that any ref type works here, we just do as we are told even though tags for instance aren't usually
+                // pointed to by HEAD.
+                s.ref_name = Some(desired_ref_name);
+            }
+        } else {
+            tracing::warn!(
+                "Couldn't find any segment that was named after the entrypoint ref name or contained its name '{desired_ref_name}'",
+            );
+        };
+        Ok(())
+    }
+
     /// This is a post-process as only in the end we are sure what is a remote commit.
     /// On remote commits, we want to further segment remote tracking segments to avoid picking
     /// up too many remote commits later.
@@ -570,7 +675,7 @@ impl Graph {
                     s.ref_name = Some(name);
                     s.metadata = md;
                 }
-                reconnect_edges(
+                reconnect_outgoing_edges(
                     self,
                     edges_from_segment_above,
                     (new_sidx_above_base_sidx, None),
@@ -675,6 +780,11 @@ impl Graph {
     /// Name ambiguous segments if they are reachable by remote tracking branch and
     /// if the first commit has (unambiguously) the matching local tracking branch.
     /// Also, link up all remote segments with their local ones, and vice versa.
+    ///
+    /// Additionally, restore possibly broken linkage to their siblings
+    /// (from remote to local tracking branch), as this connection might have been destroyed by
+    /// the insertion of empty segments. Again, instead of making that smarter, we fix it up here
+    /// because it's simpler.
     fn improve_remote_segments(
         &mut self,
         repo: &OverlayRepo<'_>,
@@ -1125,11 +1235,11 @@ fn reconnect_outgoing(
         (source_sidx, Some(source_cidx)),
         Direction::Outgoing,
     );
-    reconnect_edges(graph, edges, (target_sidx, Some(target_cidx)))
+    reconnect_outgoing_edges(graph, edges, (target_sidx, Some(target_cidx)))
 }
 
 /// Delete all `edges` and recreate the edges with `target` as new source.
-fn reconnect_edges(
+fn reconnect_outgoing_edges(
     graph: &mut PetGraph,
     edges: Vec<EdgeOwned>,
     (target_sidx, target_first_commit_id): (SegmentIndex, Option<gix::ObjectId>),

crates/but-graph/src/lib.rs

@@ -233,6 +233,10 @@ pub struct Graph {
     /// The [`CommitIndex`] is empty if the entry point is an empty segment, one that is supposed to receive
     /// commits later.
     entrypoint: Option<(SegmentIndex, Option<CommitIndex>)>,
+    /// The ref_name used when starting the graph traversal. It is set to help assure that the entrypoint stays
+    /// on the correctly named segment, knowing that the post-process may alter segments quite substantially
+    /// when crating independent and dependent branches.
+    entrypoint_ref: Option<gix::refs::FullName>,
     /// The segment index of the extra target as provided for traversal.
     extra_target: Option<SegmentIndex>,
     /// It's `true` only if we have stopped the traversal due to a hard limit.

crates/but-graph/src/projection/workspace.rs

@@ -519,7 +519,6 @@ impl Graph {
         if ws.has_managed_ref() {
             let (lowest_base, lowest_base_sidx) =
                 ws_lower_bound.map_or((None, None), |(base, sidx)| (Some(base), Some(sidx)));
-            // dbg!(lowest_base, lowest_base_sidx);
             for stack_top_sidx in self
                 .inner
                 .neighbors_directed(ws_tip_segment.id, Direction::Outgoing)

crates/but-graph/tests/fixtures/scenarios.sh

@@ -738,6 +738,13 @@ EOF
     git checkout gitbutler/workspace
   )
 
+  cp -R no-target-without-ws-commit-ambiguous no-target-without-ws-commit-ambiguous-with-remotes
+  (cd no-target-without-ws-commit-ambiguous-with-remotes
+    add_main_remote_setup
+    remote_tracking_caught_up A
+    remote_tracking_caught_up B
+  )
+
   git init no-target-with-ws-commit
   (cd no-target-with-ws-commit
     commit init

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

@@ -34,6 +34,7 @@ fn unborn() -> anyhow::Result<()> {
                 None,
             ),
         ),
+        entrypoint_ref: None,
         extra_target: None,
         hard_limit_hit: false,
         options: Options {
@@ -138,6 +139,7 @@ fn detached() -> anyhow::Result<()> {
                 ),
             ),
         ),
+        entrypoint_ref: None,
         extra_target: None,
         hard_limit_hit: false,
         options: Options {