forked from rojo-rbx/rojo
Compare commits
6 Commits
name-prop
...
feature/in
| Author | SHA1 | Date | |
|---|---|---|---|
|
110b9f0df3
|
|||
|
917d17a738
|
|||
|
14bbdaf560
|
|||
|
5b1b5db06c
|
|||
|
33dd0f5ed1
|
|||
|
95fe993de3
|
@@ -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
|
source: tests/rojo_test/syncback_util.rs
|
||||||
|
assertion_line: 101
|
||||||
expression: "String::from_utf8_lossy(&output.stdout)"
|
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/ChildWithoutDuplicates/Child/.gitkeep
|
||||||
|
Writing src/ChildWithDuplicates/DuplicateChild
|
||||||
|
Writing src/ChildWithDuplicates/DuplicateChild1
|
||||||
Writing src/ChildWithoutDuplicates
|
Writing src/ChildWithoutDuplicates
|
||||||
Writing src/ChildWithoutDuplicates/Child
|
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
|
||||||
|
---
|
||||||
|
|
||||||
@@ -109,8 +109,14 @@ pub fn syncback_csv<'sync>(
|
|||||||
|
|
||||||
if !meta.is_empty() {
|
if !meta.is_empty() {
|
||||||
let parent = snapshot.path.parent_err()?;
|
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(
|
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")?,
|
serde_json::to_vec_pretty(&meta).context("cannot serialize metadata")?,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -8,10 +8,13 @@ use memofs::{DirEntry, Vfs};
|
|||||||
|
|
||||||
use crate::{
|
use crate::{
|
||||||
snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource},
|
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";
|
const EMPTY_DIR_KEEP_NAME: &str = ".gitkeep";
|
||||||
|
|
||||||
@@ -91,6 +94,22 @@ pub fn snapshot_dir_no_meta(
|
|||||||
Ok(Some(snapshot))
|
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>(
|
pub fn syncback_dir<'sync>(
|
||||||
snapshot: &SyncbackSnapshot<'sync>,
|
snapshot: &SyncbackSnapshot<'sync>,
|
||||||
) -> anyhow::Result<SyncbackReturn<'sync>> {
|
) -> anyhow::Result<SyncbackReturn<'sync>> {
|
||||||
@@ -134,65 +153,128 @@ pub fn syncback_dir_no_meta<'sync>(
|
|||||||
let mut children = Vec::new();
|
let mut children = Vec::new();
|
||||||
let mut removed_children = Vec::new();
|
let mut removed_children = Vec::new();
|
||||||
|
|
||||||
// We have to enforce unique child names for the file system.
|
// Build the old child map early so it can be used for deduplication below.
|
||||||
let mut child_names = HashSet::with_capacity(new_inst.children().len());
|
let mut old_child_map = HashMap::new();
|
||||||
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");
|
|
||||||
}
|
|
||||||
|
|
||||||
if let Some(old_inst) = snapshot.old_inst() {
|
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() {
|
for child in old_inst.children() {
|
||||||
let inst = snapshot.get_old_instance(*child).unwrap();
|
let inst = snapshot.get_old_instance(*child).unwrap();
|
||||||
old_child_map.insert(inst.name(), inst);
|
old_child_map.insert(inst.name(), inst);
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
for new_child_ref in new_inst.children() {
|
// --- Two-pass collision resolution ---
|
||||||
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()) {
|
// Pass 1: Collect each child's base filesystem name and old ref, applying
|
||||||
if old_child.metadata().relevant_paths.is_empty() {
|
// skip conditions. Track which names are used (lowercased) so we can
|
||||||
log::debug!(
|
// detect collisions.
|
||||||
"Skipping instance {} because it doesn't exist on the disk",
|
struct ChildEntry {
|
||||||
old_child.name()
|
new_ref: rbx_dom_weak::types::Ref,
|
||||||
);
|
old_ref: Option<rbx_dom_weak::types::Ref>,
|
||||||
continue;
|
base_name: String,
|
||||||
} else if matches!(
|
middleware: Middleware,
|
||||||
old_child.metadata().instigating_source,
|
skip: bool,
|
||||||
Some(InstigatingSource::ProjectNode { .. })
|
}
|
||||||
) {
|
|
||||||
log::debug!(
|
let mut entries = Vec::with_capacity(new_inst.children().len());
|
||||||
"Skipping instance {} because it originates in a project file",
|
let mut used_names: HashSet<String> = HashSet::with_capacity(new_inst.children().len());
|
||||||
old_child.name()
|
let mut collision_indices: Vec<usize> = Vec::new();
|
||||||
);
|
|
||||||
continue;
|
for new_child_ref in new_inst.children() {
|
||||||
}
|
let new_child = snapshot.get_new_instance(*new_child_ref).unwrap();
|
||||||
// This child exists in both doms. Pass it on.
|
|
||||||
children.push(snapshot.with_joined_path(*new_child_ref, Some(old_child.id()))?);
|
// Determine old_ref and apply skip conditions.
|
||||||
} else {
|
let old_child = if snapshot.old_inst().is_some() {
|
||||||
// The child only exists in the the new dom
|
old_child_map.remove(new_child.name.as_str())
|
||||||
children.push(snapshot.with_joined_path(*new_child_ref, None)?);
|
} 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());
|
let old_ref = old_child.as_ref().map(|o| o.id());
|
||||||
} else {
|
|
||||||
// There is no old instance. Just add every child.
|
if skip {
|
||||||
for new_child_ref in new_inst.children() {
|
entries.push(ChildEntry {
|
||||||
children.push(snapshot.with_joined_path(*new_child_ref, None)?);
|
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();
|
let mut fs_snapshot = FsSnapshot::new();
|
||||||
|
|
||||||
@@ -225,6 +307,12 @@ pub fn syncback_dir_no_meta<'sync>(
|
|||||||
mod test {
|
mod test {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
|
use std::path::PathBuf;
|
||||||
|
|
||||||
|
use crate::{
|
||||||
|
snapshot::{InstanceMetadata, InstanceSnapshot},
|
||||||
|
Project, RojoTree, SyncbackData, SyncbackSnapshot,
|
||||||
|
};
|
||||||
use memofs::{InMemoryFs, VfsSnapshot};
|
use memofs::{InMemoryFs, VfsSnapshot};
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
@@ -261,4 +349,302 @@ mod test {
|
|||||||
|
|
||||||
insta::assert_yaml_snapshot!(instance_snapshot);
|
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() {
|
if !meta.is_empty() {
|
||||||
let parent_location = snapshot.path.parent_err()?;
|
let parent_location = snapshot.path.parent_err()?;
|
||||||
let instance_name = &snapshot.new_inst().name;
|
let file_name = snapshot
|
||||||
let slugified;
|
.path
|
||||||
let meta_name = if crate::syncback::validate_file_name(instance_name).is_err() {
|
.file_name()
|
||||||
slugified = crate::syncback::slugify_name(instance_name);
|
.and_then(|n| n.to_str())
|
||||||
&slugified
|
.unwrap_or("");
|
||||||
} else {
|
let meta_stem = file_name
|
||||||
instance_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(
|
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")?,
|
serde_json::to_vec_pretty(&meta).context("cannot serialize metadata")?,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -154,11 +154,13 @@ impl AdjacentMetadata {
|
|||||||
.old_inst()
|
.old_inst()
|
||||||
.and_then(|inst| inst.metadata().specified_name.clone())
|
.and_then(|inst| inst.metadata().specified_name.clone())
|
||||||
.or_else(|| {
|
.or_else(|| {
|
||||||
// If this is a new instance and its name is invalid for the filesystem,
|
// Write name when name_for_inst would produce a different
|
||||||
// we need to specify the name in meta.json so it can be preserved
|
// filesystem stem (slugification or init-prefix).
|
||||||
if snapshot.old_inst().is_none() {
|
if snapshot.old_inst().is_none() {
|
||||||
let instance_name = &snapshot.new_inst().name;
|
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())
|
Some(instance_name.clone())
|
||||||
} else {
|
} else {
|
||||||
None
|
None
|
||||||
@@ -421,11 +423,13 @@ impl DirectoryMetadata {
|
|||||||
.old_inst()
|
.old_inst()
|
||||||
.and_then(|inst| inst.metadata().specified_name.clone())
|
.and_then(|inst| inst.metadata().specified_name.clone())
|
||||||
.or_else(|| {
|
.or_else(|| {
|
||||||
// If this is a new instance and its name is invalid for the filesystem,
|
// Write name when name_for_inst would produce a different
|
||||||
// we need to specify the name in meta.json so it can be preserved
|
// directory name (slugification or init-prefix).
|
||||||
if snapshot.old_inst().is_none() {
|
if snapshot.old_inst().is_none() {
|
||||||
let instance_name = &snapshot.new_inst().name;
|
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())
|
Some(instance_name.clone())
|
||||||
} else {
|
} else {
|
||||||
None
|
None
|
||||||
|
|||||||
@@ -58,8 +58,14 @@ pub fn syncback_txt<'sync>(
|
|||||||
|
|
||||||
if !meta.is_empty() {
|
if !meta.is_empty() {
|
||||||
let parent = snapshot.path.parent_err()?;
|
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(
|
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")?,
|
serde_json::to_vec_pretty(&meta).context("could not serialize metadata")?,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -36,23 +36,33 @@ pub fn name_for_inst<'a>(
|
|||||||
| Middleware::ServerScriptDir
|
| Middleware::ServerScriptDir
|
||||||
| Middleware::ClientScriptDir
|
| Middleware::ClientScriptDir
|
||||||
| Middleware::ModuleScriptDir => {
|
| 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))
|
Cow::Owned(slugify_name(&new_inst.name))
|
||||||
} else {
|
} 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 extension = extension_for_middleware(middleware);
|
||||||
let slugified;
|
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 = slugify_name(&new_inst.name);
|
||||||
&slugified
|
&slugified
|
||||||
} else {
|
} else {
|
||||||
&new_inst.name
|
&new_inst.name
|
||||||
};
|
};
|
||||||
|
// Prefix "init" stems to avoid colliding with reserved init files.
|
||||||
Cow::Owned(format!("{final_name}.{extension}"))
|
if stem.to_lowercase() == "init" {
|
||||||
|
Cow::Owned(format!("_{stem}.{extension}"))
|
||||||
|
} else {
|
||||||
|
Cow::Owned(format!("{stem}.{extension}"))
|
||||||
|
}
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -28,7 +28,7 @@ use crate::{
|
|||||||
Project,
|
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 fs_snapshot::FsSnapshot;
|
||||||
pub use hash::*;
|
pub use hash::*;
|
||||||
pub use property_filter::{filter_properties, filter_properties_preallocated};
|
pub use property_filter::{filter_properties, filter_properties_preallocated};
|
||||||
|
|||||||
@@ -31,6 +31,25 @@ pub struct SyncbackSnapshot<'sync> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl<'sync> 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
|
/// Constructs a SyncbackSnapshot from the provided refs
|
||||||
/// while inheriting this snapshot's path and data. This should be used for
|
/// while inheriting this snapshot's path and data. This should be used for
|
||||||
/// directories.
|
/// directories.
|
||||||
@@ -237,6 +256,25 @@ pub fn inst_path(dom: &WeakDom, referent: Ref) -> String {
|
|||||||
path.join("/")
|
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)]
|
#[cfg(test)]
|
||||||
mod test {
|
mod test {
|
||||||
use rbx_dom_weak::{InstanceBuilder, WeakDom};
|
use rbx_dom_weak::{InstanceBuilder, WeakDom};
|
||||||
|
|||||||
@@ -60,8 +60,8 @@ syncback_tests! {
|
|||||||
// Ensures that projects can be reserialized by syncback and that
|
// Ensures that projects can be reserialized by syncback and that
|
||||||
// default.project.json doesn't change unexpectedly.
|
// default.project.json doesn't change unexpectedly.
|
||||||
project_reserialize => ["attribute_mismatch.luau", "property_mismatch.project.json"],
|
project_reserialize => ["attribute_mismatch.luau", "property_mismatch.project.json"],
|
||||||
// Confirms that Instances that cannot serialize as directories serialize as rbxms
|
// Confirms that duplicate children are resolved with incrementing suffixes
|
||||||
rbxm_fallback => ["src/ChildWithDuplicates.rbxm"],
|
rbxm_fallback => ["src/ChildWithDuplicates/DuplicateChild/.gitkeep", "src/ChildWithDuplicates/DuplicateChild1/.gitkeep"],
|
||||||
// Ensures that ref properties are linked properly on the file system
|
// Ensures that ref properties are linked properly on the file system
|
||||||
ref_properties => ["src/pointer.model.json", "src/target.model.json"],
|
ref_properties => ["src/pointer.model.json", "src/target.model.json"],
|
||||||
// Ensures that ref properties are linked when no attributes are manually
|
// Ensures that ref properties are linked when no attributes are manually
|
||||||
|
|||||||
Reference in New Issue
Block a user