revset_graph: preserve original parents order

It seemed awkward if merged PR is sometimes rendered as a first branch.
Instead of sorting edges in index order, let's build a HashSet only when
deduplication is needed.
This commit is contained in:
Yuya Nishihara 2023-07-28 15:51:08 +09:00
parent 1bf6ab5370
commit b3ae7e7657
12 changed files with 169 additions and 163 deletions

View file

@ -14,7 +14,7 @@
#![allow(missing_docs)]
use std::cmp::{min, Reverse};
use std::cmp::min;
use std::collections::{BTreeMap, HashSet};
use crate::backend::CommitId;
@ -132,7 +132,7 @@ pub struct RevsetGraphIterator<'revset, 'index> {
min_position: IndexPosition,
/// Edges for commits not in the input set.
// TODO: Remove unneeded entries here as we go (that's why it's an ordered map)?
edges: BTreeMap<IndexPosition, HashSet<IndexGraphEdge>>,
edges: BTreeMap<IndexPosition, Vec<IndexGraphEdge>>,
skip_transitive_edges: bool,
}
@ -166,25 +166,30 @@ impl<'revset, 'index> RevsetGraphIterator<'revset, 'index> {
fn edges_from_internal_commit(
&mut self,
index_entry: &IndexEntry<'index>,
) -> HashSet<IndexGraphEdge> {
) -> Vec<IndexGraphEdge> {
if let Some(edges) = self.edges.get(&index_entry.position()) {
return edges.clone();
}
let mut edges = HashSet::new();
let mut edges = Vec::new();
let mut known_ancestors = HashSet::new();
for parent in index_entry.parents() {
let parent_position = parent.position();
self.consume_to(parent_position);
if self.look_ahead.contains_key(&parent_position) {
edges.insert(IndexGraphEdge::direct(parent_position));
edges.push(IndexGraphEdge::direct(parent_position));
} else {
let parent_edges = self.edges_from_external_commit(parent);
if parent_edges
.iter()
.all(|edge| edge.edge_type == RevsetGraphEdgeType::Missing)
{
edges.insert(IndexGraphEdge::missing(parent_position));
edges.push(IndexGraphEdge::missing(parent_position));
} else {
edges.extend(parent_edges);
edges.extend(
parent_edges
.into_iter()
.filter(|edge| known_ancestors.insert(edge.target)),
)
}
}
}
@ -195,7 +200,7 @@ impl<'revset, 'index> RevsetGraphIterator<'revset, 'index> {
fn edges_from_external_commit(
&mut self,
index_entry: IndexEntry<'index>,
) -> HashSet<IndexGraphEdge> {
) -> Vec<IndexGraphEdge> {
let position = index_entry.position();
let mut stack = vec![index_entry];
while let Some(entry) = stack.last() {
@ -204,26 +209,32 @@ impl<'revset, 'index> RevsetGraphIterator<'revset, 'index> {
stack.pop().unwrap();
continue;
}
let mut edges = HashSet::new();
let mut edges = Vec::new();
let mut known_ancestors = HashSet::new();
let mut parents_complete = true;
for parent in entry.parents() {
let parent_position = parent.position();
self.consume_to(parent_position);
if self.look_ahead.contains_key(&parent_position) {
// We have found a path back into the input set
edges.insert(IndexGraphEdge::indirect(parent_position));
edges.push(IndexGraphEdge::indirect(parent_position));
} else if let Some(parent_edges) = self.edges.get(&parent_position) {
if parent_edges
.iter()
.all(|edge| edge.edge_type == RevsetGraphEdgeType::Missing)
{
edges.insert(IndexGraphEdge::missing(parent_position));
edges.push(IndexGraphEdge::missing(parent_position));
} else {
edges.extend(parent_edges.iter().cloned());
edges.extend(
parent_edges
.iter()
.filter(|edge| known_ancestors.insert(edge.target))
.cloned(),
);
}
} else if parent_position < self.min_position {
// The parent is not in the input set
edges.insert(IndexGraphEdge::missing(parent_position));
edges.push(IndexGraphEdge::missing(parent_position));
} else {
// The parent is not in the input set but it's somewhere in the range
// where we have commits in the input set, so continue searching.
@ -239,10 +250,7 @@ impl<'revset, 'index> RevsetGraphIterator<'revset, 'index> {
self.edges.get(&position).unwrap().clone()
}
fn remove_transitive_edges(
&mut self,
edges: HashSet<IndexGraphEdge>,
) -> HashSet<IndexGraphEdge> {
fn remove_transitive_edges(&mut self, edges: Vec<IndexGraphEdge>) -> Vec<IndexGraphEdge> {
if !edges
.iter()
.any(|edge| edge.edge_type == RevsetGraphEdgeType::Indirect)
@ -310,8 +318,6 @@ impl<'revset, 'index> Iterator for RevsetGraphIterator<'revset, 'index> {
if self.skip_transitive_edges {
edges = self.remove_transitive_edges(edges);
}
let mut edges: Vec<_> = edges.into_iter().collect();
edges.sort_unstable_by_key(|edge| Reverse(edge.target));
let edges = edges
.iter()
.map(|edge| edge.to_revset_edge(self.index))

View file

@ -126,9 +126,9 @@ fn test_graph_iterator_virtual_octopus(skip_transitive_edges: bool) {
assert_eq!(
commits[0].1,
vec![
indirect(&commit_c),
indirect(&commit_b),
indirect(&commit_a),
indirect(&commit_b),
indirect(&commit_c),
]
);
assert_eq!(commits[1].1, vec![missing(&root_commit)]);
@ -215,7 +215,7 @@ fn test_graph_iterator_multiple_missing(skip_transitive_edges: bool) {
assert_eq!(commits[1].0, *commit_b.id());
assert_eq!(
commits[0].1,
vec![missing(&commit_c), indirect(&commit_b), missing(&commit_a),]
vec![missing(&commit_a), indirect(&commit_b), missing(&commit_c)]
);
assert_eq!(commits[1].1, vec![missing(&root_commit)]);
}
@ -263,7 +263,7 @@ fn test_graph_iterator_edge_to_ancestor(skip_transitive_edges: bool) {
} else {
assert_eq!(commits[0].1, vec![direct(&commit_d), indirect(&commit_c),]);
}
assert_eq!(commits[1].1, vec![direct(&commit_c), missing(&commit_b),]);
assert_eq!(commits[1].1, vec![missing(&commit_b), direct(&commit_c)]);
assert_eq!(commits[2].1, vec![missing(&commit_a)]);
}
@ -320,12 +320,12 @@ fn test_graph_iterator_edge_escapes_from_(skip_transitive_edges: bool) {
assert_eq!(commits[3].0, *commit_d.id());
assert_eq!(commits[4].0, *commit_a.id());
if skip_transitive_edges {
assert_eq!(commits[0].1, vec![indirect(&commit_h), direct(&commit_g),]);
assert_eq!(commits[0].1, vec![direct(&commit_g), indirect(&commit_h)]);
assert_eq!(commits[1].1, vec![indirect(&commit_d)]);
} else {
assert_eq!(
commits[0].1,
vec![indirect(&commit_h), direct(&commit_g), indirect(&commit_d),]
vec![direct(&commit_g), indirect(&commit_d), indirect(&commit_h)]
);
assert_eq!(commits[1].1, vec![indirect(&commit_d), indirect(&commit_a)]);
}

View file

@ -45,11 +45,11 @@ fn test_rebase_branch_with_merge() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ e
d
c
d
c
b
a
a
"###);
@ -66,10 +66,10 @@ fn test_rebase_branch_with_merge() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ e
c d
c d
b
a
a
"###);
@ -121,9 +121,9 @@ fn test_rebase_branch_with_merge() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ e
a b
d
c
d
c
a b
"###);

View file

@ -149,13 +149,13 @@ fn test_chmod_file_dir_deletion_conflicts() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ file_deletion
deletion
deletion
file_dir
file
dir
dir
file
base
"###);

View file

@ -43,8 +43,8 @@ fn test_duplicate() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 17a00fc21654 c
2443ea76b0b1 a
d370aee184ba b
d370aee184ba b
2443ea76b0b1 a
000000000000
"###);
@ -62,9 +62,9 @@ fn test_duplicate() {
2f6dc5a1ffc2 a
@ 17a00fc21654 c
2443ea76b0b1 a
d370aee184ba b
d370aee184ba b
2443ea76b0b1 a
000000000000
"###);
@ -79,8 +79,8 @@ fn test_duplicate() {
@ 17a00fc21654 c
2443ea76b0b1 a
d370aee184ba b
d370aee184ba b
2443ea76b0b1 a
000000000000
"###);
@ -101,9 +101,9 @@ fn test_duplicate_many() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 921dde6e55c0 e
1394f625cbbd b
ebd06dba20ec d
c0cb3a0b73e7 c
ebd06dba20ec d
c0cb3a0b73e7 c
1394f625cbbd b
2443ea76b0b1 a
000000000000
@ -120,11 +120,11 @@ fn test_duplicate_many() {
3b74d9691015 b
@ 921dde6e55c0 e
1394f625cbbd b
ebd06dba20ec d
c0cb3a0b73e7 c
1394f625cbbd b
ebd06dba20ec d
c0cb3a0b73e7 c
2443ea76b0b1 a
000000000000
"###);
@ -139,10 +139,10 @@ fn test_duplicate_many() {
0276d3d7c24d b
@ 921dde6e55c0 e
1394f625cbbd b
ebd06dba20ec d
c0cb3a0b73e7 c
ebd06dba20ec d
c0cb3a0b73e7 c
1394f625cbbd b
2443ea76b0b1 a
000000000000
@ -159,16 +159,16 @@ fn test_duplicate_many() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
0f7430f2727a e
fa167d18a83a b
2181781b4f81 d
2181781b4f81 d
fa167d18a83a b
@ 921dde6e55c0 e
1394f625cbbd b
ebd06dba20ec d
ebd06dba20ec d
c0cb3a0b73e7 c
1394f625cbbd b
c0cb3a0b73e7 c
2443ea76b0b1 a
000000000000
"###);
@ -178,9 +178,9 @@ fn test_duplicate_many() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 921dde6e55c0 e
1394f625cbbd b
ebd06dba20ec d
c0cb3a0b73e7 c
ebd06dba20ec d
c0cb3a0b73e7 c
1394f625cbbd b
2443ea76b0b1 a
000000000000
@ -194,13 +194,13 @@ fn test_duplicate_many() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
9bd4389f5d47 e
d94e4c55a68b d
d94e4c55a68b d
@ 921dde6e55c0 e
1394f625cbbd b
ebd06dba20ec d
c0cb3a0b73e7 c
c0cb3a0b73e7 c
1394f625cbbd b
2443ea76b0b1 a
c6f7f8c4512e a
@ -221,16 +221,16 @@ fn test_duplicate_many() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
ee8fe64ed254 e
e13ac0adabdf b
2f2442db08eb d
df53fa589286 c
2f2442db08eb d
df53fa589286 c
e13ac0adabdf b
0fe67a05989e a
@ 921dde6e55c0 e
1394f625cbbd b
ebd06dba20ec d
c0cb3a0b73e7 c
ebd06dba20ec d
c0cb3a0b73e7 c
1394f625cbbd b
2443ea76b0b1 a

View file

@ -956,11 +956,11 @@ fn test_graph_styles() {
insta::assert_snapshot!(stdout, @r###"
@ merge
side branch
side branch
with
long
description
main branch 2
main branch 2
main branch 1
initial
@ -973,11 +973,11 @@ fn test_graph_styles() {
insta::assert_snapshot!(stdout, @r###"
@ merge
|\
o | side branch
| o side branch
| | with
| | long
| | description
o | main branch 2
| o main branch 2
|/
o main branch 1
o initial
@ -991,11 +991,11 @@ fn test_graph_styles() {
@ merge
|\
| \
o | side branch
| o side branch
| | with
| | long
| | description
o | main branch 2
| o main branch 2
| /
|/
o main branch 1
@ -1009,11 +1009,11 @@ fn test_graph_styles() {
insta::assert_snapshot!(stdout, @r###"
@ merge
side branch
side branch
with
long
description
main branch 2
main branch 2
main branch 1
initial
@ -1026,11 +1026,11 @@ fn test_graph_styles() {
insta::assert_snapshot!(stdout, @r###"
@ merge
side branch
side branch
with
long
description
main branch 2
main branch 2
main branch 1
initial
@ -1100,11 +1100,11 @@ fn test_log_word_wrap() {
3 4 5
6 7 8
9
0 1 2
0 1 2
3 4 5
6 7 8
9
0 1 2
0 1 2
3 4 5
6 7 8
9
@ -1122,11 +1122,11 @@ fn test_log_word_wrap() {
|\ 3 4 5
| | 6 7 8
| | 9
o | 0 1 2
| o 0 1 2
| | 3 4 5
| | 6 7 8
| | 9
o | 0 1 2
| o 0 1 2
|/ 3 4 5
| 6 7 8
| 9

View file

@ -61,8 +61,8 @@ fn test_new_merge() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 0c4e5b9b68ae0cbe7ce3c61042619513d09005bf
38e8e2f6c92ffb954961fc391b515ff551b41636 add file1
f399209d9dda06e8a25a0c8e9a0cde9f421ff35d add file2
f399209d9dda06e8a25a0c8e9a0cde9f421ff35d add file2
38e8e2f6c92ffb954961fc391b515ff551b41636 add file1
0000000000000000000000000000000000000000
"###);
@ -77,8 +77,8 @@ fn test_new_merge() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 200ed1a14c8acf09783dafefe5bebf2ff58f12fd
38e8e2f6c92ffb954961fc391b515ff551b41636 add file1
f399209d9dda06e8a25a0c8e9a0cde9f421ff35d add file2
f399209d9dda06e8a25a0c8e9a0cde9f421ff35d add file2
38e8e2f6c92ffb954961fc391b515ff551b41636 add file1
0000000000000000000000000000000000000000
"###);
@ -115,8 +115,8 @@ fn test_new_insert_after() {
insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
@ F
D
E
E
D
C
B
@ -137,13 +137,13 @@ fn test_new_insert_after() {
C
F
E
@ G
B
A
D
D
B
A
E
root
"###);
@ -158,14 +158,14 @@ fn test_new_insert_after() {
C
F
E
G
B
A
@ H
D
@ H
D
B
A
E
root
"###);
@ -180,8 +180,8 @@ fn test_new_insert_after_children() {
insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
@ F
D
E
E
D
C
B
@ -203,15 +203,15 @@ fn test_new_insert_after_children() {
insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
@ G
C
B
C
B
A
F
D
E
E
D
root
"###);
@ -226,8 +226,8 @@ fn test_new_insert_before() {
insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
@ F
D
E
E
D
C
B
@ -270,8 +270,8 @@ fn test_new_insert_before_root_successors() {
insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
@ F
D
E
E
D
C
B
@ -313,8 +313,8 @@ fn test_new_insert_before_no_loop() {
insta::assert_snapshot!(stdout, @r###"
@ 7705d353bf5d F
c9257eff5bf9 D
41a89ffcbba2 E
41a89ffcbba2 E
c9257eff5bf9 D
ec18c57d72d8 C
6041917ceeb5 B
@ -339,8 +339,8 @@ fn test_new_insert_before_no_root_merge() {
insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
@ F
D
E
E
D
C
B
@ -380,8 +380,8 @@ fn test_new_insert_before_root() {
insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
@ F
D
E
E
D
C
B

View file

@ -190,11 +190,11 @@ fn test_rebase_branch_with_merge() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ e
d
c
d
c
b
a
a
"###);
@ -248,8 +248,8 @@ fn test_rebase_single_revision() {
@ d
c
a
b
b
a
"###);
@ -313,9 +313,9 @@ fn test_rebase_single_revision_merge_parent() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ d
a
c
b
c
b
a
"###);
@ -366,8 +366,8 @@ fn test_rebase_multiple_destinations() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
a
b
@ c
@ c
b
"###);
@ -467,8 +467,8 @@ fn test_rebase_with_descendants() {
@ d
c
a
b
b
a
"###);
@ -512,8 +512,8 @@ fn test_rebase_with_descendants() {
@ d
c
a
b
b
a
"###);

View file

@ -56,8 +56,8 @@ fn test_resolution() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
a
b
b
a
base
@ -307,8 +307,8 @@ fn test_normal_conflict_input_files() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
a
b
b
a
base
@ -348,8 +348,8 @@ fn test_baseless_conflict_input_files() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
a
b
b
a
base
@ -416,8 +416,8 @@ fn test_edit_delete_conflict_input_files() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
a
b
b
a
base
@ -459,8 +459,8 @@ fn test_file_vs_dir() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
a
b
b
a
base
@ -506,10 +506,10 @@ fn test_description_with_dir_and_deletion() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
edit
del
dir
del
edit
base
@ -586,8 +586,8 @@ fn test_multiple_conflicts() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
a
b
b
a
base

View file

@ -123,8 +123,8 @@ fn test_restore_conflicted_merge() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
a
b
b
a
base

View file

@ -91,8 +91,8 @@ fn test_squash() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ c7a11b36d333 e
90fe0a96fc90 c
5658521e0f8b d
5658521e0f8b d
90fe0a96fc90 c
fa5efbdf533c b
90aeefd03044 a
@ -115,8 +115,8 @@ fn test_squash() {
@ 959145c11426
80960125bb96 e
90fe0a96fc90 c
5658521e0f8b d
5658521e0f8b d
90fe0a96fc90 c
fa5efbdf533c b
90aeefd03044 a

View file

@ -90,8 +90,8 @@ fn test_unsquash() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 1f8f152ff48e e
90fe0a96fc90 c
5658521e0f8b d
5658521e0f8b d
90fe0a96fc90 c
fa5efbdf533c b
90aeefd03044 a
@ -114,8 +114,8 @@ fn test_unsquash() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 3217340cb761
90fe0a96fc90 c e??
5658521e0f8b d e??
5658521e0f8b d e??
90fe0a96fc90 c e??
fa5efbdf533c b
90aeefd03044 a