Skip to content

Commit 3b52cfd

Browse files
ByronByron
committed
revert previous to FAILed commits (#26)
1 parent 4d53b04 commit 3b52cfd

File tree

3 files changed

+61
-76
lines changed

3 files changed

+61
-76
lines changed

‎src/index/diff/delegate.rs

+34-27
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@ use crate::index::diff::Error;
22
use crate::{Change, CrateVersion};
33
use bstr::BStr;
44
use git_repository as git;
5-
use std::collections::BTreeMap;
5+
use std::collections::BTreeSet;
66
use std::ops::Range;
77

88
#[derive(Default)]
99
pub(crate) struct Delegate {
10-
map: BTreeMap<[u8; 32], CrateVersion>,
11-
deletions: Vec<Change>,
10+
changes: Vec<Change>,
11+
deleted_version_ids: BTreeSet<u64>,
1212
err: Option<Error>,
1313
}
1414

@@ -37,7 +37,11 @@ impl Delegate {
3737
if let Some(obj) = entry_data(entry_mode, id)? {
3838
for line in obj.data.lines() {
3939
let version = version_from_json_line(line, change.location)?;
40-
self.map.insert(version.checksum, version.into());
40+
self.changes.push(if version.yanked {
41+
Change::Yanked(version)
42+
} else {
43+
Change::Added(version)
44+
});
4145
}
4246
}
4347
}
@@ -48,7 +52,7 @@ impl Delegate {
4852
for line in obj.data.lines() {
4953
deleted.push(version_from_json_line(line, change.location)?);
5054
}
51-
self.deletions.push(Change::Deleted {
55+
self.changes.push(Change::Deleted {
5256
name: change.location.to_string(),
5357
versions: deleted,
5458
});
@@ -77,14 +81,21 @@ impl Delegate {
7781
.iter()
7882
.map(|&line| input.interner[line].as_bstr())
7983
.peekable();
84+
let mut remember = |version: CrateVersion| {
85+
self.changes.push(if version.yanked {
86+
Change::Yanked(version)
87+
} else {
88+
Change::Added(version)
89+
});
90+
};
8091
'outer: loop {
8192
match (lines_before.peek().is_some(), lines_after.peek().is_some())
8293
{
8394
(true, false) => {
8495
for removed in lines_before {
8596
match version_from_json_line(removed, location) {
8697
Ok(version) => {
87-
self.map.insert(version.checksum, version);
98+
self.deleted_version_ids.insert(version.id());
8899
}
89100
Err(e) => {
90101
err = Some(e);
@@ -97,10 +108,7 @@ impl Delegate {
97108
(false, true) => {
98109
for inserted in lines_after {
99110
match version_from_json_line(inserted, location) {
100-
Ok(version) => {
101-
self.map
102-
.insert(version.checksum, version.into());
103-
}
111+
Ok(version) => remember(version),
104112
Err(e) => {
105113
err = Some(e);
106114
break;
@@ -119,14 +127,11 @@ impl Delegate {
119127
.map(|removed| (removed, inserted))
120128
}) {
121129
Ok((removed_version, inserted_version)) => {
122-
self.map.insert(
123-
removed_version.checksum,
124-
removed_version,
125-
);
126-
self.map.insert(
127-
inserted_version.checksum,
128-
inserted_version,
129-
);
130+
if removed_version.yanked
131+
!= inserted_version.yanked
132+
{
133+
remember(inserted_version);
134+
}
130135
}
131136
Err(e) => {
132137
err = Some(e);
@@ -149,18 +154,20 @@ impl Delegate {
149154
Ok(Default::default())
150155
}
151156

152-
pub fn into_result(self) -> Result<Vec<Change>, Error> {
157+
pub fn into_result(mut self) -> Result<Vec<Change>, Error> {
153158
match self.err {
154159
Some(err) => Err(err),
155160
None => {
156-
let mut changes = self.map.into_values().map(Into::into).collect::<Vec<_>>();
157-
changes.sort_by(|a: &Change, b: &Change| {
158-
let a = a.version();
159-
let b = b.version();
160-
a.name.cmp(&b.name).then_with(|| a.version.cmp(&b.version))
161-
});
162-
changes.extend(self.deletions);
163-
Ok(changes)
161+
if !self.deleted_version_ids.is_empty() {
162+
let deleted_version_ids = &self.deleted_version_ids;
163+
self.changes.retain(|change| match change {
164+
Change::Added(v) | Change::Yanked(v) => {
165+
!deleted_version_ids.contains(&v.id())
166+
}
167+
Change::Deleted { .. } => true,
168+
})
169+
}
170+
Ok(self.changes)
164171
}
165172
}
166173
}

‎src/types.rs

+8-14
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,6 @@ impl Change {
5959
_ => None,
6060
}
6161
}
62-
63-
pub(crate) fn version(&self) -> &CrateVersion {
64-
match self {
65-
Change::Added(v) | Change::Yanked(v) => v,
66-
Change::Deleted { .. } => unreachable!("BUG: must not call this on deletion"),
67-
}
68-
}
6962
}
7063

7164
impl fmt::Display for Change {
@@ -116,13 +109,14 @@ pub struct CrateVersion {
116109
pub dependencies: Vec<Dependency>,
117110
}
118111

119-
impl From<CrateVersion> for Change {
120-
fn from(v: CrateVersion) -> Self {
121-
if v.yanked {
122-
Change::Yanked(v)
123-
} else {
124-
Change::Added(v)
125-
}
112+
impl CrateVersion {
113+
pub(crate) fn id(&self) -> u64 {
114+
let mut s = std::collections::hash_map::DefaultHasher::new();
115+
self.name.hash(&mut s);
116+
self.yanked.hash(&mut s);
117+
self.version.hash(&mut s);
118+
self.checksum.hash(&mut s);
119+
s.finish()
126120
}
127121
}
128122

‎tests/baseline.rs

+19-35
Original file line numberDiff line numberDiff line change
@@ -19,48 +19,32 @@ fn all_aggregrated_diffs_equal_latest_version(
1919

2020
let actual = scope.spawn(|| -> Result<_, Box<dyn std::error::Error + Send + Sync>> {
2121
use crates_index_diff::git;
22-
use crates_index_diff::Change::*;
2322

2423
let start = std::time::Instant::now();
2524
let repo_path = crates_index::Index::new_cargo_default()?.path().to_owned();
2625
let index = crates_index_diff::Index::from_path_or_cloned(repo_path)?;
27-
let commits = index
28-
.repository()
29-
.find_reference("refs/remotes/origin/HEAD")?
30-
.id()
31-
.ancestors()
32-
.first_parent_only()
33-
.all()?
34-
.map(|id| id.map(|id| id.detach()))
35-
.collect::<Result<Vec<_>, _>>()?;
36-
37-
// This could be more complex, like jumping to landmarks like 'Delete crate(s)' and so forth.
38-
let partitions = 4;
39-
let chunk_size = (commits.len() / partitions).max(1);
40-
let mut steps = (0..commits.len()).step_by(chunk_size).collect::<Vec<_>>();
41-
if *steps.last().expect("at least 1") != commits.len() - 1 {
42-
steps.push(commits.len() - 1);
43-
}
26+
let changes = index.changes_between_commits(
27+
git::hash::ObjectId::empty_tree(git::hash::Kind::Sha1),
28+
index
29+
.repository()
30+
.find_reference("refs/remotes/origin/HEAD")?
31+
.target()
32+
.id(),
33+
)?;
4434

35+
use crates_index_diff::Change::*;
4536
let mut versions = HashSet::new();
46-
let mut previous = None;
47-
for current in steps.into_iter().map(|idx| commits[idx].to_owned()) {
48-
let old = previous
49-
.unwrap_or_else(|| git::hash::ObjectId::empty_tree(git::hash::Kind::Sha1));
50-
previous = Some(current);
5137

52-
let changes = index.changes_between_commits(old, current)?;
53-
for change in changes {
54-
match change {
55-
Added(v) | Yanked(v) => {
56-
versions.insert(v.checksum.to_owned());
57-
}
58-
Deleted {
59-
versions: deleted, ..
60-
} => {
61-
for deleted_version in deleted {
62-
versions.remove(&deleted_version.checksum);
63-
}
38+
for change in changes {
39+
match change {
40+
Added(v) | Yanked(v) => {
41+
versions.insert(v.checksum.to_owned());
42+
}
43+
Deleted {
44+
versions: deleted, ..
45+
} => {
46+
for deleted_version in deleted {
47+
versions.remove(&deleted_version.checksum);
6448
}
6549
}
6650
}

0 commit comments

Comments
 (0)