Fix off-by-one error in :prune filter (#1522)

christian-schilling · web-flow · commit 9ad455b6c7a7 · 2025-10-12T08:55:13.000Z
As the docs say this should only apply to merge
commits.
Unfortunately this means we have to bump CACHE_VERSION
in case someone has already used this.

Change: fix-prune
diff --git a/josh-core/src/cache.rs b/josh-core/src/cache.rs
@@ -1,7 +1,7 @@
 use super::*;
 use std::collections::HashMap;
 
-const CACHE_VERSION: u64 = 22;
+const CACHE_VERSION: u64 = 23;
 
 lazy_static! {
     static ref DB: std::sync::Mutex<Option<sled::Db>> = std::sync::Mutex::new(None);
diff --git a/josh-core/src/filter/mod.rs b/josh-core/src/filter/mod.rs
@@ -816,7 +816,7 @@ fn apply_to_commit2(
         Op::Prune => {
             let p: Vec<_> = commit.parent_ids().collect();
 
-            if p.len() > 0 {
+            if p.len() > 1 {
                 let parent = some_or!(transaction.get(filter, p[0]), {
                     return Ok(None);
                 });
diff --git a/tests/filter/prune_trivial_merge.t b/tests/filter/prune_trivial_merge.t
@@ -21,27 +21,36 @@
   $ git add .
   $ git commit -m "mod file1" 1> /dev/null
 
+  $ git commit --allow-empty -m "empty commit" 1> /dev/null
+
   $ git merge -q  branch1 --no-ff
   $ git log --graph --pretty=%s
   *   Merge branch 'branch1'
   |\  
   | * add file2
+  * | empty commit
   * | mod file1
   |/  
   * add file1
 
   $ josh-filter -s ::file1
-  [3] ::file1
+  [4] ::file1
   $ git log --graph --pretty=%s FILTERED_HEAD
   *   Merge branch 'branch1'
   |\  
+  * | empty commit
   * | mod file1
   |/  
   * add file1
   $ josh-filter -s ::file1:prune=trivial-merge
-  [2] :prune=trivial-merge
-  [3] ::file1
+  [3] :prune=trivial-merge
+  [4] ::file1
 
   $ git log --graph --pretty=%s FILTERED_HEAD
+  * empty commit
   * mod file1
   * add file1
+
+
+
+
diff --git a/tests/proxy/amend_patchset.t b/tests/proxy/amend_patchset.t
@@ -124,7 +124,7 @@
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/authentication.t b/tests/proxy/authentication.t
@@ -124,7 +124,7 @@
   "real_repo.git" = ["::sub1/"]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/caching.t b/tests/proxy/caching.t
@@ -51,7 +51,7 @@
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
@@ -162,7 +162,7 @@
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_absent_head.t b/tests/proxy/clone_absent_head.t
@@ -85,7 +85,7 @@
   $ bash ${TESTDIR}/destroy_test_env.sh
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- 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
-  |   `-- 22
+  |   `-- 23
   |       `-- 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
-  |   `-- 22
+  |   `-- 23
   |       `-- 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
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_locked_refs.t b/tests/proxy/clone_locked_refs.t
@@ -111,7 +111,7 @@
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_prefix.t b/tests/proxy/clone_prefix.t
@@ -74,7 +74,7 @@
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_sha.t b/tests/proxy/clone_sha.t
@@ -93,7 +93,7 @@ Check (2) and (3) but with a branch ref
   "real_repo.git" = ["::sub1/"]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_subsubtree.t b/tests/proxy/clone_subsubtree.t
@@ -87,7 +87,7 @@
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_subtree.t b/tests/proxy/clone_subtree.t
@@ -85,7 +85,7 @@
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- 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
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_subtree_tags.t b/tests/proxy/clone_subtree_tags.t
@@ -117,7 +117,7 @@
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/clone_with_meta.t b/tests/proxy/clone_with_meta.t
@@ -129,7 +129,7 @@
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- 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
-  |   `-- 22
+  |   `-- 23
   |       `-- 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
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/import_export.t b/tests/proxy/import_export.t
@@ -301,7 +301,7 @@ Flushed credential cache
   "repo2.git" = [":prefix=repo2"]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/join_subtree.t b/tests/proxy/join_subtree.t
@@ -132,7 +132,7 @@ Make sure all temporary namespace got removed
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- 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
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/markers.t b/tests/proxy/markers.t
@@ -340,7 +340,7 @@
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- 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
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/no_proxy_lfs.t b/tests/proxy/no_proxy_lfs.t
@@ -50,7 +50,7 @@
   $ bash ${TESTDIR}/destroy_test_env.sh
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_graphql.t b/tests/proxy/push_graphql.t
@@ -69,7 +69,7 @@
   "real_repo.git" = ["::sub1/"]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_new_branch.t b/tests/proxy/push_new_branch.t
@@ -114,7 +114,7 @@ Check the branch again
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_prefix.t b/tests/proxy/push_prefix.t
@@ -56,7 +56,7 @@
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_review.t b/tests/proxy/push_review.t
@@ -87,7 +87,7 @@ Make sure all temporary namespace got removed
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- 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
-  |   `-- 22
+  |   `-- 23
   |       `-- 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
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_review_old.t b/tests/proxy/push_review_old.t
@@ -85,7 +85,7 @@ Flushed credential cache
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_review_topic.t b/tests/proxy/push_review_topic.t
@@ -42,7 +42,7 @@ Make sure all temporary namespace got removed
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_stacked.t b/tests/proxy/push_stacked.t
@@ -151,7 +151,7 @@ Make sure all temporary namespace got removed
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_stacked_gerrit.t b/tests/proxy/push_stacked_gerrit.t
@@ -134,7 +134,7 @@ Make sure all temporary namespace got removed
   "real_repo.git" = ["::sub1/"]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- 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
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_stacked_sub.t b/tests/proxy/push_stacked_sub.t
@@ -132,7 +132,7 @@ Make sure all temporary namespace got removed
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_subdir_prefix.t b/tests/proxy/push_subdir_prefix.t
@@ -48,7 +48,7 @@
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_subtree.t b/tests/proxy/push_subtree.t
@@ -90,7 +90,7 @@ Make sure all temporary namespace got removed
   ]
   .
   |-- josh
-  |   `-- 22
+  |   `-- 23
   |       `-- sled
   |           |-- blobs
   |           |-- conf
diff --git a/tests/proxy/push_subtree_two_repos.t b/tests/proxy/push_subtree_two_repos.t
diff --git a/tests/proxy/unrelated_leak.t b/tests/proxy/unrelated_leak.t
diff --git a/tests/proxy/workspace.t b/tests/proxy/workspace.t
diff --git a/tests/proxy/workspace_create.t b/tests/proxy/workspace_create.t
diff --git a/tests/proxy/workspace_discover.t b/tests/proxy/workspace_discover.t
diff --git a/tests/proxy/workspace_edit_commit.t b/tests/proxy/workspace_edit_commit.t
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
`@@ -124,7 +124,7 @@`
`124`	`124`	`]`
`125`	`125`	`.`
`126`	`126`	`\|-- josh`
`127`		- \| `-- 22
	`127`	+ \| `-- 23
`128`	`128`	\| `-- sled
`129`	`129`	`\| \|-- blobs`
`130`	`130`	`\| \|-- conf`
Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,7 @@`
`51`	`51`	`]`
`52`	`52`	`.`
`53`	`53`	`\|-- josh`
`54`		- \| `-- 22
	`54`	+ \| `-- 23
`55`	`55`	\| `-- sled
`56`	`56`	`\| \|-- blobs`
`57`	`57`	`\| \|-- conf`
`@@ -162,7 +162,7 @@`
`162`	`162`	`]`
`163`	`163`	`.`
`164`	`164`	`\|-- josh`
`165`		- \| `-- 22
	`165`	+ \| `-- 23
`166`	`166`	\| `-- sled
`167`	`167`	`\| \|-- blobs`
`168`	`168`	`\| \|-- conf`
Original file line number	Diff line number	Diff line change
`@@ -85,7 +85,7 @@`
`85`	`85`	`$ bash ${TESTDIR}/destroy_test_env.sh`
`86`	`86`	`.`
`87`	`87`	`\|-- josh`
`88`		- \| `-- 22
	`88`	+ \| `-- 23
`89`	`89`	\| `-- sled
`90`	`90`	`\| \|-- blobs`
`91`	`91`	`\| \|-- 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`		- \| `-- 22
	`56`	+ \| `-- 23
`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`		- \| `-- 22
	`12`	+ \| `-- 23
`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`		- \| `-- 22
	`35`	+ \| `-- 23
`36`	`36`	\| `-- sled
`37`	`37`	`\| \|-- blobs`
`38`	`38`	`\| \|-- conf`