diff --git a/CHANGELOG.md b/CHANGELOG.md index ea04bb33..8ceff750 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,8 +40,10 @@ Making a new release? Simply add the new header with the version and date undern * 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]) * Fixed missing support for init.plugin.lua and init.plugin.luau. ([#1252]) +* Add support for gitignore-style negation in `globIgnorePaths` and syncback's `ignorePaths` ([#1256]) * Fixed the sync fallback scrambling sibling order; replacements are now re-parented ancestors-first and in their original child order. ([#1265]) * Instances that share a name and class are now robustly matched on resync by comparing their properties, instead of relying on child order alone. ([#1266]) +* Rojo now reports a clear error instead of panicking in several cases, including when the `serve` port is already in use, when a synced file is read-only or locked, when the filesystem watcher can't be created, and when the working directory is inaccessible. ([#1267]) [#1176]: https://github.com/rojo-rbx/rojo/pull/1176 [#1179]: https://github.com/rojo-rbx/rojo/pull/1179 @@ -52,8 +54,10 @@ Making a new release? Simply add the new header with the version and date undern [#1215]: https://github.com/rojo-rbx/rojo/pull/1215 [#1217]: https://github.com/rojo-rbx/rojo/pull/1217 [#1252]: https://github.com/rojo-rbx/rojo/pull/1252 +[#1256]: https://github.com/rojo-rbx/rojo/pull/1256 [#1265]: https://github.com/rojo-rbx/rojo/pull/1265 [#1266]: https://github.com/rojo-rbx/rojo/pull/1266 +[#1267]: https://github.com/rojo-rbx/rojo/pull/1267 ## [7.7.0-rc.1] (November 27th, 2025) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0063e448..bf11ae72 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -13,12 +13,27 @@ 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 +* Rust 1.88 or newer * 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.) +Rokit installs the pinned Rojo, Selene, StyLua, Lune, and run-in-roblox versions listed in [`rokit.toml`](rokit.toml): + +```bash +rokit install +``` + +Before opening a pull request, run the relevant checks: + +```bash +cargo test +cargo fmt -- --check +cargo clippy +stylua --check plugin/src +selene plugin/src +``` + When working on the Studio plugin, we recommend using this command to automatically rebuild the plugin when you save a change: *(Make sure you've enabled the Studio setting to reload plugins on file change!)* @@ -29,7 +44,7 @@ bash scripts/watch-build-plugin.sh You can also run the plugin's unit tests with the following: -*(Make sure you have `run-in-roblox` installed first!)* +*(If you are not using Rokit, make sure you have `run-in-roblox` installed first!)* ```bash bash scripts/unit-test-plugin.sh @@ -49,27 +64,27 @@ Please file issues and we'll try to help figure out what the best way forward is ## Local Development Gotchas -If your build fails with "Error: failed to open file `D:\code\rojo\plugin\modules\roact\src`" you need to update your Git submodules. +If your build fails with an error about a missing path under `plugin/Packages`, such as `plugin/Packages/Roact`, you need to update your Git submodules. Run the command and try building again: `git submodule update --init --recursive`. ## Pushing a Rojo Release -The Rojo release process is pretty manual right now. If you need to do it, here's how: +The Rojo release process is driven by the GitHub Actions release workflow. If you need to do it, here's how: 1. Bump server version in [`Cargo.toml`](Cargo.toml) -2. Bump plugin version in [`plugin/src/Config.lua`](plugin/src/Config.lua) -3. Run `cargo test` to update `Cargo.lock` and run tests +2. Bump plugin version in [`plugin/Version.txt`](plugin/Version.txt) + * The build checks that the Cargo and plugin versions match. +3. Run `cargo test` to update `Cargo.lock` after the version bump and run tests 4. Update [`CHANGELOG.md`](CHANGELOG.md) 5. Commit! * `git add . && git commit -m "Release vX.Y.Z"` 6. Tag the commit * `git tag vX.Y.Z` -7. Publish the CLI - * `cargo publish` -8. Publish the Plugin - * `cargo run -- upload plugin --asset_id 6415005344` -9. Push commits and tags +7. Push commits and tags * `git push && git push --tags` +8. Wait for the GitHub Actions release workflow to create the draft release and upload CLI/plugin artifacts +9. Publish the CLI crate + * `cargo publish` 10. Copy GitHub release content from previous release * Update the leading text with a summary about the release * Paste the changelog notes (as-is!) from [`CHANGELOG.md`](CHANGELOG.md) - * Write a small summary of each major feature \ No newline at end of file + * Write a small summary of each major feature diff --git a/README.md b/README.md index c150ecb7..f3d1aeeb 100644 --- a/README.md +++ b/README.md @@ -25,12 +25,11 @@ Rojo enables: * Versioning your game, library, or plugin using Git or another VCS * Streaming `rbxmx` and `rbxm` models into your game in real time * Packaging and deploying your project to Roblox.com from the command line +* Pulling Instances from Roblox place and model files back into an existing Rojo project with `rojo syncback` -In the future, Rojo will be able to: +Rojo also has an optional two-way sync setting in the Studio plugin for syncing supported Studio edits back to the filesystem. -* Sync instances from Roblox Studio to the filesystem -* Automatically convert your existing game to work with Rojo -* Import custom instances like MoonScript code +Some workflows, like fully automatic conversion of every existing game into a Rojo project, are still limited and may require manual project configuration. ## [Documentation](https://rojo.space/docs) Documentation is hosted in the [rojo.space repository](https://github.com/rojo-rbx/rojo.space). diff --git a/crates/memofs/CHANGELOG.md b/crates/memofs/CHANGELOG.md index c35b5399..ec0b71f0 100644 --- a/crates/memofs/CHANGELOG.md +++ b/crates/memofs/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased Changes * Added `Vfs::canonicalize`. [#1201] +* **Breaking:** `StdBackend::new` and `Vfs::new_default` now return `io::Result`, so a failure to create the filesystem watcher is reported as an error instead of panicking. The `Default` implementation for `StdBackend` has been removed as a result. [#1267] + +[#1267]: https://github.com/rojo-rbx/rojo/pull/1267 ## 0.3.1 (2025-11-27) * Added `Vfs::exists`. [#1169] diff --git a/crates/memofs/src/lib.rs b/crates/memofs/src/lib.rs index 41d21100..11acf810 100644 --- a/crates/memofs/src/lib.rs +++ b/crates/memofs/src/lib.rs @@ -255,8 +255,11 @@ pub struct Vfs { impl Vfs { /// Creates a new `Vfs` with the default backend, `StdBackend`. - pub fn new_default() -> Self { - Self::new(StdBackend::new()) + /// + /// Returns an error if the filesystem watcher could not be initialized, + /// which can happen in restricted or sandboxed environments. + pub fn new_default() -> io::Result { + Ok(Self::new(StdBackend::new()?)) } /// Creates a new `Vfs` with the given backend. @@ -639,7 +642,7 @@ mod test { let file_path = dir.path().join("file.txt"); fs_err::write(&file_path, contents.to_string()).unwrap(); - let vfs = Vfs::new(StdBackend::new()); + let vfs = Vfs::new(StdBackend::new().unwrap()); let canonicalized = vfs.canonicalize(&file_path).unwrap(); assert_eq!(canonicalized, file_path.canonicalize().unwrap()); assert_eq!( @@ -653,7 +656,7 @@ mod test { let dir = tempfile::tempdir().unwrap(); let file_path = dir.path().join("test"); - let vfs = Vfs::new(StdBackend::new()); + let vfs = Vfs::new(StdBackend::new().unwrap()); let err = vfs.canonicalize(&file_path).unwrap_err(); assert_eq!(err.kind(), io::ErrorKind::NotFound); } diff --git a/crates/memofs/src/std_backend.rs b/crates/memofs/src/std_backend.rs index a25fa39d..d684bde5 100644 --- a/crates/memofs/src/std_backend.rs +++ b/crates/memofs/src/std_backend.rs @@ -17,9 +17,9 @@ pub struct StdBackend { } impl StdBackend { - pub fn new() -> StdBackend { + pub fn new() -> io::Result { let (notify_tx, notify_rx) = mpsc::channel(); - let watcher = watcher(notify_tx, Duration::from_millis(50)).unwrap(); + let watcher = watcher(notify_tx, Duration::from_millis(50)).map_err(io::Error::other)?; let (tx, rx) = crossbeam_channel::unbounded(); @@ -46,11 +46,11 @@ impl StdBackend { Result::<(), crossbeam_channel::SendError>::Ok(()) }); - Self { + Ok(Self { watcher, watcher_receiver: rx, watches: HashSet::new(), - } + }) } } @@ -134,9 +134,3 @@ impl VfsBackend for StdBackend { self.watcher.unwatch(path).map_err(io::Error::other) } } - -impl Default for StdBackend { - fn default() -> Self { - Self::new() - } -} diff --git a/src/change_processor.rs b/src/change_processor.rs index 6c5c8224..c1f245f2 100644 --- a/src/change_processor.rs +++ b/src/change_processor.rs @@ -200,7 +200,15 @@ impl JobThreadContext { if let Some(instance) = tree.get_instance(id) { if let Some(instigating_source) = &instance.metadata().instigating_source { match instigating_source { - InstigatingSource::Path(path) => fs::remove_file(path).unwrap(), + InstigatingSource::Path(path) => { + if let Err(err) = fs::remove_file(path) { + log::error!( + "Failed to remove file {}: {}", + path.display(), + err + ); + } + } InstigatingSource::ProjectNode { .. } => { log::warn!( "Cannot remove instance {:?}, it's from a project file", @@ -244,7 +252,13 @@ impl JobThreadContext { match instigating_source { InstigatingSource::Path(path) => { if let Some(Variant::String(value)) = changed_value { - fs::write(path, value).unwrap(); + if let Err(err) = fs::write(path, value) { + log::error!( + "Failed to write file {}: {}", + path.display(), + err + ); + } } else { log::warn!("Cannot change Source to non-string value."); } diff --git a/src/cli/build.rs b/src/cli/build.rs index c2ddebb3..82e81387 100644 --- a/src/cli/build.rs +++ b/src/cli/build.rs @@ -75,10 +75,10 @@ impl BuildCommand { _ => unreachable!(), }; - let project_path = resolve_path(&self.project); + let project_path = resolve_path(&self.project)?; log::trace!("Constructing in-memory filesystem"); - let vfs = Vfs::new_default(); + let vfs = Vfs::new_default()?; vfs.set_watch_enabled(self.watch); let session = ServeSession::new(vfs, project_path)?; @@ -87,11 +87,16 @@ impl BuildCommand { write_model(&session, &output_path, output_kind)?; if self.watch { - let rt = Runtime::new().unwrap(); + let rt = Runtime::new().context("Failed to start the async runtime for watch mode")?; loop { let receiver = session.message_queue().subscribe(cursor); - let (new_cursor, _patch_set) = rt.block_on(receiver).unwrap(); + let (new_cursor, _patch_set) = match rt.block_on(receiver) { + Ok(message) => message, + // The message queue was dropped, so there is nothing left + // to watch. Stop watching gracefully. + Err(_) => break, + }; cursor = new_cursor; write_model(&session, &output_path, output_kind)?; diff --git a/src/cli/fmt_project.rs b/src/cli/fmt_project.rs index 1c165974..e1c7adaa 100644 --- a/src/cli/fmt_project.rs +++ b/src/cli/fmt_project.rs @@ -18,10 +18,10 @@ pub struct FmtProjectCommand { impl FmtProjectCommand { pub fn run(self) -> anyhow::Result<()> { - let vfs = Vfs::new_default(); + let vfs = Vfs::new_default()?; vfs.set_watch_enabled(false); - let base_path = resolve_path(&self.project); + let base_path = resolve_path(&self.project)?; let project = Project::load_fuzzy(&vfs, &base_path)? .context("A project file is required to run 'rojo fmt-project'")?; diff --git a/src/cli/init.rs b/src/cli/init.rs index eb323310..434c2e34 100644 --- a/src/cli/init.rs +++ b/src/cli/init.rs @@ -9,7 +9,7 @@ use std::{ io::{self, Write}, }; -use anyhow::{bail, format_err}; +use anyhow::{bail, format_err, Context}; use clap::Parser; use fs_err as fs; use fs_err::OpenOptions; @@ -42,9 +42,9 @@ pub struct InitCommand { impl InitCommand { pub fn run(self) -> anyhow::Result<()> { - let template = self.kind.template(); + let template = self.kind.template()?; - let base_path = resolve_path(&self.path); + let base_path = resolve_path(&self.path)?; fs::create_dir_all(&base_path)?; let canonical = fs::canonicalize(&base_path)?; @@ -128,7 +128,7 @@ pub enum InitKind { } impl InitKind { - fn template(&self) -> InMemoryFs { + fn template(&self) -> anyhow::Result { let template_path = match self { Self::Place => "place", Self::Model => "model", @@ -136,20 +136,24 @@ impl InitKind { }; let snapshot: VfsSnapshot = bincode::deserialize(TEMPLATE_BINCODE) - .expect("Rojo's templates were not properly packed into Rojo's binary"); + .context("Rojo's templates were not properly packed into Rojo's binary. This is a bug in Rojo; please file an issue.")?; - if let VfsSnapshot::Dir { mut children } = snapshot { - if let Some(template) = children.remove(template_path) { - let mut fs = InMemoryFs::new(); - fs.load_snapshot("", template) - .expect("loading a template in memory should never fail"); - fs - } else { - panic!("template for project type {:?} is missing", self) - } - } else { - panic!("Rojo's templates were packed as a file instead of a directory") - } + let VfsSnapshot::Dir { mut children } = snapshot else { + bail!("Rojo's templates were packed as a file instead of a directory. This is a bug in Rojo; please file an issue."); + }; + + let template = children.remove(template_path).ok_or_else(|| { + format_err!( + "The template for project type {:?} is missing. This is a bug in Rojo; please file an issue.", + self + ) + })?; + + let mut fs = InMemoryFs::new(); + fs.load_snapshot("", template) + .context("Failed to load Rojo's bundled template into memory")?; + + Ok(fs) } } diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 7d3d87b1..caef95aa 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -12,6 +12,7 @@ mod upload; use std::{borrow::Cow, env, path::Path, str::FromStr}; +use anyhow::Context; use clap::Parser; use thiserror::Error; @@ -125,10 +126,14 @@ pub enum Subcommand { Syncback(SyncbackCommand), } -pub(super) fn resolve_path(path: &Path) -> Cow<'_, Path> { +pub(super) fn resolve_path(path: &Path) -> anyhow::Result> { if path.is_absolute() { - Cow::Borrowed(path) + Ok(Cow::Borrowed(path)) } else { - Cow::Owned(env::current_dir().unwrap().join(path)) + let current_dir = env::current_dir().context( + "Could not determine the current working directory. \ + It may have been deleted, or Rojo may not have permission to access it.", + )?; + Ok(Cow::Owned(current_dir.join(path))) } } diff --git a/src/cli/serve.rs b/src/cli/serve.rs index f89e4387..e33c3fc5 100644 --- a/src/cli/serve.rs +++ b/src/cli/serve.rs @@ -35,9 +35,9 @@ pub struct ServeCommand { impl ServeCommand { pub fn run(self, global: GlobalOptions) -> anyhow::Result<()> { - let project_path = resolve_path(&self.project); + let project_path = resolve_path(&self.project)?; - let vfs = Vfs::new_default(); + let vfs = Vfs::new_default()?; let session = Arc::new(ServeSession::new(vfs, project_path)?); @@ -53,8 +53,9 @@ impl ServeCommand { let server = LiveServer::new(session); - let _ = show_start_message(ip, port, global.color.into()); - server.start((ip, port).into()); + server.start((ip, port).into(), || { + let _ = show_start_message(ip, port, global.color.into()); + })?; Ok(()) } diff --git a/src/cli/sourcemap.rs b/src/cli/sourcemap.rs index 94369e7c..10fff590 100644 --- a/src/cli/sourcemap.rs +++ b/src/cli/sourcemap.rs @@ -5,6 +5,7 @@ use std::{ path::{self, Path, PathBuf}, }; +use anyhow::Context; use clap::Parser; use fs_err::File; use memofs::Vfs; @@ -71,10 +72,10 @@ pub struct SourcemapCommand { impl SourcemapCommand { pub fn run(self) -> anyhow::Result<()> { - let project_path = fs_err::canonicalize(resolve_path(&self.project))?; + let project_path = fs_err::canonicalize(resolve_path(&self.project)?)?; log::trace!("Constructing filesystem with StdBackend"); - let vfs = Vfs::new_default(); + let vfs = Vfs::new_default()?; vfs.set_watch_enabled(self.watch); log::trace!("Setting up session for sourcemap generation"); @@ -100,11 +101,16 @@ impl SourcemapCommand { if self.watch { log::trace!("Setting up runtime for watch mode"); - let rt = Runtime::new().unwrap(); + let rt = Runtime::new().context("Failed to start the async runtime for watch mode")?; loop { let receiver = session.message_queue().subscribe(cursor); - let (new_cursor, patch_set) = rt.block_on(receiver).unwrap(); + let (new_cursor, patch_set) = match rt.block_on(receiver) { + Ok(message) => message, + // The message queue was dropped, so there is nothing left + // to watch. Stop watching gracefully. + Err(_) => break, + }; cursor = new_cursor; if patch_set_affects_sourcemap(&session, &patch_set, filter) { diff --git a/src/cli/syncback.rs b/src/cli/syncback.rs index 282d55c8..7bb274fb 100644 --- a/src/cli/syncback.rs +++ b/src/cli/syncback.rs @@ -58,8 +58,8 @@ pub struct SyncbackCommand { impl SyncbackCommand { pub fn run(&self, global: GlobalOptions) -> anyhow::Result<()> { - let path_old = resolve_path(&self.project); - let path_new = resolve_path(&self.input); + let path_old = resolve_path(&self.project)?; + let path_new = resolve_path(&self.input)?; let input_kind = FileKind::from_path(&path_new).context(UNKNOWN_INPUT_KIND_ERR)?; let dom_start_timer = Instant::now(); @@ -69,7 +69,7 @@ impl SyncbackCommand { dom_start_timer.elapsed().as_secs_f32() ); - let vfs = Vfs::new_default(); + let vfs = Vfs::new_default()?; vfs.set_watch_enabled(false); let project_start_timer = Instant::now(); diff --git a/src/cli/upload.rs b/src/cli/upload.rs index 3c3610fd..bc9d427e 100644 --- a/src/cli/upload.rs +++ b/src/cli/upload.rs @@ -38,9 +38,9 @@ pub struct UploadCommand { impl UploadCommand { pub fn run(self) -> Result<(), anyhow::Error> { - let project_path = resolve_path(&self.project); + let project_path = resolve_path(&self.project)?; - let vfs = Vfs::new_default(); + let vfs = Vfs::new_default()?; let session = ServeSession::new(vfs, project_path)?; diff --git a/src/glob.rs b/src/glob.rs index 96437c38..c0146863 100644 --- a/src/glob.rs +++ b/src/glob.rs @@ -48,3 +48,61 @@ impl<'de> Deserialize<'de> for Glob { Glob::new(&glob).map_err(D::Error::custom) } } + +/// A glob with optional gitignore-style negation. A leading `!` marks the +/// pattern as a negation (re-includes paths that an earlier rule excluded). +/// To match a literal `!` at the start of a pattern, escape it with `\!`. +#[derive(Debug, Clone)] +pub struct IgnorableGlob { + glob: Glob, + negated: bool, + raw: String, +} + +impl IgnorableGlob { + pub fn new(pattern: &str) -> Result { + let (negated, body) = if let Some(rest) = pattern.strip_prefix('!') { + (true, rest) + } else if pattern.starts_with(r"\!") { + (false, &pattern[1..]) + } else { + (false, pattern) + }; + + Ok(IgnorableGlob { + glob: Glob::new(body)?, + negated, + raw: pattern.to_owned(), + }) + } + + pub fn is_match>(&self, path: P) -> bool { + self.glob.is_match(path) + } + + pub fn is_negation(&self) -> bool { + self.negated + } +} + +impl PartialEq for IgnorableGlob { + fn eq(&self, other: &Self) -> bool { + self.negated == other.negated && self.glob == other.glob + } +} + +impl Eq for IgnorableGlob {} + +impl Serialize for IgnorableGlob { + fn serialize(&self, serializer: S) -> Result { + serializer.serialize_str(&self.raw) + } +} + +impl<'de> Deserialize<'de> for IgnorableGlob { + fn deserialize>(deserializer: D) -> Result { + let pattern = String::deserialize(deserializer)?; + + IgnorableGlob::new(&pattern).map_err(D::Error::custom) + } +} diff --git a/src/project.rs b/src/project.rs index 5cc8278e..bfcc7f0e 100644 --- a/src/project.rs +++ b/src/project.rs @@ -12,7 +12,8 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use crate::{ - glob::Glob, json, resolution::UnresolvedValue, snapshot::SyncRule, syncback::SyncbackRules, + glob::IgnorableGlob, json, resolution::UnresolvedValue, snapshot::SyncRule, + syncback::SyncbackRules, }; /// Represents 'default' project names that act as `init` files @@ -114,7 +115,7 @@ pub struct Project { /// A list of globs, relative to the folder the project file is in, that /// match files that should be excluded if Rojo encounters them. #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub glob_ignore_paths: Vec, + pub glob_ignore_paths: Vec, /// A list of rules for syncback with this project file. #[serde(skip_serializing_if = "Option::is_none")] @@ -593,4 +594,55 @@ mod test { assert!(project.sync_rules[0].include.is_match("data.data.json")); assert!(project.sync_rules[1].include.is_match("init.module.lua")); } + + #[test] + fn glob_ignore_paths_negation() { + let project_json = r#"{ + "name": "TestProject", + "tree": { "$path": "src" }, + "globIgnorePaths": [ + "**/*.spec.lua", + "!keep.spec.lua", + "\\!literal.lua" + ] + }"#; + + let project = Project::load_from_slice( + project_json.as_bytes(), + PathBuf::from("/test/default.project.json"), + None, + ) + .expect("project should parse"); + + let paths = &project.glob_ignore_paths; + assert_eq!(paths.len(), 3); + + assert!(!paths[0].is_negation()); + assert!(paths[0].is_match("foo.spec.lua")); + + assert!(paths[1].is_negation()); + assert!(paths[1].is_match("keep.spec.lua")); + + // `\!literal.lua` should match a file literally named `!literal.lua`, + // not be parsed as a negation. + assert!(!paths[2].is_negation()); + assert!(paths[2].is_match("!literal.lua")); + + let rules: Vec<_> = paths + .iter() + .map(|g| crate::snapshot::PathIgnoreRule { + base_path: PathBuf::from("/test"), + glob: g.clone(), + }) + .collect(); + assert!(crate::snapshot::is_path_ignored( + &rules, + "/test/foo.spec.lua" + )); + assert!(!crate::snapshot::is_path_ignored( + &rules, + "/test/keep.spec.lua" + )); + assert!(!crate::snapshot::is_path_ignored(&rules, "/test/plain.lua")); + } } diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index 79f7a2d0..a1bf0afe 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -8,7 +8,7 @@ use anyhow::Context; use serde::{Deserialize, Serialize}; use crate::{ - glob::Glob, + glob::{Glob, IgnorableGlob}, path_serializer, project::ProjectNode, snapshot_middleware::{emit_legacy_scripts_default, Middleware}, @@ -222,18 +222,37 @@ pub struct PathIgnoreRule { pub base_path: PathBuf, /// The actual glob that can be matched against the input path. - pub glob: Glob, + pub glob: IgnorableGlob, } impl PathIgnoreRule { - pub fn passes>(&self, path: P) -> bool { + pub fn matches>(&self, path: P) -> bool { let path = path.as_ref(); match path.strip_prefix(&self.base_path) { - Ok(suffix) => !self.glob.is_match(suffix), - Err(_) => true, + Ok(suffix) => self.glob.is_match(suffix), + Err(_) => false, } } + + pub fn is_negation(&self) -> bool { + self.glob.is_negation() + } +} + +/// Evaluates an ordered list of [`PathIgnoreRule`]s against a path using +/// gitignore-style "last match wins" semantics: a path is ignored if the last +/// rule whose pattern matches it is non-negated. Paths matched by no rule are +/// not ignored. +pub fn is_path_ignored>(rules: &[PathIgnoreRule], path: P) -> bool { + let path = path.as_ref(); + let mut ignored = false; + for rule in rules { + if rule.matches(path) { + ignored = !rule.is_negation(); + } + } + ignored } /// Represents where a particular Instance or InstanceSnapshot came from. diff --git a/src/snapshot_middleware/csv.rs b/src/snapshot_middleware/csv.rs index 18fa1424..b4a86efc 100644 --- a/src/snapshot_middleware/csv.rs +++ b/src/snapshot_middleware/csv.rs @@ -195,7 +195,7 @@ struct LocalizationEntry<'a> { /// https://github.com/BurntSushi/rust-csv/issues/151 /// /// This function operates in one step in order to minimize data-copying. -fn convert_localization_csv(contents: &[u8]) -> Result { +fn convert_localization_csv(contents: &[u8]) -> anyhow::Result { let mut reader = csv::Reader::from_reader(contents); let headers = reader.headers()?.clone(); @@ -237,7 +237,7 @@ fn convert_localization_csv(contents: &[u8]) -> Result { } let encoded = - serde_json::to_string(&entries).expect("Could not encode JSON for localization table"); + serde_json::to_string(&entries).context("Could not encode JSON for localization table")?; Ok(encoded) } diff --git a/src/snapshot_middleware/dir.rs b/src/snapshot_middleware/dir.rs index 201f5620..b9f62743 100644 --- a/src/snapshot_middleware/dir.rs +++ b/src/snapshot_middleware/dir.rs @@ -7,7 +7,9 @@ use anyhow::Context; use memofs::{DirEntry, Vfs}; use crate::{ - snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource}, + snapshot::{ + is_path_ignored, InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource, + }, syncback::{hash_instance, FsSnapshot, SyncbackReturn, SyncbackSnapshot}, }; @@ -41,12 +43,8 @@ pub fn snapshot_dir_no_meta( path: &Path, name: &str, ) -> anyhow::Result> { - let passes_filter_rules = |child: &DirEntry| { - context - .path_ignore_rules - .iter() - .all(|rule| rule.passes(child.path())) - }; + let passes_filter_rules = + |child: &DirEntry| !is_path_ignored(&context.path_ignore_rules, child.path()); let mut snapshot_children = Vec::new(); diff --git a/src/syncback/mod.rs b/src/syncback/mod.rs index 5373ba4f..7fe3992d 100644 --- a/src/syncback/mod.rs +++ b/src/syncback/mod.rs @@ -21,7 +21,7 @@ use std::{ }; use crate::{ - glob::Glob, + glob::{Glob, IgnorableGlob}, snapshot::{InstanceWithMeta, RojoTree}, snapshot_middleware::Middleware, syncback::ref_properties::{collect_referents, link_referents}, @@ -415,18 +415,18 @@ pub struct SyncbackRules { } impl SyncbackRules { - pub fn compile_globs(&self) -> anyhow::Result> { + pub fn compile_globs(&self) -> anyhow::Result> { let mut globs = Vec::with_capacity(self.ignore_paths.len()); let dir_ignore_paths = self.create_ignore_dir_paths.unwrap_or(true); for pattern in &self.ignore_paths { - let glob = Glob::new(pattern) + let glob = IgnorableGlob::new(pattern) .with_context(|| format!("the pattern '{pattern}' is not a valid glob"))?; globs.push(glob); if dir_ignore_paths { if let Some(dir_pattern) = pattern.strip_suffix("/**") { - if let Ok(glob) = Glob::new(dir_pattern) { + if let Ok(glob) = IgnorableGlob::new(dir_pattern) { globs.push(glob) } } @@ -437,7 +437,7 @@ impl SyncbackRules { } } -fn is_valid_path(globs: &Option>, base_path: &Path, path: &Path) -> bool { +fn is_valid_path(globs: &Option>, base_path: &Path, path: &Path) -> bool { let git_glob = GIT_IGNORE_GLOB.get_or_init(|| Glob::new(".git/**").unwrap()); let test_path = match path.strip_prefix(base_path) { Ok(suffix) => suffix, @@ -447,11 +447,16 @@ fn is_valid_path(globs: &Option>, base_path: &Path, path: &Path) -> bo return false; } if let Some(ref ignore_paths) = globs { + // Gitignore-style "last match wins" + let mut ignored = false; for glob in ignore_paths { if glob.is_match(test_path) { - return false; + ignored = !glob.is_negation(); } } + if ignored { + return false; + } } true } @@ -539,3 +544,71 @@ fn strip_unknown_root_children(new: &mut WeakDom, old: &RojoTree) { new.destroy(child_ref); } } + +#[cfg(test)] +mod test { + use super::*; + + fn rules(ignore_paths: &[&str], create_ignore_dir_paths: Option) -> SyncbackRules { + SyncbackRules { + ignore_trees: Vec::new(), + ignore_paths: ignore_paths.iter().map(|s| s.to_string()).collect(), + ignore_properties: IndexMap::new(), + sync_current_camera: None, + sync_unscriptable: None, + ignore_referents: None, + create_ignore_dir_paths, + } + } + + #[test] + fn ignore_paths_negation() { + let globs = Some( + rules(&["**/*.lua", "!keep.lua"], Some(false)) + .compile_globs() + .unwrap(), + ); + let base = Path::new("/test"); + + // A later negation re-includes a path matched by an earlier pattern. + assert!(!is_valid_path(&globs, base, Path::new("/test/foo.lua"))); + assert!(is_valid_path(&globs, base, Path::new("/test/keep.lua"))); + // Paths matched by no rule are valid. + assert!(is_valid_path(&globs, base, Path::new("/test/plain.txt"))); + } + + #[test] + fn ignore_paths_negation_with_dir_expansion() { + // With `create_ignore_dir_paths`, a negated `foo/**` pattern should also + // re-include the `foo` directory itself, mirroring the file rule. + let globs = Some( + rules(&["**/*", "!keep/**"], Some(true)) + .compile_globs() + .unwrap(), + ); + let base = Path::new("/test"); + + assert!(!is_valid_path(&globs, base, Path::new("/test/drop/a.lua"))); + assert!(is_valid_path(&globs, base, Path::new("/test/keep"))); + assert!(is_valid_path(&globs, base, Path::new("/test/keep/a.lua"))); + } + + #[test] + fn ignore_paths_escaped_bang_is_literal() { + // `\!literal.lua` should ignore a file literally named `!literal.lua` + // rather than being parsed as a negation. + let globs = Some( + rules(&[r"\!literal.lua"], Some(false)) + .compile_globs() + .unwrap(), + ); + let base = Path::new("/test"); + + assert!(!globs.as_ref().unwrap()[0].is_negation()); + assert!(!is_valid_path( + &globs, + base, + Path::new("/test/!literal.lua") + )); + } +} diff --git a/src/web/mod.rs b/src/web/mod.rs index d6df4b15..8e33d9c5 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -12,6 +12,7 @@ use std::convert::Infallible; use std::net::SocketAddr; use std::sync::Arc; +use anyhow::Context; use hyper::{ server::Server, service::{make_service_fn, service_fn}, @@ -30,7 +31,12 @@ impl LiveServer { LiveServer { serve_session } } - pub fn start(self, address: SocketAddr) { + /// Starts the server on the given address, blocking until it stops. + /// + /// `on_listening` is invoked once the server has successfully bound to the + /// address, so callers can defer printing any "listening" message until + /// after binding can no longer fail (e.g. due to the port being in use). + pub fn start(self, address: SocketAddr, on_listening: impl FnOnce()) -> anyhow::Result<()> { let serve_session = Arc::clone(&self.serve_session); let make_service = make_service_fn(move |_conn| { @@ -53,9 +59,25 @@ impl LiveServer { } }); - let rt = Runtime::new().unwrap(); + let rt = Runtime::new().context("Failed to start the async runtime for the web server")?; let _guard = rt.enter(); - let server = Server::bind(&address).serve(make_service); - rt.block_on(server).unwrap(); + let server = Server::try_bind(&address) + .with_context(|| { + format!( + "Could not start the Rojo server on {address}.\n\ + The address may already be in use or reserved. Another Rojo server might already \ + be running, or another program may be using that port.\n\ + You can pick a different port with the --port option." + ) + })? + .serve(make_service); + + // Binding succeeded, so it's now safe to tell the user we're listening. + on_listening(); + + rt.block_on(server) + .context("The Rojo web server encountered a fatal error")?; + + Ok(()) } } diff --git a/src/web/ui.rs b/src/web/ui.rs index 4d63eaad..f6361b4f 100644 --- a/src/web/ui.rs +++ b/src/web/ui.rs @@ -6,7 +6,7 @@ use std::{borrow::Cow, sync::Arc, time::Duration}; -use hyper::{header, Body, Method, Request, Response, StatusCode}; +use hyper::{Body, Method, Request, Response, StatusCode}; use rbx_dom_weak::types::{Ref, Variant}; use ritz::{html, Fragment, HtmlContent, HtmlSelfClosingTag}; @@ -16,7 +16,7 @@ use crate::{ web::{ assets, interface::{ErrorResponse, SERVER_VERSION}, - util::json, + util::{json, response}, }, }; @@ -45,17 +45,11 @@ impl UiService { } fn handle_logo(&self) -> Response { - Response::builder() - .header(header::CONTENT_TYPE, "image/png") - .body(Body::from(assets::logo())) - .unwrap() + response(StatusCode::OK, "image/png", assets::logo()) } fn handle_icon(&self) -> Response { - Response::builder() - .header(header::CONTENT_TYPE, "image/png") - .body(Body::from(assets::icon())) - .unwrap() + response(StatusCode::OK, "image/png", assets::icon()) } fn handle_home(&self) -> Response { @@ -66,10 +60,11 @@ impl UiService { }); - Response::builder() - .header(header::CONTENT_TYPE, "text/html") - .body(Body::from(format!("{}", page))) - .unwrap() + response( + StatusCode::OK, + "text/html", + format!("{}", page), + ) } fn handle_show_instances(&self) -> Response { @@ -80,10 +75,11 @@ impl UiService { { Self::instance(&tree, root_id) } }); - Response::builder() - .header(header::CONTENT_TYPE, "text/html") - .body(Body::from(format!("{}", page))) - .unwrap() + response( + StatusCode::OK, + "text/html", + format!("{}", page), + ) } fn instance(tree: &RojoTree, id: Ref) -> HtmlContent<'_> { diff --git a/src/web/util.rs b/src/web/util.rs index 1c1cbb50..bd1f6b81 100644 --- a/src/web/util.rs +++ b/src/web/util.rs @@ -1,6 +1,27 @@ use hyper::{header::CONTENT_TYPE, Body, Response, StatusCode}; use serde::{Deserialize, Serialize}; +/// Builds an HTTP response, falling back to an empty `500` response (rather than +/// panicking) if the response could not be constructed. With constant headers +/// and a valid status code this never actually fails, but routing every +/// response through here means a malformed response can never crash the server. +pub fn response( + code: StatusCode, + content_type: &'static str, + body: impl Into, +) -> Response { + Response::builder() + .status(code) + .header(CONTENT_TYPE, content_type) + .body(body.into()) + .unwrap_or_else(|err| { + log::error!("Failed to build HTTP response: {}", err); + let mut fallback = Response::new(Body::empty()); + *fallback.status_mut() = StatusCode::INTERNAL_SERVER_ERROR; + fallback + }) +} + pub fn msgpack_ok(value: T) -> Response { msgpack(value, StatusCode::OK) } @@ -12,18 +33,14 @@ pub fn msgpack(value: T, code: StatusCode) -> Response { .with_struct_map(); if let Err(err) = value.serialize(&mut serializer) { - return Response::builder() - .status(StatusCode::INTERNAL_SERVER_ERROR) - .header(CONTENT_TYPE, "text/plain") - .body(Body::from(err.to_string())) - .unwrap(); + return response( + StatusCode::INTERNAL_SERVER_ERROR, + "text/plain", + err.to_string(), + ); }; - Response::builder() - .status(code) - .header(CONTENT_TYPE, "application/msgpack") - .body(Body::from(serialized)) - .unwrap() + response(code, "application/msgpack", serialized) } pub fn serialize_msgpack(value: T) -> anyhow::Result> { @@ -49,17 +66,13 @@ pub fn json(value: T, code: StatusCode) -> Response { let serialized = match serde_json::to_string(&value) { Ok(v) => v, Err(err) => { - return Response::builder() - .status(StatusCode::INTERNAL_SERVER_ERROR) - .header(CONTENT_TYPE, "text/plain") - .body(Body::from(err.to_string())) - .unwrap(); + return response( + StatusCode::INTERNAL_SERVER_ERROR, + "text/plain", + err.to_string(), + ); } }; - Response::builder() - .status(code) - .header(CONTENT_TYPE, "application/json") - .body(Body::from(serialized)) - .unwrap() + response(code, "application/json", serialized) }