This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7909

rust-filepatterns: add support for `include` and `subinclude` patterns
ClosedPublic

Authored by Alphare on Jan 16 2020, 6:33 AM.

Details

Reviewers
kevincox
Group Reviewers
hg-reviewers
Commits
rHG2fe89bec8011: rust-filepatterns: add support for `include` and `subinclude` patterns
rHGd1fb8bdf3384: rust-filepatterns: add support for `include` and `subinclude` patterns
Summary

This prepares a future patch for IncludeMatcher on the road to bare
hg status support.

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

Alphare created this revision.Jan 16 2020, 6:33 AM
Alphare added a comment.Jan 16 2020, 7:19 AM

Note to reviewers: this stack is part of the larger series of getting a full Rust dirstate.status which is most of the performance hit in bare hg status. More patches are coming, but I figured I would send the patches as I go to help with review timing.

kevincox requested changes to this revision.Jan 16 2020, 10:30 AM
kevincox added inline comments.
rust/hg-core/src/filepatterns.rs
455

I don't see anywhere where this returns None. Should this just be .map() or .flat_map()?

461

I would write this as Some(Ok(match syntax { .. })) as the last expression of this map. Then instead of res.extend() and res.push() you just return the vecs.

524

How about:

if !p.is_empty() {
  p.push(b'/');
}
Ok(p)
This revision now requires changes to proceed.Jan 16 2020, 10:30 AM
Alphare marked 2 inline comments as done.Jan 16 2020, 1:21 PM
Alphare added inline comments.
rust/hg-core/src/filepatterns.rs
461

The match arms would be incompatible. Or maybe I'm missing something.

kevincox added inline comments.Jan 16 2020, 2:40 PM
rust/hg-core/src/filepatterns.rs
461

You would have to make sure you return a Vec on both arms. You will probably want inner_pats.into_iter().collect() on one branch and vec![entry] on the other branch.

Alphare marked an inline comment as done.Jan 16 2020, 4:50 PM
Alphare retitled this revision from rust-filepatterns: add support for `ignore` and `subignore` patterns to rust-filepatterns: add support for `include` and `subinclude` patterns.
Alphare added a child revision: D7910: rust-re2: add wrapper for calling Re2 from Rust.
Alphare updated this revision to Diff 19385.
Alphare updated this revision to Diff 20039.Feb 10 2020, 10:46 AM
kevincox accepted this revision.Feb 10 2020, 1:14 PM
Alphare added a commit: rHGd1fb8bdf3384: rust-filepatterns: add support for `include` and `subinclude` patterns.Feb 10 2020, 6:30 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGd1fb8bdf3384: rust-filepatterns: add support for `include` and `subinclude` patterns (authored by Alphare). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 20079

rust/hg-core/src/filepatterns.rs

// filepatterns.rs // filepatterns.rs
// //
// Copyright 2019 Raphaël Gomès <rgomes@octobus.net> // Copyright 2019 Raphaël Gomès <rgomes@octobus.net>
// //
// This software may be used and distributed according to the terms of the // This software may be used and distributed according to the terms of the
// GNU General Public License version 2 or any later version. // GNU General Public License version 2 or any later version.
//! Handling of Mercurial-specific patterns. //! Handling of Mercurial-specific patterns.
use crate::{utils::SliceExt, FastHashMap, PatternError}; use crate::{
utils::{
files::{canonical_path, get_bytes_from_path, get_path_from_bytes},
hg_path::{path_to_hg_path_buf, HgPathBuf, HgPathError},
SliceExt,
},
FastHashMap, PatternError,
};
use lazy_static::lazy_static; use lazy_static::lazy_static;
use regex::bytes::{NoExpand, Regex}; use regex::bytes::{NoExpand, Regex};
use std::fs::File; use std::fs::File;
use std::io::Read; use std::io::Read;
use std::ops::Deref;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::vec::Vec; use std::vec::Vec;
lazy_static! { lazy_static! {
static ref RE_ESCAPE: Vec<Vec<u8>> = { static ref RE_ESCAPE: Vec<Vec<u8>> = {
let mut v: Vec<Vec<u8>> = (0..=255).map(|byte| vec![byte]).collect(); let mut v: Vec<Vec<u8>> = (0..=255).map(|byte| vec![byte]).collect();
let to_escape = b"()[]{}?*+-|^$\\.&~# \t\n\r\x0b\x0c"; let to_escape = b"()[]{}?*+-|^$\\.&~# \t\n\r\x0b\x0c";
for byte in to_escape { for byte in to_escape {
RelPath, RelPath,
/// an unrooted glob (*.rs matches Rust files in all dirs) /// an unrooted glob (*.rs matches Rust files in all dirs)
RelGlob, RelGlob,
/// A regexp that needn't match the start of a name /// A regexp that needn't match the start of a name
RelRegexp, RelRegexp,
/// A path relative to repository root, which is matched non-recursively /// A path relative to repository root, which is matched non-recursively
/// (will not match subdirectories) /// (will not match subdirectories)
RootFiles, RootFiles,
/// A file of patterns to read and include
Include,
/// A file of patterns to match against files under the same directory
SubInclude,
} }
/// Transforms a glob pattern into a regex /// Transforms a glob pattern into a regex
fn glob_to_re(pat: &[u8]) -> Vec<u8> { fn glob_to_re(pat: &[u8]) -> Vec<u8> {
let mut input = pat; let mut input = pat;
let mut res: Vec<u8> = vec![]; let mut res: Vec<u8> = vec![];
let mut group_depth = 0; let mut group_depth = 0;
b"re:" => Ok(PatternSyntax::Regexp), b"re:" => Ok(PatternSyntax::Regexp),
b"path:" => Ok(PatternSyntax::Path), b"path:" => Ok(PatternSyntax::Path),
b"relpath:" => Ok(PatternSyntax::RelPath), b"relpath:" => Ok(PatternSyntax::RelPath),
b"rootfilesin:" => Ok(PatternSyntax::RootFiles), b"rootfilesin:" => Ok(PatternSyntax::RootFiles),
b"relglob:" => Ok(PatternSyntax::RelGlob), b"relglob:" => Ok(PatternSyntax::RelGlob),
b"relre:" => Ok(PatternSyntax::RelRegexp), b"relre:" => Ok(PatternSyntax::RelRegexp),
b"glob:" => Ok(PatternSyntax::Glob), b"glob:" => Ok(PatternSyntax::Glob),
b"rootglob:" => Ok(PatternSyntax::RootGlob), b"rootglob:" => Ok(PatternSyntax::RootGlob),
b"include:" => Ok(PatternSyntax::Include),
b"subinclude:" => Ok(PatternSyntax::SubInclude),
_ => Err(PatternError::UnsupportedSyntax( _ => Err(PatternError::UnsupportedSyntax(
String::from_utf8_lossy(kind).to_string(), String::from_utf8_lossy(kind).to_string(),
)), )),
} }
} }
/// Builds the regex that corresponds to the given pattern. /// Builds the regex that corresponds to the given pattern.
/// If within a `syntax: regexp` context, returns the pattern, /// If within a `syntax: regexp` context, returns the pattern,
[b".*", rest, GLOB_SUFFIX].concat() [b".*", rest, GLOB_SUFFIX].concat()
} else { } else {
[b"(?:|.*/)", glob_re.as_slice(), GLOB_SUFFIX].concat() [b"(?:|.*/)", glob_re.as_slice(), GLOB_SUFFIX].concat()
} }
} }
PatternSyntax::Glob | PatternSyntax::RootGlob => { PatternSyntax::Glob | PatternSyntax::RootGlob => {
[glob_to_re(pattern).as_slice(), GLOB_SUFFIX].concat() [glob_to_re(pattern).as_slice(), GLOB_SUFFIX].concat()
} }
PatternSyntax::Include | PatternSyntax::SubInclude => unreachable!(),
} }
} }
const GLOB_SPECIAL_CHARACTERS: [u8; 7] = const GLOB_SPECIAL_CHARACTERS: [u8; 7] =
[b'*', b'?', b'[', b']', b'{', b'}', b'\\']; [b'*', b'?', b'[', b']', b'{', b'}', b'\\'];
/// TODO support other platforms /// TODO support other platforms
#[cfg(unix)] #[cfg(unix)]
let IgnorePattern { let IgnorePattern {
pattern, syntax, .. pattern, syntax, ..
} = entry; } = entry;
let pattern = match syntax { let pattern = match syntax {
PatternSyntax::RootGlob PatternSyntax::RootGlob
| PatternSyntax::Path | PatternSyntax::Path
| PatternSyntax::RelGlob | PatternSyntax::RelGlob
| PatternSyntax::RootFiles => normalize_path_bytes(&pattern), | PatternSyntax::RootFiles => normalize_path_bytes(&pattern),
PatternSyntax::Include | PatternSyntax::SubInclude => {
return Err(PatternError::NonRegexPattern(entry.clone()))
}
_ => pattern.to_owned(), _ => pattern.to_owned(),
}; };
if *syntax == PatternSyntax::RootGlob if *syntax == PatternSyntax::RootGlob
&& !pattern.iter().any(|b| GLOB_SPECIAL_CHARACTERS.contains(b)) && !pattern.iter().any(|b| GLOB_SPECIAL_CHARACTERS.contains(b))
{ {
let mut escaped = escape_pattern(&pattern); let mut escaped = escape_pattern(&pattern);
escaped.extend(GLOB_SUFFIX); escaped.extend(GLOB_SUFFIX);
Ok(escaped) Ok(escaped)
pattern: pattern.to_owned(), pattern: pattern.to_owned(),
source: source.as_ref().to_owned(), source: source.as_ref().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:`
/// patterns.
///
/// `subinclude:` is not treated as a special pattern here: unraveling them
/// needs to occur in the "ignore" phase.
pub fn get_patterns_from_file(
pattern_file: impl AsRef<Path>,
root_dir: impl AsRef<Path>,
) -> PatternResult<(Vec<IgnorePattern>, Vec<PatternFileWarning>)> {
let (patterns, mut warnings) = read_pattern_file(&pattern_file, true)?;
let patterns = patterns
.into_iter()
.flat_map(|entry| -> PatternResult<_> {
kevincoxUnsubmitted
Done

I don't see anywhere where this returns None. Should this just be .map() or .flat_map()?

kevincox: I don't see anywhere where this returns `None`. Should this just be `.map()` or `.flat_map()`?
let IgnorePattern {
syntax,
pattern,
source: _,
} = &entry;
Ok(match syntax {
kevincoxUnsubmitted
Not Done

I would write this as Some(Ok(match syntax { .. })) as the last expression of this map. Then instead of res.extend() and res.push() you just return the vecs.

kevincox: I would write this as `Some(Ok(match syntax { .. }))` as the last expression of this map. Then…
AlphareAuthorUnsubmitted
Done

The match arms would be incompatible. Or maybe I'm missing something.

Alphare: The `match` arms would be incompatible. Or maybe I'm missing something.
kevincoxUnsubmitted
Done

You would have to make sure you return a Vec on both arms. You will probably want inner_pats.into_iter().collect() on one branch and vec![entry] on the other branch.

kevincox: You would have to make sure you return a Vec on both arms. You will probably want `inner_pats.
PatternSyntax::Include => {
let inner_include =
root_dir.as_ref().join(get_path_from_bytes(&pattern));
let (inner_pats, inner_warnings) = get_patterns_from_file(
&inner_include,
root_dir.as_ref(),
)?;
warnings.extend(inner_warnings);
inner_pats
}
_ => vec![entry],
})
})
.flatten()
.collect();
Ok((patterns, warnings))
}
/// Holds all the information needed to handle a `subinclude:` pattern.
pub struct SubInclude {
/// Will be used for repository (hg) paths that start with this prefix.
/// It is relative to the current working directory, so comparing against
/// repository paths is painless.
pub prefix: HgPathBuf,
/// The file itself, containing the patterns
pub path: PathBuf,
/// Folder in the filesystem where this it applies
pub root: PathBuf,
}
impl SubInclude {
pub fn new(
root_dir: impl AsRef<Path>,
pattern: &[u8],
source: impl AsRef<Path>,
) -> Result<SubInclude, HgPathError> {
let normalized_source =
normalize_path_bytes(&get_bytes_from_path(source));
let source_root = get_path_from_bytes(&normalized_source);
let source_root = source_root.parent().unwrap_or(source_root.deref());
let path = source_root.join(get_path_from_bytes(pattern));
let new_root = path.parent().unwrap_or(path.deref());
let prefix = canonical_path(&root_dir, &root_dir, new_root)?;
Ok(Self {
prefix: path_to_hg_path_buf(prefix).and_then(|mut p| {
if !p.is_empty() {
p.push(b'/');
}
Ok(p)
})?,
path: path.to_owned(),
root: new_root.to_owned(),
})
}
}
/// Separate and pre-process subincludes from other patterns for the "ignore"
/// phase.
kevincoxUnsubmitted
Done

How about:

if !p.is_empty() {
  p.push(b'/');
}
Ok(p)
kevincox: How about: ``` if !p.is_empty() { p.push(b'/'); } Ok(p) ```
pub fn filter_subincludes(
ignore_patterns: &[IgnorePattern],
root_dir: impl AsRef<Path>,
) -> Result<(Vec<SubInclude>, Vec<&IgnorePattern>), HgPathError> {
let mut subincludes = vec![];
let mut others = vec![];
for ignore_pattern in ignore_patterns.iter() {
let IgnorePattern {
syntax,
pattern,
source,
} = ignore_pattern;
if *syntax == PatternSyntax::SubInclude {
subincludes.push(SubInclude::new(&root_dir, pattern, &source)?);
} else {
others.push(ignore_pattern)
}
}
Ok((subincludes, others))
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use pretty_assertions::assert_eq; use pretty_assertions::assert_eq;
#[test] #[test]
fn escape_pattern_test() { fn escape_pattern_test() {
let untouched = let untouched =

rust/hg-core/src/lib.rs

#[derive(Debug)] #[derive(Debug)]
pub enum PatternError { pub enum PatternError {
Path(HgPathError), Path(HgPathError),
UnsupportedSyntax(String), UnsupportedSyntax(String),
UnsupportedSyntaxInFile(String, String, usize), UnsupportedSyntaxInFile(String, String, usize),
TooLong(usize), TooLong(usize),
IO(std::io::Error), IO(std::io::Error),
/// Needed a pattern that can be turned into a regex but got one that can't
/// This should only happen through programmer error.
NonRegexPattern(IgnorePattern),
} }
impl ToString for PatternError { impl ToString for PatternError {
fn to_string(&self) -> String { fn to_string(&self) -> String {
match self { match self {
PatternError::UnsupportedSyntax(syntax) => { PatternError::UnsupportedSyntax(syntax) => {
format!("Unsupported syntax {}", syntax) format!("Unsupported syntax {}", syntax)
} }
PatternError::UnsupportedSyntaxInFile(syntax, file_path, line) => { PatternError::UnsupportedSyntaxInFile(syntax, file_path, line) => {
format!( format!(
"{}:{}: unsupported syntax {}", "{}:{}: unsupported syntax {}",
file_path, line, syntax file_path, line, syntax
) )
} }
PatternError::TooLong(size) => { PatternError::TooLong(size) => {
format!("matcher pattern is too long ({} bytes)", size) format!("matcher pattern is too long ({} bytes)", size)
} }
PatternError::IO(e) => e.to_string(), PatternError::IO(e) => e.to_string(),
PatternError::Path(e) => e.to_string(), PatternError::Path(e) => e.to_string(),
PatternError::NonRegexPattern(pattern) => {
format!("'{:?}' cannot be turned into a regex", pattern)
}
} }
} }
} }
impl From<DirstateMapError> for DirstateError { impl From<DirstateMapError> for DirstateError {
fn from(e: DirstateMapError) -> Self { fn from(e: DirstateMapError) -> Self {
DirstateError::Map(e) DirstateError::Map(e)
} }