-
Notifications
You must be signed in to change notification settings - Fork 363
lore: Discard reverted uncommitted directory adds on scan #104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
lehuan5062
wants to merge
10
commits into
EpicGames:main
Choose a base branch
from
lehuan5062:discard-reverted-directory
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+475
−122
Open
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
7befc87
lore: Discard reverted uncommitted directory adds on scan
lehuan5062 d335160
lore: Address maintainer feedback on directory discard
lehuan5062 e89c9a5
lore: Fix smoke test to actually exercise the directory discard scan …
lehuan5062 409f2c7
lore: Move shutil import to module scope in discard test
lehuan5062 5cc9f50
lore: Trim organizational comments from discard test
lehuan5062 04c827d
Merge branch 'EpicGames:main' into discard-reverted-directory
lehuan5062 a15c41b
Merge branch 'EpicGames:main' into discard-reverted-directory
lehuan5062 4da0419
fix: scan discards reverted uncommitted directories and fixes ordering
lehuan5062 102f674
Merge branch 'EpicGames:main' into discard-reverted-directory
lehuan5062 6a12b52
Merge branch 'EpicGames:main' into discard-reverted-directory
lehuan5062 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,226 @@ | ||
| // SPDX-FileCopyrightText: 2026 Epic Games, Inc. | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| //! Working-tree scan handling of a reverted uncommitted directory add. | ||
| //! | ||
| //! When a directory (and its contents) is indexed as an uncommitted add and | ||
| //! then removed from disk before any commit, the next scan must discard the | ||
| //! stale node rather than report a delete. The parent has no committed base the | ||
| //! directory could be a deletion of, so a delete entry would be an unremovable | ||
| //! "zombie" — the same treatment already given to a reverted single-file add. | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| #![allow(clippy::disallowed_methods)] // Test fixture writes; not subject to repository write-token discipline. | ||
|
|
||
| use std::fs::File; | ||
| use std::io::Write; | ||
| use std::path::Path; | ||
| use std::sync::Arc; | ||
|
|
||
| use lore_base::error::NoRemote; | ||
| use lore_base::runtime::LORE_CONTEXT; | ||
| use lore_base::runtime::runtime; | ||
| use lore_base::types::Context; | ||
| use lore_revision::branch; | ||
| use lore_revision::change::FileAction; | ||
| use lore_revision::filter::FilterMode; | ||
| use lore_revision::lore::RepositoryId; | ||
| use lore_revision::repository; | ||
| use lore_revision::repository::RepositoryContext; | ||
| use lore_revision::repository::RepositoryFormat; | ||
| use lore_revision::repository::load_filter; | ||
| use lore_revision::state; | ||
| use lore_transport::ProtocolError; | ||
|
|
||
| include!("helper.rs"); | ||
|
|
||
| /// Create (or truncate) a read/write file at `path` and write `contents` to | ||
| /// it, returning the open handle. Panics if the file cannot be created or | ||
| /// written, since a failed fixture setup invalidates the test. | ||
| fn create_file(path: &Path, contents: &[u8]) -> File { | ||
| let mut file = File::options() | ||
| .create(true) | ||
| .truncate(true) | ||
| .read(true) | ||
| .write(true) | ||
| .open(path) | ||
| .unwrap_or_else(|_| panic!("Failed to create test file at {}", path.display())); | ||
| file.write_all(contents) | ||
| .unwrap_or_else(|_| panic!("Failed to write test file at {}", path.display())); | ||
| file | ||
| } | ||
|
|
||
| /// Build a fresh on-disk repository at `path` with no commits (revision 0) | ||
| /// and return a write-capable [`RepositoryContext`] for it. | ||
| async fn create_repository( | ||
| path: &Path, | ||
| repository_id: RepositoryId, | ||
| immutable_store: Arc<dyn lore_storage::ImmutableStore>, | ||
| mutable_store: Arc<dyn lore_storage::MutableStore>, | ||
| ) -> Arc<RepositoryContext> { | ||
| std::fs::create_dir_all(path).expect("Create repository directory failed"); | ||
| let default_branch = Context::from(uuid::Uuid::now_v7()); | ||
| let write_token = repository::RepositoryWriteToken::acquire(path).await; | ||
| let created_repo = repository::create_local( | ||
| path, | ||
| &write_token, | ||
| repository_id, | ||
| default_branch, | ||
| branch::DEFAULT_DEFAULT_NAME.to_string(), | ||
| repository::RepositoryConfig::default(), | ||
| false, | ||
| ) | ||
| .await | ||
| .expect("Failed to create repository"); | ||
|
|
||
| let repository = Arc::new( | ||
| RepositoryContext::new( | ||
| Some(path.to_path_buf()), | ||
| immutable_store, | ||
| mutable_store, | ||
| repository_id, | ||
| created_repo.instance_id, | ||
| Err(ProtocolError::from(NoRemote)), | ||
| load_filter(path).expect("Failed to load filter"), | ||
| RepositoryFormat::Lore, | ||
| ) | ||
| .with_write_token(write_token.share()), | ||
| ); | ||
| lore_revision::instance::store_current_anchor_branch(&repository, default_branch) | ||
| .await | ||
| .expect("Failed to store anchor branch"); | ||
| repository | ||
| } | ||
|
|
||
| /// Reconcile the working tree against the staged state, mutating `state_staged` | ||
| /// in place exactly as `lore status --scan` does, and return the detected | ||
| /// changes. | ||
| async fn scan( | ||
| repository: Arc<RepositoryContext>, | ||
| state_staged: Arc<state::State>, | ||
| state_current: Arc<state::State>, | ||
| ) -> Vec<lore_revision::change::NodeChange> { | ||
| let (changes, _stats) = state::diff_filesystem_ex( | ||
| repository.clone(), | ||
| state_staged, | ||
| repository, | ||
| state_current, | ||
| None, /* full tree */ | ||
| FilterMode::Full, | ||
| true, /* scan_dirty */ | ||
| Arc::new(Vec::new()), | ||
| ) | ||
| .await | ||
| .expect("Failed to diff filesystem"); | ||
| changes | ||
| } | ||
|
|
||
| /// A directory indexed as an uncommitted add (along with its contents) and | ||
| /// then removed from disk must be discarded on the next scan rather than | ||
| /// reported as a delete: with no committed base there is nothing to delete, | ||
| /// and a delete entry would be an unremovable "zombie". | ||
| #[tokio::test] | ||
| async fn removed_uncommitted_directory_is_discarded_not_deleted() { | ||
| let (immutable_store, mutable_store, execution) = | ||
| test_store_create().await.expect("Failed to create stores"); | ||
| let repository_id = RepositoryId::from(uuid::Uuid::now_v7()); | ||
|
|
||
| runtime() | ||
| .spawn(LORE_CONTEXT.scope(execution.clone(), async move { | ||
| let tempdir = generate_tempdir(); | ||
| let path = tempdir.to_path_buf(); | ||
| let repository = create_repository( | ||
| path.as_path(), | ||
| repository_id, | ||
| immutable_store.clone(), | ||
| mutable_store.clone(), | ||
| ) | ||
| .await; | ||
|
|
||
| // A directory with content that gets indexed as an uncommitted | ||
| // add (the directory node plus its child file). | ||
| std::fs::create_dir(path.join("ghost").as_path()) | ||
| .expect("Create ghost directory failed"); | ||
| let _ = create_file(path.join("ghost").join("inner.txt").as_path(), &[7, 7, 7]); | ||
|
|
||
| let (current_revision, _branch) = | ||
| lore_revision::instance::load_current_anchor(&repository) | ||
| .await | ||
| .expect("Failed to load current anchor"); | ||
| let state_current = state::State::deserialize(repository.clone(), current_revision) | ||
| .await | ||
| .expect("Failed to deserialize current state"); | ||
| let state_staged = state::State::deserialize(repository.clone(), current_revision) | ||
| .await | ||
| .expect("Failed to deserialize staged state"); | ||
|
|
||
| // First scan indexes the directory as an add. | ||
| let changes = scan( | ||
| repository.clone(), | ||
| state_staged.clone(), | ||
| state_current.clone(), | ||
| ) | ||
| .await; | ||
| assert!( | ||
| changes | ||
| .iter() | ||
| .any(|c| c.path.as_str() == "ghost" && c.action == FileAction::Add), | ||
| "expected the new directory to be indexed as an add, found: {:?}", | ||
| changes | ||
| .iter() | ||
| .map(|c| (c.path.as_str().to_string(), c.action)) | ||
| .collect::<Vec<_>>() | ||
| ); | ||
| assert!( | ||
| changes.iter().any(|c| c.path.as_str() == "ghost/inner.txt"), | ||
| "expected the directory's contents to be indexed too, found: {:?}", | ||
| changes | ||
| .iter() | ||
| .map(|c| (c.path.as_str().to_string(), c.action)) | ||
| .collect::<Vec<_>>() | ||
| ); | ||
|
|
||
| // Remove it from disk and rescan against the same staged state. | ||
| std::fs::remove_dir_all(path.join("ghost")) | ||
| .expect("Failed to remove ghost directory"); | ||
| let changes = scan( | ||
| repository.clone(), | ||
| state_staged.clone(), | ||
| state_current.clone(), | ||
| ) | ||
| .await; | ||
| assert!( | ||
| changes | ||
| .iter() | ||
| .all(|c| !c.path.as_str().starts_with("ghost")), | ||
| "removed uncommitted directory must be discarded, not reported, found: {:?}", | ||
| changes | ||
| .iter() | ||
| .map(|c| (c.path.as_str().to_string(), c.action)) | ||
| .collect::<Vec<_>>() | ||
| ); | ||
|
|
||
| // A further scan stays clean — the node was discarded, not merely | ||
| // hidden, so it cannot resurface. | ||
| let changes = scan( | ||
| repository.clone(), | ||
| state_staged.clone(), | ||
| state_current.clone(), | ||
| ) | ||
| .await; | ||
| assert!( | ||
| changes | ||
| .iter() | ||
| .all(|c| !c.path.as_str().starts_with("ghost")), | ||
| "discarded directory must not resurface on a later scan, found: {:?}", | ||
| changes | ||
| .iter() | ||
| .map(|c| (c.path.as_str().to_string(), c.action)) | ||
| .collect::<Vec<_>>() | ||
| ); | ||
| })) | ||
| .await | ||
| .expect("Test task panicked"); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.