Compare commits

..

30 Commits

Author SHA1 Message Date
93f808db28 Merge branch 'feature/init-name-resolution' 2026-02-26 14:30:58 +01:00
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
4fce3461f3 Merge branch 'feature/init-name-resolution' 2026-02-25 21:09: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
85bb5ec3f2 Merge branch 'feature/init-name-resolution' 2026-02-25 17:30:27 +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
667683d3b3 Merge branch 'feature/init-name-resolution' 2026-02-25 16:33:14 +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
d7a9ce55db Merge branch 'feature/init-name-resolution' 2026-02-24 21:54:31 +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
996113b177 Merge branch 'feature/init-name-resolution' 2026-02-24 01:06:22 +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
4ca26efccb Merge branch 'fix/git-since-live-sync' 2026-02-13 18:13:42 +01:00
ce0db54e0a Merge branch 'feature/dangerously-force-json' 2026-02-13 18:13:37 +01:00
c552fdc52e Add --dangerously-force-json flag for syncback
Adds a CLI flag that forces syncback to use JSON representations
instead of binary .rbxm files. Instances with children become
directories with init.meta.json; leaf instances become .model.json.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-13 17:41:42 +01:00
ari
a6e9939d6c Merge branch 'master' into name-prop 2026-01-20 01:10:20 +01:00
5957368c04 Remove redundant code
Can't remember why I added this one
2026-01-20 01:08:59 +01:00
78916c8a63 Revert 2 semantic changes 2026-01-20 00:59:34 +01:00
791ccfcfd1 Remove addition of 'Actor' to json_model_classes 2026-01-20 00:55:03 +01:00
3500ebe02a Update CHANGELOG.md 2026-01-20 00:54:18 +01:00
0e1364945f Avoid clone in src/syncback/file_names.rs 2026-01-12 14:41:12 +01:00
ari
3a6aae65f7 Avoid clone in src/syncback/file_names.rs
Co-authored-by: krakow10 <krakow20@gmail.com>
2026-01-12 14:35:46 +01:00
ari
d13d229eef Avoid clone in src/snapshot_middleware/json_model.rs
Co-authored-by: krakow10 <krakow20@gmail.com>
2026-01-12 14:35:18 +01:00
ari
9a485d88ce Avoid clone in src/snapshot_middleware/lua.rs
Co-authored-by: krakow10 <krakow20@gmail.com>
2026-01-12 14:35:06 +01:00
020d72faef fix: improve middleware selection for actor and other container classes 2025-12-18 05:10:53 +01:00
60d150f4c6 feat: optimize name handling for leaf scripts with invalid names
Prefer slugified filenames + adjacent meta files for scripts without children instead of forcing directory creation
2025-12-18 04:43:47 +01:00
73dab330b5 test: remove oudated json_model_legacy_name test 2025-12-15 20:32:28 +01:00
790312a5b0 fix: lack of .model.json support 2025-12-15 20:26:25 +01:00
5c396322d9 fix: name prop not properly syncing 2025-12-15 19:08:18 +01:00
37e44e474a feat: support name property in meta and model jsons 2025-12-15 18:45:59 +01:00
14 changed files with 584 additions and 164 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
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

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

@@ -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!",

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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