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
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

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 20039

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)
} }