[pbs-devel] [PATCH pathpatterns v3] match_list: updated `matches()`, to only retrieve file mode if necessary
Wolfgang Bumiller
w.bumiller at proxmox.com
Mon Aug 21 09:57:41 CEST 2023
On Fri, Aug 18, 2023 at 04:32:11PM +0200, Gabriel Goller wrote:
> Updated `matches()` function, which now takes the `GetFileMode` trait, that
> has a function that should return the `file_mode`. `matches()` will go
> through the patterns and match by path only until it finds a pattern which
> does not have `MatchFlag::ANY_FILE_TYPE`, in case it will call the
> `get_file_mode`, which will return a `file_mode`. This will ensure that the
> `get()` (which in our case executes `stat()`) will only be run once(at most),
> every pattern will only be processed once and that the order of the patterns
> will be respected.
>
> Signed-off-by: Gabriel Goller <g.goller at proxmox.com>
> ---
>
> changes v2:
> - updated the `matches_path()` function to take a closure and renamed
> it to `matches_mode_lazy()`. This allows us to only match once, so we
> don't need to match before and after `stat()` (without and with
> `file_mode`) anymore.
>
> changes v1:
> - added `matches_path()` function, which matches by path only and returns an
> error if the `file_mode` is required.
>
> src/lib.rs | 72 +++++-----
> src/match_list.rs | 337 ++++++++++++++++++++++++++++++++++++++--------
> 2 files changed, 315 insertions(+), 94 deletions(-)
>
> diff --git a/src/lib.rs b/src/lib.rs
> index cb89917..998cdae 100644
> --- a/src/lib.rs
> +++ b/src/lib.rs
> @@ -31,17 +31,17 @@
> //! MatchEntry::include(Pattern::path("bananas/curved.*")?),
> //! ];
> //!
> -//! assert_eq!(list.matches("/things", None), None);
> -//! assert_eq!(list.matches("/things/shop", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/things/shop/bananas", None), Some(MatchType::Exclude));
> -//! assert_eq!(list.matches("/things/shop/bananas/curved.txt", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/things/shop/bananas/curved.bak", None), Some(MatchType::Include));
> +//! assert_eq!(list.matches("/things", None), Ok(None));
> +//! assert_eq!(list.matches("/things/shop", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/things/shop/bananas", None), Ok(Some(MatchType::Exclude)));
> +//! assert_eq!(list.matches("/things/shop/bananas/curved.txt", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/things/shop/bananas/curved.bak", None), Ok(Some(MatchType::Include)));
> //!
> //! // this will exclude the curved.bak file
> //! list.push(MatchEntry::exclude(Pattern::path("curved.bak")?));
> -//! assert_eq!(list.matches("/things/shop/bananas/curved.bak", None), Some(MatchType::Exclude));
> +//! assert_eq!(list.matches("/things/shop/bananas/curved.bak", None), Ok(Some(MatchType::Exclude)));
> //! list.pop();
> -//! # assert_eq!(list.matches("/things/shop/bananas/curved.bak", None), Some(MatchType::Include));
> +//! # assert_eq!(list.matches("/things/shop/bananas/curved.bak", None), Ok(Some(MatchType::Include)));
> //!
> //! // but this will not:
> //! list.push(
> @@ -49,40 +49,40 @@
> //! .flags(MatchFlag::ANCHORED)
> //! );
> //! // or: list.push
> -//! assert_eq!(list.matches("/things/shop/bananas/curved.bak", None), Some(MatchType::Include));
> +//! assert_eq!(list.matches("/things/shop/bananas/curved.bak", None), Ok(Some(MatchType::Include)));
> //! list.pop();
> //!
> //! // let's check some patterns, anything starting with a 'c', 'f' or 's':
> //! let mut list = vec![MatchEntry::include(Pattern::path("[cfs]*")?)];
> -//! assert_eq!(list.matches("/things", None), None);
> -//! assert_eq!(list.matches("/things/file1.dat", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/things/file2.dat", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/things/shop", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/things/shop/info.txt", None), None);
> -//! assert_eq!(list.matches("/things/shop/apples", None), None);
> -//! assert_eq!(list.matches("/things/shop/apples/gala.txt", None), None);
> -//! assert_eq!(list.matches("/things/shop/apples/golden-delicious.txt", None), None);
> -//! assert_eq!(list.matches("/things/shop/bananas", None), None);
> -//! assert_eq!(list.matches("/things/shop/bananas/straight.txt", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/things/shop/bananas/curved.txt", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/shop/bananas/curved.bak", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/things/shop/bananas/other.txt", None), None);
> +//! assert_eq!(list.matches("/things", None), Ok(None));
> +//! assert_eq!(list.matches("/things/file1.dat", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/things/file2.dat", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/things/shop", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/things/shop/info.txt", None), Ok(None));
> +//! assert_eq!(list.matches("/things/shop/apples", None), Ok(None));
> +//! assert_eq!(list.matches("/things/shop/apples/gala.txt", None), Ok(None));
> +//! assert_eq!(list.matches("/things/shop/apples/golden-delicious.txt", None), Ok(None));
> +//! assert_eq!(list.matches("/things/shop/bananas", None), Ok(None));
> +//! assert_eq!(list.matches("/things/shop/bananas/straight.txt", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/things/shop/bananas/curved.txt", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/shop/bananas/curved.bak", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/things/shop/bananas/other.txt", None), Ok(None));
> //!
> //! // If we add `**` we end up including the entire `shop/` subtree:
> //! list.push(MatchEntry::include(Pattern::path("[cfs]*/**")?));
> -//! assert_eq!(list.matches("/things", None), None);
> -//! assert_eq!(list.matches("/things/file1.dat", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/things/file2.dat", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/things/shop", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/things/shop/info.txt", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/things/shop/apples", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/things/shop/apples/gala.txt", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/shop/apples/golden-delicious.txt", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/things/shop/bananas", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/things/shop/bananas/straight.txt", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/things/shop/bananas/curved.txt", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/shop/bananas/curved.bak", None), Some(MatchType::Include));
> -//! assert_eq!(list.matches("/shop/bananas/other.txt", None), Some(MatchType::Include));
> +//! assert_eq!(list.matches("/things", None), Ok(None));
> +//! assert_eq!(list.matches("/things/file1.dat", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/things/file2.dat", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/things/shop", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/things/shop/info.txt", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/things/shop/apples", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/things/shop/apples/gala.txt", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/shop/apples/golden-delicious.txt", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/things/shop/bananas", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/things/shop/bananas/straight.txt", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/things/shop/bananas/curved.txt", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/shop/bananas/curved.bak", None), Ok(Some(MatchType::Include)));
> +//! assert_eq!(list.matches("/shop/bananas/other.txt", None), Ok(Some(MatchType::Include)));
> //! # Ok(())
> //! # }
> //! # test().unwrap()
> @@ -92,7 +92,9 @@ mod match_list;
> mod pattern;
>
> #[doc(inline)]
> -pub use match_list::{MatchEntry, MatchFlag, MatchList, MatchPattern, MatchType};
> +pub use match_list::{
> + FileModeRequired, GetFileMode, MatchEntry, MatchFlag, MatchList, MatchPattern, MatchType,
> +};
>
> #[doc(inline)]
> pub use pattern::{ParseError, Pattern, PatternFlag};
> diff --git a/src/match_list.rs b/src/match_list.rs
> index c5b14e0..51aa65d 100644
> --- a/src/match_list.rs
> +++ b/src/match_list.rs
> @@ -1,5 +1,4 @@
> //! Helpers for include/exclude lists.
> -
> use bitflags::bitflags;
>
> use crate::PatternFlag;
> @@ -39,6 +38,37 @@ impl Default for MatchFlag {
> }
> }
>
> +#[derive(Debug, PartialEq)]
Aaaah shoot, can't put the `PartialEq` behind `cfg(test)` due to
doctests :(
Well, those doctests look ugly in the docs anyway, so I suppose a future
todo-item would be to move those into regular tests and guard this
derive.
But for now this is fine (definitely out of scope of this series)!
> +pub struct FileModeRequired {}
^ Why does this have an empty body now? We don't need that, please drop
it :-)
> +impl std::fmt::Display for FileModeRequired {
> + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
> + write!(f, "File mode is required for matching")
> + }
> +}
> +impl std::error::Error for FileModeRequired {}
> +
> +pub trait GetFileMode {
> + type Error;
> + fn get(self) -> Result<u32, Self::Error>;
> +}
> +
> +impl<T, E> GetFileMode for T
> +where
> + T: FnOnce() -> Result<u32, E>,
> +{
> + type Error = E;
> + fn get(self) -> Result<u32, Self::Error> {
> + self()
> + }
> +}
> +
> +impl GetFileMode for Option<u32> {
> + type Error = FileModeRequired;
> + fn get(self) -> Result<u32, Self::Error> {
> + self.ok_or(FileModeRequired {})
> + }
> +}
> +
> /// A pattern entry. (Glob patterns or literal patterns.)
> // Note:
> // For regex we'd likely use the POSIX extended REs via `regexec(3)`, since we're targetting
> @@ -211,7 +241,7 @@ impl MatchEntry {
> pub fn matches_mode(&self, file_mode: u32) -> bool {
> // bitflags' `.contains` means ALL bits must be set, if they are all set we don't
> // need to check the mode...
> - if self.flags.contains(MatchFlag::ANY_FILE_TYPE) {
> + if !self.needs_file_mode() {
> return true;
> }
>
> @@ -304,12 +334,21 @@ impl MatchEntry {
>
> self.matches_path_exact(path)
> }
> +
> + pub fn needs_file_mode(&self) -> bool {
> + let flags = (self.flags & MatchFlag::ANY_FILE_TYPE).bits();
> + if flags != 0 && flags != MatchFlag::ANY_FILE_TYPE.bits() {
> + return true;
> + }
> + false
> + }
> }
>
> #[doc(hidden)]
> pub trait MatchListEntry {
> fn entry_matches(&self, path: &[u8], file_mode: Option<u32>) -> Option<MatchType>;
> fn entry_matches_exact(&self, path: &[u8], file_mode: Option<u32>) -> Option<MatchType>;
> + fn entry_needs_file_mode(&self) -> bool;
> }
>
> impl MatchListEntry for &'_ MatchEntry {
> @@ -328,6 +367,10 @@ impl MatchListEntry for &'_ MatchEntry {
> None
> }
> }
> +
> + fn entry_needs_file_mode(&self) -> bool {
> + self.needs_file_mode()
> + }
> }
>
> impl MatchListEntry for &'_ &'_ MatchEntry {
> @@ -346,6 +389,10 @@ impl MatchListEntry for &'_ &'_ MatchEntry {
> None
> }
> }
> +
> + fn entry_needs_file_mode(&self) -> bool {
> + self.needs_file_mode()
> + }
> }
>
> /// This provides [`matches`](MatchList::matches) and [`matches_exact`](MatchList::matches_exact)
> @@ -361,19 +408,26 @@ impl MatchListEntry for &'_ &'_ MatchEntry {
> /// This makes it easier to use slices over entries or references to entries.
> pub trait MatchList {
> /// Check whether this list contains anything matching a prefix of the specified path, and the
> - /// specified file mode.
> - fn matches<T: AsRef<[u8]>>(&self, path: T, file_mode: Option<u32>) -> Option<MatchType> {
> - self.matches_do(path.as_ref(), file_mode)
> - }
> -
> - fn matches_do(&self, path: &[u8], file_mode: Option<u32>) -> Option<MatchType>;
> -
> - /// Check whether this list contains anything exactly matching the path and mode.
> - fn matches_exact<T: AsRef<[u8]>>(&self, path: T, file_mode: Option<u32>) -> Option<MatchType> {
> - self.matches_exact_do(path.as_ref(), file_mode)
> - }
> -
> - fn matches_exact_do(&self, path: &[u8], file_mode: Option<u32>) -> Option<MatchType>;
> + /// specified file mode. Gets the file_mode lazily, only if needed.
> + fn matches<T, U>(
> + &self,
> + path: T,
> + get_file_mode: U,
> + ) -> Result<Option<MatchType>, <U as GetFileMode>::Error>
Just use `U::Error`, formats cleaner.
> + where
> + T: AsRef<[u8]>,
> + U: GetFileMode;
> +
> + /// Check whether this list contains anything exactly matching the path and mode. Gets the
> + /// file_mode lazily, only if needed.
> + fn matches_exact<T, U>(
> + &self,
> + path: T,
> + get_file_mode: U,
> + ) -> Result<Option<MatchType>, <U as GetFileMode>::Error>
> + where
> + T: AsRef<[u8]>,
> + U: GetFileMode;
> }
>
> impl<'a, T> MatchList for T
> @@ -383,30 +437,56 @@ where
> <&'a T as IntoIterator>::IntoIter: DoubleEndedIterator,
> <&'a T as IntoIterator>::Item: MatchListEntry,
> {
> - fn matches_do(&self, path: &[u8], file_mode: Option<u32>) -> Option<MatchType> {
> + fn matches<U, G>(
> + &self,
> + path: U,
> + get_file_mode: G,
> + ) -> Result<Option<MatchType>, <G as GetFileMode>::Error>
> + where
> + U: AsRef<[u8]>,
> + G: GetFileMode,
> + {
> // This is an &self method on a `T where T: 'a`.
> let this: &'a Self = unsafe { std::mem::transmute(self) };
>
> + let mut get_file_mode = Some(get_file_mode);
> + let mut file_mode = None;
> +
> for m in this.into_iter().rev() {
> - if let Some(mt) = m.entry_matches(path, file_mode) {
> - return Some(mt);
> + if file_mode.is_none() && m.entry_needs_file_mode() {
> + file_mode = Some(get_file_mode.take().unwrap().get()?);
> + }
> + if let Some(mt) = m.entry_matches(path.as_ref(), file_mode) {
> + return Ok(Some(mt));
> }
> }
> -
> - None
> - }
> -
> - fn matches_exact_do(&self, path: &[u8], file_mode: Option<u32>) -> Option<MatchType> {
> + Ok(None)
> + }
> +
> + fn matches_exact<U, G>(
> + &self,
> + path: U,
> + get_file_mode: G,
> + ) -> Result<Option<MatchType>, <G as GetFileMode>::Error>
> + where
> + U: AsRef<[u8]>,
> + G: GetFileMode,
> + {
> // This is an &self method on a `T where T: 'a`.
> let this: &'a Self = unsafe { std::mem::transmute(self) };
>
> + let mut get_file_mode = Some(get_file_mode);
> + let mut file_mode = None;
> +
> for m in this.into_iter().rev() {
> - if let Some(mt) = m.entry_matches_exact(path, file_mode) {
> - return Some(mt);
> + if file_mode.is_none() && m.entry_needs_file_mode() {
> + file_mode = Some(get_file_mode.take().unwrap().get()?);
> + }
> + if let Some(mt) = m.entry_matches_exact(path.as_ref(), file_mode) {
> + return Ok(Some(mt));
> }
> }
> -
> - None
> + Ok(None)
> }
> }
>
> @@ -415,20 +495,20 @@ fn assert_containers_implement_match_list() {
> use std::iter::FromIterator;
>
> let vec = vec![MatchEntry::include(crate::Pattern::path("a*").unwrap())];
> - assert_eq!(vec.matches("asdf", None), Some(MatchType::Include));
> + assert_eq!(vec.matches("asdf", None), Ok(Some(MatchType::Include)));
>
> // FIXME: ideally we can make this work as well!
> let vd = std::collections::VecDeque::<MatchEntry>::from_iter(vec.clone());
> - assert_eq!(vd.matches("asdf", None), Some(MatchType::Include));
> + assert_eq!(vd.matches("asdf", None), Ok(Some(MatchType::Include)));
>
> let list: &[MatchEntry] = &vec[..];
> - assert_eq!(list.matches("asdf", None), Some(MatchType::Include));
> + assert_eq!(list.matches("asdf", None), Ok(Some(MatchType::Include)));
>
> let list: Vec<&MatchEntry> = vec.iter().collect();
> - assert_eq!(list.matches("asdf", None), Some(MatchType::Include));
> + assert_eq!(list.matches("asdf", None), Ok(Some(MatchType::Include)));
>
> let list: &[&MatchEntry] = &list[..];
> - assert_eq!(list.matches("asdf", None), Some(MatchType::Include));
> + assert_eq!(list.matches("asdf", None), Ok(Some(MatchType::Include)));
> }
>
> #[test]
> @@ -443,25 +523,28 @@ fn test_file_type_matches() {
> ];
> assert_eq!(
> matchlist.matches("a_dir", Some(libc::S_IFDIR)),
> - Some(MatchType::Include)
> + Ok(Some(MatchType::Include))
> );
> assert_eq!(
> matchlist.matches("/a_dir", Some(libc::S_IFDIR)),
> - Some(MatchType::Include)
> + Ok(Some(MatchType::Include))
> );
> - assert_eq!(matchlist.matches("/a_dir", Some(libc::S_IFREG)), None);
> + assert_eq!(matchlist.matches("/a_dir", Some(libc::S_IFREG)), Ok(None));
>
> assert_eq!(
> matchlist.matches("/a_file", Some(libc::S_IFREG)),
> - Some(MatchType::Exclude)
> + Ok(Some(MatchType::Exclude))
> );
> - assert_eq!(matchlist.matches("/a_file", Some(libc::S_IFDIR)), None);
> + assert_eq!(matchlist.matches("/a_file", Some(libc::S_IFDIR)), Ok(None));
>
> assert_eq!(
> matchlist.matches("/another_dir", Some(libc::S_IFDIR)),
> - Some(MatchType::Exclude)
> + Ok(Some(MatchType::Exclude))
> + );
> + assert_eq!(
> + matchlist.matches("/another_dir", Some(libc::S_IFREG)),
> + Ok(None)
> );
> - assert_eq!(matchlist.matches("/another_dir", Some(libc::S_IFREG)), None);
> }
>
> #[test]
> @@ -471,22 +554,25 @@ fn test_anchored_matches() {
> let matchlist = vec![
> MatchEntry::new(Pattern::path("file-a").unwrap(), MatchType::Include),
> MatchEntry::new(Pattern::path("some/path").unwrap(), MatchType::Include)
> - .flags(MatchFlag::ANCHORED),
> + .add_flags(MatchFlag::ANCHORED),
> ];
>
> - assert_eq!(matchlist.matches("file-a", None), Some(MatchType::Include));
> + assert_eq!(
> + matchlist.matches("file-a", None),
> + Ok(Some(MatchType::Include))
> + );
> assert_eq!(
> matchlist.matches("another/file-a", None),
> - Some(MatchType::Include)
> + Ok(Some(MatchType::Include))
> );
>
> - assert_eq!(matchlist.matches("some", None), None);
> - assert_eq!(matchlist.matches("path", None), None);
> + assert_eq!(matchlist.matches("some", None), Ok(None));
> + assert_eq!(matchlist.matches("path", None), Ok(None));
> assert_eq!(
> matchlist.matches("some/path", None),
> - Some(MatchType::Include)
> + Ok(Some(MatchType::Include))
> );
> - assert_eq!(matchlist.matches("another/some/path", None), None);
> + assert_eq!(matchlist.matches("another/some/path", None), Ok(None));
> }
>
> #[test]
> @@ -495,7 +581,10 @@ fn test_literal_matches() {
> MatchPattern::Literal(b"/bin/mv".to_vec()),
> MatchType::Include,
> )];
> - assert_eq!(matchlist.matches("/bin/mv", None), Some(MatchType::Include));
> + assert_eq!(
> + matchlist.matches("/bin/mv", None),
> + Ok(Some(MatchType::Include))
> + );
> }
>
> #[test]
> @@ -504,29 +593,159 @@ fn test_path_relativity() {
> let matchlist = vec![
> MatchEntry::new(Pattern::path("noslash").unwrap(), MatchType::Include),
> MatchEntry::new(Pattern::path("noslash-a").unwrap(), MatchType::Include)
> - .flags(MatchFlag::ANCHORED),
> + .add_flags(MatchFlag::ANCHORED),
> MatchEntry::new(Pattern::path("/slash").unwrap(), MatchType::Include),
> MatchEntry::new(Pattern::path("/slash-a").unwrap(), MatchType::Include)
> - .flags(MatchFlag::ANCHORED),
> + .add_flags(MatchFlag::ANCHORED),
> ];
> - assert_eq!(matchlist.matches("noslash", None), Some(MatchType::Include));
> + assert_eq!(
> + matchlist.matches("noslash", None),
> + Ok(Some(MatchType::Include))
> + );
> assert_eq!(
> matchlist.matches("noslash-a", None),
> - Some(MatchType::Include)
> + Ok(Some(MatchType::Include))
> );
> - assert_eq!(matchlist.matches("slash", None), None);
> - assert_eq!(matchlist.matches("/slash", None), Some(MatchType::Include));
> - assert_eq!(matchlist.matches("slash-a", None), None);
> + assert_eq!(matchlist.matches("slash", None), Ok(None));
> + assert_eq!(
> + matchlist.matches("/slash", None),
> + Ok(Some(MatchType::Include))
> + );
> + assert_eq!(matchlist.matches("slash-a", None), Ok(None));
> assert_eq!(
> matchlist.matches("/slash-a", None),
> - Some(MatchType::Include)
> + Ok(Some(MatchType::Include))
> );
>
> assert_eq!(
> matchlist.matches("foo/noslash", None),
> - Some(MatchType::Include)
> + Ok(Some(MatchType::Include))
> );
> - assert_eq!(matchlist.matches("foo/noslash-a", None), None);
> - assert_eq!(matchlist.matches("foo/slash", None), None);
> - assert_eq!(matchlist.matches("foo/slash-a", None), None);
> + assert_eq!(matchlist.matches("foo/noslash-a", None), Ok(None));
> + assert_eq!(matchlist.matches("foo/slash", None), Ok(None));
> + assert_eq!(matchlist.matches("foo/slash-a", None), Ok(None));
> +}
> +
> +#[test]
> +fn matches_path() {
> + use crate::Pattern;
> +
> + let matchlist = vec![
> + MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude),
> + MatchEntry::new(Pattern::path("b*").unwrap(), MatchType::Exclude),
> + ];
> +
> + assert_eq!(
> + matchlist.matches("ahsjdj", || Err(FileModeRequired {})),
> + Ok(Some(MatchType::Exclude))
> + );
> + let mut _test = MatchFlag::ANY_FILE_TYPE;
^ Not sure what the point of this is. If it's only to test the
assignment/mutability, maybe don't use `MatchFlag` for the contents,
that's a bit confusing/misleading, and maybe include a test that would
actually trigger the access as well and also actually validate the final
value.
> + assert_eq!(
> + matchlist.matches("bhshdf", || {
> + _test = MatchFlag::MATCH_DIRECTORIES;
> + Ok::<u32, FileModeRequired>(libc::S_IFDIR)
> + }),
> + Ok(Some(MatchType::Exclude))
> + );
> +
> + let matchlist = vec![
> + MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude)
> + .flags(MatchFlag::MATCH_DIRECTORIES),
> + MatchEntry::new(Pattern::path("b*").unwrap(), MatchType::Exclude),
> + ];
> +
> + assert_eq!(
> + matchlist.matches("ahsjdj", || Err(FileModeRequired {})),
> + Err(FileModeRequired {})
> + );
> + assert_eq!(
> + matchlist.matches("bhshdf", || Err(FileModeRequired {})),
> + Ok(Some(MatchType::Exclude))
> + );
> + assert_eq!(
> + matchlist.matches("ahsjdj", || Ok::<u32, FileModeRequired>(libc::S_IFDIR)),
> + Ok(Some(MatchType::Exclude))
> + );
Could also have a case with a different mode here.
> +
> + let matchlist = vec![
> + MatchEntry::new(Pattern::path("b*").unwrap(), MatchType::Include),
> + MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude)
> + .flags(MatchFlag::MATCH_DIRECTORIES),
> + ];
> +
> + assert_eq!(
> + matchlist.matches("ahsjdj", || Err(FileModeRequired {})),
> + Err(FileModeRequired {})
> + );
> + assert_eq!(
> + matchlist.matches("bhshdf", || Err(FileModeRequired {})),
> + Err(FileModeRequired {})
> + );
> +
> + assert_eq!(
> + matchlist.matches("ahsjdj", || Ok::<u32, FileModeRequired>(libc::S_IFDIR)),
> + Ok(Some(MatchType::Exclude))
> + );
> + assert_eq!(
> + matchlist.matches("bhshdf", || Err(FileModeRequired {})),
> + Err(FileModeRequired {})
> + );
^ This case is a duplicate.
> + assert_eq!(
> + matchlist.matches("bbb", || Ok::<u32, FileModeRequired>(libc::S_IFDIR)),
> + Ok(Some(MatchType::Include))
> + );
> +
> + let matchlist = vec![
> + MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude)
> + .flags(MatchFlag::MATCH_DIRECTORIES),
> + ];
> +
> + assert_eq!(
> + matchlist.matches("bbb", || Ok::<u32, FileModeRequired>(libc::S_IFDIR)),
> + Ok(None)
> + );
> +
> + let matchlist = vec![
> + MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude)
> + .flags(MatchFlag::MATCH_DIRECTORIES),
> + MatchEntry::new(Pattern::path("b*").unwrap(), MatchType::Exclude)
> + .flags(MatchFlag::MATCH_REGULAR_FILES),
> + ];
> +
> + assert_eq!(
> + matchlist.matches("ahsjdj", || Ok::<u32, FileModeRequired>(libc::S_IFDIR)),
> + Ok(Some(MatchType::Exclude))
> + );
> + assert_eq!(
> + matchlist.matches("ahsjdj", || Ok::<u32, FileModeRequired>(libc::S_IFREG)),
> + Ok(None)
> + );
> + assert_eq!(
> + matchlist.matches("bhsjdj", || Ok::<u32, FileModeRequired>(libc::S_IFREG)),
> + Ok(Some(MatchType::Exclude))
> + );
> +}
> +
> +#[test]
> +fn match_entry_needs_flag() {
> + use crate::Pattern;
> + let match_entry = MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude)
> + .flags(MatchFlag::MATCH_DIRECTORIES);
could replace the `.flags` call here with using `a*/` as a pattern
(trailing slash).
> + assert_eq!(match_entry.needs_file_mode(), true);
> +
> + let match_entry = MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude)
> + .flags(MatchFlag::MATCH_REGULAR_FILES);
> + assert_eq!(match_entry.needs_file_mode(), true);
> +
> + let match_entry = MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude)
> + .flags(MatchFlag::MATCH_DIRECTORIES | MatchFlag::MATCH_REGULAR_FILES);
> + assert_eq!(match_entry.needs_file_mode(), true);
> +
> + let match_entry = MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude)
> + .flags(MatchFlag::ANCHORED | MatchFlag::MATCH_DIRECTORIES);
> + assert_eq!(match_entry.needs_file_mode(), true);
> +
> + let match_entry = MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude)
> + .flags(MatchFlag::ANY_FILE_TYPE);
> + assert_eq!(match_entry.needs_file_mode(), false);
> }
> --
> 2.39.2
More information about the pbs-devel
mailing list