Don't change order of filters that share a prefix

This fixed an issue where pushing through workspace
does not work correctly:
The workspace root was sorted to the back and then
reversed first, resulting in all mapped files being
moved to the workspace dir.

Author: christian-schilling

.github/workflows/rust.yml

@@ -30,6 +30,8 @@ jobs:
     - uses: actions/checkout@v2
     - name: Check format
       run: cargo fmt -- --check
+    - name: Run Rust tests
+      run: cargo test
     - name: Configure environment
       run: |
         export CARGO_TARGET_DIR=`pwd`/target

src/cache.rs

@@ -1,7 +1,7 @@
 use super::*;
 use std::collections::HashMap;
 
-const VERSION: u64 = 4;
+const VERSION: u64 = 5;
 
 lazy_static! {
     static ref DB: std::sync::Mutex<Option<sled::Db>> =

src/filter/mod.rs

@@ -175,6 +175,34 @@ fn spec2(op: &Op) -> String {
     }
 }
 
+pub fn src_path(filter: Filter) -> std::path::PathBuf {
+    src_path2(&to_op(filter))
+}
+
+fn src_path2(op: &Op) -> std::path::PathBuf {
+    normalize_path(&match op {
+        Op::Subdir(path) => path.to_owned(),
+        Op::File(path) => path.to_owned(),
+        Op::Chain(a, b) => src_path(*a).join(src_path(*b)),
+        _ => std::path::PathBuf::new(),
+    })
+    .to_owned()
+}
+
+pub fn dst_path(filter: Filter) -> std::path::PathBuf {
+    dst_path2(&to_op(filter))
+}
+
+fn dst_path2(op: &Op) -> std::path::PathBuf {
+    normalize_path(&match op {
+        Op::Prefix(path) => path.to_owned(),
+        Op::File(path) => path.to_owned(),
+        Op::Chain(a, b) => dst_path(*b).join(dst_path(*a)),
+        _ => std::path::PathBuf::new(),
+    })
+    .to_owned()
+}
+
 /// Calculate the filtered commit for `commit`. This can take some time if done
 /// for the first time and thus should generally be done asynchronously.
 pub fn apply_to_commit(
@@ -670,3 +698,32 @@ pub fn chain(first: Filter, second: Filter) -> Filter {
 pub fn compose(first: Filter, second: Filter) -> Filter {
     opt::optimize(to_filter(Op::Compose(vec![first, second])))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::path::PathBuf;
+
+    #[test]
+    fn src_path_test() {
+        assert_eq!(PathBuf::from("x"), src_path(parse(":/x").unwrap()));
+        assert_eq!(PathBuf::from("x/y"), src_path(parse(":/x/y").unwrap()));
+        assert_eq!(PathBuf::from("x/y"), src_path(parse(":/x::y").unwrap()));
+    }
+
+    #[test]
+    fn dst_path_test() {
+        assert_eq!(PathBuf::from(""), dst_path(parse(":/x").unwrap()));
+        assert_eq!(PathBuf::from(""), dst_path(parse(":/x/y").unwrap()));
+        assert_eq!(PathBuf::from("y"), dst_path(parse(":/x::y").unwrap()));
+        assert_eq!(
+            PathBuf::from("a/y"),
+            dst_path(parse(":[a=:/x::y/]").unwrap())
+        );
+
+        assert_eq!(
+            PathBuf::from("c/a"),
+            dst_path(parse(":[a=:/x::y/,a/b=:/i]:prefix=c").unwrap())
+        );
+    }
+}

src/filter/opt.rs

@@ -201,17 +201,19 @@ fn last_chain(rest: Filter, filter: Filter) -> (Filter, Filter) {
 
 fn prefix_sort(filters: &Vec<Filter>) -> Vec<Filter> {
     let mut sorted = filters.clone();
-    let mut ok = true;
     sorted.sort_by(|a, b| {
-        if let (Op::Chain(a, _), Op::Chain(b, _)) = (to_op(*a), to_op(*b)) {
-            if let (Op::Subdir(a), Op::Subdir(b)) = (to_op(a), to_op(b)) {
-                return a.partial_cmp(&b).unwrap();
-            }
+        let (src_a, src_b) = (src_path(*a), src_path(*b));
+        if src_a.starts_with(&src_b) || src_b.starts_with(&src_a) {
+            return std::cmp::Ordering::Equal;
+        }
+        let (dst_a, dst_b) = (dst_path(*a), dst_path(*b));
+        if dst_a.starts_with(&dst_b) || dst_b.starts_with(&dst_a) {
+            return std::cmp::Ordering::Equal;
         }
-        ok = false;
-        std::cmp::Ordering::Equal
+
+        return (&src_a, &dst_a).partial_cmp(&(&src_b, &dst_b)).unwrap();
     });
-    return if ok { sorted } else { filters.clone() };
+    return sorted;
 }
 
 fn common_pre(filters: &Vec<Filter>) -> Option<(Filter, Vec<Filter>)> {

tests/filter/pretty_print.t

@@ -182,3 +182,22 @@
       ::subsub1/
       ::subsub2/
   ]
+
+Subdir only filters should not reorder filters that share a prefix
+  $ cat > f <<EOF
+  > a/subsub1 = :/sub1/subsub1
+  > :/x/subsub2
+  > EOF
+
+  $ josh-filter -p --file f
+  a/subsub1 = :/sub1/subsub1
+  :/x/subsub2
+
+  $ cat > f <<EOF
+  > :/x/subsub2
+  > a/subsub1 = :/sub1/subsub1
+  > EOF
+
+  $ josh-filter -p --file f
+  :/x/subsub2
+  a/subsub1 = :/sub1/subsub1