Skip to content

Commit 25bad62

Browse files
committed
Handle references getting specified multiple times
1 parent b776c61 commit 25bad62

2 files changed

Lines changed: 189 additions & 17 deletions

File tree

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

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,9 @@ impl Overlay {
3838
///
3939
/// For example, if the `but_rebase::graph_rebase::Editor` converts a
4040
/// `Reference` step to a `None` step which is the equivalent of running
41-
/// `git update-ref -d`, it should no longer be part of the [`Graph`], so we
42-
/// would list the particular reference as a dropped reference.
41+
/// `git update-ref -d`, it should no longer be part of the
42+
/// [`crate::Graph`], so we would list the particular reference as a dropped
43+
/// reference.
4344
pub fn with_dropped_references(
4445
mut self,
4546
refs: impl IntoIterator<Item = gix::refs::FullName>,
@@ -104,16 +105,24 @@ impl Overlay {
104105
workspace,
105106
entrypoint,
106107
} = self;
108+
// Construct BTreeMaps with a deterministic order from left to right.
109+
let mut or = BTreeMap::new();
110+
for reference in overriding_references {
111+
if !or.contains_key(&reference.name) {
112+
or.insert(reference.name.clone(), reference);
113+
}
114+
}
115+
let mut nor = BTreeMap::new();
116+
for reference in nonoverriding_references {
117+
if !nor.contains_key(&reference.name) {
118+
nor.insert(reference.name.clone(), reference);
119+
}
120+
}
121+
107122
(
108123
OverlayRepo {
109-
nonoverriding_references: nonoverriding_references
110-
.into_iter()
111-
.map(|r| (r.name.clone(), r))
112-
.collect(),
113-
overriding_references: overriding_references
114-
.into_iter()
115-
.map(|r| (r.name.clone(), r))
116-
.collect(),
124+
nonoverriding_references: nor,
125+
overriding_references: or,
117126
dropped_references: dropped_references.into_iter().collect(),
118127
inner: repo,
119128
},
@@ -236,9 +245,13 @@ impl<'repo> OverlayRepo<'repo> {
236245
prefixes: impl Iterator<Item = &'a str>,
237246
workspace_ref_names: &[&gix::refs::FullNameRef],
238247
) -> anyhow::Result<RefsById> {
239-
let mut seen = (!self.nonoverriding_references.is_empty()).then(BTreeSet::new);
248+
let mut seen = BTreeSet::new();
240249
let mut ref_filter =
241250
|r: gix::Reference<'_>| -> Option<(gix::ObjectId, gix::refs::FullName)> {
251+
if self.dropped_references.contains(r.name()) {
252+
return None;
253+
}
254+
242255
if workspace_ref_names.contains(&r.name()) {
243256
return None;
244257
}
@@ -251,11 +264,7 @@ impl<'repo> OverlayRepo<'repo> {
251264
(id.detach(), r.inner.name)
252265
};
253266
// This is only for overrides.
254-
if let Some(seen) = seen.as_mut() {
255-
seen.insert(name.clone()).then_some((id, name))
256-
} else {
257-
Some((id, name))
258-
}
267+
seen.insert(name.clone()).then_some((id, name))
259268
};
260269
let mut all_refs_by_id = gix::hashtable::HashMap::<_, Vec<_>>::default();
261270
for prefix in prefixes {

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

Lines changed: 164 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
//! Some tests that expliclity test the overlay functionality
1+
//! Some tests that explicitly test the overlay functionality
22
33
use but_graph::{Graph, init::Overlay};
44
use but_testsupport::{graph_tree, visualize_commit_graph_all};
@@ -160,3 +160,166 @@ fn drop_head_ref() -> anyhow::Result<()> {
160160

161161
Ok(())
162162
}
163+
164+
#[test]
165+
fn overriding_references() -> anyhow::Result<()> {
166+
let (repo, meta) = read_only_in_memory_scenario("four-diamond")?;
167+
insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r"
168+
* 8a6c109 (HEAD -> merged) Merge branch 'C' into merged
169+
|\
170+
| * 7ed512a (C) Merge branch 'D' into C
171+
| |\
172+
| | * ecb1877 (D) D
173+
| * | 35ee481 C
174+
| |/
175+
* | 62b409a (A) Merge branch 'B' into A
176+
|\ \
177+
| * | f16dddf (B) B
178+
| |/
179+
* / 592abec A
180+
|/
181+
* 965998b (main) base
182+
");
183+
184+
let graph = Graph::from_head(&repo, &*meta, standard_options())?;
185+
insta::assert_snapshot!(graph_tree(&graph), @r"
186+
187+
└── 👉►:0[0]:merged[🌳]
188+
└── ·8a6c109 (⌂|1)
189+
├── ►:1[1]:A
190+
│ └── ·62b409a (⌂|1)
191+
│ ├── ►:3[2]:anon:
192+
│ │ └── ·592abec (⌂|1)
193+
│ │ └── ►:7[3]:main
194+
│ │ └── ·965998b (⌂|1)
195+
│ └── ►:4[2]:B
196+
│ └── ·f16dddf (⌂|1)
197+
│ └── →:7: (main)
198+
└── ►:2[1]:C
199+
└── ·7ed512a (⌂|1)
200+
├── ►:5[2]:anon:
201+
│ └── ·35ee481 (⌂|1)
202+
│ └── →:7: (main)
203+
└── ►:6[2]:D
204+
└── ·ecb1877 (⌂|1)
205+
└── →:7: (main)
206+
");
207+
208+
let merged_a = repo.rev_parse_single("35ee481")?;
209+
let merged_b = repo.rev_parse_single("592abec")?;
210+
let merged: gix::refs::FullName = "refs/heads/merged".try_into()?;
211+
212+
// The dropped takes precedence over git or overriding references.
213+
let overlay = Overlay::default()
214+
.with_dropped_references([merged.clone()])
215+
.with_references([
216+
gix::refs::Reference {
217+
name: merged.clone(),
218+
target: gix::refs::Target::Object(merged_a.detach()),
219+
peeled: Some(merged_a.detach()),
220+
},
221+
gix::refs::Reference {
222+
name: merged.clone(),
223+
target: gix::refs::Target::Object(merged_b.detach()),
224+
peeled: Some(merged_b.detach()),
225+
},
226+
]);
227+
228+
let graph = graph.redo_traversal_with_overlay(&repo, &*meta, overlay)?;
229+
230+
insta::assert_snapshot!(graph_tree(&graph), @"
231+
232+
└── ►:0[0]:anon:
233+
└── 👉·8a6c109 (⌂|1)
234+
├── ►:1[1]:A
235+
│ └── ·62b409a (⌂|1)
236+
│ ├── ►:3[2]:anon:
237+
│ │ └── ·592abec (⌂|1)
238+
│ │ └── ►:7[3]:main
239+
│ │ └── ·965998b (⌂|1)
240+
│ └── ►:4[2]:B
241+
│ └── ·f16dddf (⌂|1)
242+
│ └── →:7: (main)
243+
└── ►:2[1]:C
244+
└── ·7ed512a (⌂|1)
245+
├── ►:5[2]:anon:
246+
│ └── ·35ee481 (⌂|1)
247+
│ └── →:7: (main)
248+
└── ►:6[2]:D
249+
└── ·ecb1877 (⌂|1)
250+
└── →:7: (main)
251+
");
252+
253+
// The first overriding reference precedence over git or other overriding references.
254+
let overlay = Overlay::default().with_references([
255+
gix::refs::Reference {
256+
name: merged.clone(),
257+
target: gix::refs::Target::Object(merged_a.detach()),
258+
peeled: Some(merged_a.detach()),
259+
},
260+
gix::refs::Reference {
261+
name: merged.clone(),
262+
target: gix::refs::Target::Object(merged_b.detach()),
263+
peeled: Some(merged_b.detach()),
264+
},
265+
]);
266+
267+
let graph = graph.redo_traversal_with_overlay(&repo, &*meta, overlay)?;
268+
269+
insta::assert_snapshot!(graph_tree(&graph), @"
270+
271+
└── ►:0[0]:anon:
272+
└── 👉·8a6c109 (⌂|1)
273+
├── ►:1[1]:A
274+
│ └── ·62b409a (⌂|1)
275+
│ ├── ►:3[2]:anon:
276+
│ │ └── ·592abec (⌂|1)
277+
│ │ └── ►:7[3]:main
278+
│ │ └── ·965998b (⌂|1)
279+
│ └── ►:4[2]:B
280+
│ └── ·f16dddf (⌂|1)
281+
│ └── →:7: (main)
282+
└── ►:2[1]:C
283+
└── ·7ed512a (⌂|1)
284+
├── ►:5[2]:merged[🌳]
285+
│ └── ·35ee481 (⌂|1)
286+
│ └── →:7: (main)
287+
└── ►:6[2]:D
288+
└── ·ecb1877 (⌂|1)
289+
└── →:7: (main)
290+
");
291+
292+
// overriding references take precedence over git.
293+
let overlay = Overlay::default().with_references([gix::refs::Reference {
294+
name: merged.clone(),
295+
target: gix::refs::Target::Object(merged_b.detach()),
296+
peeled: Some(merged_b.detach()),
297+
}]);
298+
299+
let graph = graph.redo_traversal_with_overlay(&repo, &*meta, overlay)?;
300+
301+
insta::assert_snapshot!(graph_tree(&graph), @"
302+
303+
└── ►:0[0]:anon:
304+
└── 👉·8a6c109 (⌂|1)
305+
├── ►:1[1]:A
306+
│ └── ·62b409a (⌂|1)
307+
│ ├── ►:3[2]:merged[🌳]
308+
│ │ └── ·592abec (⌂|1)
309+
│ │ └── ►:7[3]:main
310+
│ │ └── ·965998b (⌂|1)
311+
│ └── ►:4[2]:B
312+
│ └── ·f16dddf (⌂|1)
313+
│ └── →:7: (main)
314+
└── ►:2[1]:C
315+
└── ·7ed512a (⌂|1)
316+
├── ►:5[2]:anon:
317+
│ └── ·35ee481 (⌂|1)
318+
│ └── →:7: (main)
319+
└── ►:6[2]:D
320+
└── ·ecb1877 (⌂|1)
321+
└── →:7: (main)
322+
");
323+
324+
Ok(())
325+
}

0 commit comments

Comments
 (0)