Compare commits

...

6 Commits

Author SHA1 Message Date
110b9f0df3 feat: resolve duplicate sibling names with incrementing suffixes
Instead of bailing when children have duplicate filesystem names,
syncback now resolves collisions by appending incrementing suffixes
(e.g. Foo, Foo1, Foo2). This handles both init-renamed children and
any other name collisions. Meta stem derivation is now path-based
to correctly handle collision suffixes and dotted names.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-26 14:30:46 +01:00
917d17a738 fix: simplify meta stem to instance name + slugify + init-prefix
Drop the strip_suffix(extension) approach for computing adjacent meta
file names. Instead, use the instance name directly (slugified if it
has invalid filesystem chars, prefixed with '_' if it's "init"). This
is the same logic as the original code plus init-prefix handling, and
correctly preserves dots in instance names like "Name.new".

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 21:09:45 +01:00
14bbdaf560 fix: handle dotted names and .lua extension in meta path + name check
Two bugs:

1. Meta stem fallback used raw instance name (unslugged), so names with
   forbidden chars like '/' would create bogus directory components in
   the meta path. Fix: fallback now slugifies + init-prefixes, matching
   name_for_inst.

2. AdjacentMetadata name check used split('.').next() to extract the
   filesystem stem, breaking dotted names like "Name.new" (stem became
   "Name", mismatched the instance name, wrote an unnecessary name
   property). Fix: check the conditions that cause name_for_inst to
   diverge (invalid chars or init-prefix) directly instead of comparing
   path stems.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 17:30:21 +01:00
5b1b5db06c fix: derive adjacent meta stem from snapshot path, not instance name
The previous fix used split('.').next() to get the meta stem from the
snapshot path, which only takes the first dot-segment. This broke names
containing dots (e.g. "Name.new" → "Name.new.luau" would produce
"Name.meta.json" instead of "Name.new.meta.json").

Strip the full middleware extension (e.g. ".server.luau", ".txt") from
the snapshot path filename instead. This correctly handles all cases:
  Name.new.luau      → Name.new  → Name.new.meta.json
  _Init.server.luau  → _Init     → _Init.meta.json
  Name.new.txt       → Name.new  → Name.new.meta.json

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-25 16:33:09 +01:00
33dd0f5ed1 fix: derive adjacent meta path from snapshot path, not instance name
When a script/txt/csv child is renamed by name_for_inst (e.g. "Init" →
"_Init.luau"), the adjacent meta file must follow the same name. All
three callers were using the Roblox instance name to construct the meta
path, producing "Init.meta.json" instead of "_Init.meta.json" — which
collides with the parent directory's "init.meta.json" on
case-insensitive file systems.

Fix by deriving the meta stem from the first dot-segment of the
snapshot path file name, which already holds the resolved name.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-24 21:53:53 +01:00
95fe993de3 feat: auto-resolve init-name conflicts during syncback
When a child instance has a Roblox name that would produce a filesystem
name of "init" (case-insensitive), syncback now automatically prefixes
it with '_' (e.g. "Init" → "_Init.luau") instead of erroring. The
corresponding meta.json writes the original name via the `name` property
so Rojo can restore it on the next snapshot.

The sibling dedup check is updated to use actual on-disk names for
existing children and the resolved (init-prefixed) name for new ones,
so genuine collisions still error while false positives from the `name`
property are avoided.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-24 01:05:31 +01:00
13 changed files with 551 additions and 152 deletions

View File

@@ -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

View File

@@ -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

View File

@@ -0,0 +1,6 @@
---
source: tests/tests/syncback.rs
assertion_line: 31
expression: src/ChildWithDuplicates/DuplicateChild1/.gitkeep
---

View File

@@ -0,0 +1,6 @@
---
source: tests/tests/syncback.rs
assertion_line: 31
expression: src/ChildWithDuplicates/DuplicateChild/.gitkeep
---

View File

@@ -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")?,
) )
} }

View File

@@ -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"]);
}
} }

View File

@@ -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")?,
); );
} }

View File

@@ -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

View File

@@ -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")?,
); );
} }

View File

@@ -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}"))
}
} }
}) })
} }

View File

@@ -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};

View File

@@ -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};

View File

@@ -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