Merge pull request #10995 from Byron/fix

applyv3: tracking-branch usability

Author: Byron

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

@@ -128,6 +128,12 @@ impl Options {
         self.commits_limit_recharge_location.extend(commits);
         self
     }
+
+    /// Set the extra-target which should be included in the workspace.
+    pub fn with_extra_target_commit_id(mut self, id: impl Into<gix::ObjectId>) -> Self {
+        self.extra_target_commit_id = Some(id.into());
+        self
+    }
 }
 
 /// Lifecycle

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

@@ -77,7 +77,7 @@ impl Graph {
         // We perform view-related updates here for convenience, but also because the graph
         // traversal should have nothing to do with workspace details. It's just about laying
         // the foundation for figuring out our workspaces more easily.
-        self.workspace_upgrades(meta, repo)?;
+        self.workspace_upgrades(meta, repo, &refs_by_id)?;
 
         // Point entrypoint to the right spot after all the virtual branches were added.
         self.set_entrypoint_to_ref_name(meta)?;
@@ -352,6 +352,23 @@ impl Graph {
             );
             self.inner.remove_edge(edge.id);
         }
+
+        if cidx_for_new_segment == 0 {
+            // The top-segment is still connected with edges that think they link to a commit, so adjust them.
+            let edges_to_adjust: Vec<_> = self
+                .inner
+                .edges_directed(sidx, Direction::Incoming)
+                .map(|e| e.id())
+                .collect();
+            for edge_id in edges_to_adjust {
+                let edge = self
+                    .inner
+                    .edge_weight_mut(edge_id)
+                    .expect("still present as we just saw it");
+                edge.dst = None;
+                edge.dst_id = None;
+            }
+        }
         Ok(new_segment_sidx)
     }
 
@@ -532,13 +549,22 @@ impl Graph {
         &mut self,
         meta: &OverlayMetadata<'_, T>,
         repo: &OverlayRepo<'_>,
+        refs_by_id: &RefsById,
     ) -> anyhow::Result<()> {
-        let Some((ws_sidx, ws_stacks, ws_data, ws_target)) = self
+        let Some((ws_sidx, ws_stacks, ws_data, ws_target, ws_low_bound_sidx)) = self
             .to_workspace_inner(workspace::Downgrade::Disallow)
             .ok()
             .and_then(|mut ws| {
                 let md = ws.metadata.take();
-                md.map(|md| (ws.id, ws.stacks, md, ws.target))
+                md.map(|md| {
+                    let lower_bound_if_in_workspace = ws.lower_bound_segment_id.filter(|lb_sidx| {
+                        ws.stacks
+                            .iter()
+                            .flat_map(|s| s.segments.iter().map(|s| s.id))
+                            .any(|sid| sid == *lb_sidx)
+                    });
+                    (ws.id, ws.stacks, md, ws.target, lower_bound_if_in_workspace)
+                })
             })
         else {
             return Ok(());
@@ -810,6 +836,16 @@ impl Graph {
             });
             self[sidx].sibling_segment_id = named_segment_id;
         }
+
+        // The named-segment check is needed as we don't want to double-split unnamed segments.
+        // What this really does is to pass ownership of the base commit from a named segment to an unnamed one,
+        // as all algorithms kind of rely on it.
+        // So if this ever becomes a problem, we can also try to adjust said algorithms downstream.
+        if let Some(low_bound_segment_id) = ws_low_bound_sidx
+            && self[low_bound_segment_id].ref_name.is_some()
+        {
+            self.split_segment(low_bound_segment_id, 0, None, Some(refs_by_id), meta)?;
+        }
         Ok(())
     }
 

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

@@ -70,7 +70,7 @@ impl Stack {
         }
         if let Some((last_segment, last_aggregated_sidx)) = segments.last_mut().and_then(|s| {
             let sidx = s.commits_by_segment.last().map(|t| t.0)?;
-            (s, sidx).into()
+            Some((s, sidx))
         }) {
             let first_parent_sidx = graph
                 .neighbors_directed(last_aggregated_sidx, Direction::Outgoing)

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

@@ -584,7 +584,7 @@ impl Graph {
                                 return stop;
                             }
                             // Check for anonymous segments with sibling ID - these know their
-                            // named counterparts and we want to set the name, but they must
+                            // named counterparts, and we want to set the name, but they must
                             // be in their own stack-segment.
                             if s.ref_name.is_none() && s.sibling_segment_id.is_some() {
                                 return stop;
@@ -851,7 +851,7 @@ impl Graph {
     fn collect_stack_segments(
         &self,
         from: SegmentIndex,
-        entrypoint_sidx: Option<SegmentIndex>,
+        mut entrypoint_sidx: Option<SegmentIndex>,
         mut is_one_past_end_of_stack_segment: impl FnMut(&Segment) -> bool,
         mut starts_next_stack_segment: impl FnMut(&Segment) -> bool,
         mut discard_stack: impl FnMut(&StackSegment) -> bool,
@@ -866,6 +866,7 @@ impl Graph {
             let mut segment = StackSegment::from_graph_segments(&segments, self)?;
             if entrypoint_sidx.is_some_and(|id| segment.id == id) {
                 segment.is_entrypoint = true;
+                entrypoint_sidx = None;
             }
             out.push(segment);
             next = stopped_at

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

@@ -205,6 +205,65 @@ git init special-branches
 
 mkdir ws
 (cd ws
+  git init remote-and-integrated-tracking-linear
+  (cd remote-and-integrated-tracking-linear
+     commit M1
+     commit M-base
+     git branch A
+     git checkout -b soon-origin-A main
+       commit A-remote
+     git checkout main
+       commit M-advanced
+       setup_target_to_match_main
+
+     git checkout A
+     create_workspace_commit_once A
+     setup_remote_tracking soon-origin-A A "move"
+  )
+
+  git init remote-and-integrated-tracking
+  (cd remote-and-integrated-tracking
+     commit M1
+     commit M2
+     git checkout -b tmp1
+      commit X
+     git checkout main
+     commit Y
+     git merge --no-ff tmp1 -m "M-base"
+     git branch A
+     git checkout -b soon-origin-A main
+       commit A-remote
+     git checkout main
+       commit M-advanced
+       setup_target_to_match_main
+
+     git checkout A
+     create_workspace_commit_once A
+     setup_remote_tracking soon-origin-A A "move"
+  )
+
+  git init remote-and-integrated-tracking-extra-commit
+  (cd remote-and-integrated-tracking-extra-commit
+     commit M1
+     commit M2
+     git checkout -b tmp1
+      commit X
+     git checkout main
+     commit Y
+     git merge --no-ff tmp1 -m "M-base"
+     git checkout -b A
+       commit A-local
+     git checkout -b soon-origin-A main
+       commit A-remote
+     git checkout main
+       commit M-advanced
+       setup_target_to_match_main
+
+     git checkout A
+     create_workspace_commit_once A
+     setup_remote_tracking soon-origin-A A "move"
+  )
+
   git init single-stack-ambiguous
   (cd single-stack-ambiguous
      commit init

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

@@ -5218,6 +5218,109 @@ fn dependent_branch_on_base() -> anyhow::Result<()> {
     Ok(())
 }
 
+#[test]
+fn remote_and_integrated_tracking_branch_on_merge() -> anyhow::Result<()> {
+    let (repo, mut meta) = read_only_in_memory_scenario("ws/remote-and-integrated-tracking")?;
+    insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r"
+    * d018f71 (HEAD -> gitbutler/workspace) GitButler Workspace Commit
+    | * c1e26b0 (origin/main, main) M-advanced
+    |/  
+    | * 2181501 (origin/A) A-remote
+    |/  
+    *   1ee1e34 (A) M-base
+    |\  
+    | * efc3b77 (tmp1) X
+    * | c822d66 Y
+    |/  
+    * bce0c5e M2
+    * 3183e43 M1
+    ");
+    add_stack_with_segments(&mut meta, 1, "A", StackState::InWorkspace, &[]);
+
+    let graph = Graph::from_head(
+        &repo,
+        &*meta,
+        standard_options().with_extra_target_commit_id(repo.rev_parse_single("origin/main")?),
+    )?
+    .validated()?;
+    insta::assert_snapshot!(graph_workspace(&graph.to_workspace()?), @r"
+    📕🏘️:0:gitbutler/workspace <> ✓refs/remotes/origin/main⇣1 on 1ee1e34
+    └── ≡📙:3:A <> origin/A →:4:⇣1 on 1ee1e34 {1}
+        └── 📙:3:A <> origin/A →:4:⇣1
+            └── 🟣2181501
+    ");
+
+    Ok(())
+}
+
+#[test]
+fn remote_and_integrated_tracking_branch_on_linear_segment() -> anyhow::Result<()> {
+    let (repo, mut meta) =
+        read_only_in_memory_scenario("ws/remote-and-integrated-tracking-linear")?;
+    insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r"
+    * 21e584f (HEAD -> gitbutler/workspace) GitButler Workspace Commit
+    | * 8dc508f (origin/main, main) M-advanced
+    |/  
+    | * 197ddce (origin/A) A-remote
+    |/  
+    * 081bae9 (A) M-base
+    * 3183e43 M1
+    ");
+    add_stack_with_segments(&mut meta, 1, "A", StackState::InWorkspace, &[]);
+
+    let graph = Graph::from_head(
+        &repo,
+        &*meta,
+        standard_options().with_extra_target_commit_id(repo.rev_parse_single("origin/main")?),
+    )?
+    .validated()?;
+    insta::assert_snapshot!(graph_workspace(&graph.to_workspace()?), @r"
+    📕🏘️:0:gitbutler/workspace <> ✓refs/remotes/origin/main⇣1 on 081bae9
+    └── ≡📙:3:A <> origin/A →:4:⇣1 on 081bae9 {1}
+        └── 📙:3:A <> origin/A →:4:⇣1
+            └── 🟣197ddce
+    ");
+
+    Ok(())
+}
+
+#[test]
+fn remote_and_integrated_tracking_branch_on_merge_extra_target() -> anyhow::Result<()> {
+    let (repo, mut meta) =
+        read_only_in_memory_scenario("ws/remote-and-integrated-tracking-extra-commit")?;
+    insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r"
+    * 5f2810f (HEAD -> gitbutler/workspace) GitButler Workspace Commit
+    * 9f47a25 (A) A-local
+    | * c1e26b0 (origin/main, main) M-advanced
+    |/  
+    | * 2181501 (origin/A) A-remote
+    |/  
+    *   1ee1e34 M-base
+    |\  
+    | * efc3b77 (tmp1) X
+    * | c822d66 Y
+    |/  
+    * bce0c5e M2
+    * 3183e43 M1
+    ");
+    add_stack_with_segments(&mut meta, 1, "A", StackState::InWorkspace, &[]);
+    let graph = Graph::from_head(
+        &repo,
+        &*meta,
+        standard_options().with_extra_target_commit_id(repo.rev_parse_single("origin/main")?),
+    )?
+    .validated()?;
+    insta::assert_snapshot!(graph_workspace(&graph.to_workspace()?), @r"
+    📕🏘️:0:gitbutler/workspace <> ✓refs/remotes/origin/main⇣1 on 1ee1e34
+    └── ≡📙:3:A <> origin/A →:4:⇡1⇣1 on 1ee1e34 {1}
+        └── 📙:3:A <> origin/A →:4:⇡1⇣1
+            ├── 🟣2181501
+            └── ·9f47a25 (🏘️)
+    ");
+
+    Ok(())
+}
+
 #[test]
 fn unapplied_branch_on_base() -> anyhow::Result<()> {
     let (repo, mut meta) = read_only_in_memory_scenario("ws/unapplied-branch-on-base")?;

crates/gitbutler-project/src/project.rs

@@ -202,7 +202,7 @@ impl Project {
         let mut projects = crate::dangerously_list_projects_without_migration()?;
         // Sort projects by longest pathname to shortest.
         // We need to do this because users might have one gitbutler project
-        // nexted insided of another via a gitignored folder.
+        // nexted inside another via a gitignored folder.
         // We want to match on the longest project path.
         projects.sort_by(|a, b| {
             a.worktree_dir