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.
This commit is contained in:
Joe 2026-02-15 15:11:37 -06:00
parent 484b190464
commit 4564ea242f
4 changed files with 139 additions and 51 deletions

View file

@ -276,6 +276,70 @@ pub(crate) fn write_sourcemap(
Ok(()) 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<Ref, Vec<PathBuf>>,
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<Ref, Vec<PathBuf>>,
project_dir: &Path,
) -> Option<SourcemapNode<'a>> {
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<Cow<'a, Path>> = 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, /// 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 /// then renaming it to the target path. This ensures file watchers never
/// see partial file contents. /// see partial file contents.

View file

@ -22,11 +22,7 @@ use crate::{
syncback::{syncback_loop, FsSnapshot}, syncback::{syncback_loop, FsSnapshot},
}; };
use super::{ use super::{resolve_path, sourcemap::write_sourcemap_from_syncback, GlobalOptions};
resolve_path,
sourcemap::{filter_nothing, write_sourcemap},
GlobalOptions,
};
const UNKNOWN_INPUT_KIND_ERR: &str = "Could not detect what kind of file was inputted. \ 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."; Expected input file to end in .rbxl, .rbxlx, .rbxm, or .rbxmx.";
@ -195,7 +191,7 @@ impl SyncbackCommand {
} else { } else {
log::info!("Beginning syncback (clean mode)..."); log::info!("Beginning syncback (clean mode)...");
} }
let snapshot = syncback_loop( let result = syncback_loop(
session_old.vfs(), session_old.vfs(),
&mut dom_old, &mut dom_old,
dom_new, dom_new,
@ -209,19 +205,17 @@ impl SyncbackCommand {
let base_path = session_old.root_project().folder_location(); let base_path = session_old.root_project().folder_location();
if self.list { 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); drop(dom_old);
if !self.dry_run { if !self.dry_run {
if self.interactive { if self.interactive {
eprintln!( eprintln!(
"Would write {} files/folders and remove {} files/folders.", "Would write {} files/folders and remove {} files/folders.",
snapshot.added_paths().len(), result.fs_snapshot.added_paths().len(),
snapshot.removed_paths().len() result.fs_snapshot.removed_paths().len()
); );
eprint!("Is this okay? (Y/N): "); eprint!("Is this okay? (Y/N): ");
io::stderr().flush()?; io::stderr().flush()?;
@ -233,45 +227,45 @@ impl SyncbackCommand {
return Ok(()); return Ok(());
} }
} }
log::info!("Writing to the file system..."); 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!( log::info!(
"Finished syncback: wrote {} files/folders, removed {}.", "Finished syncback: wrote {} files/folders, removed {}.",
snapshot.added_paths().len(), result.fs_snapshot.added_paths().len(),
snapshot.removed_paths().len() result.fs_snapshot.removed_paths().len()
); );
// Generate sourcemap after successful syncback. match sourcemap_result {
// We need a fresh session that reads the post-syncback filesystem state, Ok(()) => log::info!("Generated sourcemap at {}", sourcemap_path.display()),
// since session_old contains the pre-syncback tree. Err(e) => log::warn!("Could not generate sourcemap: {}", e),
//
// 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
);
}
} }
// Refresh git index if in a git repository // Refresh git index if in a git repository
@ -291,8 +285,8 @@ impl SyncbackCommand {
} else { } else {
log::info!( log::info!(
"Would write {} files/folders and remove {} files/folders.", "Would write {} files/folders and remove {} files/folders.",
snapshot.added_paths().len(), result.fs_snapshot.added_paths().len(),
snapshot.removed_paths().len() result.fs_snapshot.removed_paths().len()
); );
log::info!("Aborting before writing to file system due to `--dry-run`"); log::info!("Aborting before writing to file system due to `--dry-run`");
} }

View file

@ -36,5 +36,5 @@ pub use snapshot::{
RojoDescendants, RojoTree, RojoDescendants, RojoTree,
}; };
pub use snapshot_middleware::{snapshot_from_vfs, Middleware, ScriptType}; 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; pub use web::interface as web_api;

View file

@ -43,6 +43,19 @@ pub use property_filter::{
pub use snapshot::{inst_path, SyncbackData, SyncbackSnapshot}; pub use snapshot::{inst_path, SyncbackData, SyncbackSnapshot};
pub use stats::SyncbackStats; 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<Ref, Vec<PathBuf>>,
}
/// The name of an enviroment variable to use to override the behavior of /// The name of an enviroment variable to use to override the behavior of
/// syncback on model files. /// syncback on model files.
/// By default, syncback will use `Rbxm` for model files. /// By default, syncback will use `Rbxm` for model files.
@ -81,7 +94,7 @@ pub fn syncback_loop(
new_tree: WeakDom, new_tree: WeakDom,
project: &Project, project: &Project,
incremental: bool, incremental: bool,
) -> anyhow::Result<FsSnapshot> { ) -> anyhow::Result<SyncbackResult> {
syncback_loop_with_stats(vfs, old_tree, new_tree, project, incremental, None) syncback_loop_with_stats(vfs, old_tree, new_tree, project, incremental, None)
} }
@ -94,7 +107,7 @@ pub fn syncback_loop_with_stats(
project: &Project, project: &Project,
incremental: bool, incremental: bool,
external_stats: Option<&SyncbackStats>, external_stats: Option<&SyncbackStats>,
) -> anyhow::Result<FsSnapshot> { ) -> anyhow::Result<SyncbackResult> {
// Create internal stats if not provided externally // Create internal stats if not provided externally
let internal_stats = SyncbackStats::new(); let internal_stats = SyncbackStats::new();
let stats = external_stats.unwrap_or(&internal_stats); 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 fs_snapshot = FsSnapshot::new();
let mut instance_paths: HashMap<Ref, Vec<PathBuf>> = HashMap::new();
'syncback: while let Some(snapshot) = snapshots.pop() { 'syncback: while let Some(snapshot) = snapshots.pop() {
let inst_path = snapshot.get_new_inst_path(snapshot.new); 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 // 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<PathBuf> = 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| { fs_snapshot.merge_with_filter(syncback.fs_snapshot, |path| {
is_valid_path(&ignore_patterns, project_path, path) is_valid_path(&ignore_patterns, project_path, path)
}); });
@ -895,7 +921,11 @@ pub fn syncback_loop_with_stats(
stats.log_summary(); stats.log_summary();
} }
Ok(fs_snapshot) Ok(SyncbackResult {
fs_snapshot,
new_tree,
instance_paths,
})
} }
pub struct SyncbackReturn<'sync> { pub struct SyncbackReturn<'sync> {