From 4564ea242fe446fc714f900981015ac34411a79e Mon Sep 17 00:00:00 2001 From: Joe Date: Sun, 15 Feb 2026 15:11:37 -0600 Subject: [PATCH] Enhance syncback functionality with sourcemap generation - Introduced a new function `write_sourcemap_from_syncback` to generate sourcemaps directly from in-memory data, improving performance by eliminating the need for file reads during sourcemap creation. - Updated the `syncback_loop` to return a `SyncbackResult` struct, encapsulating filesystem operations and instance paths for better post-processing. - Modified the `SyncbackCommand` to utilize the new sourcemap generation method, allowing for parallel execution of file writes and sourcemap creation. These changes streamline the syncback process and enhance the efficiency of sourcemap generation. --- src/cli/sourcemap.rs | 64 ++++++++++++++++++++++++++++++++ src/cli/syncback.rs | 88 +++++++++++++++++++++----------------------- src/lib.rs | 2 +- src/syncback/mod.rs | 36 ++++++++++++++++-- 4 files changed, 139 insertions(+), 51 deletions(-) diff --git a/src/cli/sourcemap.rs b/src/cli/sourcemap.rs index e6898e57..3c103dd7 100644 --- a/src/cli/sourcemap.rs +++ b/src/cli/sourcemap.rs @@ -276,6 +276,70 @@ pub(crate) fn write_sourcemap( Ok(()) } +/// Generates a sourcemap directly from a WeakDom and instance-to-path map, +/// without creating a ServeSession or re-reading the filesystem. +/// +/// Used by syncback to build the sourcemap from in-memory data in parallel +/// with file writes. +pub(crate) fn write_sourcemap_from_syncback( + dom: &rbx_dom_weak::WeakDom, + instance_paths: &std::collections::HashMap>, + project_dir: &Path, + output: &Path, +) -> anyhow::Result<()> { + let canonical_project_dir = + std::fs::canonicalize(project_dir).unwrap_or_else(|_| project_dir.to_path_buf()); + + let root_node = + recurse_create_node_from_dom(dom, dom.root_ref(), instance_paths, &canonical_project_dir); + + let json_output = serde_json::to_string(&root_node)?; + write_atomic(output, json_output.as_bytes())?; + + Ok(()) +} + +fn recurse_create_node_from_dom<'a>( + dom: &'a rbx_dom_weak::WeakDom, + referent: Ref, + instance_paths: &std::collections::HashMap>, + project_dir: &Path, +) -> Option> { + let instance = dom.get_by_ref(referent)?; + + let children: Vec<_> = instance + .children() + .iter() + .filter_map(|&child_ref| { + recurse_create_node_from_dom(dom, child_ref, instance_paths, project_dir) + }) + .collect(); + + if children.is_empty() && instance.class.as_str() == "DataModel" { + return None; + } + + let file_paths: Vec> = instance_paths + .get(&referent) + .map(|paths| { + paths + .iter() + .filter_map(|p| { + let canonical = std::fs::canonicalize(p).unwrap_or_else(|_| p.clone()); + pathdiff::diff_paths(&canonical, project_dir).map(Cow::Owned) + }) + .collect() + }) + .unwrap_or_default(); + + Some(SourcemapNode { + name: &instance.name, + class_name: instance.class, + file_paths, + children, + }) +} + /// Writes data to a file atomically by writing to a temporary file first, /// then renaming it to the target path. This ensures file watchers never /// see partial file contents. diff --git a/src/cli/syncback.rs b/src/cli/syncback.rs index a267be5a..775c46fb 100644 --- a/src/cli/syncback.rs +++ b/src/cli/syncback.rs @@ -22,11 +22,7 @@ use crate::{ syncback::{syncback_loop, FsSnapshot}, }; -use super::{ - resolve_path, - sourcemap::{filter_nothing, write_sourcemap}, - GlobalOptions, -}; +use super::{resolve_path, sourcemap::write_sourcemap_from_syncback, GlobalOptions}; const UNKNOWN_INPUT_KIND_ERR: &str = "Could not detect what kind of file was inputted. \ Expected input file to end in .rbxl, .rbxlx, .rbxm, or .rbxmx."; @@ -195,7 +191,7 @@ impl SyncbackCommand { } else { log::info!("Beginning syncback (clean mode)..."); } - let snapshot = syncback_loop( + let result = syncback_loop( session_old.vfs(), &mut dom_old, dom_new, @@ -209,19 +205,17 @@ impl SyncbackCommand { let base_path = session_old.root_project().folder_location(); if self.list { - list_files(&snapshot, global.color.into(), base_path)?; + list_files(&result.fs_snapshot, global.color.into(), base_path)?; } - // Drop dom_old early to release the mutex - we don't need it anymore - // and write_sourcemap needs to acquire the same lock drop(dom_old); if !self.dry_run { if self.interactive { eprintln!( "Would write {} files/folders and remove {} files/folders.", - snapshot.added_paths().len(), - snapshot.removed_paths().len() + result.fs_snapshot.added_paths().len(), + result.fs_snapshot.removed_paths().len() ); eprint!("Is this okay? (Y/N): "); io::stderr().flush()?; @@ -233,45 +227,45 @@ impl SyncbackCommand { return Ok(()); } } + log::info!("Writing to the file system..."); - snapshot.write_to_vfs_parallel(base_path, session_old.vfs())?; + let sourcemap_path = base_path.join("sourcemap.json"); + + // Run file writes and sourcemap generation in parallel. + // The sourcemap is built entirely from in-memory data (WeakDom + path map), + // so it doesn't need to wait for files to be on disk. + let (write_result, sourcemap_result) = std::thread::scope(|s| { + let write_handle = s.spawn(|| { + result + .fs_snapshot + .write_to_vfs_parallel(base_path, session_old.vfs()) + }); + + let sourcemap_handle = s.spawn(|| { + write_sourcemap_from_syncback( + &result.new_tree, + &result.instance_paths, + base_path, + &sourcemap_path, + ) + }); + + ( + write_handle.join().unwrap(), + sourcemap_handle.join().unwrap(), + ) + }); + + write_result?; log::info!( "Finished syncback: wrote {} files/folders, removed {}.", - snapshot.added_paths().len(), - snapshot.removed_paths().len() + result.fs_snapshot.added_paths().len(), + result.fs_snapshot.removed_paths().len() ); - // Generate sourcemap after successful syncback. - // We need a fresh session that reads the post-syncback filesystem state, - // since session_old contains the pre-syncback tree. - // - // Note: This may fail if clean mode removed directories that the project - // references. This is expected for projects that have meta-only directories - // (e.g., `$path` pointing to a folder with only init.meta.json5). In such - // cases, we log a warning but don't fail the syncback. - let sourcemap_path = base_path.join("sourcemap.json"); - let sourcemap_vfs = Vfs::new_oneshot(); - match ServeSession::new_oneshot(sourcemap_vfs, path_old.clone()) { - Ok(sourcemap_session) => { - write_sourcemap( - &sourcemap_session, - Some(&sourcemap_path), - filter_nothing, - false, - true, - )?; - log::info!("Generated sourcemap at {}", sourcemap_path.display()); - // Avoid expensive drop - forget(sourcemap_session); - } - Err(e) => { - log::warn!( - "Could not generate sourcemap after syncback: {}. \ - This can happen when clean mode removes directories that \ - only contain meta files.", - e - ); - } + match sourcemap_result { + Ok(()) => log::info!("Generated sourcemap at {}", sourcemap_path.display()), + Err(e) => log::warn!("Could not generate sourcemap: {}", e), } // Refresh git index if in a git repository @@ -291,8 +285,8 @@ impl SyncbackCommand { } else { log::info!( "Would write {} files/folders and remove {} files/folders.", - snapshot.added_paths().len(), - snapshot.removed_paths().len() + result.fs_snapshot.added_paths().len(), + result.fs_snapshot.removed_paths().len() ); log::info!("Aborting before writing to file system due to `--dry-run`"); } diff --git a/src/lib.rs b/src/lib.rs index 8a878581..b626f337 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -36,5 +36,5 @@ pub use snapshot::{ RojoDescendants, RojoTree, }; pub use snapshot_middleware::{snapshot_from_vfs, Middleware, ScriptType}; -pub use syncback::{syncback_loop, FsSnapshot, SyncbackData, SyncbackSnapshot}; +pub use syncback::{syncback_loop, FsSnapshot, SyncbackData, SyncbackResult, SyncbackSnapshot}; pub use web::interface as web_api; diff --git a/src/syncback/mod.rs b/src/syncback/mod.rs index 0ca5c123..69d16805 100644 --- a/src/syncback/mod.rs +++ b/src/syncback/mod.rs @@ -43,6 +43,19 @@ pub use property_filter::{ pub use snapshot::{inst_path, SyncbackData, SyncbackSnapshot}; pub use stats::SyncbackStats; +/// Result of a syncback operation, containing everything needed for +/// post-processing (file writes, sourcemap generation, etc.). +pub struct SyncbackResult { + /// Filesystem operations to perform (file writes, directory creation, removals). + pub fs_snapshot: FsSnapshot, + /// The new instance tree from the Roblox file, after pruning and filtering. + /// Returned so callers can build sourcemaps without re-reading from disk. + pub new_tree: WeakDom, + /// Maps each instance Ref (in `new_tree`) to the file paths written for it. + /// Used to generate sourcemaps from in-memory data. + pub instance_paths: HashMap>, +} + /// The name of an enviroment variable to use to override the behavior of /// syncback on model files. /// By default, syncback will use `Rbxm` for model files. @@ -81,7 +94,7 @@ pub fn syncback_loop( new_tree: WeakDom, project: &Project, incremental: bool, -) -> anyhow::Result { +) -> anyhow::Result { syncback_loop_with_stats(vfs, old_tree, new_tree, project, incremental, None) } @@ -94,7 +107,7 @@ pub fn syncback_loop_with_stats( project: &Project, incremental: bool, external_stats: Option<&SyncbackStats>, -) -> anyhow::Result { +) -> anyhow::Result { // Create internal stats if not provided externally let internal_stats = SyncbackStats::new(); let stats = external_stats.unwrap_or(&internal_stats); @@ -457,6 +470,7 @@ pub fn syncback_loop_with_stats( }]; let mut fs_snapshot = FsSnapshot::new(); + let mut instance_paths: HashMap> = HashMap::new(); 'syncback: while let Some(snapshot) = snapshots.pop() { let inst_path = snapshot.get_new_inst_path(snapshot.new); @@ -574,6 +588,18 @@ pub fn syncback_loop_with_stats( // TODO provide replacement snapshots for e.g. two way sync + // Collect file paths for this instance before merging into the main snapshot. + // This builds the instance-to-path map used for sourcemap generation. + let files_for_instance: Vec = syncback + .fs_snapshot + .added_files() + .into_iter() + .map(|p| p.to_path_buf()) + .collect(); + if !files_for_instance.is_empty() { + instance_paths.insert(snapshot.new, files_for_instance); + } + fs_snapshot.merge_with_filter(syncback.fs_snapshot, |path| { is_valid_path(&ignore_patterns, project_path, path) }); @@ -895,7 +921,11 @@ pub fn syncback_loop_with_stats( stats.log_summary(); } - Ok(fs_snapshot) + Ok(SyncbackResult { + fs_snapshot, + new_tree, + instance_paths, + }) } pub struct SyncbackReturn<'sync> {