From 6de088be31bb1cdf0a1d2d40155bfe79c07c8b7d Mon Sep 17 00:00:00 2001 From: Joe Date: Tue, 17 Feb 2026 21:27:10 -0600 Subject: [PATCH] Add batch script files and enhance git staging logic - Introduced 20 new batch script files (BatchScript01.luau to BatchScript20.luau) for testing purposes. - Enhanced the `change_processor` to consolidate git staging operations, reducing the risk of index.lock races by batching paths for staging. - Updated the `PatchSet` struct to include a new field for pre-resolved filesystem paths to stage via git add. - Modified the API to handle batch staging more efficiently, ensuring all files are staged correctly in mixed write scenarios. These changes improve the testing framework and optimize git integration for batch operations. --- .../git_sync_defaults/src/BatchScript01.luau | 2 + .../git_sync_defaults/src/BatchScript02.luau | 2 + .../git_sync_defaults/src/BatchScript03.luau | 2 + .../git_sync_defaults/src/BatchScript04.luau | 2 + .../git_sync_defaults/src/BatchScript05.luau | 2 + .../git_sync_defaults/src/BatchScript06.luau | 2 + .../git_sync_defaults/src/BatchScript07.luau | 2 + .../git_sync_defaults/src/BatchScript08.luau | 2 + .../git_sync_defaults/src/BatchScript09.luau | 2 + .../git_sync_defaults/src/BatchScript10.luau | 2 + .../git_sync_defaults/src/BatchScript11.luau | 2 + .../git_sync_defaults/src/BatchScript12.luau | 2 + .../git_sync_defaults/src/BatchScript13.luau | 2 + .../git_sync_defaults/src/BatchScript14.luau | 2 + .../git_sync_defaults/src/BatchScript15.luau | 2 + .../git_sync_defaults/src/BatchScript16.luau | 2 + .../git_sync_defaults/src/BatchScript17.luau | 2 + .../git_sync_defaults/src/BatchScript18.luau | 2 + .../git_sync_defaults/src/BatchScript19.luau | 2 + .../git_sync_defaults/src/BatchScript20.luau | 2 + src/change_processor.rs | 22 +- src/snapshot/patch.rs | 7 + src/web/api.rs | 7 +- tests/tests/git_sync_defaults.rs | 209 ++++++++++++++++++ 24 files changed, 272 insertions(+), 13 deletions(-) create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript01.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript02.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript03.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript04.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript05.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript06.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript07.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript08.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript09.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript10.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript11.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript12.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript13.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript14.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript15.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript16.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript17.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript18.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript19.luau create mode 100644 rojo-test/serve-tests/git_sync_defaults/src/BatchScript20.luau diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript01.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript01.luau new file mode 100644 index 00000000..06e0c946 --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript01.luau @@ -0,0 +1,2 @@ +local BatchScript01 = {} +return BatchScript01 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript02.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript02.luau new file mode 100644 index 00000000..909a59bd --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript02.luau @@ -0,0 +1,2 @@ +local BatchScript02 = {} +return BatchScript02 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript03.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript03.luau new file mode 100644 index 00000000..94d767ad --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript03.luau @@ -0,0 +1,2 @@ +local BatchScript03 = {} +return BatchScript03 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript04.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript04.luau new file mode 100644 index 00000000..77f95530 --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript04.luau @@ -0,0 +1,2 @@ +local BatchScript04 = {} +return BatchScript04 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript05.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript05.luau new file mode 100644 index 00000000..e9cf8932 --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript05.luau @@ -0,0 +1,2 @@ +local BatchScript05 = {} +return BatchScript05 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript06.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript06.luau new file mode 100644 index 00000000..96e56c10 --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript06.luau @@ -0,0 +1,2 @@ +local BatchScript06 = {} +return BatchScript06 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript07.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript07.luau new file mode 100644 index 00000000..89fc6ec8 --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript07.luau @@ -0,0 +1,2 @@ +local BatchScript07 = {} +return BatchScript07 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript08.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript08.luau new file mode 100644 index 00000000..026fa1f2 --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript08.luau @@ -0,0 +1,2 @@ +local BatchScript08 = {} +return BatchScript08 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript09.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript09.luau new file mode 100644 index 00000000..57ccd37e --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript09.luau @@ -0,0 +1,2 @@ +local BatchScript09 = {} +return BatchScript09 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript10.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript10.luau new file mode 100644 index 00000000..9a2dd32d --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript10.luau @@ -0,0 +1,2 @@ +local BatchScript10 = {} +return BatchScript10 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript11.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript11.luau new file mode 100644 index 00000000..8ea7033c --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript11.luau @@ -0,0 +1,2 @@ +local BatchScript11 = {} +return BatchScript11 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript12.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript12.luau new file mode 100644 index 00000000..0d25ddc1 --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript12.luau @@ -0,0 +1,2 @@ +local BatchScript12 = {} +return BatchScript12 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript13.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript13.luau new file mode 100644 index 00000000..1fb0d90e --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript13.luau @@ -0,0 +1,2 @@ +local BatchScript13 = {} +return BatchScript13 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript14.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript14.luau new file mode 100644 index 00000000..de006942 --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript14.luau @@ -0,0 +1,2 @@ +local BatchScript14 = {} +return BatchScript14 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript15.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript15.luau new file mode 100644 index 00000000..06a8adea --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript15.luau @@ -0,0 +1,2 @@ +local BatchScript15 = {} +return BatchScript15 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript16.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript16.luau new file mode 100644 index 00000000..9d7a9548 --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript16.luau @@ -0,0 +1,2 @@ +local BatchScript16 = {} +return BatchScript16 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript17.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript17.luau new file mode 100644 index 00000000..203a88d4 --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript17.luau @@ -0,0 +1,2 @@ +local BatchScript17 = {} +return BatchScript17 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript18.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript18.luau new file mode 100644 index 00000000..687ff57e --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript18.luau @@ -0,0 +1,2 @@ +local BatchScript18 = {} +return BatchScript18 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript19.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript19.luau new file mode 100644 index 00000000..c7ba3de9 --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript19.luau @@ -0,0 +1,2 @@ +local BatchScript19 = {} +return BatchScript19 diff --git a/rojo-test/serve-tests/git_sync_defaults/src/BatchScript20.luau b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript20.luau new file mode 100644 index 00000000..f4a27c68 --- /dev/null +++ b/rojo-test/serve-tests/git_sync_defaults/src/BatchScript20.luau @@ -0,0 +1,2 @@ +local BatchScript20 = {} +return BatchScript20 diff --git a/src/change_processor.rs b/src/change_processor.rs index 4a27b907..c04fc14d 100644 --- a/src/change_processor.rs +++ b/src/change_processor.rs @@ -1013,6 +1013,7 @@ impl JobThreadContext { removed_instances: patch_set.removed_instances, updated_instances: Vec::new(), stage_ids: HashSet::new(), + stage_paths: Vec::new(), }; let applied = apply_patch_set(&mut tree, structural_patch); @@ -1027,7 +1028,7 @@ impl JobThreadContext { ); } - fn handle_tree_event(&self, patch_set: PatchSet) { + fn handle_tree_event(&self, mut patch_set: PatchSet) { // Log incoming patch summary at debug level log::debug!( "Processing client patch: {} removed, {} added, {} updated", @@ -1308,6 +1309,10 @@ impl JobThreadContext { // Applied after the PatchSet to keep metadata in sync. let mut metadata_updates: Vec<(Ref, PathBuf)> = Vec::new(); + // Paths to stage via git add after all writes complete. + // Starts with pre-resolved paths from api.rs, then Source writes are appended. + let mut pending_stage_paths = std::mem::take(&mut patch_set.stage_paths); + for update in &patch_set.updated_instances { let id = update.id; @@ -1821,12 +1826,7 @@ impl JobThreadContext { err ); } else if patch_set.stage_ids.contains(&id) { - if let Some(ref repo_root) = self.git_repo_root { - crate::git::git_add( - repo_root, - std::slice::from_ref(write_path), - ); - } + pending_stage_paths.push(write_path.clone()); } } else { log::warn!("Cannot change Source to non-string value."); @@ -1898,6 +1898,14 @@ impl JobThreadContext { } } + // Consolidated git staging: one git_add call for all paths + // (pre-resolved from api.rs + Source writes from this function). + if !pending_stage_paths.is_empty() { + if let Some(ref repo_root) = self.git_repo_root { + crate::git::git_add(repo_root, &pending_stage_paths); + } + } + applied }; diff --git a/src/snapshot/patch.rs b/src/snapshot/patch.rs index 7a4195ea..235ed12d 100644 --- a/src/snapshot/patch.rs +++ b/src/snapshot/patch.rs @@ -24,6 +24,12 @@ pub struct PatchSet { /// Populated from WriteRequest.stage_ids, consumed by change_processor. #[serde(default, skip)] pub stage_ids: HashSet, + /// Pre-resolved filesystem paths to stage via git add. + /// Populated by api.rs (for files already on disk), consumed by change_processor + /// together with any Source write paths from stage_ids. + /// All staging is consolidated into a single git_add call to avoid index.lock races. + #[serde(default, skip)] + pub stage_paths: Vec, } impl PatchSet { @@ -33,6 +39,7 @@ impl PatchSet { added_instances: Vec::new(), updated_instances: Vec::new(), stage_ids: HashSet::new(), + stage_paths: Vec::new(), } } } diff --git a/src/web/api.rs b/src/web/api.rs index 0f1ed75a..380873f5 100644 --- a/src/web/api.rs +++ b/src/web/api.rs @@ -764,15 +764,10 @@ impl ApiService { added_instances, updated_instances, stage_ids, + stage_paths: paths_to_stage, }) .unwrap(); - if let Some(repo_root) = self.serve_session.git_repo_root() { - if !paths_to_stage.is_empty() { - crate::git::git_add(repo_root, &paths_to_stage); - } - } - msgpack_ok(WriteResponse { session_id }) } diff --git a/tests/tests/git_sync_defaults.rs b/tests/tests/git_sync_defaults.rs index c5f2e92f..fd37c11e 100644 --- a/tests/tests/git_sync_defaults.rs +++ b/tests/tests/git_sync_defaults.rs @@ -690,3 +690,212 @@ fn init_style_script_staging_targets_init_file() { ); } } + +// =========================================================================== +// Batch staging: many files in a single write request +// =========================================================================== + +const BATCH_SCRIPT_COUNT: usize = 20; + +fn batch_script_name(i: usize) -> String { + format!("BatchScript{:02}", i) +} + +fn batch_script_rel_path(i: usize) -> String { + format!("src/{}.luau", batch_script_name(i)) +} + +#[test] +fn batch_stage_20_files_no_index_lock_race() { + let mut session = TestServeSession::new_with_git("git_sync_defaults", |path| { + git_commit_all(path, "initial commit"); + for i in 1..=BATCH_SCRIPT_COUNT { + fs::write( + path.join(batch_script_rel_path(i)), + format!("-- modified {}\nreturn {{}}\n", i), + ) + .unwrap(); + } + }); + let info = session.wait_to_come_online(); + let read = session.get_api_read(info.root_instance_id).unwrap(); + + let mut stage_ids = Vec::new(); + for i in 1..=BATCH_SCRIPT_COUNT { + if let Some((id, _)) = find_instance_by_name(&read.instances, &batch_script_name(i)) { + stage_ids.push(id); + } + } + assert!( + stage_ids.len() >= BATCH_SCRIPT_COUNT, + "Should find all {} batch scripts, found {}", + BATCH_SCRIPT_COUNT, + stage_ids.len() + ); + + let write_request = WriteRequest { + session_id: info.session_id, + removed: vec![], + added: HashMap::new(), + updated: vec![], + stage_ids, + }; + session.post_api_write(&write_request).unwrap(); + thread::sleep(Duration::from_millis(1500)); + + for i in 1..=BATCH_SCRIPT_COUNT { + assert!( + git_is_staged(session.path(), &batch_script_rel_path(i)), + "{} should be staged", + batch_script_name(i) + ); + } +} + +#[test] +fn batch_source_write_20_files_all_staged() { + let mut session = TestServeSession::new_with_git("git_sync_defaults", |path| { + git_commit_all(path, "initial commit"); + }); + let info = session.wait_to_come_online(); + let read = session.get_api_read(info.root_instance_id).unwrap(); + + let mut updates = Vec::new(); + let mut stage_ids = Vec::new(); + for i in 1..=BATCH_SCRIPT_COUNT { + if let Some((id, _)) = find_instance_by_name(&read.instances, &batch_script_name(i)) { + let mut props = UstrMap::default(); + props.insert( + ustr("Source"), + Some(Variant::String(format!( + "-- batch pull {}\nreturn {{}}\n", + i + ))), + ); + updates.push(InstanceUpdate { + id, + changed_name: None, + changed_class_name: None, + changed_properties: props, + changed_metadata: None, + }); + stage_ids.push(id); + } + } + assert!( + updates.len() >= BATCH_SCRIPT_COUNT, + "Should find all {} batch scripts, found {}", + BATCH_SCRIPT_COUNT, + updates.len() + ); + + let write_request = WriteRequest { + session_id: info.session_id, + removed: vec![], + added: HashMap::new(), + updated: updates, + stage_ids, + }; + session.post_api_write(&write_request).unwrap(); + thread::sleep(Duration::from_millis(2000)); + + for i in 1..=BATCH_SCRIPT_COUNT { + let rel = batch_script_rel_path(i); + assert!( + git_is_staged(session.path(), &rel), + "{} should be staged after batch Source write", + batch_script_name(i) + ); + let content = fs::read_to_string(session.path().join(&rel)).unwrap(); + assert!( + content.contains(&format!("batch pull {}", i)), + "{} should have new content on disk", + batch_script_name(i) + ); + } +} + +#[test] +fn batch_mixed_stage_and_source_write() { + let mut session = TestServeSession::new_with_git("git_sync_defaults", |path| { + git_commit_all(path, "initial commit"); + // Modify the first 10 files on disk (push-accepted: already on disk, just stage) + for i in 1..=10 { + fs::write( + path.join(batch_script_rel_path(i)), + format!("-- push modified {}\nreturn {{}}\n", i), + ) + .unwrap(); + } + }); + let info = session.wait_to_come_online(); + let read = session.get_api_read(info.root_instance_id).unwrap(); + + let mut updates = Vec::new(); + let mut stage_ids = Vec::new(); + + // Files 1-10: push-accepted (already modified on disk, just stage) + for i in 1..=10 { + if let Some((id, _)) = find_instance_by_name(&read.instances, &batch_script_name(i)) { + stage_ids.push(id); + } + } + + // Files 11-20: pull-accepted (Source write + stage) + for i in 11..=BATCH_SCRIPT_COUNT { + if let Some((id, _)) = find_instance_by_name(&read.instances, &batch_script_name(i)) { + let mut props = UstrMap::default(); + props.insert( + ustr("Source"), + Some(Variant::String(format!( + "-- pull source {}\nreturn {{}}\n", + i + ))), + ); + updates.push(InstanceUpdate { + id, + changed_name: None, + changed_class_name: None, + changed_properties: props, + changed_metadata: None, + }); + stage_ids.push(id); + } + } + + assert!( + stage_ids.len() >= BATCH_SCRIPT_COUNT, + "Should have {} stage_ids, got {}", + BATCH_SCRIPT_COUNT, + stage_ids.len() + ); + + let write_request = WriteRequest { + session_id: info.session_id, + removed: vec![], + added: HashMap::new(), + updated: updates, + stage_ids, + }; + session.post_api_write(&write_request).unwrap(); + thread::sleep(Duration::from_millis(2000)); + + // All 20 files should be staged + for i in 1..=BATCH_SCRIPT_COUNT { + assert!( + git_is_staged(session.path(), &batch_script_rel_path(i)), + "{} should be staged in mixed batch", + batch_script_name(i) + ); + } + + // Files 11-20 should have the new pull content + for i in 11..=BATCH_SCRIPT_COUNT { + let content = fs::read_to_string(session.path().join(batch_script_rel_path(i))).unwrap(); + assert!( + content.contains(&format!("pull source {}", i)), + "{} should have pull content on disk", + batch_script_name(i) + ); + } +}