Skip to content

Commit

Permalink
Fix optimisation bug for compose filter (#1159)
Browse files Browse the repository at this point in the history
The optimiser incorrectly transformed :[:/y,:/x/y] into :[:/,:/x]:/y
This is not correct because :/y is more restrictive than :/ and will
therefore not match all the files that :/ does.
Due to the compose filter guaranteeing uniqueness of mappings
the led to files missing from the first filter.

Change: fix-opt-bug
  • Loading branch information
christian-schilling authored Mar 3, 2023
1 parent a1c4ba4 commit fc857af
Show file tree
Hide file tree
Showing 51 changed files with 254 additions and 70 deletions.
2 changes: 1 addition & 1 deletion josh-core/src/cache.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::*;
use std::collections::HashMap;

const CACHE_VERSION: u64 = 14;
const CACHE_VERSION: u64 = 15;

lazy_static! {
static ref DB: std::sync::Mutex<Option<sled::Db>> = std::sync::Mutex::new(None);
Expand Down
7 changes: 3 additions & 4 deletions josh-core/src/filter/opt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,10 +248,9 @@ fn common_post(filters: &Vec<Filter>) -> Option<(Filter, Vec<Filter>)> {
}
}
}
if Some(to_filter(Op::Nop)) == c {
None
} else {
c.map(|c| (c, rest))
match c.map(to_op) {
Some(Op::Prefix(_)) => c.map(|c| (c, rest)),
_ => None,
}
}

Expand Down
131 changes: 131 additions & 0 deletions tests/filter/compose_shadow_dir_same_name.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
$ export TESTTMP=${PWD}

$ cd ${TESTTMP}
$ git init -q real_repo 1> /dev/null
$ cd real_repo

$ mkdir sub1
$ echo contents1 > sub1/file1
$ git add sub1
$ git commit -m "add file1" 1> /dev/null

$ mkdir xx
$ echo contents1 > xx/file2
$ git add xx
$ git commit -m "add file2" 1> /dev/null

$ mkdir -p sub/xx
$ echo contents1 > sub/xx/file3
$ echo contents1 > sub/xx/file4
$ git add sub
$ git commit -m "add file3" 1> /dev/null

$ git diff ${EMPTY_TREE}..HEAD
diff --git a/sub/xx/file3 b/sub/xx/file3
new file mode 100644
index 0000000..a024003
--- /dev/null
+++ b/sub/xx/file3
@@ -0,0 +1 @@
+contents1
diff --git a/sub/xx/file4 b/sub/xx/file4
new file mode 100644
index 0000000..a024003
--- /dev/null
+++ b/sub/xx/file4
@@ -0,0 +1 @@
+contents1
diff --git a/sub1/file1 b/sub1/file1
new file mode 100644
index 0000000..a024003
--- /dev/null
+++ b/sub1/file1
@@ -0,0 +1 @@
+contents1
diff --git a/xx/file2 b/xx/file2
new file mode 100644
index 0000000..a024003
--- /dev/null
+++ b/xx/file2
@@ -0,0 +1 @@
+contents1


$ josh-filter ":[:/sub1,:/xx]"
$ git diff ${EMPTY_TREE}..FILTERED_HEAD
diff --git a/file1 b/file1
new file mode 100644
index 0000000..a024003
--- /dev/null
+++ b/file1
@@ -0,0 +1 @@
+contents1
diff --git a/file2 b/file2
new file mode 100644
index 0000000..a024003
--- /dev/null
+++ b/file2
@@ -0,0 +1 @@
+contents1

$ josh-filter ":[:/xx,:/sub1]"
$ git diff ${EMPTY_TREE}..FILTERED_HEAD
diff --git a/file1 b/file1
new file mode 100644
index 0000000..a024003
--- /dev/null
+++ b/file1
@@ -0,0 +1 @@
+contents1
diff --git a/file2 b/file2
new file mode 100644
index 0000000..a024003
--- /dev/null
+++ b/file2
@@ -0,0 +1 @@
+contents1

$ josh-filter -s ":[:/sub/xx::file3,:/sub1,:/xx,:/sub/xx]"
[1] :/sub1
[1] ::file3
[2] :/sub
[3] :/xx
[3] :[
:/sub/xx::file3
:/sub1
:/xx
:/sub/xx
]
[3] :[
:/xx
:/sub/xx
]
$ git diff ${EMPTY_TREE}..FILTERED_HEAD
diff --git a/file1 b/file1
new file mode 100644
index 0000000..a024003
--- /dev/null
+++ b/file1
@@ -0,0 +1 @@
+contents1
diff --git a/file2 b/file2
new file mode 100644
index 0000000..a024003
--- /dev/null
+++ b/file2
@@ -0,0 +1 @@
+contents1
diff --git a/file3 b/file3
new file mode 100644
index 0000000..a024003
--- /dev/null
+++ b/file3
@@ -0,0 +1 @@
+contents1
diff --git a/file4 b/file4
new file mode 100644
index 0000000..a024003
--- /dev/null
+++ b/file4
@@ -0,0 +1 @@
+contents1
2 changes: 1 addition & 1 deletion tests/proxy/amend_patchset.t
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@
]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/authentication.t
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@
"real_repo.git" = ["::sub1/"]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
4 changes: 2 additions & 2 deletions tests/proxy/caching.t
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down Expand Up @@ -162,7 +162,7 @@
]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_absent_head.t
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
$ bash ${TESTDIR}/destroy_test_env.sh
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_all.t
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
"real_repo.git" = ["::sub1/"]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_blocked.t
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
$ bash ${TESTDIR}/destroy_test_env.sh
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_invalid_url.t
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
$ bash ${TESTDIR}/destroy_test_env.sh
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_locked_refs.t
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_prefix.t
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_sha.t
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ Check (2) and (3) but with a branch ref
"real_repo.git" = ["::sub1/"]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_subsubtree.t
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_subtree.t
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_subtree_no_master.t
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
"real_repo.git" = [":/sub1"]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_subtree_tags.t
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@
]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_with_meta.t
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@
]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/empty_commit.t
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ should still be included.
"real_repo.git" = ["::sub1/"]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/get_version.t
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
$ bash ${TESTDIR}/destroy_test_env.sh
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/import_export.t
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ Flushed credential cache
"repo2.git" = [":prefix=repo2"]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/join_with_merge.t
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
"real_repo.git" = [":prefix=sub1"]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/markers.t
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@
]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/no_proxy.t
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
$ bash ${TESTDIR}/destroy_test_env.sh
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_graphql.t
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
"real_repo.git" = ["::sub1/"]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_new_branch.t
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ Check the branch again
]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_prefix.t
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_review.t
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ Make sure all temporary namespace got removed
]
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_review_already_in.t
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ test for that.
"real_repo.git" = []
.
|-- josh
| `-- 14
| `-- 15
| `-- sled
| |-- blobs
| |-- conf
Expand Down
Loading

0 comments on commit fc857af

Please sign in to comment.