forked from rojo-rbx/rojo
Compare commits
30 Commits
fix/git-si
...
master
| Author | SHA1 | Date | |
|---|---|---|---|
|
93f808db28
|
|||
|
110b9f0df3
|
|||
|
4fce3461f3
|
|||
|
917d17a738
|
|||
|
85bb5ec3f2
|
|||
|
14bbdaf560
|
|||
|
667683d3b3
|
|||
|
5b1b5db06c
|
|||
|
d7a9ce55db
|
|||
|
33dd0f5ed1
|
|||
|
996113b177
|
|||
|
95fe993de3
|
|||
|
4ca26efccb
|
|||
|
ce0db54e0a
|
|||
|
c552fdc52e
|
|||
| a6e9939d6c | |||
|
5957368c04
|
|||
|
78916c8a63
|
|||
|
791ccfcfd1
|
|||
|
3500ebe02a
|
|||
|
0e1364945f
|
|||
| 3a6aae65f7 | |||
| d13d229eef | |||
| 9a485d88ce | |||
|
020d72faef
|
|||
|
60d150f4c6
|
|||
|
73dab330b5
|
|||
|
790312a5b0
|
|||
|
5c396322d9
|
|||
|
37e44e474a
|
@@ -1,73 +0,0 @@
|
||||
---
|
||||
source: tests/rojo_test/syncback_util.rs
|
||||
expression: src/ChildWithDuplicates.rbxm
|
||||
---
|
||||
num_types: 1
|
||||
num_instances: 3
|
||||
chunks:
|
||||
- Inst:
|
||||
type_id: 0
|
||||
type_name: Folder
|
||||
object_format: 0
|
||||
referents:
|
||||
- 0
|
||||
- 1
|
||||
- 2
|
||||
- Prop:
|
||||
type_id: 0
|
||||
prop_name: AttributesSerialize
|
||||
prop_type: String
|
||||
values:
|
||||
- ""
|
||||
- ""
|
||||
- ""
|
||||
- Prop:
|
||||
type_id: 0
|
||||
prop_name: Capabilities
|
||||
prop_type: SecurityCapabilities
|
||||
values:
|
||||
- 0
|
||||
- 0
|
||||
- 0
|
||||
- Prop:
|
||||
type_id: 0
|
||||
prop_name: Name
|
||||
prop_type: String
|
||||
values:
|
||||
- DuplicateChild
|
||||
- DuplicateChild
|
||||
- ChildWithDuplicates
|
||||
- Prop:
|
||||
type_id: 0
|
||||
prop_name: DefinesCapabilities
|
||||
prop_type: Bool
|
||||
values:
|
||||
- false
|
||||
- false
|
||||
- false
|
||||
- Prop:
|
||||
type_id: 0
|
||||
prop_name: SourceAssetId
|
||||
prop_type: Int64
|
||||
values:
|
||||
- -1
|
||||
- -1
|
||||
- -1
|
||||
- Prop:
|
||||
type_id: 0
|
||||
prop_name: Tags
|
||||
prop_type: String
|
||||
values:
|
||||
- ""
|
||||
- ""
|
||||
- ""
|
||||
- Prnt:
|
||||
version: 0
|
||||
links:
|
||||
- - 0
|
||||
- 2
|
||||
- - 1
|
||||
- 2
|
||||
- - 2
|
||||
- -1
|
||||
- End
|
||||
@@ -1,9 +1,12 @@
|
||||
---
|
||||
source: tests/rojo_test/syncback_util.rs
|
||||
assertion_line: 101
|
||||
expression: "String::from_utf8_lossy(&output.stdout)"
|
||||
---
|
||||
Writing src/ChildWithDuplicates.rbxm
|
||||
Writing src/ChildWithDuplicates/DuplicateChild/.gitkeep
|
||||
Writing src/ChildWithDuplicates/DuplicateChild1/.gitkeep
|
||||
Writing src/ChildWithoutDuplicates/Child/.gitkeep
|
||||
Writing src/ChildWithDuplicates/DuplicateChild
|
||||
Writing src/ChildWithDuplicates/DuplicateChild1
|
||||
Writing src/ChildWithoutDuplicates
|
||||
Writing src/ChildWithoutDuplicates/Child
|
||||
Removing src/ChildWithDuplicates
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
---
|
||||
source: tests/tests/syncback.rs
|
||||
assertion_line: 31
|
||||
expression: src/ChildWithDuplicates/DuplicateChild1/.gitkeep
|
||||
---
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
---
|
||||
source: tests/tests/syncback.rs
|
||||
assertion_line: 31
|
||||
expression: src/ChildWithDuplicates/DuplicateChild/.gitkeep
|
||||
---
|
||||
|
||||
@@ -54,6 +54,11 @@ pub struct SyncbackCommand {
|
||||
/// If provided, the prompt for writing to the file system is skipped.
|
||||
#[clap(long, short = 'y')]
|
||||
pub non_interactive: bool,
|
||||
|
||||
/// If provided, forces syncback to use JSON model files instead of binary
|
||||
/// .rbxm files for instances that would otherwise serialize as binary.
|
||||
#[clap(long)]
|
||||
pub dangerously_force_json: bool,
|
||||
}
|
||||
|
||||
impl SyncbackCommand {
|
||||
@@ -104,6 +109,7 @@ impl SyncbackCommand {
|
||||
&mut dom_old,
|
||||
dom_new,
|
||||
session_old.root_project(),
|
||||
self.dangerously_force_json,
|
||||
)?;
|
||||
log::debug!(
|
||||
"Syncback finished in {:.02}s!",
|
||||
|
||||
@@ -109,8 +109,14 @@ pub fn syncback_csv<'sync>(
|
||||
|
||||
if !meta.is_empty() {
|
||||
let parent = snapshot.path.parent_err()?;
|
||||
let file_name = snapshot
|
||||
.path
|
||||
.file_name()
|
||||
.and_then(|n| n.to_str())
|
||||
.unwrap_or("");
|
||||
let meta_stem = file_name.strip_suffix(".csv").unwrap_or(file_name);
|
||||
fs_snapshot.add_file(
|
||||
parent.join(format!("{}.meta.json", new_inst.name)),
|
||||
parent.join(format!("{meta_stem}.meta.json")),
|
||||
serde_json::to_vec_pretty(&meta).context("cannot serialize metadata")?,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -8,10 +8,13 @@ use memofs::{DirEntry, Vfs};
|
||||
|
||||
use crate::{
|
||||
snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource},
|
||||
syncback::{hash_instance, FsSnapshot, SyncbackReturn, SyncbackSnapshot},
|
||||
syncback::{
|
||||
extension_for_middleware, hash_instance, FsSnapshot, SyncbackReturn,
|
||||
SyncbackSnapshot,
|
||||
},
|
||||
};
|
||||
|
||||
use super::{meta_file::DirectoryMetadata, snapshot_from_vfs};
|
||||
use super::{meta_file::DirectoryMetadata, snapshot_from_vfs, Middleware};
|
||||
|
||||
const EMPTY_DIR_KEEP_NAME: &str = ".gitkeep";
|
||||
|
||||
@@ -91,6 +94,22 @@ pub fn snapshot_dir_no_meta(
|
||||
Ok(Some(snapshot))
|
||||
}
|
||||
|
||||
/// Splits a filesystem name into (stem, extension) based on middleware type.
|
||||
/// For directory middleware, the extension is empty. For file middleware,
|
||||
/// the extension comes from `extension_for_middleware`.
|
||||
fn split_name_and_ext(name: &str, middleware: Middleware) -> (&str, &str) {
|
||||
if middleware.is_dir() {
|
||||
(name, "")
|
||||
} else {
|
||||
let ext = extension_for_middleware(middleware);
|
||||
if let Some(stem) = name.strip_suffix(&format!(".{ext}")) {
|
||||
(stem, ext)
|
||||
} else {
|
||||
(name, "")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub fn syncback_dir<'sync>(
|
||||
snapshot: &SyncbackSnapshot<'sync>,
|
||||
) -> anyhow::Result<SyncbackReturn<'sync>> {
|
||||
@@ -134,65 +153,128 @@ pub fn syncback_dir_no_meta<'sync>(
|
||||
let mut children = Vec::new();
|
||||
let mut removed_children = Vec::new();
|
||||
|
||||
// We have to enforce unique child names for the file system.
|
||||
let mut child_names = HashSet::with_capacity(new_inst.children().len());
|
||||
let mut duplicate_set = HashSet::new();
|
||||
for child_ref in new_inst.children() {
|
||||
let child = snapshot.get_new_instance(*child_ref).unwrap();
|
||||
if !child_names.insert(child.name.to_lowercase()) {
|
||||
duplicate_set.insert(child.name.as_str());
|
||||
}
|
||||
}
|
||||
if !duplicate_set.is_empty() {
|
||||
if duplicate_set.len() <= 25 {
|
||||
anyhow::bail!(
|
||||
"Instance has children with duplicate name (case may not exactly match):\n {}",
|
||||
duplicate_set.into_iter().collect::<Vec<&str>>().join(", ")
|
||||
);
|
||||
}
|
||||
anyhow::bail!("Instance has more than 25 children with duplicate names");
|
||||
}
|
||||
|
||||
// Build the old child map early so it can be used for deduplication below.
|
||||
let mut old_child_map = HashMap::new();
|
||||
if let Some(old_inst) = snapshot.old_inst() {
|
||||
let mut old_child_map = HashMap::with_capacity(old_inst.children().len());
|
||||
for child in old_inst.children() {
|
||||
let inst = snapshot.get_old_instance(*child).unwrap();
|
||||
old_child_map.insert(inst.name(), inst);
|
||||
}
|
||||
}
|
||||
|
||||
for new_child_ref in new_inst.children() {
|
||||
let new_child = snapshot.get_new_instance(*new_child_ref).unwrap();
|
||||
if let Some(old_child) = old_child_map.remove(new_child.name.as_str()) {
|
||||
if old_child.metadata().relevant_paths.is_empty() {
|
||||
log::debug!(
|
||||
"Skipping instance {} because it doesn't exist on the disk",
|
||||
old_child.name()
|
||||
);
|
||||
continue;
|
||||
} else if matches!(
|
||||
old_child.metadata().instigating_source,
|
||||
Some(InstigatingSource::ProjectNode { .. })
|
||||
) {
|
||||
log::debug!(
|
||||
"Skipping instance {} because it originates in a project file",
|
||||
old_child.name()
|
||||
);
|
||||
continue;
|
||||
}
|
||||
// This child exists in both doms. Pass it on.
|
||||
children.push(snapshot.with_joined_path(*new_child_ref, Some(old_child.id()))?);
|
||||
} else {
|
||||
// The child only exists in the the new dom
|
||||
children.push(snapshot.with_joined_path(*new_child_ref, None)?);
|
||||
// --- Two-pass collision resolution ---
|
||||
//
|
||||
// Pass 1: Collect each child's base filesystem name and old ref, applying
|
||||
// skip conditions. Track which names are used (lowercased) so we can
|
||||
// detect collisions.
|
||||
struct ChildEntry {
|
||||
new_ref: rbx_dom_weak::types::Ref,
|
||||
old_ref: Option<rbx_dom_weak::types::Ref>,
|
||||
base_name: String,
|
||||
middleware: Middleware,
|
||||
skip: bool,
|
||||
}
|
||||
|
||||
let mut entries = Vec::with_capacity(new_inst.children().len());
|
||||
let mut used_names: HashSet<String> = HashSet::with_capacity(new_inst.children().len());
|
||||
let mut collision_indices: Vec<usize> = Vec::new();
|
||||
|
||||
for new_child_ref in new_inst.children() {
|
||||
let new_child = snapshot.get_new_instance(*new_child_ref).unwrap();
|
||||
|
||||
// Determine old_ref and apply skip conditions.
|
||||
let old_child = if snapshot.old_inst().is_some() {
|
||||
old_child_map.remove(new_child.name.as_str())
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
let mut skip = false;
|
||||
if let Some(ref old) = old_child {
|
||||
if old.metadata().relevant_paths.is_empty() {
|
||||
log::debug!(
|
||||
"Skipping instance {} because it doesn't exist on the disk",
|
||||
old.name()
|
||||
);
|
||||
skip = true;
|
||||
} else if matches!(
|
||||
old.metadata().instigating_source,
|
||||
Some(InstigatingSource::ProjectNode { .. })
|
||||
) {
|
||||
log::debug!(
|
||||
"Skipping instance {} because it originates in a project file",
|
||||
old.name()
|
||||
);
|
||||
skip = true;
|
||||
}
|
||||
}
|
||||
// Any children that are in the old dom but not the new one are removed.
|
||||
removed_children.extend(old_child_map.into_values());
|
||||
} else {
|
||||
// There is no old instance. Just add every child.
|
||||
for new_child_ref in new_inst.children() {
|
||||
children.push(snapshot.with_joined_path(*new_child_ref, None)?);
|
||||
|
||||
let old_ref = old_child.as_ref().map(|o| o.id());
|
||||
|
||||
if skip {
|
||||
entries.push(ChildEntry {
|
||||
new_ref: *new_child_ref,
|
||||
old_ref,
|
||||
base_name: String::new(),
|
||||
middleware: Middleware::Dir,
|
||||
skip: true,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
let (middleware, base_name) =
|
||||
snapshot.child_middleware_and_name(*new_child_ref, old_ref)?;
|
||||
|
||||
let idx = entries.len();
|
||||
let lower = base_name.to_lowercase();
|
||||
if !used_names.insert(lower) {
|
||||
// Name already claimed — needs resolution.
|
||||
collision_indices.push(idx);
|
||||
}
|
||||
|
||||
entries.push(ChildEntry {
|
||||
new_ref: *new_child_ref,
|
||||
old_ref,
|
||||
base_name,
|
||||
middleware,
|
||||
skip: false,
|
||||
});
|
||||
}
|
||||
|
||||
// Pass 2: Resolve collisions by appending incrementing suffixes.
|
||||
for idx in collision_indices {
|
||||
let entry = &entries[idx];
|
||||
let (stem, ext) = split_name_and_ext(&entry.base_name, entry.middleware);
|
||||
let mut counter = 1u32;
|
||||
loop {
|
||||
let candidate = if ext.is_empty() {
|
||||
format!("{stem}{counter}")
|
||||
} else {
|
||||
format!("{stem}{counter}.{ext}")
|
||||
};
|
||||
let lower = candidate.to_lowercase();
|
||||
if used_names.insert(lower) {
|
||||
// Safe to mutate — we only visit each collision index once.
|
||||
let entry = &mut entries[idx];
|
||||
entry.base_name = candidate;
|
||||
break;
|
||||
}
|
||||
counter += 1;
|
||||
}
|
||||
}
|
||||
|
||||
// Create snapshots from resolved entries.
|
||||
for entry in &entries {
|
||||
if entry.skip {
|
||||
continue;
|
||||
}
|
||||
let resolved_path = snapshot.path.join(&entry.base_name);
|
||||
children.push(snapshot.with_new_path(resolved_path, entry.new_ref, entry.old_ref));
|
||||
}
|
||||
|
||||
// Any children that are in the old dom but not the new one are removed.
|
||||
if snapshot.old_inst().is_some() {
|
||||
removed_children.extend(old_child_map.into_values());
|
||||
}
|
||||
let mut fs_snapshot = FsSnapshot::new();
|
||||
|
||||
@@ -225,6 +307,12 @@ pub fn syncback_dir_no_meta<'sync>(
|
||||
mod test {
|
||||
use super::*;
|
||||
|
||||
use std::path::PathBuf;
|
||||
|
||||
use crate::{
|
||||
snapshot::{InstanceMetadata, InstanceSnapshot},
|
||||
Project, RojoTree, SyncbackData, SyncbackSnapshot,
|
||||
};
|
||||
use memofs::{InMemoryFs, VfsSnapshot};
|
||||
|
||||
#[test]
|
||||
@@ -261,4 +349,302 @@ mod test {
|
||||
|
||||
insta::assert_yaml_snapshot!(instance_snapshot);
|
||||
}
|
||||
|
||||
fn make_project() -> Project {
|
||||
serde_json::from_str(r#"{"tree": {"$className": "DataModel"}}"#).unwrap()
|
||||
}
|
||||
|
||||
fn make_vfs() -> Vfs {
|
||||
let mut imfs = InMemoryFs::new();
|
||||
imfs.load_snapshot("/root", VfsSnapshot::empty_dir()).unwrap();
|
||||
Vfs::new(imfs)
|
||||
}
|
||||
|
||||
/// Two children whose Roblox names are identical when lowercased ("Alpha"
|
||||
/// and "alpha") but live at different filesystem paths because of the
|
||||
/// `name` property ("Beta/" and "Alpha/" respectively). The dedup check
|
||||
/// must use the actual filesystem paths, not the raw Roblox names, to
|
||||
/// avoid a false-positive duplicate error.
|
||||
#[test]
|
||||
fn syncback_no_false_duplicate_with_name_prop() {
|
||||
use rbx_dom_weak::{InstanceBuilder, WeakDom};
|
||||
|
||||
// Old child A: Roblox name "Alpha", on disk at "/root/Beta"
|
||||
// (name property maps "Alpha" → "Beta" on the filesystem)
|
||||
let old_child_a = InstanceSnapshot::new()
|
||||
.name("Alpha")
|
||||
.class_name("Folder")
|
||||
.metadata(
|
||||
InstanceMetadata::new()
|
||||
.instigating_source(PathBuf::from("/root/Beta"))
|
||||
.relevant_paths(vec![PathBuf::from("/root/Beta")]),
|
||||
);
|
||||
// Old child B: Roblox name "alpha", on disk at "/root/Alpha"
|
||||
let old_child_b = InstanceSnapshot::new()
|
||||
.name("alpha")
|
||||
.class_name("Folder")
|
||||
.metadata(
|
||||
InstanceMetadata::new()
|
||||
.instigating_source(PathBuf::from("/root/Alpha"))
|
||||
.relevant_paths(vec![PathBuf::from("/root/Alpha")]),
|
||||
);
|
||||
let old_parent = InstanceSnapshot::new()
|
||||
.name("Parent")
|
||||
.class_name("Folder")
|
||||
.children(vec![old_child_a, old_child_b])
|
||||
.metadata(
|
||||
InstanceMetadata::new()
|
||||
.instigating_source(PathBuf::from("/root"))
|
||||
.relevant_paths(vec![PathBuf::from("/root")]),
|
||||
);
|
||||
let old_tree = RojoTree::new(old_parent);
|
||||
|
||||
// New state: same two children in Roblox.
|
||||
let mut new_tree = WeakDom::new(InstanceBuilder::new("ROOT"));
|
||||
let new_parent = new_tree.insert(
|
||||
new_tree.root_ref(),
|
||||
InstanceBuilder::new("Folder").with_name("Parent"),
|
||||
);
|
||||
new_tree.insert(new_parent, InstanceBuilder::new("Folder").with_name("Alpha"));
|
||||
new_tree.insert(new_parent, InstanceBuilder::new("Folder").with_name("alpha"));
|
||||
|
||||
let vfs = make_vfs();
|
||||
let project = make_project();
|
||||
let data = SyncbackData::for_test(&vfs, &old_tree, &new_tree, &project);
|
||||
let snapshot = SyncbackSnapshot {
|
||||
data,
|
||||
old: Some(old_tree.get_root_id()),
|
||||
new: new_parent,
|
||||
path: PathBuf::from("/root"),
|
||||
middleware: None,
|
||||
};
|
||||
|
||||
let result = syncback_dir_no_meta(&snapshot);
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"should not error when two children have the same lowercased Roblox \
|
||||
name but map to distinct filesystem paths: {:?}",
|
||||
result.as_ref().err(),
|
||||
);
|
||||
}
|
||||
|
||||
/// Two completely new children with the same name get resolved via
|
||||
/// incrementing suffixes instead of erroring.
|
||||
#[test]
|
||||
fn syncback_resolves_sibling_duplicate_names() {
|
||||
use rbx_dom_weak::{InstanceBuilder, WeakDom};
|
||||
|
||||
let old_parent = InstanceSnapshot::new()
|
||||
.name("Parent")
|
||||
.class_name("Folder")
|
||||
.metadata(
|
||||
InstanceMetadata::new()
|
||||
.instigating_source(PathBuf::from("/root"))
|
||||
.relevant_paths(vec![PathBuf::from("/root")]),
|
||||
);
|
||||
let old_tree = RojoTree::new(old_parent);
|
||||
|
||||
let mut new_tree = WeakDom::new(InstanceBuilder::new("ROOT"));
|
||||
let new_parent = new_tree.insert(
|
||||
new_tree.root_ref(),
|
||||
InstanceBuilder::new("Folder").with_name("Parent"),
|
||||
);
|
||||
new_tree.insert(new_parent, InstanceBuilder::new("Folder").with_name("Foo"));
|
||||
new_tree.insert(new_parent, InstanceBuilder::new("Folder").with_name("Foo"));
|
||||
|
||||
let vfs = make_vfs();
|
||||
let project = make_project();
|
||||
let data = SyncbackData::for_test(&vfs, &old_tree, &new_tree, &project);
|
||||
let snapshot = SyncbackSnapshot {
|
||||
data,
|
||||
old: Some(old_tree.get_root_id()),
|
||||
new: new_parent,
|
||||
path: PathBuf::from("/root"),
|
||||
middleware: None,
|
||||
};
|
||||
|
||||
let result = syncback_dir_no_meta(&snapshot);
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"should resolve duplicate names with suffixes, not error: {:?}",
|
||||
result.as_ref().err(),
|
||||
);
|
||||
let children = result.unwrap().children;
|
||||
let mut names: Vec<String> = children
|
||||
.iter()
|
||||
.map(|c| c.path.file_name().unwrap().to_string_lossy().into_owned())
|
||||
.collect();
|
||||
names.sort();
|
||||
assert_eq!(names, vec!["Foo", "Foo1"]);
|
||||
}
|
||||
|
||||
/// A new child named "Init" (as a ModuleScript) would naively become
|
||||
/// "Init.luau", which case-insensitively matches the parent's reserved
|
||||
/// "init.luau". Syncback must resolve this automatically by prefixing the
|
||||
/// filesystem name with '_' (→ "_Init.luau") rather than erroring.
|
||||
#[test]
|
||||
fn syncback_resolves_init_name_conflict() {
|
||||
use rbx_dom_weak::{InstanceBuilder, WeakDom};
|
||||
|
||||
let old_parent = InstanceSnapshot::new()
|
||||
.name("Parent")
|
||||
.class_name("Folder")
|
||||
.metadata(
|
||||
InstanceMetadata::new()
|
||||
.instigating_source(PathBuf::from("/root"))
|
||||
.relevant_paths(vec![PathBuf::from("/root")]),
|
||||
);
|
||||
let old_tree = RojoTree::new(old_parent);
|
||||
|
||||
let mut new_tree = WeakDom::new(InstanceBuilder::new("ROOT"));
|
||||
let new_parent = new_tree.insert(
|
||||
new_tree.root_ref(),
|
||||
InstanceBuilder::new("Folder").with_name("Parent"),
|
||||
);
|
||||
new_tree.insert(
|
||||
new_parent,
|
||||
InstanceBuilder::new("ModuleScript").with_name("Init"),
|
||||
);
|
||||
|
||||
let vfs = make_vfs();
|
||||
let project = make_project();
|
||||
let data = SyncbackData::for_test(&vfs, &old_tree, &new_tree, &project);
|
||||
let snapshot = SyncbackSnapshot {
|
||||
data,
|
||||
old: Some(old_tree.get_root_id()),
|
||||
new: new_parent,
|
||||
path: PathBuf::from("/root"),
|
||||
middleware: None,
|
||||
};
|
||||
|
||||
let result = syncback_dir_no_meta(&snapshot);
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"should resolve init-name conflict by prefixing '_', not error: {:?}",
|
||||
result.as_ref().err(),
|
||||
);
|
||||
// The child should have been placed at "_Init.luau", not "Init.luau".
|
||||
let child_file_name = result
|
||||
.unwrap()
|
||||
.children
|
||||
.into_iter()
|
||||
.next()
|
||||
.and_then(|c| c.path.file_name().map(|n| n.to_string_lossy().into_owned()))
|
||||
.unwrap_or_default();
|
||||
assert!(
|
||||
child_file_name.starts_with('_'),
|
||||
"child filesystem name should start with '_' to avoid init collision, \
|
||||
got: {child_file_name}",
|
||||
);
|
||||
}
|
||||
|
||||
/// A child whose filesystem name is stored with a slugified prefix (e.g.
|
||||
/// "_Init") must NOT be blocked — only the bare "init" stem is reserved.
|
||||
#[test]
|
||||
fn syncback_allows_slugified_init_name() {
|
||||
use rbx_dom_weak::{InstanceBuilder, WeakDom};
|
||||
|
||||
// Existing child: on disk as "_Init" (slugified from a name with an
|
||||
// illegal character), its stem is "_init" which is not reserved.
|
||||
let old_child = InstanceSnapshot::new()
|
||||
.name("Init")
|
||||
.class_name("Folder")
|
||||
.metadata(
|
||||
InstanceMetadata::new()
|
||||
.instigating_source(PathBuf::from("/root/_Init"))
|
||||
.relevant_paths(vec![PathBuf::from("/root/_Init")]),
|
||||
);
|
||||
let old_parent = InstanceSnapshot::new()
|
||||
.name("Parent")
|
||||
.class_name("Folder")
|
||||
.children(vec![old_child])
|
||||
.metadata(
|
||||
InstanceMetadata::new()
|
||||
.instigating_source(PathBuf::from("/root"))
|
||||
.relevant_paths(vec![PathBuf::from("/root")]),
|
||||
);
|
||||
let old_tree = RojoTree::new(old_parent);
|
||||
|
||||
let mut new_tree = WeakDom::new(InstanceBuilder::new("ROOT"));
|
||||
let new_parent = new_tree.insert(
|
||||
new_tree.root_ref(),
|
||||
InstanceBuilder::new("Folder").with_name("Parent"),
|
||||
);
|
||||
new_tree.insert(new_parent, InstanceBuilder::new("Folder").with_name("Init"));
|
||||
|
||||
let vfs = make_vfs();
|
||||
let project = make_project();
|
||||
let data = SyncbackData::for_test(&vfs, &old_tree, &new_tree, &project);
|
||||
let snapshot = SyncbackSnapshot {
|
||||
data,
|
||||
old: Some(old_tree.get_root_id()),
|
||||
new: new_parent,
|
||||
path: PathBuf::from("/root"),
|
||||
middleware: None,
|
||||
};
|
||||
|
||||
let result = syncback_dir_no_meta(&snapshot);
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"should allow a child whose filesystem name is slugified away from \
|
||||
the reserved 'init' stem: {:?}",
|
||||
result.as_ref().err(),
|
||||
);
|
||||
}
|
||||
|
||||
/// Two new children both named "Init" (ModuleScripts) should get
|
||||
/// "_Init.luau" and "_Init1.luau" respectively.
|
||||
#[test]
|
||||
fn syncback_resolves_multiple_init_conflicts() {
|
||||
use rbx_dom_weak::{InstanceBuilder, WeakDom};
|
||||
|
||||
let old_parent = InstanceSnapshot::new()
|
||||
.name("Parent")
|
||||
.class_name("Folder")
|
||||
.metadata(
|
||||
InstanceMetadata::new()
|
||||
.instigating_source(PathBuf::from("/root"))
|
||||
.relevant_paths(vec![PathBuf::from("/root")]),
|
||||
);
|
||||
let old_tree = RojoTree::new(old_parent);
|
||||
|
||||
let mut new_tree = WeakDom::new(InstanceBuilder::new("ROOT"));
|
||||
let new_parent = new_tree.insert(
|
||||
new_tree.root_ref(),
|
||||
InstanceBuilder::new("Folder").with_name("Parent"),
|
||||
);
|
||||
new_tree.insert(
|
||||
new_parent,
|
||||
InstanceBuilder::new("ModuleScript").with_name("Init"),
|
||||
);
|
||||
new_tree.insert(
|
||||
new_parent,
|
||||
InstanceBuilder::new("ModuleScript").with_name("Init"),
|
||||
);
|
||||
|
||||
let vfs = make_vfs();
|
||||
let project = make_project();
|
||||
let data = SyncbackData::for_test(&vfs, &old_tree, &new_tree, &project);
|
||||
let snapshot = SyncbackSnapshot {
|
||||
data,
|
||||
old: Some(old_tree.get_root_id()),
|
||||
new: new_parent,
|
||||
path: PathBuf::from("/root"),
|
||||
middleware: None,
|
||||
};
|
||||
|
||||
let result = syncback_dir_no_meta(&snapshot);
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"should resolve multiple init conflicts with suffixes: {:?}",
|
||||
result.as_ref().err(),
|
||||
);
|
||||
let children = result.unwrap().children;
|
||||
let mut names: Vec<String> = children
|
||||
.iter()
|
||||
.map(|c| c.path.file_name().unwrap().to_string_lossy().into_owned())
|
||||
.collect();
|
||||
names.sort();
|
||||
assert_eq!(names, vec!["_Init.luau", "_Init1.luau"]);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -158,16 +158,23 @@ pub fn syncback_lua<'sync>(
|
||||
|
||||
if !meta.is_empty() {
|
||||
let parent_location = snapshot.path.parent_err()?;
|
||||
let instance_name = &snapshot.new_inst().name;
|
||||
let slugified;
|
||||
let meta_name = if crate::syncback::validate_file_name(instance_name).is_err() {
|
||||
slugified = crate::syncback::slugify_name(instance_name);
|
||||
&slugified
|
||||
} else {
|
||||
instance_name
|
||||
};
|
||||
let file_name = snapshot
|
||||
.path
|
||||
.file_name()
|
||||
.and_then(|n| n.to_str())
|
||||
.unwrap_or("");
|
||||
let meta_stem = file_name
|
||||
.strip_suffix(".server.luau")
|
||||
.or_else(|| file_name.strip_suffix(".server.lua"))
|
||||
.or_else(|| file_name.strip_suffix(".client.luau"))
|
||||
.or_else(|| file_name.strip_suffix(".client.lua"))
|
||||
.or_else(|| file_name.strip_suffix(".plugin.luau"))
|
||||
.or_else(|| file_name.strip_suffix(".plugin.lua"))
|
||||
.or_else(|| file_name.strip_suffix(".luau"))
|
||||
.or_else(|| file_name.strip_suffix(".lua"))
|
||||
.unwrap_or(file_name);
|
||||
fs_snapshot.add_file(
|
||||
parent_location.join(format!("{}.meta.json", meta_name)),
|
||||
parent_location.join(format!("{meta_stem}.meta.json")),
|
||||
serde_json::to_vec_pretty(&meta).context("cannot serialize metadata")?,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -154,11 +154,13 @@ impl AdjacentMetadata {
|
||||
.old_inst()
|
||||
.and_then(|inst| inst.metadata().specified_name.clone())
|
||||
.or_else(|| {
|
||||
// If this is a new instance and its name is invalid for the filesystem,
|
||||
// we need to specify the name in meta.json so it can be preserved
|
||||
// Write name when name_for_inst would produce a different
|
||||
// filesystem stem (slugification or init-prefix).
|
||||
if snapshot.old_inst().is_none() {
|
||||
let instance_name = &snapshot.new_inst().name;
|
||||
if validate_file_name(instance_name).is_err() {
|
||||
if validate_file_name(instance_name).is_err()
|
||||
|| instance_name.to_lowercase() == "init"
|
||||
{
|
||||
Some(instance_name.clone())
|
||||
} else {
|
||||
None
|
||||
@@ -421,11 +423,13 @@ impl DirectoryMetadata {
|
||||
.old_inst()
|
||||
.and_then(|inst| inst.metadata().specified_name.clone())
|
||||
.or_else(|| {
|
||||
// If this is a new instance and its name is invalid for the filesystem,
|
||||
// we need to specify the name in meta.json so it can be preserved
|
||||
// Write name when name_for_inst would produce a different
|
||||
// directory name (slugification or init-prefix).
|
||||
if snapshot.old_inst().is_none() {
|
||||
let instance_name = &snapshot.new_inst().name;
|
||||
if validate_file_name(instance_name).is_err() {
|
||||
if validate_file_name(instance_name).is_err()
|
||||
|| instance_name.to_lowercase() == "init"
|
||||
{
|
||||
Some(instance_name.clone())
|
||||
} else {
|
||||
None
|
||||
|
||||
@@ -58,8 +58,14 @@ pub fn syncback_txt<'sync>(
|
||||
|
||||
if !meta.is_empty() {
|
||||
let parent = snapshot.path.parent_err()?;
|
||||
let file_name = snapshot
|
||||
.path
|
||||
.file_name()
|
||||
.and_then(|n| n.to_str())
|
||||
.unwrap_or("");
|
||||
let meta_stem = file_name.strip_suffix(".txt").unwrap_or(file_name);
|
||||
fs_snapshot.add_file(
|
||||
parent.join(format!("{}.meta.json", new_inst.name)),
|
||||
parent.join(format!("{meta_stem}.meta.json")),
|
||||
serde_json::to_vec_pretty(&meta).context("could not serialize metadata")?,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -36,23 +36,33 @@ pub fn name_for_inst<'a>(
|
||||
| Middleware::ServerScriptDir
|
||||
| Middleware::ClientScriptDir
|
||||
| Middleware::ModuleScriptDir => {
|
||||
if validate_file_name(&new_inst.name).is_err() {
|
||||
let name = if validate_file_name(&new_inst.name).is_err() {
|
||||
Cow::Owned(slugify_name(&new_inst.name))
|
||||
} else {
|
||||
Cow::Borrowed(&new_inst.name)
|
||||
Cow::Borrowed(new_inst.name.as_str())
|
||||
};
|
||||
// Prefix "init" to avoid colliding with reserved init files.
|
||||
if name.to_lowercase() == "init" {
|
||||
Cow::Owned(format!("_{name}"))
|
||||
} else {
|
||||
name
|
||||
}
|
||||
}
|
||||
_ => {
|
||||
let extension = extension_for_middleware(middleware);
|
||||
let slugified;
|
||||
let final_name = if validate_file_name(&new_inst.name).is_err() {
|
||||
let stem: &str = if validate_file_name(&new_inst.name).is_err() {
|
||||
slugified = slugify_name(&new_inst.name);
|
||||
&slugified
|
||||
} else {
|
||||
&new_inst.name
|
||||
};
|
||||
|
||||
Cow::Owned(format!("{final_name}.{extension}"))
|
||||
// Prefix "init" stems to avoid colliding with reserved init files.
|
||||
if stem.to_lowercase() == "init" {
|
||||
Cow::Owned(format!("_{stem}.{extension}"))
|
||||
} else {
|
||||
Cow::Owned(format!("{stem}.{extension}"))
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -28,7 +28,7 @@ use crate::{
|
||||
Project,
|
||||
};
|
||||
|
||||
pub use file_names::{extension_for_middleware, name_for_inst, slugify_name, validate_file_name};
|
||||
pub use file_names::{extension_for_middleware, name_for_inst, validate_file_name};
|
||||
pub use fs_snapshot::FsSnapshot;
|
||||
pub use hash::*;
|
||||
pub use property_filter::{filter_properties, filter_properties_preallocated};
|
||||
@@ -52,6 +52,7 @@ pub fn syncback_loop(
|
||||
old_tree: &mut RojoTree,
|
||||
mut new_tree: WeakDom,
|
||||
project: &Project,
|
||||
force_json: bool,
|
||||
) -> anyhow::Result<FsSnapshot> {
|
||||
let ignore_patterns = project
|
||||
.syncback_rules
|
||||
@@ -153,6 +154,7 @@ pub fn syncback_loop(
|
||||
old_tree,
|
||||
new_tree: &new_tree,
|
||||
project,
|
||||
force_json,
|
||||
};
|
||||
|
||||
let mut snapshots = vec![SyncbackSnapshot {
|
||||
@@ -197,7 +199,7 @@ pub fn syncback_loop(
|
||||
}
|
||||
}
|
||||
|
||||
let middleware = get_best_middleware(&snapshot);
|
||||
let middleware = get_best_middleware(&snapshot, force_json);
|
||||
|
||||
log::trace!(
|
||||
"Middleware for {inst_path} is {:?} (path is {})",
|
||||
@@ -213,10 +215,14 @@ pub fn syncback_loop(
|
||||
let syncback = match middleware.syncback(&snapshot) {
|
||||
Ok(syncback) => syncback,
|
||||
Err(err) if middleware == Middleware::Dir => {
|
||||
let new_middleware = match env::var(DEBUG_MODEL_FORMAT_VAR) {
|
||||
Ok(value) if value == "1" => Middleware::Rbxmx,
|
||||
Ok(value) if value == "2" => Middleware::JsonModel,
|
||||
_ => Middleware::Rbxm,
|
||||
let new_middleware = if force_json {
|
||||
Middleware::JsonModel
|
||||
} else {
|
||||
match env::var(DEBUG_MODEL_FORMAT_VAR) {
|
||||
Ok(value) if value == "1" => Middleware::Rbxmx,
|
||||
Ok(value) if value == "2" => Middleware::JsonModel,
|
||||
_ => Middleware::Rbxm,
|
||||
}
|
||||
};
|
||||
let file_name = snapshot
|
||||
.path
|
||||
@@ -295,7 +301,7 @@ pub struct SyncbackReturn<'sync> {
|
||||
pub removed_children: Vec<InstanceWithMeta<'sync>>,
|
||||
}
|
||||
|
||||
pub fn get_best_middleware(snapshot: &SyncbackSnapshot) -> Middleware {
|
||||
pub fn get_best_middleware(snapshot: &SyncbackSnapshot, force_json: bool) -> Middleware {
|
||||
// At some point, we're better off using an O(1) method for checking
|
||||
// equality for classes like this.
|
||||
static JSON_MODEL_CLASSES: OnceLock<HashSet<&str>> = OnceLock::new();
|
||||
@@ -367,10 +373,18 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot) -> Middleware {
|
||||
}
|
||||
|
||||
if middleware == Middleware::Rbxm {
|
||||
middleware = match env::var(DEBUG_MODEL_FORMAT_VAR) {
|
||||
Ok(value) if value == "1" => Middleware::Rbxmx,
|
||||
Ok(value) if value == "2" => Middleware::JsonModel,
|
||||
_ => Middleware::Rbxm,
|
||||
middleware = if force_json {
|
||||
if !inst.children().is_empty() {
|
||||
Middleware::Dir
|
||||
} else {
|
||||
Middleware::JsonModel
|
||||
}
|
||||
} else {
|
||||
match env::var(DEBUG_MODEL_FORMAT_VAR) {
|
||||
Ok(value) if value == "1" => Middleware::Rbxmx,
|
||||
Ok(value) if value == "2" => Middleware::JsonModel,
|
||||
_ => Middleware::Rbxm,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -20,6 +20,7 @@ pub struct SyncbackData<'sync> {
|
||||
pub(super) old_tree: &'sync RojoTree,
|
||||
pub(super) new_tree: &'sync WeakDom,
|
||||
pub(super) project: &'sync Project,
|
||||
pub(super) force_json: bool,
|
||||
}
|
||||
|
||||
pub struct SyncbackSnapshot<'sync> {
|
||||
@@ -31,6 +32,25 @@ pub struct SyncbackSnapshot<'sync> {
|
||||
}
|
||||
|
||||
impl<'sync> SyncbackSnapshot<'sync> {
|
||||
/// Computes the middleware and filesystem name for a child without
|
||||
/// creating a full snapshot. Uses the same logic as `with_joined_path`.
|
||||
pub fn child_middleware_and_name(
|
||||
&self,
|
||||
new_ref: Ref,
|
||||
old_ref: Option<Ref>,
|
||||
) -> anyhow::Result<(Middleware, String)> {
|
||||
let temp = Self {
|
||||
data: self.data,
|
||||
old: old_ref,
|
||||
new: new_ref,
|
||||
path: PathBuf::new(),
|
||||
middleware: None,
|
||||
};
|
||||
let middleware = get_best_middleware(&temp, self.data.force_json);
|
||||
let name = name_for_inst(middleware, temp.new_inst(), temp.old_inst())?;
|
||||
Ok((middleware, name.into_owned()))
|
||||
}
|
||||
|
||||
/// Constructs a SyncbackSnapshot from the provided refs
|
||||
/// while inheriting this snapshot's path and data. This should be used for
|
||||
/// directories.
|
||||
@@ -43,7 +63,7 @@ impl<'sync> SyncbackSnapshot<'sync> {
|
||||
path: PathBuf::new(),
|
||||
middleware: None,
|
||||
};
|
||||
let middleware = get_best_middleware(&snapshot);
|
||||
let middleware = get_best_middleware(&snapshot, self.data.force_json);
|
||||
let name = name_for_inst(middleware, snapshot.new_inst(), snapshot.old_inst())?;
|
||||
snapshot.path = self.path.join(name.as_ref());
|
||||
|
||||
@@ -69,7 +89,7 @@ impl<'sync> SyncbackSnapshot<'sync> {
|
||||
path: PathBuf::new(),
|
||||
middleware: None,
|
||||
};
|
||||
let middleware = get_best_middleware(&snapshot);
|
||||
let middleware = get_best_middleware(&snapshot, self.data.force_json);
|
||||
let name = name_for_inst(middleware, snapshot.new_inst(), snapshot.old_inst())?;
|
||||
snapshot.path = base_path.join(name.as_ref());
|
||||
|
||||
@@ -237,6 +257,25 @@ pub fn inst_path(dom: &WeakDom, referent: Ref) -> String {
|
||||
path.join("/")
|
||||
}
|
||||
|
||||
impl<'sync> SyncbackData<'sync> {
|
||||
/// Constructs a `SyncbackData` for use in unit tests.
|
||||
#[cfg(test)]
|
||||
pub fn for_test(
|
||||
vfs: &'sync Vfs,
|
||||
old_tree: &'sync RojoTree,
|
||||
new_tree: &'sync WeakDom,
|
||||
project: &'sync Project,
|
||||
) -> Self {
|
||||
Self {
|
||||
vfs,
|
||||
old_tree,
|
||||
new_tree,
|
||||
project,
|
||||
force_json: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod test {
|
||||
use rbx_dom_weak::{InstanceBuilder, WeakDom};
|
||||
|
||||
@@ -60,8 +60,8 @@ syncback_tests! {
|
||||
// Ensures that projects can be reserialized by syncback and that
|
||||
// default.project.json doesn't change unexpectedly.
|
||||
project_reserialize => ["attribute_mismatch.luau", "property_mismatch.project.json"],
|
||||
// Confirms that Instances that cannot serialize as directories serialize as rbxms
|
||||
rbxm_fallback => ["src/ChildWithDuplicates.rbxm"],
|
||||
// Confirms that duplicate children are resolved with incrementing suffixes
|
||||
rbxm_fallback => ["src/ChildWithDuplicates/DuplicateChild/.gitkeep", "src/ChildWithDuplicates/DuplicateChild1/.gitkeep"],
|
||||
// Ensures that ref properties are linked properly on the file system
|
||||
ref_properties => ["src/pointer.model.json", "src/target.model.json"],
|
||||
// Ensures that ref properties are linked when no attributes are manually
|
||||
|
||||
Reference in New Issue
Block a user