Skip to content

lore: Discard reverted uncommitted directory adds on scan#104

Open
lehuan5062 wants to merge 5 commits into
EpicGames:mainfrom
lehuan5062:discard-reverted-directory
Open

lore: Discard reverted uncommitted directory adds on scan#104
lehuan5062 wants to merge 5 commits into
EpicGames:mainfrom
lehuan5062:discard-reverted-directory

Conversation

@lehuan5062

Copy link
Copy Markdown
Member

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.

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>
@lehuan5062 lehuan5062 force-pushed the discard-reverted-directory branch from 9facad1 to 7befc87 Compare July 9, 2026 03:50
@mjansson mjansson self-requested a review July 9, 2026 18:01
Comment thread lore-revision/src/state.rs Outdated
@mjansson

Copy link
Copy Markdown
Collaborator

Could you also add a smoke test for it in some appropriate file in scripts/test/? Ideally using json output and verifying the status output after a scan that does such a directory discard is exactly as intended (no stray delete reported directores, no double reported items, no missing items from other changes that are still present etc).

- Condense verbose comment in state.rs explaining reverted uncommitted directory handling to a single sentence
- Add smoke test to verify directory discard behavior with JSON output validation (no stray deletes, no duplicates, concurrent changes properly reported)

Signed-off-by: Huân Lê-Vương <65440815+lehuan5062@users.noreply.github.com>
…path

The smoke test previously called status() without --scan, so the
filesystem-diff walk containing the discard logic never ran, and it
filtered results to type == "file", hiding the directory-node entries
where a regression would actually surface. Verified by disabling the
discard logic and confirming the old test still passed; it now fails
as expected. The fix drives status(scan=True) and checks a follow-up
call (since the scan that performs the discard reports its own stale
pre-discard snapshot) with no type filter.

Signed-off-by: Huân Lê-Vương <65440815+lehuan5062@users.noreply.github.com>
Local mid-function import didn't match the file's existing convention
of importing at module scope.

Signed-off-by: Huân Lê-Vương <65440815+lehuan5062@users.noreply.github.com>
Per Lore's comment standards, remove comments that only restate what
the adjacent code already says; keep the ones explaining non-obvious
behavior (the revert simulation, the two-scan reconciliation quirk,
and why no type filter is applied).

Signed-off-by: Huân Lê-Vương <65440815+lehuan5062@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants