Skip to content

Commit 3bbd1f7

Browse files
committed
fix: status-iterator won't swallow legitimate modification during 'racy-git'.
When a modification is marked as being racy, then previously the iterator would have kept the whole modification even though it should just have tracked the single change. This made the legitimate modification disappear.
1 parent a03bde5 commit 3bbd1f7

File tree

4 files changed

+43
-7
lines changed

4 files changed

+43
-7
lines changed

gix/src/status/iter/mod.rs

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,20 @@ where
1919
///
2020
/// * `patterns`
2121
/// - Optional patterns to use to limit the paths to look at. If empty, all paths are considered.
22+
///
23+
/// ### Important: Undefined ordering
24+
///
25+
/// When compiled with the `parallel` feature, three operations are run at once:
26+
///
27+
/// * a dirwalk to find untracked and possibly ignored files
28+
/// * an entry-by-entry check to see which of the tracked files changed, and how
29+
/// * a tree-index comparison
30+
///
31+
/// All of these generate distinct events which may now happen in any order, so consumers
32+
/// that are ordering dependent have to restore their desired order.
33+
///
34+
/// This isn't feasible to do here as it would mean that returned items would have to be delayed,
35+
/// degrading performance for everyone who isn't order-dependent.
2236
#[doc(alias = "diff_index_to_workdir", alias = "git2")]
2337
pub fn into_iter(
2438
self,
@@ -274,12 +288,15 @@ impl Iter {
274288

275289
impl Iter {
276290
fn maybe_keep_index_change(&mut self, item: Item) -> Option<Item> {
277-
let change = match item {
291+
match item {
278292
Item::IndexWorktree(index_worktree::Item::Modification {
279293
status: EntryStatus::NeedsUpdate(stat),
280294
entry_index,
281295
..
282-
}) => (entry_index, ApplyChange::NewStat(stat)),
296+
}) => {
297+
self.index_changes.push((entry_index, ApplyChange::NewStat(stat)));
298+
return None;
299+
}
283300
Item::IndexWorktree(index_worktree::Item::Modification {
284301
status:
285302
EntryStatus::Change(Change::Modification {
@@ -288,12 +305,12 @@ impl Iter {
288305
}),
289306
entry_index,
290307
..
291-
}) if set_entry_stat_size_zero => (entry_index, ApplyChange::SetSizeToZero),
292-
_ => return Some(item),
308+
}) if set_entry_stat_size_zero => {
309+
self.index_changes.push((entry_index, ApplyChange::SetSizeToZero));
310+
}
311+
_ => {}
293312
};
294-
295-
self.index_changes.push(change);
296-
None
313+
Some(item)
297314
}
298315
}
299316

Binary file not shown.

gix/tests/fixtures/make_status_repos.sh

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,12 @@ git init git-mv
2121

2222
git mv file renamed
2323
)
24+
25+
git init racy-git
26+
(cd racy-git
27+
echo hi >file
28+
git add file && git commit -m "init"
29+
30+
echo ho >file && git add file
31+
echo ha >file
32+
)

gix/tests/gix/status.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,16 @@ mod into_iter {
7373
Ok(())
7474
}
7575

76+
#[test]
77+
fn tree_index_modification_worktree_modification_racy_git() -> crate::Result {
78+
let repo = repo("racy-git")?;
79+
let mut status = repo.status(gix::progress::Discard)?.into_iter(None)?;
80+
let mut items: Vec<_> = status.by_ref().filter_map(Result::ok).collect();
81+
items.sort_by(|a, b| a.location().cmp(b.location()));
82+
assert_eq!(items.len(), 2, "1 modified in index, the same in worktree");
83+
Ok(())
84+
}
85+
7686
#[test]
7787
fn error_during_tree_traversal_causes_failure() -> crate::Result {
7888
let repo = repo("untracked-only")?;

0 commit comments

Comments
 (0)