Skip to content

Commit 9facad1

Browse files
committed
lore: Discard reverted uncommitted directory adds on scan
A working-tree scan (`status --scan` / `stage --scan`) already discards a reverted uncommitted *file* add so `state_staged` matches the filesystem: a file that was staged and then removed before any commit has no committed base to delete from, so reporting a `Delete` would leave an unremovable "zombie" entry. A reverted uncommitted *directory* add was not handled the same way. Extend the same treatment to directories. When a directory node exists in `state_from` but neither in `state_current` (never committed) nor on disk, queue it for discard instead of emitting a meaningless `Delete`. Discarding a directory node must also reclaim its subtree: `apply_pending_discards` now recursively discards every child below a directory node before unlinking the node itself, so no stale descendant slots are left behind. Includes a Rust test covering the index-then-remove cycle for a directory add, asserting the node is discarded (and does not resurface on a later scan) rather than reported as a delete. Signed-off-by: Huân Lê-Vương <65440815+lehuan5062@users.noreply.github.com>
1 parent 28728f2 commit 9facad1

2 files changed

Lines changed: 275 additions & 0 deletions

File tree

lore-revision/src/state.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4684,6 +4684,30 @@ async fn apply_pending_discards(
46844684
}
46854685

46864686
let initial_ancestor = discard_node.parent;
4687+
4688+
// For a directory, discard the whole subtree below it first so its node
4689+
// slots are reclaimed; the node itself is unlinked from its parent and
4690+
// discarded by node_discard_patch below. Each child's sibling pointer is
4691+
// captured before discarding it, since discard_node repurposes that
4692+
// pointer for the block's free list.
4693+
if discard_node.is_directory() {
4694+
let mut child_ref = discard_node.child();
4695+
while let Some(child_id) = child_ref {
4696+
let child_node = state.node(repository.clone(), child_id).await?;
4697+
let next_sibling = child_node.sibling();
4698+
node_discard_recurse(
4699+
state.clone(),
4700+
repository.clone(),
4701+
child_id,
4702+
true, /* recurse */
4703+
true, /* discard */
4704+
|_, _| {},
4705+
)
4706+
.await?;
4707+
child_ref = next_sibling;
4708+
}
4709+
}
4710+
46874711
node_discard_patch(
46884712
state.clone(),
46894713
repository.clone(),
@@ -5937,6 +5961,31 @@ async fn diff_filesystem_directory_walk(
59375961
continue;
59385962
};
59395963

5964+
// A directory node that exists in state_from but neither in state_current
5965+
// (never committed) nor on disk is a reverted, uncommitted add: the
5966+
// directory was staged and then removed from disk before any commit,
5967+
// together with whatever of its contents had been staged under it.
5968+
// Reporting it as a `Delete` is meaningless because there is no committed
5969+
// base to delete from, and no mutation verb can clear it (the "zombie"
5970+
// entry). Discard the whole subtree so state_staged matches the filesystem
5971+
// instead, the same way a reverted single-file add is discarded below.
5972+
if ctx.scan_dirty && from_node.node.is_directory() {
5973+
let in_current = current_node_list
5974+
.children
5975+
.as_slice()
5976+
.binary_search_by(|child| child.name.cmp(&from_named_node.name))
5977+
.is_ok();
5978+
if !in_current {
5979+
lore_trace!(
5980+
"Queueing reverted uncommitted directory node {} (no entry at {}, not in current)",
5981+
from_named_node.node,
5982+
from_node.path
5983+
);
5984+
pending_discards.push(from_named_node.node);
5985+
continue;
5986+
}
5987+
}
5988+
59405989
// Emit deletes only for the materialized portion of the subtree,
59415990
// suppressing directories the filter merely descended through but never
59425991
// wrote to disk (see emit_filesystem_subtree_deletes).
Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
// SPDX-FileCopyrightText: 2026 Epic Games, Inc.
2+
// SPDX-License-Identifier: MIT
3+
4+
//! Working-tree scan handling of a reverted uncommitted directory add.
5+
//!
6+
//! When a directory (and its contents) is indexed as an uncommitted add and
7+
//! then removed from disk before any commit, the next scan must discard the
8+
//! stale node rather than report a delete. The parent has no committed base the
9+
//! directory could be a deletion of, so a delete entry would be an unremovable
10+
//! "zombie" — the same treatment already given to a reverted single-file add.
11+
12+
#[cfg(test)]
13+
mod tests {
14+
#![allow(clippy::disallowed_methods)] // Test fixture writes; not subject to repository write-token discipline.
15+
16+
use std::fs::File;
17+
use std::io::Write;
18+
use std::path::Path;
19+
use std::sync::Arc;
20+
21+
use lore_base::error::NoRemote;
22+
use lore_base::runtime::LORE_CONTEXT;
23+
use lore_base::runtime::runtime;
24+
use lore_base::types::Context;
25+
use lore_revision::branch;
26+
use lore_revision::change::FileAction;
27+
use lore_revision::filter::FilterMode;
28+
use lore_revision::lore::RepositoryId;
29+
use lore_revision::repository;
30+
use lore_revision::repository::RepositoryContext;
31+
use lore_revision::repository::RepositoryFormat;
32+
use lore_revision::repository::load_filter;
33+
use lore_revision::state;
34+
use lore_transport::ProtocolError;
35+
36+
include!("helper.rs");
37+
38+
/// Create (or truncate) a read/write file at `path` and write `contents` to
39+
/// it, returning the open handle. Panics if the file cannot be created or
40+
/// written, since a failed fixture setup invalidates the test.
41+
fn create_file(path: &Path, contents: &[u8]) -> File {
42+
let mut file = File::options()
43+
.create(true)
44+
.truncate(true)
45+
.read(true)
46+
.write(true)
47+
.open(path)
48+
.unwrap_or_else(|_| panic!("Failed to create test file at {}", path.display()));
49+
file.write_all(contents)
50+
.unwrap_or_else(|_| panic!("Failed to write test file at {}", path.display()));
51+
file
52+
}
53+
54+
/// Build a fresh on-disk repository at `path` with no commits (revision 0)
55+
/// and return a write-capable [`RepositoryContext`] for it.
56+
async fn create_repository(
57+
path: &Path,
58+
repository_id: RepositoryId,
59+
immutable_store: Arc<dyn lore_storage::ImmutableStore>,
60+
mutable_store: Arc<dyn lore_storage::MutableStore>,
61+
) -> Arc<RepositoryContext> {
62+
std::fs::create_dir_all(path).expect("Create repository directory failed");
63+
let default_branch = Context::from(uuid::Uuid::now_v7());
64+
let write_token = repository::RepositoryWriteToken::acquire(path).await;
65+
let created_repo = repository::create_local(
66+
path,
67+
&write_token,
68+
repository_id,
69+
default_branch,
70+
branch::DEFAULT_DEFAULT_NAME.to_string(),
71+
repository::RepositoryConfig::default(),
72+
false,
73+
)
74+
.await
75+
.expect("Failed to create repository");
76+
77+
let repository = Arc::new(
78+
RepositoryContext::new(
79+
Some(path.to_path_buf()),
80+
immutable_store,
81+
mutable_store,
82+
repository_id,
83+
created_repo.instance_id,
84+
Err(ProtocolError::from(NoRemote)),
85+
load_filter(path).expect("Failed to load filter"),
86+
RepositoryFormat::Lore,
87+
)
88+
.with_write_token(write_token.share()),
89+
);
90+
lore_revision::instance::store_current_anchor_branch(&repository, default_branch)
91+
.await
92+
.expect("Failed to store anchor branch");
93+
repository
94+
}
95+
96+
/// Reconcile the working tree against the staged state, mutating `state_staged`
97+
/// in place exactly as `lore status --scan` does, and return the detected
98+
/// changes.
99+
async fn scan(
100+
repository: Arc<RepositoryContext>,
101+
state_staged: Arc<state::State>,
102+
state_current: Arc<state::State>,
103+
) -> Vec<lore_revision::change::NodeChange> {
104+
let (changes, _stats) = state::diff_filesystem_ex(
105+
repository.clone(),
106+
state_staged,
107+
repository,
108+
state_current,
109+
None, /* full tree */
110+
FilterMode::Full,
111+
true, /* scan_dirty */
112+
Arc::new(Vec::new()),
113+
)
114+
.await
115+
.expect("Failed to diff filesystem");
116+
changes
117+
}
118+
119+
/// A directory indexed as an uncommitted add (along with its contents) and
120+
/// then removed from disk must be discarded on the next scan rather than
121+
/// reported as a delete: with no committed base there is nothing to delete,
122+
/// and a delete entry would be an unremovable "zombie".
123+
#[tokio::test]
124+
async fn removed_uncommitted_directory_is_discarded_not_deleted() {
125+
let (immutable_store, mutable_store, execution) =
126+
test_store_create().await.expect("Failed to create stores");
127+
let repository_id = RepositoryId::from(uuid::Uuid::now_v7());
128+
129+
runtime()
130+
.spawn(LORE_CONTEXT.scope(execution.clone(), async move {
131+
let tempdir = generate_tempdir();
132+
let path = tempdir.to_path_buf();
133+
let repository = create_repository(
134+
path.as_path(),
135+
repository_id,
136+
immutable_store.clone(),
137+
mutable_store.clone(),
138+
)
139+
.await;
140+
141+
// A directory with content that gets indexed as an uncommitted
142+
// add (the directory node plus its child file).
143+
std::fs::create_dir(path.join("ghost").as_path())
144+
.expect("Create ghost directory failed");
145+
let _ = create_file(path.join("ghost").join("inner.txt").as_path(), &[7, 7, 7]);
146+
147+
let (current_revision, _branch) =
148+
lore_revision::instance::load_current_anchor(&repository)
149+
.await
150+
.expect("Failed to load current anchor");
151+
let state_current = state::State::deserialize(repository.clone(), current_revision)
152+
.await
153+
.expect("Failed to deserialize current state");
154+
let state_staged = state::State::deserialize(repository.clone(), current_revision)
155+
.await
156+
.expect("Failed to deserialize staged state");
157+
158+
// First scan indexes the directory as an add.
159+
let changes = scan(
160+
repository.clone(),
161+
state_staged.clone(),
162+
state_current.clone(),
163+
)
164+
.await;
165+
assert!(
166+
changes
167+
.iter()
168+
.any(|c| c.path.as_str() == "ghost" && c.action == FileAction::Add),
169+
"expected the new directory to be indexed as an add, found: {:?}",
170+
changes
171+
.iter()
172+
.map(|c| (c.path.as_str().to_string(), c.action))
173+
.collect::<Vec<_>>()
174+
);
175+
assert!(
176+
changes.iter().any(|c| c.path.as_str() == "ghost/inner.txt"),
177+
"expected the directory's contents to be indexed too, found: {:?}",
178+
changes
179+
.iter()
180+
.map(|c| (c.path.as_str().to_string(), c.action))
181+
.collect::<Vec<_>>()
182+
);
183+
184+
// Remove it from disk and rescan against the same staged state.
185+
std::fs::remove_dir_all(path.join("ghost"))
186+
.expect("Failed to remove ghost directory");
187+
let changes = scan(
188+
repository.clone(),
189+
state_staged.clone(),
190+
state_current.clone(),
191+
)
192+
.await;
193+
assert!(
194+
changes
195+
.iter()
196+
.all(|c| !c.path.as_str().starts_with("ghost")),
197+
"removed uncommitted directory must be discarded, not reported, found: {:?}",
198+
changes
199+
.iter()
200+
.map(|c| (c.path.as_str().to_string(), c.action))
201+
.collect::<Vec<_>>()
202+
);
203+
204+
// A further scan stays clean — the node was discarded, not merely
205+
// hidden, so it cannot resurface.
206+
let changes = scan(
207+
repository.clone(),
208+
state_staged.clone(),
209+
state_current.clone(),
210+
)
211+
.await;
212+
assert!(
213+
changes
214+
.iter()
215+
.all(|c| !c.path.as_str().starts_with("ghost")),
216+
"discarded directory must not resurface on a later scan, found: {:?}",
217+
changes
218+
.iter()
219+
.map(|c| (c.path.as_str().to_string(), c.action))
220+
.collect::<Vec<_>>()
221+
);
222+
}))
223+
.await
224+
.expect("Test task panicked");
225+
}
226+
}

0 commit comments

Comments
 (0)