mirror of
https://github.com/rojo-rbx/rojo.git
synced 2026-10-23 14:01:37 +00:00
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.
This commit is contained in:
parent
bb4d40c4d3
commit
6de088be31
24 changed files with 272 additions and 13 deletions
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript01 = {}
|
||||||
|
return BatchScript01
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript02 = {}
|
||||||
|
return BatchScript02
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript03 = {}
|
||||||
|
return BatchScript03
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript04 = {}
|
||||||
|
return BatchScript04
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript05 = {}
|
||||||
|
return BatchScript05
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript06 = {}
|
||||||
|
return BatchScript06
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript07 = {}
|
||||||
|
return BatchScript07
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript08 = {}
|
||||||
|
return BatchScript08
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript09 = {}
|
||||||
|
return BatchScript09
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript10 = {}
|
||||||
|
return BatchScript10
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript11 = {}
|
||||||
|
return BatchScript11
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript12 = {}
|
||||||
|
return BatchScript12
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript13 = {}
|
||||||
|
return BatchScript13
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript14 = {}
|
||||||
|
return BatchScript14
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript15 = {}
|
||||||
|
return BatchScript15
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript16 = {}
|
||||||
|
return BatchScript16
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript17 = {}
|
||||||
|
return BatchScript17
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript18 = {}
|
||||||
|
return BatchScript18
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript19 = {}
|
||||||
|
return BatchScript19
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
local BatchScript20 = {}
|
||||||
|
return BatchScript20
|
||||||
|
|
@ -1013,6 +1013,7 @@ impl JobThreadContext {
|
||||||
removed_instances: patch_set.removed_instances,
|
removed_instances: patch_set.removed_instances,
|
||||||
updated_instances: Vec::new(),
|
updated_instances: Vec::new(),
|
||||||
stage_ids: HashSet::new(),
|
stage_ids: HashSet::new(),
|
||||||
|
stage_paths: Vec::new(),
|
||||||
};
|
};
|
||||||
|
|
||||||
let applied = apply_patch_set(&mut tree, structural_patch);
|
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 incoming patch summary at debug level
|
||||||
log::debug!(
|
log::debug!(
|
||||||
"Processing client patch: {} removed, {} added, {} updated",
|
"Processing client patch: {} removed, {} added, {} updated",
|
||||||
|
|
@ -1308,6 +1309,10 @@ impl JobThreadContext {
|
||||||
// Applied after the PatchSet to keep metadata in sync.
|
// Applied after the PatchSet to keep metadata in sync.
|
||||||
let mut metadata_updates: Vec<(Ref, PathBuf)> = Vec::new();
|
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 {
|
for update in &patch_set.updated_instances {
|
||||||
let id = update.id;
|
let id = update.id;
|
||||||
|
|
||||||
|
|
@ -1821,12 +1826,7 @@ impl JobThreadContext {
|
||||||
err
|
err
|
||||||
);
|
);
|
||||||
} else if patch_set.stage_ids.contains(&id) {
|
} else if patch_set.stage_ids.contains(&id) {
|
||||||
if let Some(ref repo_root) = self.git_repo_root {
|
pending_stage_paths.push(write_path.clone());
|
||||||
crate::git::git_add(
|
|
||||||
repo_root,
|
|
||||||
std::slice::from_ref(write_path),
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
log::warn!("Cannot change Source to non-string value.");
|
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
|
applied
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -24,6 +24,12 @@ pub struct PatchSet {
|
||||||
/// Populated from WriteRequest.stage_ids, consumed by change_processor.
|
/// Populated from WriteRequest.stage_ids, consumed by change_processor.
|
||||||
#[serde(default, skip)]
|
#[serde(default, skip)]
|
||||||
pub stage_ids: HashSet<Ref>,
|
pub stage_ids: HashSet<Ref>,
|
||||||
|
/// 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<std::path::PathBuf>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl PatchSet {
|
impl PatchSet {
|
||||||
|
|
@ -33,6 +39,7 @@ impl PatchSet {
|
||||||
added_instances: Vec::new(),
|
added_instances: Vec::new(),
|
||||||
updated_instances: Vec::new(),
|
updated_instances: Vec::new(),
|
||||||
stage_ids: HashSet::new(),
|
stage_ids: HashSet::new(),
|
||||||
|
stage_paths: Vec::new(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -764,15 +764,10 @@ impl ApiService {
|
||||||
added_instances,
|
added_instances,
|
||||||
updated_instances,
|
updated_instances,
|
||||||
stage_ids,
|
stage_ids,
|
||||||
|
stage_paths: paths_to_stage,
|
||||||
})
|
})
|
||||||
.unwrap();
|
.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 })
|
msgpack_ok(WriteResponse { session_id })
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -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)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue