Compare commits

..

1 Commits

Author SHA1 Message Date
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
59 changed files with 273 additions and 1098 deletions

3
.gitignore vendored
View File

@@ -23,6 +23,3 @@
# Macos file system junk
._*
.DS_STORE
# JetBrains IDEs
/.idea/

View File

@@ -31,14 +31,10 @@ Making a new release? Simply add the new header with the version and date undern
## Unreleased
* Fixed a bug caused by having reference properties (such as `ObjectValue.Value`) that point to an Instance not included in syncback. ([#1179])
* Implemented support for the "name" property in meta/model JSON files. ([#1187])
* Fixed instance replacement fallback failing when too many instances needed to be replaced. ([#1192])
* Fixed a bug where MacOS paths weren't being handled correctly. ([#1201])
[#1179]: https://github.com/rojo-rbx/rojo/pull/1179
[#1187]: https://github.com/rojo-rbx/rojo/pull/1187
[#1192]: https://github.com/rojo-rbx/rojo/pull/1192
[#1201]: https://github.com/rojo-rbx/rojo/pull/1201
## [7.7.0-rc.1] (November 27th, 2025)

1
Cargo.lock generated
View File

@@ -1319,7 +1319,6 @@ dependencies = [
"fs-err",
"notify",
"serde",
"tempfile",
]
[[package]]

View File

@@ -1,7 +1,6 @@
# memofs Changelog
## Unreleased Changes
* Added `Vfs::canonicalize`. [#1201]
## 0.3.1 (2025-11-27)
* Added `Vfs::exists`. [#1169]

View File

@@ -19,6 +19,3 @@ crossbeam-channel = "0.5.12"
fs-err = "2.11.0"
notify = "4.0.17"
serde = { version = "1.0.197", features = ["derive"] }
[dev-dependencies]
tempfile = "3.10.1"

View File

@@ -232,33 +232,6 @@ impl VfsBackend for InMemoryFs {
}
}
// TODO: We rely on Rojo to prepend cwd to any relative path before storing paths
// in MemoFS. The current implementation will error if no prepended absolute path
// is found. It really only normalizes paths within the provided path's context.
// Example: "/Users/username/project/../other/file.txt" ->
// "/Users/username/other/file.txt"
// Erroneous example: "/Users/../../other/file.txt" -> "/other/file.txt"
// This is not very robust. We should implement proper path normalization here or otherwise
// warn if we are missing context and can not fully canonicalize the path correctly.
fn canonicalize(&mut self, path: &Path) -> io::Result<PathBuf> {
let mut normalized = PathBuf::new();
for component in path.components() {
match component {
std::path::Component::ParentDir => {
normalized.pop();
}
std::path::Component::CurDir => {}
_ => normalized.push(component),
}
}
let inner = self.inner.lock().unwrap();
match inner.entries.get(&normalized) {
Some(_) => Ok(normalized),
None => not_found(&normalized),
}
}
fn event_receiver(&self) -> crossbeam_channel::Receiver<VfsEvent> {
let inner = self.inner.lock().unwrap();

View File

@@ -77,7 +77,6 @@ pub trait VfsBackend: sealed::Sealed + Send + 'static {
fn metadata(&mut self, path: &Path) -> io::Result<Metadata>;
fn remove_file(&mut self, path: &Path) -> io::Result<()>;
fn remove_dir_all(&mut self, path: &Path) -> io::Result<()>;
fn canonicalize(&mut self, path: &Path) -> io::Result<PathBuf>;
fn event_receiver(&self) -> crossbeam_channel::Receiver<VfsEvent>;
fn watch(&mut self, path: &Path) -> io::Result<()>;
@@ -226,11 +225,6 @@ impl VfsInner {
self.backend.metadata(path)
}
fn canonicalize<P: AsRef<Path>>(&mut self, path: P) -> io::Result<PathBuf> {
let path = path.as_ref();
self.backend.canonicalize(path)
}
fn event_receiver(&self) -> crossbeam_channel::Receiver<VfsEvent> {
self.backend.event_receiver()
}
@@ -419,19 +413,6 @@ impl Vfs {
self.inner.lock().unwrap().metadata(path)
}
/// Normalize a path via the underlying backend.
///
/// Roughly equivalent to [`std::fs::canonicalize`][std::fs::canonicalize]. Relative paths are
/// resolved against the backend's current working directory (if applicable) and errors are
/// surfaced directly from the backend.
///
/// [std::fs::canonicalize]: https://doc.rust-lang.org/stable/std/fs/fn.canonicalize.html
#[inline]
pub fn canonicalize<P: AsRef<Path>>(&self, path: P) -> io::Result<PathBuf> {
let path = path.as_ref();
self.inner.lock().unwrap().canonicalize(path)
}
/// Retrieve a handle to the event receiver for this `Vfs`.
#[inline]
pub fn event_receiver(&self) -> crossbeam_channel::Receiver<VfsEvent> {
@@ -559,13 +540,6 @@ impl VfsLock<'_> {
self.inner.metadata(path)
}
/// Normalize a path via the underlying backend.
#[inline]
pub fn normalize<P: AsRef<Path>>(&mut self, path: P) -> io::Result<PathBuf> {
let path = path.as_ref();
self.inner.canonicalize(path)
}
/// Retrieve a handle to the event receiver for this `Vfs`.
#[inline]
pub fn event_receiver(&self) -> crossbeam_channel::Receiver<VfsEvent> {
@@ -581,9 +555,7 @@ impl VfsLock<'_> {
#[cfg(test)]
mod test {
use crate::{InMemoryFs, StdBackend, Vfs, VfsSnapshot};
use std::io;
use std::path::PathBuf;
use crate::{InMemoryFs, Vfs, VfsSnapshot};
/// https://github.com/rojo-rbx/rojo/issues/899
#[test]
@@ -599,62 +571,4 @@ mod test {
"bar\nfoo\n\n"
);
}
/// https://github.com/rojo-rbx/rojo/issues/1200
#[test]
fn canonicalize_in_memory_success() {
let mut imfs = InMemoryFs::new();
let contents = "Lorem ipsum dolor sit amet.".to_string();
imfs.load_snapshot("/test/file.txt", VfsSnapshot::file(contents.to_string()))
.unwrap();
let vfs = Vfs::new(imfs);
assert_eq!(
vfs.canonicalize("/test/nested/../file.txt").unwrap(),
PathBuf::from("/test/file.txt")
);
assert_eq!(
vfs.read_to_string(vfs.canonicalize("/test/nested/../file.txt").unwrap())
.unwrap()
.to_string(),
contents.to_string()
);
}
#[test]
fn canonicalize_in_memory_missing_errors() {
let imfs = InMemoryFs::new();
let vfs = Vfs::new(imfs);
let err = vfs.canonicalize("test").unwrap_err();
assert_eq!(err.kind(), io::ErrorKind::NotFound);
}
#[test]
fn canonicalize_std_backend_success() {
let contents = "Lorem ipsum dolor sit amet.".to_string();
let dir = tempfile::tempdir().unwrap();
let file_path = dir.path().join("file.txt");
fs_err::write(&file_path, contents.to_string()).unwrap();
let vfs = Vfs::new(StdBackend::new());
let canonicalized = vfs.canonicalize(&file_path).unwrap();
assert_eq!(canonicalized, file_path.canonicalize().unwrap());
assert_eq!(
vfs.read_to_string(&canonicalized).unwrap().to_string(),
contents.to_string()
);
}
#[test]
fn canonicalize_std_backend_missing_errors() {
let dir = tempfile::tempdir().unwrap();
let file_path = dir.path().join("test");
let vfs = Vfs::new(StdBackend::new());
let err = vfs.canonicalize(&file_path).unwrap_err();
assert_eq!(err.kind(), io::ErrorKind::NotFound);
}
}

View File

@@ -1,5 +1,5 @@
use std::io;
use std::path::{Path, PathBuf};
use std::path::Path;
use crate::{Metadata, ReadDir, VfsBackend, VfsEvent};
@@ -50,10 +50,6 @@ impl VfsBackend for NoopBackend {
Err(io::Error::other("NoopBackend doesn't do anything"))
}
fn canonicalize(&mut self, _path: &Path) -> io::Result<PathBuf> {
Err(io::Error::other("NoopBackend doesn't do anything"))
}
fn event_receiver(&self) -> crossbeam_channel::Receiver<VfsEvent> {
crossbeam_channel::never()
}

View File

@@ -106,10 +106,6 @@ impl VfsBackend for StdBackend {
})
}
fn canonicalize(&mut self, path: &Path) -> io::Result<PathBuf> {
fs_err::canonicalize(path)
}
fn event_receiver(&self) -> crossbeam_channel::Receiver<VfsEvent> {
self.watcher_receiver.clone()
}

View File

@@ -0,0 +1,16 @@
---
source: tests/tests/build.rs
expression: contents
---
<roblox version="4">
<Item class="Folder" referent="0">
<Properties>
<string name="Name">json_model_legacy_name</string>
</Properties>
<Item class="Folder" referent="1">
<Properties>
<string name="Name">Expected Name</string>
</Properties>
</Item>
</Item>
</roblox>

View File

@@ -1,23 +0,0 @@
---
source: tests/tests/build.rs
assertion_line: 109
expression: contents
---
<roblox version="4">
<Item class="DataModel" referent="0">
<Properties>
<string name="Name">model_json_name_input</string>
</Properties>
<Item class="Workspace" referent="1">
<Properties>
<string name="Name">Workspace</string>
<bool name="NeedsPivotMigration">false</bool>
</Properties>
<Item class="StringValue" referent="2">
<Properties>
<string name="Name">/Bar</string>
</Properties>
</Item>
</Item>
</Item>
</roblox>

View File

@@ -1,20 +0,0 @@
---
source: tests/tests/build.rs
assertion_line: 108
expression: contents
---
<roblox version="4">
<Item class="Folder" referent="0">
<Properties>
<string name="Name">slugified_name_roundtrip</string>
</Properties>
<Item class="Script" referent="1">
<Properties>
<string name="Name">/Script</string>
<token name="RunContext">0</token>
<string name="Source"><![CDATA[print("Hello world!")
]]></string>
</Properties>
</Item>
</Item>
</roblox>

View File

@@ -0,0 +1,6 @@
{
"name": "json_model_legacy_name",
"tree": {
"$path": "folder"
}
}

View File

@@ -0,0 +1,4 @@
{
"Name": "Overridden Name",
"ClassName": "Folder"
}

View File

@@ -1,11 +0,0 @@
{
"name": "model_json_name_input",
"tree": {
"$className": "DataModel",
"Workspace": {
"$className": "Workspace",
"$path": "src"
}
}
}

View File

@@ -1,5 +0,0 @@
{
"name": "/Bar",
"className": "StringValue"
}

View File

@@ -1,4 +0,0 @@
{
"name": "/Script"
}

View File

@@ -1,2 +0,0 @@
print("Hello world!")

View File

@@ -1,6 +0,0 @@
{
"name": "slugified_name_roundtrip",
"tree": {
"$path": "src"
}
}

View File

@@ -1,3 +0,0 @@
{
"name": "/Script"
}

View File

@@ -1 +0,0 @@
print("Hello world!")

View File

@@ -1,6 +0,0 @@
---
source: tests/rojo_test/syncback_util.rs
assertion_line: 101
expression: "String::from_utf8_lossy(&output.stdout)"
---

View File

@@ -0,0 +1,73 @@
---
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,12 +1,9 @@
---
source: tests/rojo_test/syncback_util.rs
assertion_line: 101
expression: "String::from_utf8_lossy(&output.stdout)"
---
Writing src/ChildWithDuplicates/DuplicateChild/.gitkeep
Writing src/ChildWithDuplicates/DuplicateChild1/.gitkeep
Writing src/ChildWithDuplicates.rbxm
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

@@ -1,13 +0,0 @@
---
source: tests/rojo_test/syncback_util.rs
assertion_line: 101
expression: "String::from_utf8_lossy(&output.stdout)"
---
Writing default.project.json
Writing src/Camera.rbxm
Writing src/Terrain.rbxm
Writing src/_Folder/init.meta.json
Writing src/_Script.meta.json
Writing src/_Script.server.luau
Writing src
Writing src/_Folder

View File

@@ -1,9 +0,0 @@
---
source: tests/tests/syncback.rs
assertion_line: 31
expression: src/foo.model.json
---
{
"name": "/Bar",
"className": "StringValue"
}

View File

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

View File

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

View File

@@ -1,8 +0,0 @@
---
source: tests/tests/syncback.rs
assertion_line: 31
expression: src/_Folder.model.json
---
{
"className": "Folder"
}

View File

@@ -1,8 +0,0 @@
---
source: tests/tests/syncback.rs
assertion_line: 31
expression: src/_Folder/init.meta.json
---
{
"name": "/Folder"
}

View File

@@ -1,8 +0,0 @@
---
source: tests/tests/syncback.rs
assertion_line: 31
expression: src/_Script.meta.json
---
{
"name": "/Script"
}

View File

@@ -1,6 +0,0 @@
---
source: tests/tests/syncback.rs
assertion_line: 31
expression: src/_Script.server.luau
---
print("Hello world!")

View File

@@ -1,8 +0,0 @@
---
source: tests/tests/syncback.rs
assertion_line: 31
expression: src/_Script/init.meta.json
---
{
"name": "/Script"
}

View File

@@ -1,6 +0,0 @@
---
source: tests/tests/syncback.rs
assertion_line: 31
expression: src/_Script/init.server.luau
---
print("Hello world!")

View File

@@ -1,11 +0,0 @@
{
"name": "model_json_name",
"tree": {
"$className": "DataModel",
"Workspace": {
"$className": "Workspace",
"$path": "src"
}
}
}

View File

@@ -1,5 +0,0 @@
{
"name": "/Bar",
"className": "StringValue"
}

View File

@@ -1,10 +0,0 @@
{
"name": "slugified_name",
"tree": {
"$className": "DataModel",
"Workspace": {
"$className": "Workspace",
"$path": "src"
}
}
}

View File

@@ -1,13 +1,13 @@
use crossbeam_channel::{select, Receiver, RecvError, Sender};
use jod_thread::JoinHandle;
use memofs::{IoResultExt, Vfs, VfsEvent};
use rbx_dom_weak::types::{Ref, Variant};
use std::path::PathBuf;
use std::{
fs,
sync::{Arc, Mutex},
};
use crossbeam_channel::{select, Receiver, RecvError, Sender};
use jod_thread::JoinHandle;
use memofs::{IoResultExt, Vfs, VfsEvent};
use rbx_dom_weak::types::{Ref, Variant};
use crate::{
message_queue::MessageQueue,
snapshot::{
@@ -114,49 +114,6 @@ struct JobThreadContext {
}
impl JobThreadContext {
/// Computes and applies patches to the DOM for a given file path.
///
/// This function finds the nearest ancestor to the given path that has associated instances
/// in the tree.
/// It then computes and applies changes for each affected instance ID and
/// returns a vector of applied patch sets.
fn apply_patches(&self, path: PathBuf) -> Vec<AppliedPatchSet> {
let mut tree = self.tree.lock().unwrap();
let mut applied_patches = Vec::new();
// Find the nearest ancestor to this path that has
// associated instances in the tree. This helps make sure
// that we handle additions correctly, especially if we
// receive events for descendants of a large tree being
// created all at once.
let mut current_path = path.as_path();
let affected_ids = loop {
let ids = tree.get_ids_at_path(current_path);
log::trace!("Path {} affects IDs {:?}", current_path.display(), ids);
if !ids.is_empty() {
break ids.to_vec();
}
log::trace!("Trying parent path...");
match current_path.parent() {
Some(parent) => current_path = parent,
None => break Vec::new(),
}
};
for id in affected_ids {
if let Some(patch) = compute_and_apply_changes(&mut tree, &self.vfs, id) {
if !patch.is_empty() {
applied_patches.push(patch);
}
}
}
applied_patches
}
fn handle_vfs_event(&self, event: VfsEvent) {
log::trace!("Vfs event: {:?}", event);
@@ -168,16 +125,41 @@ impl JobThreadContext {
// For a given VFS event, we might have many changes to different parts
// of the tree. Calculate and apply all of these changes.
let applied_patches = match event {
VfsEvent::Create(path) | VfsEvent::Write(path) => {
self.apply_patches(self.vfs.canonicalize(&path).unwrap())
}
VfsEvent::Remove(path) => {
// MemoFS does not track parent removals yet, so we can canonicalize
// the parent path safely and then append the removed path's file name.
let parent = path.parent().unwrap();
let file_name = path.file_name().unwrap();
let parent_normalized = self.vfs.canonicalize(parent).unwrap();
self.apply_patches(parent_normalized.join(file_name))
VfsEvent::Create(path) | VfsEvent::Remove(path) | VfsEvent::Write(path) => {
let mut tree = self.tree.lock().unwrap();
let mut applied_patches = Vec::new();
// Find the nearest ancestor to this path that has
// associated instances in the tree. This helps make sure
// that we handle additions correctly, especially if we
// receive events for descendants of a large tree being
// created all at once.
let mut current_path = path.as_path();
let affected_ids = loop {
let ids = tree.get_ids_at_path(current_path);
log::trace!("Path {} affects IDs {:?}", current_path.display(), ids);
if !ids.is_empty() {
break ids.to_vec();
}
log::trace!("Trying parent path...");
match current_path.parent() {
Some(parent) => current_path = parent,
None => break Vec::new(),
}
};
for id in affected_ids {
if let Some(patch) = compute_and_apply_changes(&mut tree, &self.vfs, id) {
if !patch.is_empty() {
applied_patches.push(patch);
}
}
}
applied_patches
}
_ => {
log::warn!("Unhandled VFS event: {:?}", event);

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

@@ -70,12 +70,6 @@ pub struct InstanceMetadata {
/// A schema provided via a JSON file, if one exists. Will be `None` for
/// all non-JSON middleware.
pub schema: Option<String>,
/// A custom name specified via meta.json or model.json files. If present,
/// this name will be used for the instance while the filesystem name will
/// be slugified to remove illegal characters.
#[serde(skip_serializing_if = "Option::is_none")]
pub specified_name: Option<String>,
}
impl InstanceMetadata {
@@ -88,7 +82,6 @@ impl InstanceMetadata {
specified_id: None,
middleware: None,
schema: None,
specified_name: None,
}
}
@@ -137,13 +130,6 @@ impl InstanceMetadata {
pub fn schema(self, schema: Option<String>) -> Self {
Self { schema, ..self }
}
pub fn specified_name(self, specified_name: Option<String>) -> Self {
Self {
specified_name,
..self
}
}
}
impl Default for InstanceMetadata {

View File

@@ -42,7 +42,7 @@ pub fn snapshot_csv(
.metadata(
InstanceMetadata::new()
.instigating_source(path)
.relevant_paths(vec![vfs.canonicalize(path)?]),
.relevant_paths(vec![path.to_path_buf()]),
);
AdjacentMetadata::read_and_apply_all(vfs, path, name, &mut snapshot)?;
@@ -109,14 +109,8 @@ 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_stem}.meta.json")),
parent.join(format!("{}.meta.json", new_inst.name)),
serde_json::to_vec_pretty(&meta).context("cannot serialize metadata")?,
)
}

View File

@@ -8,13 +8,10 @@ use memofs::{DirEntry, Vfs};
use crate::{
snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource},
syncback::{
extension_for_middleware, hash_instance, FsSnapshot, SyncbackReturn,
SyncbackSnapshot,
},
syncback::{hash_instance, FsSnapshot, SyncbackReturn, SyncbackSnapshot},
};
use super::{meta_file::DirectoryMetadata, snapshot_from_vfs, Middleware};
use super::{meta_file::DirectoryMetadata, snapshot_from_vfs};
const EMPTY_DIR_KEEP_NAME: &str = ".gitkeep";
@@ -65,19 +62,18 @@ pub fn snapshot_dir_no_meta(
}
}
let normalized_path = vfs.canonicalize(path)?;
let relevant_paths = vec![
normalized_path.clone(),
path.to_path_buf(),
// TODO: We shouldn't need to know about Lua existing in this
// middleware. Should we figure out a way for that function to add
// relevant paths to this middleware?
normalized_path.join("init.lua"),
normalized_path.join("init.luau"),
normalized_path.join("init.server.lua"),
normalized_path.join("init.server.luau"),
normalized_path.join("init.client.lua"),
normalized_path.join("init.client.luau"),
normalized_path.join("init.csv"),
path.join("init.lua"),
path.join("init.luau"),
path.join("init.server.lua"),
path.join("init.server.luau"),
path.join("init.client.lua"),
path.join("init.client.luau"),
path.join("init.csv"),
];
let snapshot = InstanceSnapshot::new()
@@ -94,22 +90,6 @@ 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>> {
@@ -153,128 +133,65 @@ pub fn syncback_dir_no_meta<'sync>(
let mut children = Vec::new();
let mut removed_children = Vec::new();
// Build the old child map early so it can be used for deduplication below.
let mut old_child_map = HashMap::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");
}
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);
}
}
// --- 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;
}
}
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}")
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 {
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;
// The child only exists in the the new dom
children.push(snapshot.with_joined_path(*new_child_ref, None)?);
}
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() {
// 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 mut fs_snapshot = FsSnapshot::new();
@@ -307,12 +224,6 @@ 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]
@@ -349,302 +260,4 @@ 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

@@ -32,7 +32,7 @@ pub fn snapshot_json(
.metadata(
InstanceMetadata::new()
.instigating_source(path)
.relevant_paths(vec![vfs.canonicalize(path)?])
.relevant_paths(vec![path.to_path_buf()])
.context(context),
);

View File

@@ -35,14 +35,20 @@ pub fn snapshot_json_model(
format!("File is not a valid JSON model: {}", path.display())
})?;
// If the JSON has a name property, preserve it in metadata for syncback
let specified_name = instance.name.clone();
if let Some(top_level_name) = &instance.name {
let new_name = format!("{}.model.json", top_level_name);
// Use the name from JSON if present, otherwise fall back to filename-derived name
if instance.name.is_none() {
instance.name = Some(name.to_owned());
log::warn!(
"Model at path {} had a top-level Name field. \
This field has been ignored since Rojo 6.0.\n\
Consider removing this field and renaming the file to {}.",
new_name,
path.display()
);
}
instance.name = Some(name.to_owned());
let id = instance.id.take().map(RojoRef::new);
let schema = instance.schema.take();
@@ -53,11 +59,10 @@ pub fn snapshot_json_model(
snapshot.metadata = snapshot
.metadata
.instigating_source(path)
.relevant_paths(vec![vfs.canonicalize(path)?])
.relevant_paths(vec![path.to_path_buf()])
.context(context)
.specified_id(id)
.schema(schema)
.specified_name(specified_name);
.schema(schema);
Ok(Some(snapshot))
}
@@ -76,7 +81,6 @@ pub fn syncback_json_model<'sync>(
// schemas will ever exist in one project for it to matter, but it
// could have a performance cost.
model.schema = old_inst.metadata().schema.clone();
model.name = old_inst.metadata().specified_name.clone();
}
Ok(SyncbackReturn {

View File

@@ -88,7 +88,7 @@ pub fn snapshot_lua(
.metadata(
InstanceMetadata::new()
.instigating_source(path)
.relevant_paths(vec![vfs.canonicalize(path)?])
.relevant_paths(vec![path.to_path_buf()])
.context(context),
);
@@ -158,23 +158,8 @@ pub fn syncback_lua<'sync>(
if !meta.is_empty() {
let parent_location = 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(".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_stem}.meta.json")),
parent_location.join(format!("{}.meta.json", new_inst.name)),
serde_json::to_vec_pretty(&meta).context("cannot serialize metadata")?,
);
}

View File

@@ -10,10 +10,7 @@ use rbx_dom_weak::{
use serde::{Deserialize, Serialize};
use crate::{
json,
resolution::UnresolvedValue,
snapshot::InstanceSnapshot,
syncback::{validate_file_name, SyncbackSnapshot},
json, resolution::UnresolvedValue, snapshot::InstanceSnapshot, syncback::SyncbackSnapshot,
RojoRef,
};
@@ -39,9 +36,6 @@ pub struct AdjacentMetadata {
#[serde(default, skip_serializing_if = "IndexMap::is_empty")]
pub attributes: IndexMap<String, UnresolvedValue>,
#[serde(skip_serializing_if = "Option::is_none")]
pub name: Option<String>,
#[serde(skip)]
pub path: PathBuf,
}
@@ -150,26 +144,6 @@ impl AdjacentMetadata {
}
}
let name = snapshot
.old_inst()
.and_then(|inst| inst.metadata().specified_name.clone())
.or_else(|| {
// 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()
|| instance_name.to_lowercase() == "init"
{
Some(instance_name.clone())
} else {
None
}
} else {
None
}
});
Ok(Some(Self {
ignore_unknown_instances: if ignore_unknown_instances {
Some(true)
@@ -181,7 +155,6 @@ impl AdjacentMetadata {
path,
id: None,
schema,
name,
}))
}
@@ -240,26 +213,11 @@ impl AdjacentMetadata {
Ok(())
}
fn apply_name(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
if self.name.is_some() && snapshot.metadata.specified_name.is_some() {
anyhow::bail!(
"cannot specify a name using {} (instance has a name from somewhere else)",
self.path.display()
);
}
if let Some(name) = &self.name {
snapshot.name = name.clone().into();
}
snapshot.metadata.specified_name = self.name.take();
Ok(())
}
pub fn apply_all(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
self.apply_ignore_unknown_instances(snapshot);
self.apply_properties(snapshot)?;
self.apply_id(snapshot)?;
self.apply_schema(snapshot)?;
self.apply_name(snapshot)?;
Ok(())
}
@@ -268,13 +226,11 @@ impl AdjacentMetadata {
///
/// - The number of properties and attributes is 0
/// - `ignore_unknown_instances` is None
/// - `name` is None
#[inline]
pub fn is_empty(&self) -> bool {
self.attributes.is_empty()
&& self.properties.is_empty()
&& self.ignore_unknown_instances.is_none()
&& self.name.is_none()
}
// TODO: Add method to allow selectively applying parts of metadata and
@@ -306,9 +262,6 @@ pub struct DirectoryMetadata {
#[serde(skip_serializing_if = "Option::is_none")]
pub class_name: Option<Ustr>,
#[serde(skip_serializing_if = "Option::is_none")]
pub name: Option<String>,
#[serde(skip)]
pub path: PathBuf,
}
@@ -419,26 +372,6 @@ impl DirectoryMetadata {
}
}
let name = snapshot
.old_inst()
.and_then(|inst| inst.metadata().specified_name.clone())
.or_else(|| {
// 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()
|| instance_name.to_lowercase() == "init"
{
Some(instance_name.clone())
} else {
None
}
} else {
None
}
});
Ok(Some(Self {
ignore_unknown_instances: if ignore_unknown_instances {
Some(true)
@@ -451,7 +384,6 @@ impl DirectoryMetadata {
path,
id: None,
schema,
name,
}))
}
@@ -461,7 +393,6 @@ impl DirectoryMetadata {
self.apply_properties(snapshot)?;
self.apply_id(snapshot)?;
self.apply_schema(snapshot)?;
self.apply_name(snapshot)?;
Ok(())
}
@@ -533,33 +464,17 @@ impl DirectoryMetadata {
snapshot.metadata.schema = self.schema.take();
Ok(())
}
fn apply_name(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
if self.name.is_some() && snapshot.metadata.specified_name.is_some() {
anyhow::bail!(
"cannot specify a name using {} (instance has a name from somewhere else)",
self.path.display()
);
}
if let Some(name) = &self.name {
snapshot.name = name.clone().into();
}
snapshot.metadata.specified_name = self.name.take();
Ok(())
}
/// Returns whether the metadata is 'empty', meaning it doesn't have anything
/// worth persisting in it. Specifically:
///
/// - The number of properties and attributes is 0
/// - `ignore_unknown_instances` is None
/// - `class_name` is either None or not Some("Folder")
/// - `name` is None
#[inline]
pub fn is_empty(&self) -> bool {
self.attributes.is_empty()
&& self.properties.is_empty()
&& self.ignore_unknown_instances.is_none()
&& self.name.is_none()
&& if let Some(class) = &self.class_name {
class == "Folder"
} else {

View File

@@ -28,7 +28,7 @@ pub fn snapshot_rbxm(
.metadata(
InstanceMetadata::new()
.instigating_source(path)
.relevant_paths(vec![vfs.canonicalize(path)?])
.relevant_paths(vec![path.to_path_buf()])
.context(context),
);

View File

@@ -31,7 +31,7 @@ pub fn snapshot_rbxmx(
.metadata(
InstanceMetadata::new()
.instigating_source(path)
.relevant_paths(vec![vfs.canonicalize(path)?])
.relevant_paths(vec![path.to_path_buf()])
.context(context),
);

View File

@@ -31,7 +31,7 @@ pub fn snapshot_toml(
.metadata(
InstanceMetadata::new()
.instigating_source(path)
.relevant_paths(vec![vfs.canonicalize(path)?])
.relevant_paths(vec![path.to_path_buf()])
.context(context),
);

View File

@@ -28,7 +28,7 @@ pub fn snapshot_txt(
.metadata(
InstanceMetadata::new()
.instigating_source(path)
.relevant_paths(vec![vfs.canonicalize(path)?])
.relevant_paths(vec![path.to_path_buf()])
.context(context),
);
@@ -58,14 +58,8 @@ 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_stem}.meta.json")),
parent.join(format!("{}.meta.json", new_inst.name)),
serde_json::to_vec_pretty(&meta).context("could not serialize metadata")?,
);
}

View File

@@ -37,7 +37,7 @@ pub fn snapshot_yaml(
.metadata(
InstanceMetadata::new()
.instigating_source(path)
.relevant_paths(vec![vfs.canonicalize(path)?])
.relevant_paths(vec![path.to_path_buf()])
.context(context),
);

View File

@@ -8,11 +8,11 @@ use rbx_dom_weak::Instance;
use crate::{snapshot::InstanceWithMeta, snapshot_middleware::Middleware};
pub fn name_for_inst<'a>(
pub fn name_for_inst<'old>(
middleware: Middleware,
new_inst: &'a Instance,
old_inst: Option<InstanceWithMeta<'a>>,
) -> anyhow::Result<Cow<'a, str>> {
new_inst: &Instance,
old_inst: Option<InstanceWithMeta<'old>>,
) -> anyhow::Result<Cow<'old, str>> {
if let Some(old_inst) = old_inst {
if let Some(source) = old_inst.metadata().relevant_paths.first() {
source
@@ -35,34 +35,14 @@ pub fn name_for_inst<'a>(
| Middleware::CsvDir
| Middleware::ServerScriptDir
| Middleware::ClientScriptDir
| Middleware::ModuleScriptDir => {
let name = if validate_file_name(&new_inst.name).is_err() {
Cow::Owned(slugify_name(&new_inst.name))
} else {
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
}
}
| Middleware::ModuleScriptDir => Cow::Owned(new_inst.name.clone()),
_ => {
let extension = extension_for_middleware(middleware);
let slugified;
let stem: &str = if validate_file_name(&new_inst.name).is_err() {
slugified = slugify_name(&new_inst.name);
&slugified
} else {
&new_inst.name
};
// 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}"))
}
let name = &new_inst.name;
validate_file_name(name).with_context(|| {
format!("name '{name}' is not legal to write to the file system")
})?;
Cow::Owned(format!("{name}.{extension}"))
}
})
}
@@ -114,39 +94,6 @@ const INVALID_WINDOWS_NAMES: [&str; 22] = [
/// in a file's name.
const FORBIDDEN_CHARS: [char; 9] = ['<', '>', ':', '"', '/', '|', '?', '*', '\\'];
/// Slugifies a name by replacing forbidden characters with underscores
/// and ensuring the result is a valid file name
pub fn slugify_name(name: &str) -> String {
let mut result = String::with_capacity(name.len());
for ch in name.chars() {
if FORBIDDEN_CHARS.contains(&ch) {
result.push('_');
} else {
result.push(ch);
}
}
// Handle Windows reserved names by appending an underscore
let result_lower = result.to_lowercase();
for forbidden in INVALID_WINDOWS_NAMES {
if result_lower == forbidden.to_lowercase() {
result.push('_');
break;
}
}
while result.ends_with(' ') || result.ends_with('.') {
result.pop();
}
if result.is_empty() || result.chars().all(|c| c == '_') {
result = "instance".to_string();
}
result
}
/// Validates a provided file name to ensure it's allowed on the file system. An
/// error is returned if the name isn't allowed, indicating why.
/// This takes into account rules for Windows, MacOS, and Linux.

View File

@@ -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();
@@ -361,10 +367,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,25 +32,6 @@ 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.
@@ -62,7 +44,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());
@@ -88,7 +70,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());
@@ -256,25 +238,6 @@ 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

@@ -41,6 +41,7 @@ gen_build_tests! {
issue_546,
json_as_lua,
json_model_in_folder,
json_model_legacy_name,
module_in_folder,
module_init,
nested_runcontext,
@@ -54,8 +55,6 @@ gen_build_tests! {
script_meta_disabled,
server_in_folder,
server_init,
slugified_name_roundtrip,
model_json_name_input,
txt,
txt_in_folder,
unresolved_values,

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 duplicate children are resolved with incrementing suffixes
rbxm_fallback => ["src/ChildWithDuplicates/DuplicateChild/.gitkeep", "src/ChildWithDuplicates/DuplicateChild1/.gitkeep"],
// Confirms that Instances that cannot serialize as directories serialize as rbxms
rbxm_fallback => ["src/ChildWithDuplicates.rbxm"],
// 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
@@ -86,9 +86,4 @@ syncback_tests! {
sync_rules => ["src/module.modulescript", "src/text.text"],
// Ensures that the `syncUnscriptable` setting works
unscriptable_properties => ["default.project.json"],
// Ensures that instances with names containing illegal characters get slugified filenames
// and preserve their original names in meta.json without forcing directories for leaf scripts
slugified_name => ["src/_Script.meta.json", "src/_Script.server.luau", "src/_Folder/init.meta.json"],
// Ensures that .model.json files preserve the name property
model_json_name => ["src/foo.model.json"],
}