v3 apply: remote handling

crates/but-core/src/ref_metadata.rs

@@ -61,16 +61,19 @@ impl Workspace {
 
     /// Insert `branch` as new stack if it's not yet contained in the workspace and if `order` is not `None` or push
     /// it to the end of the stack list.
+    /// Use `relation` to indicate how the new stack should be seen.
     /// If a new stack is created, it's considered to be *in* the workspace. If there is an existing stack, it
     /// may also be marked as *outside the workspace*, we do not change this.
-    /// Note that `order` is only relevant at insertion time, not if the branch already exists.
+    /// Note that `order` is only relevant at insertion time, not if the branch already exists, and `relation`
+    /// is only used if the stack is newly created.
     /// Returns `(stack_id, segment_idx)` of the stack that was either newly created, or already present.
     /// Note that `segment_idx` may be non-0 if `branch` already existed as segment, and the caller has to
     /// deal with this.
     pub fn add_or_insert_new_stack_if_not_present(
         &mut self,
         branch: &FullNameRef,
         order: Option<usize>,
+        relation: WorkspaceCommitRelation,
     ) -> (usize, usize) {
         if let Some(owners) =
             self.find_owner_indexes_by_name(branch, StackKind::AppliedAndUnapplied)
@@ -80,7 +83,7 @@ impl Workspace {
 
         let stack = WorkspaceStack {
             id: StackId::generate(),
-            in_workspace: true,
+            workspacecommit_relation: relation,
             branches: vec![WorkspaceStackBranch {
                 ref_name: branch.to_owned(),
                 archived: false,
@@ -140,7 +143,7 @@ impl Workspace {
     pub fn stacks(&self, kind: StackKind) -> impl Iterator<Item = &WorkspaceStack> {
         self.stacks.iter().filter(move |s| {
             if matches!(kind, StackKind::Applied) {
-                s.in_workspace
+                s.workspacecommit_relation.is_in_workspace()
             } else {
                 true
             }
@@ -152,7 +155,7 @@ impl Workspace {
     pub fn stacks_mut(&mut self, kind: StackKind) -> impl Iterator<Item = &mut WorkspaceStack> {
         self.stacks.iter_mut().filter(move |s| {
             if matches!(kind, StackKind::Applied) {
-                s.in_workspace
+                s.workspacecommit_relation.is_in_workspace()
             } else {
                 true
             }
@@ -380,15 +383,39 @@ pub struct WorkspaceStack {
     ///
     /// Thus, branches are stored in traversal order, from the tip towards the base.
     pub branches: Vec<WorkspaceStackBranch>,
-    /// If `true`, the entire stack is expected to be visible in the workspace.
-    /// If `false`, it's considered unapplied, and is not supposed to be reachable from the workspace commit
-    /// nor is it usually shown.
+    /// How the stack acts in relation to the workspace commit.
+    pub workspacecommit_relation: WorkspaceCommitRelation,
+}
+
+/// The relationship that a [WorkspaceStack] *supposedly* has with a workspace commit.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum WorkspaceCommitRelation {
+    /// The stack is considered to be merged into the workspace commit, with its tree being observable
+    /// in the worktree associated with the workspace reference.
+    Merged,
+    /// The stack is supposed to be in the workspace commit, but its tree isn't observable
+    /// in the worktree associated with the workspace reference.
+    ///
+    /// Stacks with this relationship can never conflict with any other `Merged` stat.
+    UnmergedTree,
+    /// The stack may have previously been merged into the workspace commit,
+    /// and is considered *outside* of the workspace.
     ///
     /// The reason we have to keep stacks that aren't in the workspace is to keep
     /// information about their constituent branches, as well as their stack-ids which should remain as stable as possible.
     /// It's notable that stack-ids will change, and it's not possible overall to have a stack identity as such as
     /// the contained branches can be reshuffled at will.
-    pub in_workspace: bool,
+    Outside,
+}
+
+impl WorkspaceCommitRelation {
+    /// Return `true` if this relation suggests that the owning stack is reachable from the workspace commit.
+    pub fn is_in_workspace(&self) -> bool {
+        match self {
+            WorkspaceCommitRelation::Merged | WorkspaceCommitRelation::UnmergedTree => true,
+            WorkspaceCommitRelation::Outside => false,
+        }
+    }
 }
 
 /// A branch within a [`WorkspaceStack`], holding per-branch metadata that is
@@ -426,6 +453,11 @@ impl WorkspaceStack {
     pub fn name(&self) -> Option<&gix::refs::FullNameRef> {
         self.ref_name().map(|rn| rn.as_ref())
     }
+
+    /// Return `true` if this relation suggests that the owning stack is reachable from the workspace commit.
+    pub fn is_in_workspace(&self) -> bool {
+        self.workspacecommit_relation.is_in_workspace()
+    }
 }
 
 /// Metadata about branches, associated with any Git branch.

crates/but-core/tests/core/ref_metadata.rs

@@ -1,4 +1,5 @@
 mod workspace {
+    use but_core::ref_metadata::WorkspaceCommitRelation::Merged;
     use but_core::ref_metadata::{StackKind::AppliedAndUnapplied, Workspace};
 
     #[test]
@@ -8,18 +9,18 @@ mod workspace {
 
         let a_ref = r("refs/heads/A");
         assert_eq!(
-            ws.add_or_insert_new_stack_if_not_present(a_ref, Some(100)),
+            ws.add_or_insert_new_stack_if_not_present(a_ref, Some(100), Merged),
             (0, 0)
         );
         assert_eq!(
-            ws.add_or_insert_new_stack_if_not_present(a_ref, Some(200)),
+            ws.add_or_insert_new_stack_if_not_present(a_ref, Some(200), Merged),
             (0, 0)
         );
         assert_eq!(ws.stacks.len(), 1);
 
         let b_ref = r("refs/heads/B");
         assert_eq!(
-            ws.add_or_insert_new_stack_if_not_present(b_ref, Some(0)),
+            ws.add_or_insert_new_stack_if_not_present(b_ref, Some(0), Merged),
             (0, 0)
         );
         assert_eq!(
@@ -29,7 +30,7 @@ mod workspace {
 
         let c_ref = r("refs/heads/C");
         assert_eq!(
-            ws.add_or_insert_new_stack_if_not_present(c_ref, None),
+            ws.add_or_insert_new_stack_if_not_present(c_ref, None, Merged),
             (2, 0)
         );
         assert_eq!(
@@ -67,7 +68,7 @@ mod workspace {
             "anchor doesn't exist"
         );
         assert_eq!(
-            ws.add_or_insert_new_stack_if_not_present(a_ref, None),
+            ws.add_or_insert_new_stack_if_not_present(a_ref, None, Merged),
             (0, 0)
         );
         assert_eq!(
@@ -88,7 +89,7 @@ mod workspace {
         );
 
         assert_eq!(
-            ws.add_or_insert_new_stack_if_not_present(a_ref, None),
+            ws.add_or_insert_new_stack_if_not_present(a_ref, None, Merged),
             (0, 2),
             "adding a new stack can 'fail' if the segment is already present, but not as stack tip"
         );
@@ -113,7 +114,7 @@ mod workspace {
                             archived: false,
                         },
                     ],
-                    in_workspace: true,
+                    workspacecommit_relation: Merged,
                 },
             ],
             target_ref: None,

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

@@ -1,4 +1,5 @@
 use anyhow::{Context, bail};
+use bstr::ByteSlice;
 use but_core::{RefMetadata, ref_metadata};
 use gix::{
     hashtable::hash_map::Entry,
@@ -226,7 +227,7 @@ impl Graph {
     /// * The traversal is cut short when there is only tips which are integrated
     /// * The traversal is always as long as it needs to be to fully reconcile possibly disjoint branches, despite
     ///   this sometimes costing some time when the remote is far ahead in a huge repository.
-    #[instrument(level = tracing::Level::DEBUG, skip(meta, ref_name), err(Debug))]
+    #[instrument(level = tracing::Level::DEBUG, skip_all, fields(tip = ?tip, options = ?options, ref_name), err(Debug))]
     pub fn from_commit_traversal(
         tip: gix::Id<'_>,
         ref_name: impl Into<Option<gix::refs::FullName>>,
@@ -245,6 +246,12 @@ impl Graph {
         options: Options,
     ) -> anyhow::Result<Self> {
         let ref_name = ref_name.into();
+        {
+            if let Some(name) = &ref_name {
+                let span = tracing::Span::current();
+                span.record("ref_name", name.as_bstr().to_str_lossy().as_ref());
+            }
+        }
         let mut graph = Graph {
             options: options.clone(),
             entrypoint_ref: ref_name.clone(),
@@ -515,7 +522,7 @@ impl Graph {
             for segment in ws_metadata
                 .stacks
                 .into_iter()
-                .filter(|s| s.in_workspace)
+                .filter(|s| s.is_in_workspace())
                 .flat_map(|s| s.branches.into_iter())
             {
                 let Some(segment_tip) = repo

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

@@ -739,7 +739,7 @@ impl Graph {
             .collect();
         edges_pointing_to_named_segment.sort_by_key(|(_e, sidx, rn)| {
             let res = ws_data.stacks.iter().position(|s| {
-                s.in_workspace
+                s.is_in_workspace()
                     && s.branches
                         .first()
                         .is_some_and(|b| Some(&b.ref_name) == rn.as_ref())

crates/but-graph/src/ref_metadata_legacy.rs

@@ -7,8 +7,10 @@ use std::{
     time::Instant,
 };
 
+use crate::virtual_branches_legacy_types::{CommitOrChangeId, Stack, StackBranch, VirtualBranches};
 use anyhow::{Context, bail};
 use bstr::ByteSlice;
+use but_core::ref_metadata::WorkspaceCommitRelation;
 use but_core::{
     RefMetadata,
     ref_metadata::{
@@ -25,8 +27,6 @@ use gix::{
 };
 use itertools::Itertools;
 
-use crate::virtual_branches_legacy_types::{CommitOrChangeId, Stack, StackBranch, VirtualBranches};
-
 #[derive(Debug)]
 struct Snapshot {
     /// The time at which the `content` was changed, before it was written to disk.
@@ -368,7 +368,7 @@ impl RefMetadata for VirtualBranchesTomlMetadata {
                     branch.archived,
                 ))
             }
-            vb_stack.in_workspace = stack.in_workspace;
+            vb_stack.in_workspace = stack.is_in_workspace();
             vb_stack.heads.sort_by_key(|head| {
                 stack.branches.iter().enumerate().find_map(|(idx, branch)| {
                     (branch.ref_name.shorten() == head.name.as_str()).then_some(idx)
@@ -483,7 +483,7 @@ impl RefMetadata for VirtualBranchesTomlMetadata {
                 *review_id = value.review.review_id.clone();
                 if let Some((stack_idx, segment_idx)) = metadata_stack_indices {
                     let meta_stack = &ws.stacks[stack_idx];
-                    stack.in_workspace = meta_stack.in_workspace;
+                    stack.in_workspace = meta_stack.is_in_workspace();
                     *archived = meta_stack.branches[segment_idx].archived;
                 }
                 Ok(())
@@ -574,7 +574,11 @@ impl VirtualBranchesTomlMetadata {
                 .sorted_by_key(|s| s.order)
                 .map(|s| WorkspaceStack {
                     id: s.id,
-                    in_workspace: s.in_workspace,
+                    workspacecommit_relation: if s.in_workspace {
+                        WorkspaceCommitRelation::Merged
+                    } else {
+                        WorkspaceCommitRelation::Outside
+                    },
                     branches: s
                         .heads
                         .iter()

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

@@ -1,3 +1,4 @@
+use but_core::ref_metadata::WorkspaceCommitRelation;
 use but_core::{
     RefMetadata,
     ref_metadata::{StackId, WorkspaceStack, WorkspaceStackBranch},
@@ -2312,7 +2313,7 @@ fn workspace_without_target_can_see_remote() -> anyhow::Result<()> {
                 ref_name: ref_name.try_into()?,
                 archived: false,
             }],
-            in_workspace: true,
+            workspacecommit_relation: WorkspaceCommitRelation::Merged,
         });
         meta.branches.push((
             ref_name.try_into()?,