Skip to content

Commit 1e077ac

Browse files
committed
Let the local and remote branches find each other.
Otherwise, if the remote branch is far in the past, it will keep looking for the local tracking branch forever. But if the local tracking branch doesn't also search for the remote, then it will be allowed to stop and the two graph partitions might never meet. This leads to the upstream/remote ending up with all commits to the beginning of time, something that is relatively slow and more importantly, would easily overwhelm the UI.
1 parent 688b2b3 commit 1e077ac

File tree

5 files changed

+231
-104
lines changed

5 files changed

+231
-104
lines changed

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

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,7 @@ impl Options {
9090
pub fn limited() -> Self {
9191
Options {
9292
collect_tags: false,
93-
// TODO: put this back down once the underlying issue is fixed.
94-
commits_limit_hint: Some(600),
93+
commits_limit_hint: Some(300),
9594
..Default::default()
9695
}
9796
}
@@ -620,7 +619,11 @@ impl Graph {
620619
};
621620

622621
let refs_at_commit_before_removal = ctx.refs_by_id.remove(&id).unwrap_or_default();
623-
let (remote_items, maybe_goal_for_id) = try_queue_remote_tracking_branches(
622+
let RemoteQueueOutcome {
623+
items_to_queue_later,
624+
maybe_make_id_a_goal_so_remote_can_find_local,
625+
limit_to_let_local_find_remote,
626+
} = try_queue_remote_tracking_branches(
624627
repo,
625628
&refs_at_commit_before_removal,
626629
&mut graph,
@@ -635,14 +638,14 @@ impl Graph {
635638

636639
let segment = &mut graph[segment_idx_for_id];
637640
let commit_idx_for_possible_fork = segment.commits.len();
638-
let propagated_flags = propagated_flags | maybe_goal_for_id;
641+
let propagated_flags = propagated_flags | maybe_make_id_a_goal_so_remote_can_find_local;
639642
let hard_limit_hit = queue_parents(
640643
&mut next,
641644
&info.parent_ids,
642645
propagated_flags,
643646
segment_idx_for_id,
644647
commit_idx_for_possible_fork,
645-
limit,
648+
limit.additional_goal(limit_to_let_local_find_remote),
646649
);
647650
if hard_limit_hit {
648651
return graph.post_processed(meta, tip, ctx.with_hard_limit());
@@ -664,13 +667,13 @@ impl Graph {
664667
)?,
665668
);
666669

667-
for item in remote_items {
670+
for item in items_to_queue_later {
668671
if next.push_back_exhausted(item) {
669672
return graph.post_processed(meta, tip, ctx.with_hard_limit());
670673
}
671674
}
672675

673-
prune_integrated_tips(&mut graph, &mut next);
676+
prune_integrated_tips(&mut graph, &mut next)?;
674677
}
675678

676679
graph.post_processed(meta, tip, ctx)

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

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -689,6 +689,16 @@ pub fn propagate_flags_downward(
689689
topo.leafs.take().filter(|v| !v.is_empty())
690690
}
691691

692+
pub(crate) struct RemoteQueueOutcome {
693+
/// The new tips to queue officially later.
694+
pub items_to_queue_later: Vec<QueueItem>,
695+
/// A way for the remote to find the local tracking branch.
696+
pub maybe_make_id_a_goal_so_remote_can_find_local: CommitFlags,
697+
/// A way for the local tracking branch to find the remote.
698+
/// Only set if `items_to_queue_later` is also set.
699+
pub limit_to_let_local_find_remote: CommitFlags,
700+
}
701+
692702
/// Check `refs` for refs with remote tracking branches, and return a queue for them for traversal after creating a segment
693703
/// named after the tracking branch.
694704
/// 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>(
708718
id: gix::ObjectId,
709719
limit: Limit,
710720
goals: &mut Goals,
711-
) -> anyhow::Result<(Vec<QueueItem>, CommitFlags)> {
721+
) -> anyhow::Result<RemoteQueueOutcome> {
712722
let mut goal_flags = CommitFlags::empty();
723+
let mut limit_flags = CommitFlags::empty();
713724
let mut queue = Vec::new();
714725
for rn in refs {
715726
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>(
737748
)?);
738749

739750
let remote_limit = limit.with_indirect_goal(id, goals);
751+
let self_flags = goals.flag_for(remote_tip).unwrap_or_default();
752+
limit_flags |= self_flags;
740753
// These flags are to be attached to `id` so it can propagate itself later.
741754
// The remote limit is for searching `id`.
755+
// Also, this makes the local tracking branch look for its remote, which is important
756+
// if the remote is far away as the local branch was rebased somewhere far ahead of the remote.
742757
goal_flags |= remote_limit.goal_flags();
743758
queue.push((
744759
remote_tip,
745-
CommitFlags::empty(),
760+
self_flags,
746761
Instruction::CollectCommit {
747762
into: remote_segment,
748763
},
749764
remote_limit,
750765
));
751766
}
752-
Ok((queue, goal_flags))
767+
Ok(RemoteQueueOutcome {
768+
items_to_queue_later: queue,
769+
maybe_make_id_a_goal_so_remote_can_find_local: goal_flags,
770+
limit_to_let_local_find_remote: limit_flags,
771+
})
753772
}
754773

755774
pub fn possibly_split_occupied_segment(
@@ -881,20 +900,20 @@ fn propagate_goals_of_reachable_tips(
881900
/// as these are uninteresting.
882901
/// However, do so only if our entrypoint isn't integrated itself and is not in a workspace. The reason for this is that we
883902
/// always also traverse workspaces and their targets, even if the traversal starts outside a workspace.
884-
pub fn prune_integrated_tips(graph: &mut Graph, next: &mut Queue) {
903+
pub fn prune_integrated_tips(graph: &mut Graph, next: &mut Queue) -> anyhow::Result<()> {
885904
let all_integated_and_done = next.iter().all(|(_id, flags, _instruction, tip_limit)| {
886905
flags.contains(CommitFlags::Integrated) && tip_limit.goal_reached()
887906
});
888907
if !all_integated_and_done {
889-
return;
908+
return Ok(());
890909
}
891-
if graph
892-
.lookup_entrypoint()
893-
.ok()
894-
.and_then(|ep| ep.segment.non_empty_flags_of_first_commit())
910+
let ep = graph.lookup_entrypoint()?;
911+
if ep
912+
.segment
913+
.non_empty_flags_of_first_commit()
895914
.is_some_and(|flags| flags.contains(CommitFlags::Integrated))
896915
{
897-
return;
916+
return Ok(());
898917
}
899918

900919
next.inner
@@ -906,4 +925,5 @@ pub fn prune_integrated_tips(graph: &mut Graph, next: &mut Queue) {
906925
}
907926
false
908927
});
928+
Ok(())
909929
}

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1124,5 +1124,32 @@ EOF
11241124
setup_target_to_match_main
11251125
create_workspace_commit_once main
11261126
)
1127+
1128+
git init remote-far-in-ancestry
1129+
(cd remote-far-in-ancestry
1130+
commit M1
1131+
git checkout -b soon-A-remote
1132+
commit R2
1133+
commit R3
1134+
setup_remote_tracking soon-A-remote A move
1135+
git checkout main
1136+
commit M2
1137+
commit M3
1138+
commit M4
1139+
commit M5
1140+
commit M6
1141+
commit M7
1142+
commit M8
1143+
commit M9
1144+
commit M10
1145+
commit M11
1146+
commit M12
1147+
setup_target_to_match_main
1148+
git checkout -b A
1149+
commit A1
1150+
commit A2
1151+
commit A3
1152+
create_workspace_commit_once A
1153+
)
11271154
)
11281155

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

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -297,13 +297,13 @@ fn stacked_rebased_remotes() -> anyhow::Result<()> {
297297
├── 👉►:0[0]:B <> origin/B →:1:
298298
│ └── ·312f819 (⌂|1)
299299
│ └── ►:2[1]:A <> origin/A →:3:
300-
│ └── ·e255adc (⌂|11)
300+
│ └── ·e255adc (⌂|101)
301301
│ └── ►:4[2]:main
302-
│ └── ·fafd9d0 (⌂|11)
302+
│ └── ·fafd9d0 (⌂|1111)
303303
└── ►:1[0]:origin/B →:0:
304-
└── 🟣682be32
304+
└── 🟣682be32 (0x0|10)
305305
└── ►:3[1]:origin/A →:2:
306-
└── 🟣e29c23d
306+
└── 🟣e29c23d (0x0|1010)
307307
└── →:4: (main)
308308
");
309309

@@ -328,11 +328,11 @@ fn stacked_rebased_remotes() -> anyhow::Result<()> {
328328
├── 👉►:0[0]:B <> origin/B →:1:
329329
│ └── ·312f819 (⌂|1)
330330
│ └── ►:2[1]:A <> origin/A →:3:
331-
│ └── ·e255adc (⌂|11)
331+
│ └── ·e255adc (⌂|101)
332332
│ └── ►:4[2]:main
333-
│ └── ·fafd9d0 (⌂|11)
333+
│ └── ·fafd9d0 (⌂|101)
334334
├── ►:1[0]:origin/B →:0:
335-
│ └── ❌🟣682be32
335+
│ └── ❌🟣682be32 (0x0|10)
336336
└── ►:3[0]:origin/A →:2:
337337
");
338338
// As the remotes don't connect, they are entirely unknown.
@@ -354,13 +354,13 @@ fn stacked_rebased_remotes() -> anyhow::Result<()> {
354354
├── 👉►:0[0]:B <> origin/B →:1:
355355
│ └── ·312f819 (⌂|1)
356356
│ └── ►:2[1]:A <> origin/A →:3:
357-
│ └── ·e255adc (⌂|11)
357+
│ └── ·e255adc (⌂|101)
358358
│ └── ►:4[2]:main
359-
│ └── ·fafd9d0 (⌂|11)
359+
│ └── ·fafd9d0 (⌂|1111)
360360
└── ►:1[0]:origin/B →:0:
361-
└── 🟣682be32
361+
└── 🟣682be32 (0x0|10)
362362
└── ►:3[1]:origin/A →:2:
363-
└── 🟣e29c23d
363+
└── 🟣e29c23d (0x0|1010)
364364
└── →:4: (main)
365365
");
366366

@@ -371,9 +371,9 @@ fn stacked_rebased_remotes() -> anyhow::Result<()> {
371371
├── 👉►:0[0]:A <> origin/A →:1:
372372
│ └── ·e255adc (⌂|1)
373373
│ └── ►:2[1]:main
374-
│ └── ·fafd9d0 (⌂|1)
374+
│ └── ·fafd9d0 (⌂|11)
375375
└── ►:1[0]:origin/A →:0:
376-
└── 🟣e29c23d
376+
└── 🟣e29c23d (0x0|10)
377377
└── →:2: (main)
378378
");
379379
insta::assert_snapshot!(graph_workspace(&graph.to_workspace()?), @r"

0 commit comments

Comments
 (0)