From a2adf2b5173e947598c256c5e975ac2b875c8bc7 Mon Sep 17 00:00:00 2001 From: Ivan Matthew <76107136+Ivanmatthew@users.noreply.github.com> Date: Fri, 13 Feb 2026 04:17:28 +0100 Subject: [PATCH] Improves sourcemap path handling with pathdiff (#1217) --- CHANGELOG.md | 2 + CONTRIBUTING.md | 1 + Cargo.lock | 7 ++ Cargo.toml | 3 +- ..._sourcemap__test__maps_absolute_paths.snap | 35 +++++++ ..._sourcemap__test__maps_relative_paths.snap | 41 ++++++++ src/cli/sourcemap.rs | 96 +++++++++++++++++-- .../relative_paths/default.project.json | 14 +++ .../relative_paths/module/module.luau | 1 + .../project/default.project.json | 14 +++ .../relative_paths/project/src/init.luau | 1 + 11 files changed, 207 insertions(+), 8 deletions(-) create mode 100644 src/cli/snapshots/librojo__cli__sourcemap__test__maps_absolute_paths.snap create mode 100644 src/cli/snapshots/librojo__cli__sourcemap__test__maps_relative_paths.snap create mode 100644 test-projects/relative_paths/default.project.json create mode 100644 test-projects/relative_paths/module/module.luau create mode 100644 test-projects/relative_paths/project/default.project.json create mode 100644 test-projects/relative_paths/project/src/init.luau diff --git a/CHANGELOG.md b/CHANGELOG.md index 36d08fdf..1c090788 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ Making a new release? Simply add the new header with the version and date undern * Fixed a bug where MacOS paths weren't being handled correctly. ([#1201]) * Fixed a bug where the notification timeout thread would fail to cancel on unmount ([#1211]) * Added a "Forget" option to the sync reminder notification to avoid being reminded for that place in the future ([#1215]) +* Improves relative path calculation for sourcemap generation to avoid issues with Windows UNC paths. ([#1217]) [#1176]: https://github.com/rojo-rbx/rojo/pull/1176 [#1179]: https://github.com/rojo-rbx/rojo/pull/1179 @@ -46,6 +47,7 @@ Making a new release? Simply add the new header with the version and date undern [#1201]: https://github.com/rojo-rbx/rojo/pull/1201 [#1211]: https://github.com/rojo-rbx/rojo/pull/1211 [#1215]: https://github.com/rojo-rbx/rojo/pull/1215 +[#1217]: https://github.com/rojo-rbx/rojo/pull/1217 ## [7.7.0-rc.1] (November 27th, 2025) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c050a223..0063e448 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -14,6 +14,7 @@ Code contributions are welcome for features and bugs that have been reported in You'll want these tools to work on Rojo: * Latest stable Rust compiler + * Rustfmt and Clippy are used for code formatting and linting. * Latest stable [Rojo](https://github.com/rojo-rbx/rojo) * [Rokit](https://github.com/rojo-rbx/rokit) * [Luau Language Server](https://github.com/JohnnyMorganz/luau-lsp) (Only needed if working on the Studio plugin.) diff --git a/Cargo.lock b/Cargo.lock index 4451b3bf..0ffd1266 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1520,6 +1520,12 @@ version = "1.0.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" +[[package]] +name = "pathdiff" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df94ce210e5bc13cb6651479fa48d14f601d9858cfe0467f43ae157023b938d3" + [[package]] name = "percent-encoding" version = "2.3.2" @@ -2068,6 +2074,7 @@ dependencies = [ "num_cpus", "opener", "paste", + "pathdiff", "pretty_assertions", "profiling", "rayon", diff --git a/Cargo.toml b/Cargo.toml index 08879ab6..edf211a1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -100,6 +100,7 @@ clap = { version = "3.2.25", features = ["derive"] } profiling = "1.0.15" yaml-rust2 = "0.10.3" data-encoding = "2.8.0" +pathdiff = "0.2.3" blake3 = "1.5.0" float-cmp = "0.9.0" @@ -124,7 +125,7 @@ semver = "1.0.22" rojo-insta-ext = { path = "crates/rojo-insta-ext" } criterion = "0.3.6" -insta = { version = "1.36.1", features = ["redactions", "yaml"] } +insta = { version = "1.36.1", features = ["redactions", "yaml", "json"] } paste = "1.0.14" pretty_assertions = "1.4.0" serde_yaml = "0.8.26" diff --git a/src/cli/snapshots/librojo__cli__sourcemap__test__maps_absolute_paths.snap b/src/cli/snapshots/librojo__cli__sourcemap__test__maps_absolute_paths.snap new file mode 100644 index 00000000..33e9cf31 --- /dev/null +++ b/src/cli/snapshots/librojo__cli__sourcemap__test__maps_absolute_paths.snap @@ -0,0 +1,35 @@ +--- +source: src/cli/sourcemap.rs +expression: sourcemap_contents +--- +{ + "name": "default", + "className": "DataModel", + "filePaths": "[...1 path omitted...]", + "children": [ + { + "name": "ReplicatedStorage", + "className": "ReplicatedStorage", + "children": [ + { + "name": "Project", + "className": "ModuleScript", + "filePaths": "[...1 path omitted...]", + "children": [ + { + "name": "Module", + "className": "Folder", + "children": [ + { + "name": "module", + "className": "ModuleScript", + "filePaths": "[...1 path omitted...]" + } + ] + } + ] + } + ] + } + ] +} diff --git a/src/cli/snapshots/librojo__cli__sourcemap__test__maps_relative_paths.snap b/src/cli/snapshots/librojo__cli__sourcemap__test__maps_relative_paths.snap new file mode 100644 index 00000000..7a314b8c --- /dev/null +++ b/src/cli/snapshots/librojo__cli__sourcemap__test__maps_relative_paths.snap @@ -0,0 +1,41 @@ +--- +source: src/cli/sourcemap.rs +expression: sourcemap_contents +--- +{ + "name": "default", + "className": "DataModel", + "filePaths": [ + "default.project.json" + ], + "children": [ + { + "name": "ReplicatedStorage", + "className": "ReplicatedStorage", + "children": [ + { + "name": "Project", + "className": "ModuleScript", + "filePaths": [ + "src/init.luau" + ], + "children": [ + { + "name": "Module", + "className": "Folder", + "children": [ + { + "name": "module", + "className": "ModuleScript", + "filePaths": [ + "../module/module.luau" + ] + } + ] + } + ] + } + ] + } + ] +} diff --git a/src/cli/sourcemap.rs b/src/cli/sourcemap.rs index 95fe6631..94369e7c 100644 --- a/src/cli/sourcemap.rs +++ b/src/cli/sourcemap.rs @@ -10,7 +10,7 @@ use fs_err::File; use memofs::Vfs; use rayon::prelude::*; use rbx_dom_weak::{types::Ref, Ustr}; -use serde::Serialize; +use serde::{Deserialize, Serialize}; use tokio::runtime::Runtime; use crate::{ @@ -24,19 +24,20 @@ const PATH_STRIP_FAILED_ERR: &str = "Failed to create relative paths for project const ABSOLUTE_PATH_FAILED_ERR: &str = "Failed to turn relative path into absolute path!"; /// Representation of a node in the generated sourcemap tree. -#[derive(Serialize)] +#[derive(Serialize, Deserialize)] #[serde(rename_all = "camelCase")] struct SourcemapNode<'a> { name: &'a str, class_name: Ustr, #[serde( + default, skip_serializing_if = "Vec::is_empty", serialize_with = "crate::path_serializer::serialize_vec_absolute" )] file_paths: Vec>, - #[serde(skip_serializing_if = "Vec::is_empty")] + #[serde(default, skip_serializing_if = "Vec::is_empty")] children: Vec>, } @@ -70,12 +71,13 @@ pub struct SourcemapCommand { impl SourcemapCommand { pub fn run(self) -> anyhow::Result<()> { - let project_path = resolve_path(&self.project); + let project_path = fs_err::canonicalize(resolve_path(&self.project))?; - log::trace!("Constructing in-memory filesystem"); + log::trace!("Constructing filesystem with StdBackend"); let vfs = Vfs::new_default(); vfs.set_watch_enabled(self.watch); + log::trace!("Setting up session for sourcemap generation"); let session = ServeSession::new(vfs, project_path)?; let mut cursor = session.message_queue().cursor(); @@ -87,14 +89,17 @@ impl SourcemapCommand { // Pre-build a rayon threadpool with a low number of threads to avoid // dynamic creation overhead on systems with a high number of cpus. + log::trace!("Setting rayon global threadpool"); rayon::ThreadPoolBuilder::new() .num_threads(num_cpus::get().min(6)) .build_global() - .unwrap(); + .ok(); + log::trace!("Writing initial sourcemap"); write_sourcemap(&session, self.output.as_deref(), filter, self.absolute)?; if self.watch { + log::trace!("Setting up runtime for watch mode"); let rt = Runtime::new().unwrap(); loop { @@ -208,7 +213,7 @@ fn recurse_create_node<'a>( } else { for val in file_paths { output_file_paths.push(Cow::from( - val.strip_prefix(project_dir).expect(PATH_STRIP_FAILED_ERR), + pathdiff::diff_paths(val, project_dir).expect(PATH_STRIP_FAILED_ERR), )); } }; @@ -250,3 +255,80 @@ fn write_sourcemap( Ok(()) } + +#[cfg(test)] +mod test { + use crate::cli::sourcemap::SourcemapNode; + use crate::cli::SourcemapCommand; + use insta::internals::Content; + use std::path::Path; + + #[test] + fn maps_relative_paths() { + let sourcemap_dir = tempfile::tempdir().unwrap(); + let sourcemap_output = sourcemap_dir.path().join("sourcemap.json"); + let project_path = fs_err::canonicalize( + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("test-projects") + .join("relative_paths") + .join("project"), + ) + .unwrap(); + let sourcemap_command = SourcemapCommand { + project: project_path, + output: Some(sourcemap_output.clone()), + include_non_scripts: false, + watch: false, + absolute: false, + }; + assert!(sourcemap_command.run().is_ok()); + + let raw_sourcemap_contents = fs_err::read_to_string(sourcemap_output.as_path()).unwrap(); + let sourcemap_contents = + serde_json::from_str::(&raw_sourcemap_contents).unwrap(); + insta::assert_json_snapshot!(sourcemap_contents); + } + + #[test] + fn maps_absolute_paths() { + let sourcemap_dir = tempfile::tempdir().unwrap(); + let sourcemap_output = sourcemap_dir.path().join("sourcemap.json"); + let project_path = fs_err::canonicalize( + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("test-projects") + .join("relative_paths") + .join("project"), + ) + .unwrap(); + let sourcemap_command = SourcemapCommand { + project: project_path, + output: Some(sourcemap_output.clone()), + include_non_scripts: false, + watch: false, + absolute: true, + }; + assert!(sourcemap_command.run().is_ok()); + + let raw_sourcemap_contents = fs_err::read_to_string(sourcemap_output.as_path()).unwrap(); + let sourcemap_contents = + serde_json::from_str::(&raw_sourcemap_contents).unwrap(); + insta::assert_json_snapshot!(sourcemap_contents, { + ".**.filePaths" => insta::dynamic_redaction(|mut value, _path| { + let mut paths_count = 0; + + match value { + Content::Seq(ref mut vec) => { + for path in vec.iter().map(|i| i.as_str().unwrap()) { + assert_eq!(fs_err::canonicalize(path).is_ok(), true, "path was not valid"); + assert_eq!(Path::new(path).is_absolute(), true, "path was not absolute"); + + paths_count += 1; + } + } + _ => panic!("Expected filePaths to be a sequence"), + } + format!("[...{} path{} omitted...]", paths_count, if paths_count != 1 { "s" } else { "" } ) + }) + }); + } +} diff --git a/test-projects/relative_paths/default.project.json b/test-projects/relative_paths/default.project.json new file mode 100644 index 00000000..af85c143 --- /dev/null +++ b/test-projects/relative_paths/default.project.json @@ -0,0 +1,14 @@ +{ + "name": "default", + "tree": { + "$className": "DataModel", + "ReplicatedStorage": { + "Project": { + "$path": "project/src", + "Module": { + "$path": "module" + } + } + } + } +} diff --git a/test-projects/relative_paths/module/module.luau b/test-projects/relative_paths/module/module.luau new file mode 100644 index 00000000..15e53c4a --- /dev/null +++ b/test-projects/relative_paths/module/module.luau @@ -0,0 +1 @@ +return nil diff --git a/test-projects/relative_paths/project/default.project.json b/test-projects/relative_paths/project/default.project.json new file mode 100644 index 00000000..a7d8d46c --- /dev/null +++ b/test-projects/relative_paths/project/default.project.json @@ -0,0 +1,14 @@ +{ + "name": "default", + "tree": { + "$className": "DataModel", + "ReplicatedStorage": { + "Project": { + "$path": "src/", + "Module": { + "$path": "../module" + } + } + } + } +} diff --git a/test-projects/relative_paths/project/src/init.luau b/test-projects/relative_paths/project/src/init.luau new file mode 100644 index 00000000..15e53c4a --- /dev/null +++ b/test-projects/relative_paths/project/src/init.luau @@ -0,0 +1 @@ +return nil