Fix prefix/subdir optimisation bug

christian-schilling · LMG · commit 9bd80f4dd11c · 2022-11-15T16:13:56.000+01:00
The optimiser rule was wrong in case the subdir filter path was a
prefix of the prefix filter. It resulted in :empty in this case,
which is wrong.

Change: fix-prefix-subdir
diff --git a/src/cache.rs b/src/cache.rs
@@ -1,7 +1,7 @@
 use super::*;
 use std::collections::HashMap;
 
-const CACHE_VERSION: u64 = 13;
+const CACHE_VERSION: u64 = 14;
 
 lazy_static! {
     static ref DB: std::sync::Mutex<Option<sled::Db>> = std::sync::Mutex::new(None);
diff --git a/src/filter/opt.rs b/src/filter/opt.rs
@@ -353,7 +353,14 @@ fn step(filter: Filter) -> Filter {
         Op::Chain(a, b) => match (to_op(a), to_op(b)) {
             (Op::Chain(x, y), b) => Op::Chain(x, to_filter(Op::Chain(y, to_filter(b)))),
             (Op::Prefix(a), Op::Subdir(b)) if a == b => Op::Nop,
-            (Op::Prefix(a), Op::Subdir(b)) if a != b => Op::Empty,
+            (Op::Prefix(a), Op::Subdir(b))
+                if a != b && a.components().count() == b.components().count() =>
+            {
+                Op::Empty
+            }
+            (Op::Prefix(a), Op::Subdir(b)) if a != b => {
+                Op::Prefix(a.strip_prefix(&b).unwrap_or(&a).to_owned())
+            }
             (Op::Nop, b) => b,
             (a, Op::Nop) => a,
             (Op::Empty, _) => Op::Empty,
diff --git a/tests/filter/pretty_print.t b/tests/filter/pretty_print.t
@@ -12,6 +12,8 @@
   :/"a%\"$"
   $ josh-filter -p :/a:/b
   :/a/b
+  $ josh-filter -p :prefix=x/y:/x
+  :prefix=y
   $ josh-filter -p :[:/a:/b,:/a/b]
   :/a/b
   $ josh-filter -p :[:empty,:/a]
diff --git a/tests/proxy/amend_patchset.t b/tests/proxy/amend_patchset.t
@@ -121,7 +121,7 @@
   "real_repo.git" = [':/sub3']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/authentication.t b/tests/proxy/authentication.t
@@ -124,7 +124,7 @@
   "real_repo.git" = [':/sub1']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/caching.t b/tests/proxy/caching.t
@@ -48,7 +48,7 @@
   "real_repo.git" = [':/sub1']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
@@ -156,7 +156,7 @@
   "real_repo.git" = [':/sub1']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_absent_head.t b/tests/proxy/clone_absent_head.t
@@ -87,7 +87,7 @@
   $ bash ${TESTDIR}/destroy_test_env.sh
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_all.t b/tests/proxy/clone_all.t
@@ -53,7 +53,7 @@
   "real_repo.git" = [':/sub1']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_blocked.t b/tests/proxy/clone_blocked.t
@@ -9,7 +9,7 @@
   $ bash ${TESTDIR}/destroy_test_env.sh
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_invalid_url.t b/tests/proxy/clone_invalid_url.t
@@ -32,7 +32,7 @@
   $ bash ${TESTDIR}/destroy_test_env.sh
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_locked_refs.t b/tests/proxy/clone_locked_refs.t
@@ -111,7 +111,7 @@
   ]
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_prefix.t b/tests/proxy/clone_prefix.t
@@ -74,7 +74,7 @@
   ]
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_sha.t b/tests/proxy/clone_sha.t
@@ -72,7 +72,7 @@
   "real_repo.git" = [':/sub1']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_subsubtree.t b/tests/proxy/clone_subsubtree.t
@@ -85,7 +85,7 @@
   ]
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_subtree.t b/tests/proxy/clone_subtree.t
@@ -84,7 +84,7 @@
   ]
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_subtree_no_master.t b/tests/proxy/clone_subtree_no_master.t
@@ -78,7 +78,7 @@
   "real_repo.git" = [':/sub1']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_subtree_tags.t b/tests/proxy/clone_subtree_tags.t
@@ -114,7 +114,7 @@
   ]
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_with_meta.t b/tests/proxy/clone_with_meta.t
@@ -129,7 +129,7 @@
   ]
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/empty_commit.t b/tests/proxy/empty_commit.t
@@ -83,7 +83,7 @@ should still be included.
   "real_repo.git" = [':/sub1']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/get_version.t b/tests/proxy/get_version.t
@@ -7,7 +7,7 @@
   $ bash ${TESTDIR}/destroy_test_env.sh
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/import_export.t b/tests/proxy/import_export.t
@@ -300,7 +300,7 @@ Flushed credential cache
   "repo2.git" = [':prefix=repo2']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/join_with_merge.t b/tests/proxy/join_with_merge.t
@@ -59,7 +59,7 @@
   "real_repo.git" = [':prefix=sub1']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/markers.t b/tests/proxy/markers.t
@@ -328,7 +328,7 @@
   ]
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/no_proxy.t b/tests/proxy/no_proxy.t
@@ -35,7 +35,7 @@
   $ bash ${TESTDIR}/destroy_test_env.sh
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_new_branch.t b/tests/proxy/push_new_branch.t
@@ -94,7 +94,7 @@ Check the branch again
   "real_repo.git" = [':/sub']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_prefix.t b/tests/proxy/push_prefix.t
@@ -56,7 +56,7 @@
   ]
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_review.t b/tests/proxy/push_review.t
@@ -84,7 +84,7 @@ Make sure all temporary namespace got removed
   "real_repo.git" = [':/sub1']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_review_already_in.t b/tests/proxy/push_review_already_in.t
@@ -48,7 +48,7 @@ test for that.
   "real_repo.git" = []
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_review_nop_behind.t b/tests/proxy/push_review_nop_behind.t
@@ -55,7 +55,7 @@ This is a regression test for that problem.
   "real_repo.git" = []
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_review_old.t b/tests/proxy/push_review_old.t
@@ -82,7 +82,7 @@ Flushed credential cache
   "real_repo.git" = [':/sub1']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_review_topic.t b/tests/proxy/push_review_topic.t
@@ -39,7 +39,7 @@ Make sure all temporary namespace got removed
   "real_repo.git" = [':/sub1']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_stacked.t b/tests/proxy/push_stacked.t
@@ -145,7 +145,7 @@ Make sure all temporary namespace got removed
   ]
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_stacked_split.t b/tests/proxy/push_stacked_split.t
@@ -76,7 +76,7 @@ Make sure all temporary namespace got removed
   "real_repo.git" = [':/sub1']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_stacked_sub.t b/tests/proxy/push_stacked_sub.t
@@ -129,7 +129,7 @@ Make sure all temporary namespace got removed
   "real_repo.git" = [':/sub1']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_subdir_prefix.t b/tests/proxy/push_subdir_prefix.t
@@ -48,7 +48,7 @@
   ]
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_subtree.t b/tests/proxy/push_subtree.t
@@ -87,7 +87,7 @@ Make sure all temporary namespace got removed
   "real_repo.git" = [':/sub1']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_subtree_two_repos.t b/tests/proxy/push_subtree_two_repos.t
@@ -112,7 +112,7 @@ Put a double slash in the URL to see that it also works
   "real_repo.git" = [':/sub1']
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/unrelated_leak.t b/tests/proxy/unrelated_leak.t
@@ -132,7 +132,7 @@ Flushed credential cache
   ]
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/workspace.t b/tests/proxy/workspace.t
@@ -225,7 +225,7 @@
   ]
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/workspace_create.t b/tests/proxy/workspace_create.t
@@ -442,7 +442,7 @@ Note that ws/d/ is now present in the ws
   ]
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/workspace_discover.t b/tests/proxy/workspace_discover.t
@@ -102,7 +102,7 @@
   ]
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/workspace_edit_commit.t b/tests/proxy/workspace_edit_commit.t
@@ -181,7 +181,7 @@
   ]
   .
   |-- josh
-  |   `-- 13
+  |   `-- 14
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/workspace_in_workspace.t b/tests/proxy/workspace_in_workspace.t
diff --git a/tests/proxy/workspace_invalid_trailing_slash.t b/tests/proxy/workspace_invalid_trailing_slash.t
diff --git a/tests/proxy/workspace_modify.t b/tests/proxy/workspace_modify.t
diff --git a/tests/proxy/workspace_modify_chain.t b/tests/proxy/workspace_modify_chain.t
diff --git a/tests/proxy/workspace_modify_chain_prefix_subtree.t b/tests/proxy/workspace_modify_chain_prefix_subtree.t
diff --git a/tests/proxy/workspace_pre_history.t b/tests/proxy/workspace_pre_history.t
diff --git a/tests/proxy/workspace_publish.t b/tests/proxy/workspace_publish.t
diff --git a/tests/proxy/workspace_tags.t b/tests/proxy/workspace_tags.t

Original file line number	Diff line number	Diff line change
`@@ -121,7 +121,7 @@`
`121`	`121`	`"real_repo.git" = [':/sub3']`
`122`	`122`	`.`
`123`	`123`	`\|-- josh`
`124`		- \| `-- 13
	`124`	+ \| `-- 14
`125`	`125`	\| `-- sled
`126`	`126`	`\| \|-- blobs`
`127`	`127`	`\| \|-- conf`
Original file line number	Diff line number	Diff line change
`@@ -124,7 +124,7 @@`
`124`	`124`	`"real_repo.git" = [':/sub1']`
`125`	`125`	`.`
`126`	`126`	`\|-- josh`
`127`		- \| `-- 13
	`127`	+ \| `-- 14
`128`	`128`	\| `-- sled
`129`	`129`	`\| \|-- blobs`
`130`	`130`	`\| \|-- conf`
Original file line number	Diff line number	Diff line change
`@@ -87,7 +87,7 @@`
`87`	`87`	`$ bash ${TESTDIR}/destroy_test_env.sh`
`88`	`88`	`.`
`89`	`89`	`\|-- josh`
`90`		- \| `-- 13
	`90`	+ \| `-- 14
`91`	`91`	\| `-- sled
`92`	`92`	`\| \|-- blobs`
`93`	`93`	`\| \|-- conf`
Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@`
`53`	`53`	`"real_repo.git" = [':/sub1']`
`54`	`54`	`.`
`55`	`55`	`\|-- josh`
`56`		- \| `-- 13
	`56`	+ \| `-- 14
`57`	`57`	\| `-- sled
`58`	`58`	`\| \|-- blobs`
`59`	`59`	`\| \|-- conf`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@`
`9`	`9`	`$ bash ${TESTDIR}/destroy_test_env.sh`
`10`	`10`	`.`
`11`	`11`	`\|-- josh`
`12`		- \| `-- 13
	`12`	+ \| `-- 14
`13`	`13`	\| `-- sled
`14`	`14`	`\| \|-- blobs`
`15`	`15`	`\| \|-- conf`
Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,7 @@`
`32`	`32`	`$ bash ${TESTDIR}/destroy_test_env.sh`
`33`	`33`	`.`
`34`	`34`	`\|-- josh`
`35`		- \| `-- 13
	`35`	+ \| `-- 14
`36`	`36`	\| `-- sled
`37`	`37`	`\| \|-- blobs`
`38`	`38`	`\| \|-- conf`
Original file line number	Diff line number	Diff line change
`@@ -111,7 +111,7 @@`
`111`	`111`	`]`
`112`	`112`	`.`
`113`	`113`	`\|-- josh`
`114`		- \| `-- 13
	`114`	+ \| `-- 14
`115`	`115`	\| `-- sled
`116`	`116`	`\| \|-- blobs`
`117`	`117`	`\| \|-- conf`
Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@`
`74`	`74`	`]`
`75`	`75`	`.`
`76`	`76`	`\|-- josh`
`77`		- \| `-- 13
	`77`	+ \| `-- 14
`78`	`78`	\| `-- sled
`79`	`79`	`\| \|-- blobs`
`80`	`80`	`\| \|-- conf`
Original file line number	Diff line number	Diff line change
`@@ -72,7 +72,7 @@`
`72`	`72`	`"real_repo.git" = [':/sub1']`
`73`	`73`	`.`
`74`	`74`	`\|-- josh`
`75`		- \| `-- 13
	`75`	+ \| `-- 14
`76`	`76`	\| `-- sled
`77`	`77`	`\| \|-- blobs`
`78`	`78`	`\| \|-- conf`
Original file line number	Diff line number	Diff line change
`@@ -85,7 +85,7 @@`
`85`	`85`	`]`
`86`	`86`	`.`
`87`	`87`	`\|-- josh`
`88`		- \| `-- 13
	`88`	+ \| `-- 14
`89`	`89`	\| `-- sled
`90`	`90`	`\| \|-- blobs`
`91`	`91`	`\| \|-- conf`