Implement glob ignores (#272)

* Add Glob wrapper type with better serialization

* Introduce PathIgnoreRule struct

* Implement equality for Glob type

* Add PathIgnoreRule to InstanceContext

* Implement glob ignores in directory middleware

* Fix up filters

* Use Iterator::all instead of loop

* Add project-level configuration for glob ignores

* Add test project for glob ignores

* Wire up project file and snapshots to make glob ignores work

* Better codepaths for adding ignore rules with empty iterators

* Add test for globs inherited from parent projects

* Add test details, support glob ignores in nested projects

* Add feature flag for globs

* Switch to use ExactSizeIterator instead of size_hint

* Remove glob visitor

.github/workflows/ci.yml

@@ -28,3 +28,9 @@ jobs:
         cargo fmt -- --check
         cargo clippy
       if: matrix.rust_version == 'stable'
+
+    - name: Build (All Features)
+      run: cargo build --locked --verbose --all-features
+
+    - name: Run tests (All Features)
+      run: cargo test --locked --verbose --all-features

Cargo.lock

@@ -607,6 +607,18 @@ name = "glob"
 version = "0.2.11"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
+[[package]]
+name = "globset"
+version = "0.4.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "aho-corasick 0.7.6 (registry+https://github.com/rust-lang/crates.io-index)",
+ "bstr 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)",
+ "fnv 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)",
+ "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
+ "regex 1.3.1 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "h2"
 version = "0.1.26"
@@ -1652,6 +1664,7 @@ dependencies = [
  "csv 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "env_logger 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "futures 0.1.29 (registry+https://github.com/rust-lang/crates.io-index)",
+ "globset 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "humantime 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "hyper 0.12.35 (registry+https://github.com/rust-lang/crates.io-index)",
  "insta 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -2519,6 +2532,7 @@ dependencies = [
 "checksum generic-array 0.12.3 (registry+https://github.com/rust-lang/crates.io-index)" = "c68f0274ae0e023facc3c97b2e00f076be70e254bc851d972503b328db79b2ec"
 "checksum getrandom 0.1.13 (registry+https://github.com/rust-lang/crates.io-index)" = "e7db7ca94ed4cd01190ceee0d8a8052f08a247aa1b469a7f68c6a3b71afcf407"
 "checksum glob 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)" = "8be18de09a56b60ed0edf84bc9df007e30040691af7acd1c41874faac5895bfb"
+"checksum globset 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)" = "925aa2cac82d8834e2b2a4415b6f6879757fb5c0928fc445ae76461a12eed8f2"
 "checksum h2 0.1.26 (registry+https://github.com/rust-lang/crates.io-index)" = "a5b34c246847f938a410a03c5458c7fee2274436675e76d8b903c08efc29c462"
 "checksum heck 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "20564e78d53d2bb135c343b3f47714a56af2061f1c928fdb541dc7b9fdd94205"
 "checksum hermit-abi 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "f629dc602392d3ec14bfc8a09b5e644d7ffd725102b48b81e59f90f2633621d7"

Cargo.toml

@@ -18,6 +18,9 @@ exclude = [
 [features]
 default = []
 
+# Turn on support for specifying glob ignore path rules in the project format.
+unstable_glob_ignore_paths = []
+
 # Turn on the server half of Rojo's unstable two-way sync feature.
 unstable_two_way_sync = []
 
@@ -54,6 +57,7 @@ crossbeam-channel = "0.4.0"
 csv = "1.1.1"
 env_logger = "0.7.1"
 futures = "0.1.29"
+globset = "0.4.4"
 humantime = "1.3.0"
 hyper = "0.12.35"
 jod-thread = "0.1.0"
@@ -69,7 +73,7 @@ regex = "1.3.1"
 reqwest = "0.9.20"
 ritz = "0.1.0"
 rlua = "0.17.0"
-serde = { version = "1.0", features = ["derive"] }
+serde = { version = "1.0", features = ["derive", "rc"] }
 serde_json = "1.0"
 snafu = "0.6.0"
 structopt = "0.3.5"

rojo-test/Cargo.toml

@@ -5,6 +5,11 @@ authors = ["Lucien Greathouse <me@lpghatguy.com>"]
 edition = "2018"
 publish = false
 
+[features]
+default = []
+
+unstable_glob_ignore_paths = []
+
 [dependencies]
 env_logger = "0.6.2"
 log = "0.4.8"

rojo-test/build-test-snapshots/build_test__ignore_glob_inner.snap

@@ -0,0 +1,33 @@
+---
+source: rojo-test/src/build_test.rs
+expression: contents
+---
+<roblox version="4">
+  <Item class="Folder" referent="0">
+    <Properties>
+      <string name="Name">ignore_glob_inner</string>
+    </Properties>
+    <Item class="Folder" referent="1">
+      <Properties>
+        <string name="Name">src</string>
+      </Properties>
+      <Item class="ModuleScript" referent="2">
+        <Properties>
+          <string name="Name">outer.spec</string>
+          <string name="Source">-- This file should be included.</string>
+        </Properties>
+      </Item>
+    </Item>
+    <Item class="Folder" referent="3">
+      <Properties>
+        <string name="Name">subproject</string>
+      </Properties>
+      <Item class="ModuleScript" referent="4">
+        <Properties>
+          <string name="Name">inner</string>
+          <string name="Source">-- This file should be included.</string>
+        </Properties>
+      </Item>
+    </Item>
+  </Item>
+</roblox>

rojo-test/build-test-snapshots/build_test__ignore_glob_nested.snap

@@ -0,0 +1,17 @@
+---
+source: rojo-test/src/build_test.rs
+expression: contents
+---
+<roblox version="4">
+  <Item class="Folder" referent="0">
+    <Properties>
+      <string name="Name">ignore_glob_nested</string>
+    </Properties>
+    <Item class="ModuleScript" referent="1">
+      <Properties>
+        <string name="Name">include</string>
+        <string name="Source">-- This file must be present.</string>
+      </Properties>
+    </Item>
+  </Item>
+</roblox>

rojo-test/build-test-snapshots/build_test__ignore_glob_spec.snap

@@ -0,0 +1,17 @@
+---
+source: rojo-test/src/build_test.rs
+expression: contents
+---
+<roblox version="4">
+  <Item class="Folder" referent="0">
+    <Properties>
+      <string name="Name">ignore_glob_spec</string>
+    </Properties>
+    <Item class="ModuleScript" referent="1">
+      <Properties>
+        <string name="Name">shouldBeIncluded</string>
+        <string name="Source">-- this file should be present</string>
+      </Properties>
+    </Item>
+  </Item>
+</roblox>

rojo-test/build-tests/ignore_glob_inner/README.md

@@ -0,0 +1,2 @@
+# ignore_glob_inner
+Tests that glob ignores defined *inside* nested projects apply to those projects, but not anywhere else.

rojo-test/build-tests/ignore_glob_inner/default.project.json

@@ -0,0 +1,14 @@
+{
+  "name": "ignore_glob_inner",
+  "tree": {
+    "$className": "Folder",
+
+    "src": {
+      "$path": "src"
+    },
+
+    "subproject": {
+      "$path": "subproject"
+    }
+  }
+}

rojo-test/build-tests/ignore_glob_inner/src/outer.spec.lua

@@ -0,0 +1 @@
+-- This file should be included.

rojo-test/build-tests/ignore_glob_inner/subproject/default.project.json

@@ -0,0 +1,9 @@
+{
+  "name": "subproject",
+  "tree": {
+    "$path": "src"
+  },
+  "globIgnorePaths": [
+    "**/*.spec.lua"
+  ]
+}

rojo-test/build-tests/ignore_glob_inner/subproject/src/inner.lua

@@ -0,0 +1 @@
+-- This file should be included.

rojo-test/build-tests/ignore_glob_inner/subproject/src/inner.spec.lua

@@ -0,0 +1 @@
+-- This file should not be included.

rojo-test/build-tests/ignore_glob_nested/README.md

@@ -0,0 +1,2 @@
+# ignore_glob_nested
+Tests that glob ignores defined in the root project also apply to nested projects.

rojo-test/build-tests/ignore_glob_nested/default.project.json

@@ -0,0 +1,9 @@
+{
+  "name": "ignore_glob_nested",
+  "tree": {
+    "$path": "subproject"
+  },
+  "globIgnorePaths": [
+    "**/*.spec.lua"
+  ]
+}

rojo-test/build-tests/ignore_glob_nested/subproject/default.project.json

@@ -0,0 +1,6 @@
+{
+  "name": "subproject",
+  "tree": {
+    "$path": "src"
+  }
+}

rojo-test/build-tests/ignore_glob_nested/subproject/src/exclude.spec.lua

@@ -0,0 +1 @@
+-- This file must not be present.

rojo-test/build-tests/ignore_glob_nested/subproject/src/include.lua

@@ -0,0 +1 @@
+-- This file must be present.

rojo-test/build-tests/ignore_glob_spec/README.md

@@ -0,0 +1,2 @@
+# ignore_glob_spec
+Tests that glob ignores work for the original use case: ignoring files in the same project that end in `.spec.lua`.

rojo-test/build-tests/ignore_glob_spec/default.project.json

@@ -0,0 +1,9 @@
+{
+  "name": "ignore_glob_spec",
+  "tree": {
+    "$path": "src"
+  },
+  "globIgnorePaths": [
+    "**/*.spec.lua"
+  ]
+}

rojo-test/build-tests/ignore_glob_spec/src/shouldBeIncluded.lua

@@ -0,0 +1 @@
+-- this file should be present

rojo-test/build-tests/ignore_glob_spec/src/shouldBeRemoved.spec.lua

@@ -0,0 +1 @@
+-- this file should not be present

rojo-test/src/build_test.rs

@@ -45,6 +45,13 @@ gen_build_tests! {
     txt_in_folder,
 }
 
+#[cfg(feature = "unstable_glob_ignore_paths")]
+gen_build_tests! {
+    ignore_glob_inner,
+    ignore_glob_nested,
+    ignore_glob_spec,
+}
+
 #[test]
 fn build_plain_txt() {
     run_build_test("plain.txt");

src/common_setup.rs

@@ -8,7 +8,8 @@ use rbx_dom_weak::RbxInstanceProperties;
 use crate::{
     project::Project,
     snapshot::{
-        apply_patch_set, compute_patch_set, InstanceContext, InstancePropertiesWithMeta, RojoTree,
+        apply_patch_set, compute_patch_set, InstanceContext, InstancePropertiesWithMeta,
+        PathIgnoreRule, RojoTree,
     },
     snapshot_middleware::snapshot_from_vfs,
     vfs::{Vfs, VfsFetcher},
@@ -38,8 +39,19 @@ pub fn start<F: VfsFetcher>(
         .get(fuzzy_project_path)
         .expect("could not get project path");
 
+    let mut instance_context = InstanceContext::default();
+
+    if let Some(project) = &maybe_project {
+        let rules = project.glob_ignore_paths.iter().map(|glob| PathIgnoreRule {
+            glob: glob.clone(),
+            base_path: project.folder_location().to_path_buf(),
+        });
+
+        instance_context.add_path_ignore_rules(rules);
+    }
+
     log::trace!("Generating snapshot of instances from VFS");
-    let snapshot = snapshot_from_vfs(&InstanceContext::default(), vfs, &entry)
+    let snapshot = snapshot_from_vfs(&instance_context, vfs, &entry)
         .expect("snapshot failed")
         .expect("snapshot did not return an instance");
 

src/glob.rs

@@ -0,0 +1,50 @@
+//! Wrapper around globset's Glob type that has better serialization
+//! characteristics by coupling Glob and GlobMatcher into a single type.
+
+use std::path::Path;
+
+use globset::{Glob as InnerGlob, GlobMatcher};
+use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer};
+
+pub use globset::Error;
+
+#[derive(Debug, Clone)]
+pub struct Glob {
+    inner: InnerGlob,
+    matcher: GlobMatcher,
+}
+
+impl Glob {
+    pub fn new(glob: &str) -> Result<Self, Error> {
+        let inner = InnerGlob::new(glob)?;
+        let matcher = inner.compile_matcher();
+
+        Ok(Glob { inner, matcher })
+    }
+
+    pub fn is_match<P: AsRef<Path>>(&self, path: P) -> bool {
+        self.matcher.is_match(path)
+    }
+}
+
+impl PartialEq for Glob {
+    fn eq(&self, other: &Self) -> bool {
+        self.inner == other.inner
+    }
+}
+
+impl Eq for Glob {}
+
+impl Serialize for Glob {
+    fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
+        serializer.serialize_str(self.inner.glob())
+    }
+}
+
+impl<'de> Deserialize<'de> for Glob {
+    fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
+        let glob = <&str as Deserialize>::deserialize(deserializer)?;
+
+        Glob::new(glob).map_err(D::Error::custom)
+    }
+}

src/lib.rs

@@ -10,6 +10,7 @@ mod tree_view;
 mod auth_cookie;
 mod change_processor;
 mod common_setup;
+mod glob;
 mod message_queue;
 mod multimap;
 mod path_map;

src/project.rs

@@ -8,6 +8,8 @@ use rbx_dom_weak::UnresolvedRbxValue;
 use serde::{Deserialize, Serialize};
 use snafu::{ResultExt, Snafu};
 
+use crate::glob::Glob;
+
 static PROJECT_FILENAME: &str = "default.project.json";
 
 /// Error type returned by any function that handles projects.
@@ -52,6 +54,12 @@ pub struct Project {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub serve_place_ids: Option<HashSet<u64>>,
 
+    /// 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")]
+    #[cfg_attr(not(feature = "unstable_glob_ignore_paths"), serde(skip))]
+    pub glob_ignore_paths: Vec<Glob>,
+
     /// The path to the file that this project came from. Relative paths in the
     /// project should be considered relative to the parent of this field, also
     /// given by `Project::folder_location`.

src/snapshot/metadata.rs

@@ -1,11 +1,12 @@
 use std::{
     fmt,
     path::{Path, PathBuf},
+    sync::Arc,
 };
 
 use serde::{Deserialize, Serialize};
 
-use crate::{path_serializer, project::ProjectNode};
+use crate::{glob::Glob, path_serializer, project::ProjectNode};
 
 /// Rojo-specific metadata that can be associated with an instance or a snapshot
 /// of an instance.
@@ -99,11 +100,59 @@ impl Default for InstanceMetadata {
 }
 
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
-pub struct InstanceContext {}
+pub struct InstanceContext {
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    pub path_ignore_rules: Arc<Vec<PathIgnoreRule>>,
+}
+
+impl InstanceContext {
+    /// Extend the list of ignore rules in the context with the given new rules.
+    pub fn add_path_ignore_rules<I>(&mut self, new_rules: I)
+    where
+        I: IntoIterator<Item = PathIgnoreRule>,
+        I::IntoIter: ExactSizeIterator,
+    {
+        let new_rules = new_rules.into_iter();
+
+        // If the iterator is empty, we can skip cloning our list of ignore
+        // rules and appending to it.
+        if new_rules.len() == 0 {
+            return;
+        }
+
+        let rules = Arc::make_mut(&mut self.path_ignore_rules);
+        rules.extend(new_rules);
+    }
+}
 
 impl Default for InstanceContext {
     fn default() -> Self {
-        InstanceContext {}
+        InstanceContext {
+            path_ignore_rules: Arc::new(Vec::new()),
+        }
+    }
+}
+
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+pub struct PathIgnoreRule {
+    /// The path that this glob is relative to. Since ignore globs are defined
+    /// in project files, this will generally be the folder containing the
+    /// project file that defined this glob.
+    #[serde(serialize_with = "path_serializer::serialize_absolute")]
+    pub base_path: PathBuf,
+
+    /// The actual glob that can be matched against the input path.
+    pub glob: Glob,
+}
+
+impl PathIgnoreRule {
+    pub fn passes<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),
+            Err(_) => true,
+        }
     }
 }
 

src/snapshot_middleware/dir.rs

@@ -26,11 +26,16 @@ impl SnapshotMiddleware for SnapshotDir {
             return Ok(None);
         }
 
-        let children: Vec<VfsEntry> = entry.children(vfs)?;
+        let passes_filter_rules = |child: &VfsEntry| {
+            context
+                .path_ignore_rules
+                .iter()
+                .all(|rule| rule.passes(child.path()))
+        };
 
         let mut snapshot_children = Vec::new();
 
-        for child in children.into_iter() {
+        for child in entry.children(vfs)?.into_iter().filter(passes_filter_rules) {
             if let Some(child_snapshot) = snapshot_from_vfs(context, vfs, &child)? {
                 snapshot_children.push(child_snapshot);
             }

src/snapshot_middleware/project.rs

@@ -4,7 +4,9 @@ use rbx_reflection::try_resolve_value;
 
 use crate::{
     project::{Project, ProjectNode},
-    snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource},
+    snapshot::{
+        InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource, PathIgnoreRule,
+    },
     vfs::{FsResultExt, Vfs, VfsEntry, VfsFetcher},
 };
 
@@ -45,10 +47,19 @@ impl SnapshotMiddleware for SnapshotProject {
         let project = Project::load_from_slice(&entry.contents(vfs)?, entry.path())
             .map_err(|err| SnapshotError::malformed_project(err, entry.path()))?;
 
+        let mut context = context.clone();
+
+        let rules = project.glob_ignore_paths.iter().map(|glob| PathIgnoreRule {
+            glob: glob.clone(),
+            base_path: project.folder_location().to_path_buf(),
+        });
+
+        context.add_path_ignore_rules(rules);
+
         // Snapshotting a project should always return an instance, so this
         // unwrap is safe.
         let mut snapshot = snapshot_project_node(
-            context,
+            &context,
             project.folder_location(),
             &project.name,
             &project.tree,