This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10833

rust: Make some file path parameters less generic
ClosedPublic

Authored by SimonSapin on Jun 4 2021, 5:02 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG777c3d231913: rust: Make some file path parameters less generic
Summary

These are not widely used APIs that benefit from being maximally flexible,
taking an explicit &Path borrow is fine and simplifies their internals.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

SimonSapin created this revision.Jun 4 2021, 5:02 AM
baymax updated this revision to Diff 28435.Jun 4 2021, 7:23 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

Alphare accepted this revision.Jun 4 2021, 8:39 AM
This revision is now accepted and ready to land.Jun 4 2021, 8:39 AM
SimonSapin added a commit: rHG777c3d231913: rust: Make some file path parameters less generic.Jun 7 2021, 3:44 AM
Closed by commit rHG777c3d231913: rust: Make some file path parameters less generic (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 28466

rust/hg-core/src/filepatterns.rs

#[derive(Debug)] #[derive(Debug)]
pub enum PatternFileWarning { pub enum PatternFileWarning {
/// (file path, syntax bytes) /// (file path, syntax bytes)
InvalidSyntax(PathBuf, Vec<u8>), InvalidSyntax(PathBuf, Vec<u8>),
/// File path /// File path
NoSuchFile(PathBuf), NoSuchFile(PathBuf),
} }
pub fn parse_pattern_file_contents<P: AsRef<Path>>( pub fn parse_pattern_file_contents(
lines: &[u8], lines: &[u8],
file_path: P, file_path: &Path,
warn: bool, warn: bool,
) -> Result<(Vec<IgnorePattern>, Vec<PatternFileWarning>), PatternError> { ) -> Result<(Vec<IgnorePattern>, Vec<PatternFileWarning>), PatternError> {
let comment_regex = Regex::new(r"((?:^|[^\\])(?:\\\\)*)#.*").unwrap(); let comment_regex = Regex::new(r"((?:^|[^\\])(?:\\\\)*)#.*").unwrap();
#[allow(clippy::trivial_regex)] #[allow(clippy::trivial_regex)]
let comment_escape_regex = Regex::new(r"\\#").unwrap(); let comment_escape_regex = Regex::new(r"\\#").unwrap();
let mut inputs: Vec<IgnorePattern> = vec![]; let mut inputs: Vec<IgnorePattern> = vec![];
let mut warnings: Vec<PatternFileWarning> = vec![]; let mut warnings: Vec<PatternFileWarning> = vec![];
if let Some(syntax) = line.drop_prefix(b"syntax:") { if let Some(syntax) = line.drop_prefix(b"syntax:") {
let syntax = syntax.trim(); let syntax = syntax.trim();
if let Some(rel_syntax) = SYNTAXES.get(syntax) { if let Some(rel_syntax) = SYNTAXES.get(syntax) {
current_syntax = rel_syntax; current_syntax = rel_syntax;
} else if warn { } else if warn {
warnings.push(PatternFileWarning::InvalidSyntax( warnings.push(PatternFileWarning::InvalidSyntax(
file_path.as_ref().to_owned(), file_path.to_owned(),
syntax.to_owned(), syntax.to_owned(),
)); ));
} }
continue; continue;
} }
let mut line_syntax: &[u8] = &current_syntax; let mut line_syntax: &[u8] = &current_syntax;
} }
} }
inputs.push(IgnorePattern::new( inputs.push(IgnorePattern::new(
parse_pattern_syntax(&line_syntax).map_err(|e| match e { parse_pattern_syntax(&line_syntax).map_err(|e| match e {
PatternError::UnsupportedSyntax(syntax) => { PatternError::UnsupportedSyntax(syntax) => {
PatternError::UnsupportedSyntaxInFile( PatternError::UnsupportedSyntaxInFile(
syntax, syntax,
file_path.as_ref().to_string_lossy().into(), file_path.to_string_lossy().into(),
line_number, line_number,
) )
} }
_ => e, _ => e,
})?, })?,
&line, &line,
&file_path, file_path,
)); ));
} }
Ok((inputs, warnings)) Ok((inputs, warnings))
} }
pub fn read_pattern_file<P: AsRef<Path>>( pub fn read_pattern_file(
file_path: P, file_path: &Path,
warn: bool, warn: bool,
) -> Result<(Vec<IgnorePattern>, Vec<PatternFileWarning>), PatternError> { ) -> Result<(Vec<IgnorePattern>, Vec<PatternFileWarning>), PatternError> {
let mut f = match File::open(file_path.as_ref()) { let mut f = match File::open(file_path) {
Ok(f) => Ok(f), Ok(f) => Ok(f),
Err(e) => match e.kind() { Err(e) => match e.kind() {
std::io::ErrorKind::NotFound => { std::io::ErrorKind::NotFound => {
return Ok(( return Ok((
vec![], vec![],
vec![PatternFileWarning::NoSuchFile( vec![PatternFileWarning::NoSuchFile(file_path.to_owned())],
file_path.as_ref().to_owned(),
)],
)) ))
} }
_ => Err(e), _ => Err(e),
}, },
}?; }?;
let mut contents = Vec::new(); let mut contents = Vec::new();
f.read_to_end(&mut contents)?; f.read_to_end(&mut contents)?;
Ok(parse_pattern_file_contents(&contents, file_path, warn)?) Ok(parse_pattern_file_contents(&contents, file_path, warn)?)
} }
/// Represents an entry in an "ignore" file. /// Represents an entry in an "ignore" file.
#[derive(Debug, Eq, PartialEq, Clone)] #[derive(Debug, Eq, PartialEq, Clone)]
pub struct IgnorePattern { pub struct IgnorePattern {
pub syntax: PatternSyntax, pub syntax: PatternSyntax,
pub pattern: Vec<u8>, pub pattern: Vec<u8>,
pub source: PathBuf, pub source: PathBuf,
} }
impl IgnorePattern { impl IgnorePattern {
pub fn new( pub fn new(syntax: PatternSyntax, pattern: &[u8], source: &Path) -> Self {
syntax: PatternSyntax,
pattern: &[u8],
source: impl AsRef<Path>,
) -> Self {
Self { Self {
syntax, syntax,
pattern: pattern.to_owned(), pattern: pattern.to_owned(),
source: source.as_ref().to_owned(), source: source.to_owned(),
} }
} }
} }
pub type PatternResult<T> = Result<T, PatternError>; pub type PatternResult<T> = Result<T, PatternError>;
/// Wrapper for `read_pattern_file` that also recursively expands `include:` /// Wrapper for `read_pattern_file` that also recursively expands `include:`
/// patterns. /// patterns.
/// ///
/// `subinclude:` is not treated as a special pattern here: unraveling them /// `subinclude:` is not treated as a special pattern here: unraveling them
/// needs to occur in the "ignore" phase. /// needs to occur in the "ignore" phase.
pub fn get_patterns_from_file( pub fn get_patterns_from_file(
pattern_file: impl AsRef<Path>, pattern_file: &Path,
root_dir: impl AsRef<Path>, root_dir: &Path,
) -> PatternResult<(Vec<IgnorePattern>, Vec<PatternFileWarning>)> { ) -> PatternResult<(Vec<IgnorePattern>, Vec<PatternFileWarning>)> {
let (patterns, mut warnings) = read_pattern_file(&pattern_file, true)?; let (patterns, mut warnings) = read_pattern_file(pattern_file, true)?;
let patterns = patterns let patterns = patterns
.into_iter() .into_iter()
.flat_map(|entry| -> PatternResult<_> { .flat_map(|entry| -> PatternResult<_> {
let IgnorePattern { let IgnorePattern {
syntax, pattern, .. syntax, pattern, ..
} = &entry; } = &entry;
Ok(match syntax { Ok(match syntax {
PatternSyntax::Include => { PatternSyntax::Include => {
let inner_include = let inner_include =
root_dir.as_ref().join(get_path_from_bytes(&pattern)); root_dir.join(get_path_from_bytes(&pattern));
let (inner_pats, inner_warnings) = get_patterns_from_file( let (inner_pats, inner_warnings) =
&inner_include, get_patterns_from_file(&inner_include, root_dir)?;
root_dir.as_ref(),
)?;
warnings.extend(inner_warnings); warnings.extend(inner_warnings);
inner_pats inner_pats
} }
_ => vec![entry], _ => vec![entry],
}) })
}) })
.flatten() .flatten()
.collect(); .collect();
/// The file itself, containing the patterns /// The file itself, containing the patterns
pub path: PathBuf, pub path: PathBuf,
/// Folder in the filesystem where this it applies /// Folder in the filesystem where this it applies
pub root: PathBuf, pub root: PathBuf,
} }
impl SubInclude { impl SubInclude {
pub fn new( pub fn new(
root_dir: impl AsRef<Path>, root_dir: &Path,
pattern: &[u8], pattern: &[u8],
source: impl AsRef<Path>, source: &Path,
) -> Result<SubInclude, HgPathError> { ) -> Result<SubInclude, HgPathError> {
let normalized_source = let normalized_source =
normalize_path_bytes(&get_bytes_from_path(source)); normalize_path_bytes(&get_bytes_from_path(source));
let source_root = get_path_from_bytes(&normalized_source); let source_root = get_path_from_bytes(&normalized_source);
let source_root = let source_root =
source_root.parent().unwrap_or_else(|| source_root.deref()); source_root.parent().unwrap_or_else(|| source_root.deref());
let path = source_root.join(get_path_from_bytes(pattern)); let path = source_root.join(get_path_from_bytes(pattern));
let new_root = path.parent().unwrap_or_else(|| path.deref()); let new_root = path.parent().unwrap_or_else(|| path.deref());
let prefix = canonical_path(&root_dir, &root_dir, new_root)?; let prefix = canonical_path(root_dir, root_dir, new_root)?;
Ok(Self { Ok(Self {
prefix: path_to_hg_path_buf(prefix).and_then(|mut p| { prefix: path_to_hg_path_buf(prefix).and_then(|mut p| {
if !p.is_empty() { if !p.is_empty() {
p.push(b'/'); p.push(b'/');
} }
Ok(p) Ok(p)
})?, })?,
path: path.to_owned(), path: path.to_owned(),
root: new_root.to_owned(), root: new_root.to_owned(),
}) })
} }
} }
/// Separate and pre-process subincludes from other patterns for the "ignore" /// Separate and pre-process subincludes from other patterns for the "ignore"
/// phase. /// phase.
pub fn filter_subincludes( pub fn filter_subincludes<'a>(
ignore_patterns: &[IgnorePattern], ignore_patterns: &'a [IgnorePattern],
root_dir: impl AsRef<Path>, root_dir: &Path,
) -> Result<(Vec<SubInclude>, Vec<&IgnorePattern>), HgPathError> { ) -> Result<(Vec<SubInclude>, Vec<&'a IgnorePattern>), HgPathError> {
let mut subincludes = vec![]; let mut subincludes = vec![];
let mut others = vec![]; let mut others = vec![];
for ignore_pattern in ignore_patterns.iter() { for ignore_pattern in ignore_patterns.iter() {
let IgnorePattern { let IgnorePattern {
syntax, syntax,
pattern, pattern,
source, source,
} = ignore_pattern; } = ignore_pattern;
if *syntax == PatternSyntax::SubInclude { if *syntax == PatternSyntax::SubInclude {
subincludes.push(SubInclude::new(&root_dir, pattern, &source)?); subincludes.push(SubInclude::new(root_dir, pattern, &source)?);
} else { } else {
others.push(ignore_pattern) others.push(ignore_pattern)
} }
} }
Ok((subincludes, others)) Ok((subincludes, others))
} }
#[cfg(test)] #[cfg(test)]

rust/hg-core/src/matchers.rs

/// IgnorePattern, /// IgnorePattern,
/// PatternSyntax, /// PatternSyntax,
/// utils::hg_path::HgPath /// utils::hg_path::HgPath
/// }; /// };
/// use std::path::Path; /// use std::path::Path;
/// /// /// ///
/// let ignore_patterns = /// let ignore_patterns =
/// vec![IgnorePattern::new(PatternSyntax::RootGlob, b"this*", Path::new(""))]; /// vec![IgnorePattern::new(PatternSyntax::RootGlob, b"this*", Path::new(""))];
/// let (matcher, _) = IncludeMatcher::new(ignore_patterns, "").unwrap(); /// let (matcher, _) = IncludeMatcher::new(ignore_patterns, "".as_ref()).unwrap();
/// /// /// ///
/// assert_eq!(matcher.matches(HgPath::new(b"testing")), false); /// assert_eq!(matcher.matches(HgPath::new(b"testing")), false);
/// assert_eq!(matcher.matches(HgPath::new(b"this should work")), true); /// assert_eq!(matcher.matches(HgPath::new(b"this should work")), true);
/// assert_eq!(matcher.matches(HgPath::new(b"this also")), true); /// assert_eq!(matcher.matches(HgPath::new(b"this also")), true);
/// assert_eq!(matcher.matches(HgPath::new(b"but not this")), false); /// assert_eq!(matcher.matches(HgPath::new(b"but not this")), false);
/// ``` /// ```
pub struct IncludeMatcher<'a> { pub struct IncludeMatcher<'a> {
patterns: Vec<u8>, patterns: Vec<u8>,
parents, parents,
}) })
} }
/// Returns a function that checks whether a given file (in the general sense) /// Returns a function that checks whether a given file (in the general sense)
/// should be matched. /// should be matched.
fn build_match<'a, 'b>( fn build_match<'a, 'b>(
ignore_patterns: &'a [IgnorePattern], ignore_patterns: &'a [IgnorePattern],
root_dir: impl AsRef<Path>, root_dir: &Path,
) -> PatternResult<( ) -> PatternResult<(
Vec<u8>, Vec<u8>,
Box<dyn Fn(&HgPath) -> bool + 'b + Sync>, Box<dyn Fn(&HgPath) -> bool + 'b + Sync>,
Vec<PatternFileWarning>, Vec<PatternFileWarning>,
)> { )> {
let mut match_funcs: Vec<Box<dyn Fn(&HgPath) -> bool + Sync>> = vec![]; let mut match_funcs: Vec<Box<dyn Fn(&HgPath) -> bool + Sync>> = vec![];
// For debugging and printing // For debugging and printing
let mut patterns = vec![]; let mut patterns = vec![];
let mut all_warnings = vec![]; let mut all_warnings = vec![];
let (subincludes, ignore_patterns) = let (subincludes, ignore_patterns) =
filter_subincludes(ignore_patterns, root_dir)?; filter_subincludes(ignore_patterns, root_dir)?;
if !subincludes.is_empty() { if !subincludes.is_empty() {
// Build prefix-based matcher functions for subincludes // Build prefix-based matcher functions for subincludes
let mut submatchers = FastHashMap::default(); let mut submatchers = FastHashMap::default();
let mut prefixes = vec![]; let mut prefixes = vec![];
for SubInclude { prefix, root, path } in subincludes.into_iter() { for SubInclude { prefix, root, path } in subincludes.into_iter() {
let (match_fn, warnings) = let (match_fn, warnings) =
get_ignore_function(vec![path.to_path_buf()], root)?; get_ignore_function(vec![path.to_path_buf()], &root)?;
all_warnings.extend(warnings); all_warnings.extend(warnings);
prefixes.push(prefix.to_owned()); prefixes.push(prefix.to_owned());
submatchers.insert(prefix.to_owned(), match_fn); submatchers.insert(prefix.to_owned(), match_fn);
} }
let match_subinclude = move |filename: &HgPath| { let match_subinclude = move |filename: &HgPath| {
for prefix in prefixes.iter() { for prefix in prefixes.iter() {
if let Some(rel) = filename.relative_to(prefix) { if let Some(rel) = filename.relative_to(prefix) {
}) })
} }
/// Parses all "ignore" files with their recursive includes and returns a /// Parses all "ignore" files with their recursive includes and returns a
/// function that checks whether a given file (in the general sense) should be /// function that checks whether a given file (in the general sense) should be
/// ignored. /// ignored.
pub fn get_ignore_function<'a>( pub fn get_ignore_function<'a>(
all_pattern_files: Vec<PathBuf>, all_pattern_files: Vec<PathBuf>,
root_dir: impl AsRef<Path>, root_dir: &Path,
) -> PatternResult<( ) -> PatternResult<(
Box<dyn for<'r> Fn(&'r HgPath) -> bool + Sync + 'a>, Box<dyn for<'r> Fn(&'r HgPath) -> bool + Sync + 'a>,
Vec<PatternFileWarning>, Vec<PatternFileWarning>,
)> { )> {
let mut all_patterns = vec![]; let mut all_patterns = vec![];
let mut all_warnings = vec![]; let mut all_warnings = vec![];
for pattern_file in all_pattern_files.into_iter() { for pattern_file in &all_pattern_files {
let (patterns, warnings) = let (patterns, warnings) =
get_patterns_from_file(pattern_file, &root_dir)?; get_patterns_from_file(pattern_file, root_dir)?;
all_patterns.extend(patterns.to_owned()); all_patterns.extend(patterns.to_owned());
all_warnings.extend(warnings); all_warnings.extend(warnings);
} }
let (matcher, warnings) = IncludeMatcher::new(all_patterns, root_dir)?; let (matcher, warnings) = IncludeMatcher::new(all_patterns, root_dir)?;
all_warnings.extend(warnings); all_warnings.extend(warnings);
Ok(( Ok((
Box::new(move |path: &HgPath| matcher.matches(path)), Box::new(move |path: &HgPath| matcher.matches(path)),
all_warnings, all_warnings,
)) ))
} }
impl<'a> IncludeMatcher<'a> { impl<'a> IncludeMatcher<'a> {
pub fn new( pub fn new(
ignore_patterns: Vec<IgnorePattern>, ignore_patterns: Vec<IgnorePattern>,
root_dir: impl AsRef<Path>, root_dir: &Path,
) -> PatternResult<(Self, Vec<PatternFileWarning>)> { ) -> PatternResult<(Self, Vec<PatternFileWarning>)> {
let (patterns, match_fn, warnings) = let (patterns, match_fn, warnings) =
build_match(&ignore_patterns, root_dir)?; build_match(&ignore_patterns, root_dir)?;
let RootsDirsAndParents { let RootsDirsAndParents {
roots, roots,
dirs, dirs,
parents, parents,
} = roots_dirs_and_parents(&ignore_patterns)?; } = roots_dirs_and_parents(&ignore_patterns)?;
fn test_includematcher() { fn test_includematcher() {
// VisitchildrensetPrefix // VisitchildrensetPrefix
let (matcher, _) = IncludeMatcher::new( let (matcher, _) = IncludeMatcher::new(
vec![IgnorePattern::new( vec![IgnorePattern::new(
PatternSyntax::RelPath, PatternSyntax::RelPath,
b"dir/subdir", b"dir/subdir",
Path::new(""), Path::new(""),
)], )],
"", "".as_ref(),
) )
.unwrap(); .unwrap();
let mut set = HashSet::new(); let mut set = HashSet::new();
set.insert(HgPath::new(b"dir")); set.insert(HgPath::new(b"dir"));
assert_eq!( assert_eq!(
matcher.visit_children_set(HgPath::new(b"")), matcher.visit_children_set(HgPath::new(b"")),
VisitChildrenSet::Set(set) VisitChildrenSet::Set(set)
// VisitchildrensetRootfilesin // VisitchildrensetRootfilesin
let (matcher, _) = IncludeMatcher::new( let (matcher, _) = IncludeMatcher::new(
vec![IgnorePattern::new( vec![IgnorePattern::new(
PatternSyntax::RootFiles, PatternSyntax::RootFiles,
b"dir/subdir", b"dir/subdir",
Path::new(""), Path::new(""),
)], )],
"", "".as_ref(),
) )
.unwrap(); .unwrap();
let mut set = HashSet::new(); let mut set = HashSet::new();
set.insert(HgPath::new(b"dir")); set.insert(HgPath::new(b"dir"));
assert_eq!( assert_eq!(
matcher.visit_children_set(HgPath::new(b"")), matcher.visit_children_set(HgPath::new(b"")),
VisitChildrenSet::Set(set) VisitChildrenSet::Set(set)
// VisitchildrensetGlob // VisitchildrensetGlob
let (matcher, _) = IncludeMatcher::new( let (matcher, _) = IncludeMatcher::new(
vec![IgnorePattern::new( vec![IgnorePattern::new(
PatternSyntax::Glob, PatternSyntax::Glob,
b"dir/z*", b"dir/z*",
Path::new(""), Path::new(""),
)], )],
"", "".as_ref(),
) )
.unwrap(); .unwrap();
let mut set = HashSet::new(); let mut set = HashSet::new();
set.insert(HgPath::new(b"dir")); set.insert(HgPath::new(b"dir"));
assert_eq!( assert_eq!(
matcher.visit_children_set(HgPath::new(b"")), matcher.visit_children_set(HgPath::new(b"")),
VisitChildrenSet::Set(set) VisitChildrenSet::Set(set)