Compare commits

...

16 Commits

Author SHA1 Message Date
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
Ivan Matthew
2a1102fc55 Implement VFS Path normalization for improved cross-platform tree synchronization (#1201) 2026-01-19 15:04:59 -08: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
53 changed files with 558 additions and 111 deletions

3
.gitignore vendored
View File

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

View File

@@ -31,10 +31,14 @@ Making a new release? Simply add the new header with the version and date undern
## Unreleased ## Unreleased
* Fixed a bug caused by having reference properties (such as `ObjectValue.Value`) that point to an Instance not included in syncback. ([#1179]) * 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 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 [#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 [#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) ## [7.7.0-rc.1] (November 27th, 2025)

1
Cargo.lock generated
View File

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

View File

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

View File

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

View File

@@ -232,6 +232,33 @@ 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> { fn event_receiver(&self) -> crossbeam_channel::Receiver<VfsEvent> {
let inner = self.inner.lock().unwrap(); let inner = self.inner.lock().unwrap();

View File

@@ -77,6 +77,7 @@ pub trait VfsBackend: sealed::Sealed + Send + 'static {
fn metadata(&mut self, path: &Path) -> io::Result<Metadata>; fn metadata(&mut self, path: &Path) -> io::Result<Metadata>;
fn remove_file(&mut self, path: &Path) -> io::Result<()>; fn remove_file(&mut self, path: &Path) -> io::Result<()>;
fn remove_dir_all(&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 event_receiver(&self) -> crossbeam_channel::Receiver<VfsEvent>;
fn watch(&mut self, path: &Path) -> io::Result<()>; fn watch(&mut self, path: &Path) -> io::Result<()>;
@@ -225,6 +226,11 @@ impl VfsInner {
self.backend.metadata(path) 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> { fn event_receiver(&self) -> crossbeam_channel::Receiver<VfsEvent> {
self.backend.event_receiver() self.backend.event_receiver()
} }
@@ -413,6 +419,19 @@ impl Vfs {
self.inner.lock().unwrap().metadata(path) 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`. /// Retrieve a handle to the event receiver for this `Vfs`.
#[inline] #[inline]
pub fn event_receiver(&self) -> crossbeam_channel::Receiver<VfsEvent> { pub fn event_receiver(&self) -> crossbeam_channel::Receiver<VfsEvent> {
@@ -540,6 +559,13 @@ impl VfsLock<'_> {
self.inner.metadata(path) 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`. /// Retrieve a handle to the event receiver for this `Vfs`.
#[inline] #[inline]
pub fn event_receiver(&self) -> crossbeam_channel::Receiver<VfsEvent> { pub fn event_receiver(&self) -> crossbeam_channel::Receiver<VfsEvent> {
@@ -555,7 +581,9 @@ impl VfsLock<'_> {
#[cfg(test)] #[cfg(test)]
mod test { mod test {
use crate::{InMemoryFs, Vfs, VfsSnapshot}; use crate::{InMemoryFs, StdBackend, Vfs, VfsSnapshot};
use std::io;
use std::path::PathBuf;
/// https://github.com/rojo-rbx/rojo/issues/899 /// https://github.com/rojo-rbx/rojo/issues/899
#[test] #[test]
@@ -571,4 +599,62 @@ mod test {
"bar\nfoo\n\n" "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::io;
use std::path::Path; use std::path::{Path, PathBuf};
use crate::{Metadata, ReadDir, VfsBackend, VfsEvent}; use crate::{Metadata, ReadDir, VfsBackend, VfsEvent};
@@ -50,6 +50,10 @@ impl VfsBackend for NoopBackend {
Err(io::Error::other("NoopBackend doesn't do anything")) 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> { fn event_receiver(&self) -> crossbeam_channel::Receiver<VfsEvent> {
crossbeam_channel::never() crossbeam_channel::never()
} }

View File

@@ -106,6 +106,10 @@ 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> { fn event_receiver(&self) -> crossbeam_channel::Receiver<VfsEvent> {
self.watcher_receiver.clone() self.watcher_receiver.clone()
} }

View File

@@ -1,16 +0,0 @@
---
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

@@ -0,0 +1,23 @@
---
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

@@ -0,0 +1,20 @@
---
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

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@@ -0,0 +1,13 @@
---
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

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

Binary file not shown.

View File

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

Binary file not shown.

View File

@@ -1,12 +1,12 @@
use std::{
fs,
sync::{Arc, Mutex},
};
use crossbeam_channel::{select, Receiver, RecvError, Sender}; use crossbeam_channel::{select, Receiver, RecvError, Sender};
use jod_thread::JoinHandle; use jod_thread::JoinHandle;
use memofs::{IoResultExt, Vfs, VfsEvent}; use memofs::{IoResultExt, Vfs, VfsEvent};
use rbx_dom_weak::types::{Ref, Variant}; use rbx_dom_weak::types::{Ref, Variant};
use std::path::PathBuf;
use std::{
fs,
sync::{Arc, Mutex},
};
use crate::{ use crate::{
message_queue::MessageQueue, message_queue::MessageQueue,
@@ -114,6 +114,49 @@ struct JobThreadContext {
} }
impl 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) { fn handle_vfs_event(&self, event: VfsEvent) {
log::trace!("Vfs event: {:?}", event); log::trace!("Vfs event: {:?}", event);
@@ -125,41 +168,16 @@ impl JobThreadContext {
// For a given VFS event, we might have many changes to different parts // For a given VFS event, we might have many changes to different parts
// of the tree. Calculate and apply all of these changes. // of the tree. Calculate and apply all of these changes.
let applied_patches = match event { let applied_patches = match event {
VfsEvent::Create(path) | VfsEvent::Remove(path) | VfsEvent::Write(path) => { VfsEvent::Create(path) | VfsEvent::Write(path) => {
let mut tree = self.tree.lock().unwrap(); self.apply_patches(self.vfs.canonicalize(&path).unwrap())
let mut applied_patches = Vec::new(); }
VfsEvent::Remove(path) => {
// Find the nearest ancestor to this path that has // MemoFS does not track parent removals yet, so we can canonicalize
// associated instances in the tree. This helps make sure // the parent path safely and then append the removed path's file name.
// that we handle additions correctly, especially if we let parent = path.parent().unwrap();
// receive events for descendants of a large tree being let file_name = path.file_name().unwrap();
// created all at once. let parent_normalized = self.vfs.canonicalize(parent).unwrap();
let mut current_path = path.as_path(); self.apply_patches(parent_normalized.join(file_name))
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); log::warn!("Unhandled VFS event: {:?}", event);

View File

@@ -70,6 +70,12 @@ pub struct InstanceMetadata {
/// A schema provided via a JSON file, if one exists. Will be `None` for /// A schema provided via a JSON file, if one exists. Will be `None` for
/// all non-JSON middleware. /// all non-JSON middleware.
pub schema: Option<String>, 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 { impl InstanceMetadata {
@@ -82,6 +88,7 @@ impl InstanceMetadata {
specified_id: None, specified_id: None,
middleware: None, middleware: None,
schema: None, schema: None,
specified_name: None,
} }
} }
@@ -130,6 +137,13 @@ impl InstanceMetadata {
pub fn schema(self, schema: Option<String>) -> Self { pub fn schema(self, schema: Option<String>) -> Self {
Self { schema, ..self } Self { schema, ..self }
} }
pub fn specified_name(self, specified_name: Option<String>) -> Self {
Self {
specified_name,
..self
}
}
} }
impl Default for InstanceMetadata { impl Default for InstanceMetadata {

View File

@@ -42,7 +42,7 @@ pub fn snapshot_csv(
.metadata( .metadata(
InstanceMetadata::new() InstanceMetadata::new()
.instigating_source(path) .instigating_source(path)
.relevant_paths(vec![path.to_path_buf()]), .relevant_paths(vec![vfs.canonicalize(path)?]),
); );
AdjacentMetadata::read_and_apply_all(vfs, path, name, &mut snapshot)?; AdjacentMetadata::read_and_apply_all(vfs, path, name, &mut snapshot)?;

View File

@@ -62,18 +62,19 @@ pub fn snapshot_dir_no_meta(
} }
} }
let normalized_path = vfs.canonicalize(path)?;
let relevant_paths = vec![ let relevant_paths = vec![
path.to_path_buf(), normalized_path.clone(),
// TODO: We shouldn't need to know about Lua existing in this // TODO: We shouldn't need to know about Lua existing in this
// middleware. Should we figure out a way for that function to add // middleware. Should we figure out a way for that function to add
// relevant paths to this middleware? // relevant paths to this middleware?
path.join("init.lua"), normalized_path.join("init.lua"),
path.join("init.luau"), normalized_path.join("init.luau"),
path.join("init.server.lua"), normalized_path.join("init.server.lua"),
path.join("init.server.luau"), normalized_path.join("init.server.luau"),
path.join("init.client.lua"), normalized_path.join("init.client.lua"),
path.join("init.client.luau"), normalized_path.join("init.client.luau"),
path.join("init.csv"), normalized_path.join("init.csv"),
]; ];
let snapshot = InstanceSnapshot::new() let snapshot = InstanceSnapshot::new()

View File

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

View File

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

View File

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

View File

@@ -10,7 +10,10 @@ use rbx_dom_weak::{
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use crate::{ use crate::{
json, resolution::UnresolvedValue, snapshot::InstanceSnapshot, syncback::SyncbackSnapshot, json,
resolution::UnresolvedValue,
snapshot::InstanceSnapshot,
syncback::{validate_file_name, SyncbackSnapshot},
RojoRef, RojoRef,
}; };
@@ -36,6 +39,9 @@ pub struct AdjacentMetadata {
#[serde(default, skip_serializing_if = "IndexMap::is_empty")] #[serde(default, skip_serializing_if = "IndexMap::is_empty")]
pub attributes: IndexMap<String, UnresolvedValue>, pub attributes: IndexMap<String, UnresolvedValue>,
#[serde(skip_serializing_if = "Option::is_none")]
pub name: Option<String>,
#[serde(skip)] #[serde(skip)]
pub path: PathBuf, pub path: PathBuf,
} }
@@ -144,6 +150,24 @@ impl AdjacentMetadata {
} }
} }
let name = snapshot
.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
if snapshot.old_inst().is_none() {
let instance_name = &snapshot.new_inst().name;
if validate_file_name(instance_name).is_err() {
Some(instance_name.clone())
} else {
None
}
} else {
None
}
});
Ok(Some(Self { Ok(Some(Self {
ignore_unknown_instances: if ignore_unknown_instances { ignore_unknown_instances: if ignore_unknown_instances {
Some(true) Some(true)
@@ -155,6 +179,7 @@ impl AdjacentMetadata {
path, path,
id: None, id: None,
schema, schema,
name,
})) }))
} }
@@ -213,11 +238,26 @@ impl AdjacentMetadata {
Ok(()) 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<()> { pub fn apply_all(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
self.apply_ignore_unknown_instances(snapshot); self.apply_ignore_unknown_instances(snapshot);
self.apply_properties(snapshot)?; self.apply_properties(snapshot)?;
self.apply_id(snapshot)?; self.apply_id(snapshot)?;
self.apply_schema(snapshot)?; self.apply_schema(snapshot)?;
self.apply_name(snapshot)?;
Ok(()) Ok(())
} }
@@ -226,11 +266,13 @@ impl AdjacentMetadata {
/// ///
/// - The number of properties and attributes is 0 /// - The number of properties and attributes is 0
/// - `ignore_unknown_instances` is None /// - `ignore_unknown_instances` is None
/// - `name` is None
#[inline] #[inline]
pub fn is_empty(&self) -> bool { pub fn is_empty(&self) -> bool {
self.attributes.is_empty() self.attributes.is_empty()
&& self.properties.is_empty() && self.properties.is_empty()
&& self.ignore_unknown_instances.is_none() && self.ignore_unknown_instances.is_none()
&& self.name.is_none()
} }
// TODO: Add method to allow selectively applying parts of metadata and // TODO: Add method to allow selectively applying parts of metadata and
@@ -262,6 +304,9 @@ pub struct DirectoryMetadata {
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
pub class_name: Option<Ustr>, pub class_name: Option<Ustr>,
#[serde(skip_serializing_if = "Option::is_none")]
pub name: Option<String>,
#[serde(skip)] #[serde(skip)]
pub path: PathBuf, pub path: PathBuf,
} }
@@ -372,6 +417,24 @@ impl DirectoryMetadata {
} }
} }
let name = snapshot
.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
if snapshot.old_inst().is_none() {
let instance_name = &snapshot.new_inst().name;
if validate_file_name(instance_name).is_err() {
Some(instance_name.clone())
} else {
None
}
} else {
None
}
});
Ok(Some(Self { Ok(Some(Self {
ignore_unknown_instances: if ignore_unknown_instances { ignore_unknown_instances: if ignore_unknown_instances {
Some(true) Some(true)
@@ -384,6 +447,7 @@ impl DirectoryMetadata {
path, path,
id: None, id: None,
schema, schema,
name,
})) }))
} }
@@ -393,6 +457,7 @@ impl DirectoryMetadata {
self.apply_properties(snapshot)?; self.apply_properties(snapshot)?;
self.apply_id(snapshot)?; self.apply_id(snapshot)?;
self.apply_schema(snapshot)?; self.apply_schema(snapshot)?;
self.apply_name(snapshot)?;
Ok(()) Ok(())
} }
@@ -464,17 +529,33 @@ impl DirectoryMetadata {
snapshot.metadata.schema = self.schema.take(); snapshot.metadata.schema = self.schema.take();
Ok(()) 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 /// Returns whether the metadata is 'empty', meaning it doesn't have anything
/// worth persisting in it. Specifically: /// worth persisting in it. Specifically:
/// ///
/// - The number of properties and attributes is 0 /// - The number of properties and attributes is 0
/// - `ignore_unknown_instances` is None /// - `ignore_unknown_instances` is None
/// - `class_name` is either None or not Some("Folder") /// - `class_name` is either None or not Some("Folder")
/// - `name` is None
#[inline] #[inline]
pub fn is_empty(&self) -> bool { pub fn is_empty(&self) -> bool {
self.attributes.is_empty() self.attributes.is_empty()
&& self.properties.is_empty() && self.properties.is_empty()
&& self.ignore_unknown_instances.is_none() && self.ignore_unknown_instances.is_none()
&& self.name.is_none()
&& if let Some(class) = &self.class_name { && if let Some(class) = &self.class_name {
class == "Folder" class == "Folder"
} else { } else {

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@@ -8,11 +8,11 @@ use rbx_dom_weak::Instance;
use crate::{snapshot::InstanceWithMeta, snapshot_middleware::Middleware}; use crate::{snapshot::InstanceWithMeta, snapshot_middleware::Middleware};
pub fn name_for_inst<'old>( pub fn name_for_inst<'a>(
middleware: Middleware, middleware: Middleware,
new_inst: &Instance, new_inst: &'a Instance,
old_inst: Option<InstanceWithMeta<'old>>, old_inst: Option<InstanceWithMeta<'a>>,
) -> anyhow::Result<Cow<'old, str>> { ) -> anyhow::Result<Cow<'a, str>> {
if let Some(old_inst) = old_inst { if let Some(old_inst) = old_inst {
if let Some(source) = old_inst.metadata().relevant_paths.first() { if let Some(source) = old_inst.metadata().relevant_paths.first() {
source source
@@ -35,14 +35,24 @@ pub fn name_for_inst<'old>(
| Middleware::CsvDir | Middleware::CsvDir
| Middleware::ServerScriptDir | Middleware::ServerScriptDir
| Middleware::ClientScriptDir | Middleware::ClientScriptDir
| Middleware::ModuleScriptDir => Cow::Owned(new_inst.name.clone()), | Middleware::ModuleScriptDir => {
if validate_file_name(&new_inst.name).is_err() {
Cow::Owned(slugify_name(&new_inst.name))
} else {
Cow::Borrowed(&new_inst.name)
}
}
_ => { _ => {
let extension = extension_for_middleware(middleware); let extension = extension_for_middleware(middleware);
let name = &new_inst.name; let slugified;
validate_file_name(name).with_context(|| { let final_name = if validate_file_name(&new_inst.name).is_err() {
format!("name '{name}' is not legal to write to the file system") slugified = slugify_name(&new_inst.name);
})?; &slugified
Cow::Owned(format!("{name}.{extension}")) } else {
&new_inst.name
};
Cow::Owned(format!("{final_name}.{extension}"))
} }
}) })
} }
@@ -94,6 +104,39 @@ const INVALID_WINDOWS_NAMES: [&str; 22] = [
/// in a file's name. /// in a file's name.
const FORBIDDEN_CHARS: [char; 9] = ['<', '>', ':', '"', '/', '|', '?', '*', '\\']; 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 /// 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. /// error is returned if the name isn't allowed, indicating why.
/// This takes into account rules for Windows, MacOS, and Linux. /// This takes into account rules for Windows, MacOS, and Linux.

View File

@@ -28,7 +28,7 @@ use crate::{
Project, Project,
}; };
pub use file_names::{extension_for_middleware, name_for_inst, validate_file_name}; pub use file_names::{extension_for_middleware, name_for_inst, slugify_name, 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

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

View File

@@ -86,4 +86,9 @@ syncback_tests! {
sync_rules => ["src/module.modulescript", "src/text.text"], sync_rules => ["src/module.modulescript", "src/text.text"],
// Ensures that the `syncUnscriptable` setting works // Ensures that the `syncUnscriptable` setting works
unscriptable_properties => ["default.project.json"], 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"],
} }