Merge 'origin/master' into security/host-origin-validation

CHANGELOG.md

@@ -39,6 +39,7 @@ Making a new release? Simply add the new header with the version and date undern
 * 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])
+* 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])
@@ -52,6 +53,7 @@ Making a new release? Simply add the new header with the version and date undern
 [#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
+[#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

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)
+2. Bump plugin version in [`plugin/Version.txt`](plugin/Version.txt)
-3. Run `cargo test` to update `Cargo.lock` and run tests
+   * 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
+7. Push commits and tags
-    * `cargo publish`
-8. Publish the Plugin
-    * `cargo run -- upload plugin --asset_id 6415005344`
-9. 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
+    * Write a small summary of each major feature

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
+Some workflows, like fully automatic conversion of every existing game into a Rojo project, are still limited and may require manual project configuration.
-* Automatically convert your existing game to work with Rojo
-* Import custom instances like MoonScript code
 
 ## [Documentation](https://rojo.space/docs)
 Documentation is hosted in the [rojo.space repository](https://github.com/rojo-rbx/rojo.space).

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<Self, Error> {
+        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<P: AsRef<Path>>(&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<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
+        serializer.serialize_str(&self.raw)
+    }
+}
+
+impl<'de> Deserialize<'de> for IgnorableGlob {
+    fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
+        let pattern = String::deserialize(deserializer)?;
+
+        IgnorableGlob::new(&pattern).map_err(D::Error::custom)
+    }
+}

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
@@ -123,7 +124,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<Glob>,
+    pub glob_ignore_paths: Vec<IgnorableGlob>,
 
     /// A list of rules for syncback with this project file.
     #[serde(skip_serializing_if = "Option::is_none")]
@@ -637,4 +638,55 @@ mod test {
 
         assert!(project.serve_allowed_hosts.is_empty());
     }
+
+    #[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"));
+    }
 }

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<P: AsRef<Path>>(&self, path: P) -> bool {
+    pub fn matches<P: AsRef<Path>>(&self, path: P) -> bool {
         let path = path.as_ref();
 
         match path.strip_prefix(&self.base_path) {
-            Ok(suffix) => !self.glob.is_match(suffix),
+            Ok(suffix) => self.glob.is_match(suffix),
-            Err(_) => true,
+            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<P: AsRef<Path>>(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.

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<Option<InstanceSnapshot>> {
-    let passes_filter_rules = |child: &DirEntry| {
+    let passes_filter_rules =
-        context
+        |child: &DirEntry| !is_path_ignored(&context.path_ignore_rules, child.path());
-            .path_ignore_rules
-            .iter()
-            .all(|rule| rule.passes(child.path()))
-    };
 
     let mut snapshot_children = Vec::new();
 

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},
@@ -414,18 +414,18 @@ pub struct SyncbackRules {
 }
 
 impl SyncbackRules {
-    pub fn compile_globs(&self) -> anyhow::Result<Vec<Glob>> {
+    pub fn compile_globs(&self) -> anyhow::Result<Vec<IgnorableGlob>> {
         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)
                     }
                 }
@@ -436,7 +436,7 @@ impl SyncbackRules {
     }
 }
 
-fn is_valid_path(globs: &Option<Vec<Glob>>, base_path: &Path, path: &Path) -> bool {
+fn is_valid_path(globs: &Option<Vec<IgnorableGlob>>, 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,
@@ -446,11 +446,16 @@ fn is_valid_path(globs: &Option<Vec<Glob>>, 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
 }
@@ -538,3 +543,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<bool>) -> 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")
+        ));
+    }
+}