Merge pull request #10498 from Byron/fix4

properly fix the 'too many upstream commits' issue

Author: Byron

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

@@ -90,8 +90,7 @@ impl Options {
     pub fn limited() -> Self {
         Options {
             collect_tags: false,
-            // TODO: put this back down once the underlying issue is fixed.
-            commits_limit_hint: Some(600),
+            commits_limit_hint: Some(300),
             ..Default::default()
         }
     }
@@ -620,7 +619,11 @@ impl Graph {
             };
 
             let refs_at_commit_before_removal = ctx.refs_by_id.remove(&id).unwrap_or_default();
-            let (remote_items, maybe_goal_for_id) = try_queue_remote_tracking_branches(
+            let RemoteQueueOutcome {
+                items_to_queue_later,
+                maybe_make_id_a_goal_so_remote_can_find_local,
+                limit_to_let_local_find_remote,
+            } = try_queue_remote_tracking_branches(
                 repo,
                 &refs_at_commit_before_removal,
                 &mut graph,
@@ -635,14 +638,14 @@ impl Graph {
 
             let segment = &mut graph[segment_idx_for_id];
             let commit_idx_for_possible_fork = segment.commits.len();
-            let propagated_flags = propagated_flags | maybe_goal_for_id;
+            let propagated_flags = propagated_flags | maybe_make_id_a_goal_so_remote_can_find_local;
             let hard_limit_hit = queue_parents(
                 &mut next,
                 &info.parent_ids,
                 propagated_flags,
                 segment_idx_for_id,
                 commit_idx_for_possible_fork,
-                limit,
+                limit.additional_goal(limit_to_let_local_find_remote),
             );
             if hard_limit_hit {
                 return graph.post_processed(meta, tip, ctx.with_hard_limit());
@@ -664,13 +667,13 @@ impl Graph {
                 )?,
             );
 
-            for item in remote_items {
+            for item in items_to_queue_later {
                 if next.push_back_exhausted(item) {
                     return graph.post_processed(meta, tip, ctx.with_hard_limit());
                 }
             }
 
-            prune_integrated_tips(&mut graph, &mut next);
+            prune_integrated_tips(&mut graph, &mut next)?;
         }
 
         graph.post_processed(meta, tip, ctx)

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

@@ -689,6 +689,16 @@ pub fn propagate_flags_downward(
     topo.leafs.take().filter(|v| !v.is_empty())
 }
 
+pub(crate) struct RemoteQueueOutcome {
+    /// The new tips to queue officially later.
+    pub items_to_queue_later: Vec<QueueItem>,
+    /// A way for the remote to find the local tracking branch.
+    pub maybe_make_id_a_goal_so_remote_can_find_local: CommitFlags,
+    /// A way for the local tracking branch to find the remote.
+    /// Only set if `items_to_queue_later` is also set.
+    pub limit_to_let_local_find_remote: CommitFlags,
+}
+
 /// Check `refs` for refs with remote tracking branches, and return a queue for them for traversal after creating a segment
 /// named after the tracking branch.
 /// This eager queuing makes sure that the post-processing doesn't have to traverse again when it creates segments
@@ -708,8 +718,9 @@ pub fn try_queue_remote_tracking_branches<T: RefMetadata>(
     id: gix::ObjectId,
     limit: Limit,
     goals: &mut Goals,
-) -> anyhow::Result<(Vec<QueueItem>, CommitFlags)> {
+) -> anyhow::Result<RemoteQueueOutcome> {
     let mut goal_flags = CommitFlags::empty();
+    let mut limit_flags = CommitFlags::empty();
     let mut queue = Vec::new();
     for rn in refs {
         let Some(remote_tracking_branch) = remotes::lookup_remote_tracking_branch_or_deduce_it(
@@ -737,19 +748,27 @@ pub fn try_queue_remote_tracking_branches<T: RefMetadata>(
             )?);
 
         let remote_limit = limit.with_indirect_goal(id, goals);
+        let self_flags = goals.flag_for(remote_tip).unwrap_or_default();
+        limit_flags |= self_flags;
         // These flags are to be attached to `id` so it can propagate itself later.
         // The remote limit is for searching `id`.
+        // Also, this makes the local tracking branch look for its remote, which is important
+        // if the remote is far away as the local branch was rebased somewhere far ahead of the remote.
         goal_flags |= remote_limit.goal_flags();
         queue.push((
             remote_tip,
-            CommitFlags::empty(),
+            self_flags,
             Instruction::CollectCommit {
                 into: remote_segment,
             },
             remote_limit,
         ));
     }
-    Ok((queue, goal_flags))
+    Ok(RemoteQueueOutcome {
+        items_to_queue_later: queue,
+        maybe_make_id_a_goal_so_remote_can_find_local: goal_flags,
+        limit_to_let_local_find_remote: limit_flags,
+    })
 }
 
 pub fn possibly_split_occupied_segment(
@@ -881,20 +900,20 @@ fn propagate_goals_of_reachable_tips(
 /// as these are uninteresting.
 /// However, do so only if our entrypoint isn't integrated itself and is not in a workspace. The reason for this is that we
 /// always also traverse workspaces and their targets, even if the traversal starts outside a workspace.
-pub fn prune_integrated_tips(graph: &mut Graph, next: &mut Queue) {
+pub fn prune_integrated_tips(graph: &mut Graph, next: &mut Queue) -> anyhow::Result<()> {
     let all_integated_and_done = next.iter().all(|(_id, flags, _instruction, tip_limit)| {
         flags.contains(CommitFlags::Integrated) && tip_limit.goal_reached()
     });
     if !all_integated_and_done {
-        return;
+        return Ok(());
     }
-    if graph
-        .lookup_entrypoint()
-        .ok()
-        .and_then(|ep| ep.segment.non_empty_flags_of_first_commit())
+    let ep = graph.lookup_entrypoint()?;
+    if ep
+        .segment
+        .non_empty_flags_of_first_commit()
         .is_some_and(|flags| flags.contains(CommitFlags::Integrated))
     {
-        return;
+        return Ok(());
     }
 
     next.inner
@@ -906,4 +925,5 @@ pub fn prune_integrated_tips(graph: &mut Graph, next: &mut Queue) {
             }
             false
         });
+    Ok(())
 }

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

@@ -1124,5 +1124,32 @@ EOF
     setup_target_to_match_main
     create_workspace_commit_once main
   )
+
+  git init remote-far-in-ancestry
+  (cd remote-far-in-ancestry
+    commit M1
+    git checkout -b soon-A-remote
+      commit R2
+      commit R3
+      setup_remote_tracking soon-A-remote A move
+    git checkout main
+    commit M2
+    commit M3
+    commit M4
+    commit M5
+    commit M6
+    commit M7
+    commit M8
+    commit M9
+    commit M10
+    commit M11
+    commit M12
+    setup_target_to_match_main
+    git checkout -b A
+    commit A1
+    commit A2
+    commit A3
+    create_workspace_commit_once A
+  )
 )
 

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

@@ -297,13 +297,13 @@ fn stacked_rebased_remotes() -> anyhow::Result<()> {
     ├── 👉►:0[0]:B <> origin/B →:1:
     │   └── ·312f819 (⌂|1)
     │       └── ►:2[1]:A <> origin/A →:3:
-    │           └── ·e255adc (⌂|11)
+    │           └── ·e255adc (⌂|101)
     │               └── ►:4[2]:main
-    │                   └── ·fafd9d0 (⌂|11)
+    │                   └── ·fafd9d0 (⌂|1111)
     └── ►:1[0]:origin/B →:0:
-        └── 🟣682be32
+        └── 🟣682be32 (0x0|10)
             └── ►:3[1]:origin/A →:2:
-                └── 🟣e29c23d
+                └── 🟣e29c23d (0x0|1010)
                     └── →:4: (main)
     ");
 
@@ -328,11 +328,11 @@ fn stacked_rebased_remotes() -> anyhow::Result<()> {
     ├── 👉►:0[0]:B <> origin/B →:1:
     │   └── ·312f819 (⌂|1)
     │       └── ►:2[1]:A <> origin/A →:3:
-    │           └── ·e255adc (⌂|11)
+    │           └── ·e255adc (⌂|101)
     │               └── ►:4[2]:main
-    │                   └── ·fafd9d0 (⌂|11)
+    │                   └── ·fafd9d0 (⌂|101)
     ├── ►:1[0]:origin/B →:0:
-    │   └── ❌🟣682be32
+    │   └── ❌🟣682be32 (0x0|10)
     └── ►:3[0]:origin/A →:2:
     ");
     // As the remotes don't connect, they are entirely unknown.
@@ -354,13 +354,13 @@ fn stacked_rebased_remotes() -> anyhow::Result<()> {
     ├── 👉►:0[0]:B <> origin/B →:1:
     │   └── ·312f819 (⌂|1)
     │       └── ►:2[1]:A <> origin/A →:3:
-    │           └── ·e255adc (⌂|11)
+    │           └── ·e255adc (⌂|101)
     │               └── ►:4[2]:main
-    │                   └── ·fafd9d0 (⌂|11)
+    │                   └── ·fafd9d0 (⌂|1111)
     └── ►:1[0]:origin/B →:0:
-        └── 🟣682be32
+        └── 🟣682be32 (0x0|10)
             └── ►:3[1]:origin/A →:2:
-                └── 🟣e29c23d
+                └── 🟣e29c23d (0x0|1010)
                     └── →:4: (main)
     ");
 
@@ -371,9 +371,9 @@ fn stacked_rebased_remotes() -> anyhow::Result<()> {
     ├── 👉►:0[0]:A <> origin/A →:1:
     │   └── ·e255adc (⌂|1)
     │       └── ►:2[1]:main
-    │           └── ·fafd9d0 (⌂|1)
+    │           └── ·fafd9d0 (⌂|11)
     └── ►:1[0]:origin/A →:0:
-        └── 🟣e29c23d
+        └── 🟣e29c23d (0x0|10)
             └── →:2: (main)
     ");
     insta::assert_snapshot!(graph_workspace(&graph.to_workspace()?), @r"