This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6485

rust-filepatterns: use bytes instead of String
ClosedPublic

Authored by Alphare on Jun 6 2019, 11:24 AM.

Details

Reviewers
kevincox
Group Reviewers
hg-reviewers
Commits
rHG9609430d3625: rust-filepatterns: use bytes instead of String
Summary

In my initial patch, I introduced an unnecessary hard constraint on UTF-8
filenames and patterns which I forgot to remove. Although the performance
penalty for using String might be negligible, we don't want to break
compatibility with non-UTF-8 encodings for no reason.
Moreover, this change allows for a cleaner Rust core API.

This patch introduces a new utils module that is used with this fix.

Finally, PatternError was not put inside the Python module generated by
Rust, which would have raised a NameError.

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.

Revision Contents
Changeset List

Diff 15417

rust/hg-core/src/filepatterns.rs

use crate::{LineNumber, PatternError, PatternFileError}; use crate::{LineNumber, PatternError, PatternFileError};
use regex::Regex; use regex::bytes::Regex;
use std::collections::HashMap; use std::collections::HashMap;
use std::fs::File; use std::fs::File;
use std::io::Read; use std::io::Read;
use std::vec::Vec; use std::vec::Vec;
use utils::files::get_path_from_bytes;
use utils::{replace_slice, SliceExt};
lazy_static! { lazy_static! {
static ref reescape: Vec<Vec<u8>> = { static ref reescape: 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 {
v[*byte as usize].insert(0, b'\\'); v[*byte as usize].insert(0, b'\\');
} }
} }
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'\\'];
/// Wrapper function to `_build_single_regex` that short-circuits 'exact' globs /// Wrapper function to `_build_single_regex` that short-circuits 'exact' globs
/// that don't need to be transformed into a regex. /// that don't need to be transformed into a regex.
pub fn build_single_regex( pub fn build_single_regex(
kind: &str, kind: &[u8],
pat: &[u8], pat: &[u8],
globsuffix: &[u8], globsuffix: &[u8],
) -> Result<Vec<u8>, PatternError> { ) -> Result<Vec<u8>, PatternError> {
let enum_kind = parse_pattern_syntax(kind.as_bytes())?; let enum_kind = parse_pattern_syntax(kind)?;
if enum_kind == PatternSyntax::RootGlob if enum_kind == PatternSyntax::RootGlob
&& pat.iter().all(|b| GLOB_SPECIAL_CHARACTERS.contains(b)) && pat.iter().all(|b| GLOB_SPECIAL_CHARACTERS.contains(b))
{ {
Ok(pat.to_vec()) Ok(pat.to_vec())
} else { } else {
Ok(_build_single_regex(enum_kind, pat, globsuffix)) Ok(_build_single_regex(enum_kind, pat, globsuffix))
} }
} }
lazy_static! { lazy_static! {
static ref SYNTAXES: HashMap<&'static str, &'static str> = { static ref SYNTAXES: HashMap<&'static [u8], &'static [u8]> = {
let mut m = HashMap::new(); let mut m = HashMap::new();
m.insert("re", "relre:"); m.insert(b"re".as_ref(), b"relre:".as_ref());
m.insert("regexp", "relre:"); m.insert(b"regexp".as_ref(), b"relre:".as_ref());
m.insert("glob", "relglob:"); m.insert(b"glob".as_ref(), b"relglob:".as_ref());
m.insert("rootglob", "rootglob:"); m.insert(b"rootglob".as_ref(), b"rootglob:".as_ref());
m.insert("include", "include"); m.insert(b"include".as_ref(), b"include".as_ref());
m.insert("subinclude", "subinclude"); m.insert(b"subinclude".as_ref(), b"subinclude".as_ref());
m m
}; };
} }
pub type PatternTuple = (String, LineNumber, String); pub type PatternTuple = (Vec<u8>, LineNumber, Vec<u8>);
type WarningTuple = (String, String); type WarningTuple = (String, String);
pub fn parse_pattern_file_contents( pub fn parse_pattern_file_contents(
lines: &str, lines: &[u8],
file_path: &str, file_path: &[u8],
warn: bool, warn: bool,
) -> (Vec<PatternTuple>, Vec<WarningTuple>) { ) -> (Vec<PatternTuple>, Vec<WarningTuple>) {
let comment_regex = Regex::new(r"((?:^|[^\\])(?:\\\\)*)#.*").unwrap(); let comment_regex = Regex::new(r"((?:^|[^\\])(?:\\\\)*)#.*").unwrap();
let mut inputs: Vec<PatternTuple> = vec![]; let mut inputs: Vec<PatternTuple> = vec![];
let mut warnings: Vec<WarningTuple> = vec![]; let mut warnings: Vec<WarningTuple> = vec![];
let mut current_syntax = "relre:"; let mut current_syntax = b"relre:".as_ref();
let mut line = String::new(); for (line_number, mut line) in lines.split(|c| *c == b'\n').enumerate() {
for (line_number, line_str) in lines.split('\n').enumerate() {
let line_number = line_number + 1; let line_number = line_number + 1;
line.replace_range(.., line_str);
if line.contains('#') { if line.contains(&('#' as u8)) {
if let Some(cap) = comment_regex.captures(line.clone().as_ref()) { if let Some(cap) = comment_regex.captures(line) {
line = line[..cap.get(1).unwrap().end()].to_string() line = &line[..cap.get(1).unwrap().end()]
} }
line = line.replace(r"\#", "#"); let mut line = line.to_owned();
replace_slice(&mut line, br"\#", b"#");
} }
let mut line = line.trim_end(); let mut line = line.trim_end();
if line.is_empty() { if line.is_empty() {
continue; continue;
} }
if line.starts_with("syntax:") { if line.starts_with(b"syntax:") {
let syntax = line["syntax:".len()..].trim(); let syntax = line[b"syntax:".len()..].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((file_path.to_string(), syntax.to_string())); warnings.push((
String::from_utf8_lossy(file_path).to_string(),
String::from_utf8_lossy(syntax).to_string(),
));
} }
continue; continue;
} }
let mut line_syntax: &str = &current_syntax; let mut line_syntax: &[u8] = &current_syntax;
for (s, rels) in SYNTAXES.iter() { for (s, rels) in SYNTAXES.iter() {
if line.starts_with(rels) { if line.starts_with(rels) {
line_syntax = rels; line_syntax = rels;
line = &line[rels.len()..]; line = &line[rels.len()..];
break; break;
} else if line.starts_with(&format!("{}:", s)) { } else if line.starts_with(&[s, b":".as_ref()].concat()) {
line_syntax = rels; line_syntax = rels;
line = &line[s.len() + 1..]; line = &line[s.len() + 1..];
break; break;
} }
} }
inputs.push(( inputs.push((
format!("{}{}", line_syntax, line), [line_syntax, line].concat(),
line_number, line_number,
line.to_string(), line.to_owned(),
)); ));
} }
(inputs, warnings) (inputs, warnings)
} }
pub fn read_pattern_file( pub fn read_pattern_file(
file_path: String, file_path: &[u8],
warn: bool, warn: bool,
) -> Result<(Vec<PatternTuple>, Vec<WarningTuple>), PatternFileError> { ) -> Result<(Vec<PatternTuple>, Vec<WarningTuple>), PatternFileError> {
let mut f = File::open(&file_path)?; let mut f = File::open(get_path_from_bytes(file_path))?;
let mut contents = String::new(); let mut contents = Vec::new();
f.read_to_string(&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))
} }
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
#[test] #[test]
fn escape_pattern_test() { fn escape_pattern_test() {
assert_eq!(glob_to_re(br#"a/**/b"#), br#"a/(?:.*/)?b"#); assert_eq!(glob_to_re(br#"a/**/b"#), br#"a/(?:.*/)?b"#);
assert_eq!(glob_to_re(br#"[a*?!^][^b][!c]"#), br#"[a*?!^][\^b][^c]"#); assert_eq!(glob_to_re(br#"[a*?!^][^b][!c]"#), br#"[a*?!^][\^b][^c]"#);
assert_eq!(glob_to_re(br#"{a,b}"#), br#"(?:a|b)"#); assert_eq!(glob_to_re(br#"{a,b}"#), br#"(?:a|b)"#);
assert_eq!(glob_to_re(br#".\*\?"#), br#"\.\*\?"#); assert_eq!(glob_to_re(br#".\*\?"#), br#"\.\*\?"#);
} }
#[test] #[test]
fn test_parse_pattern_file_contents() { fn test_parse_pattern_file_contents() {
let lines = "syntax: glob\n*.elc"; let lines = b"syntax: glob\n*.elc";
assert_eq!( assert_eq!(
vec![("relglob:*.elc".to_string(), 2, "*.elc".to_string())], vec![(b"relglob:*.elc".to_vec(), 2, b"*.elc".to_vec())],
parse_pattern_file_contents(lines, "file_path", false).0, parse_pattern_file_contents(lines, b"file_path", false).0,
); );
let lines = "syntax: include\nsyntax: glob"; let lines = b"syntax: include\nsyntax: glob";
assert_eq!( assert_eq!(
parse_pattern_file_contents(lines, "file_path", false).0, parse_pattern_file_contents(lines, b"file_path", false).0,
vec![] vec![]
); );
let lines = b"glob:**.o";
assert_eq!(
parse_pattern_file_contents(lines, b"file_path", false).0,
vec![(b"relglob:**.o".to_vec(), 1, b"**.o".to_vec())]
);
} }
} }

rust/hg-core/src/lib.rs

mod dirstate; mod dirstate;
pub mod discovery; pub mod discovery;
pub mod testing; // unconditionally built, for use from integration tests pub mod testing; // unconditionally built, for use from integration tests
pub use dirstate::{ pub use dirstate::{
parsers::{pack_dirstate, parse_dirstate}, parsers::{pack_dirstate, parse_dirstate},
CopyVec, CopyVecEntry, DirstateEntry, DirstateParents, DirstateVec, CopyVec, CopyVecEntry, DirstateEntry, DirstateParents, DirstateVec,
}; };
mod filepatterns; mod filepatterns;
mod utils;
pub use filepatterns::{ pub use filepatterns::{
build_single_regex, read_pattern_file, PatternSyntax, PatternTuple, build_single_regex, read_pattern_file, PatternSyntax, PatternTuple,
}; };
/// Mercurial revision numbers /// Mercurial revision numbers
/// ///
/// As noted in revlog.c, revision numbers are actually encoded in /// As noted in revlog.c, revision numbers are actually encoded in

rust/hg-core/src/utils/files.rs

  • This file was added.
use std::path::Path;
pub fn get_path_from_bytes(bytes: &[u8]) -> &Path {
let os_str;
#[cfg(unix)]
{
use std::os::unix::ffi::OsStrExt;
os_str = std::ffi::OsStr::from_bytes(bytes);
}
#[cfg(windows)]
{
use std::os::windows::ffi::OsStrExt;
os_str = std::ffi::OsString::from_wide(bytes);
}
Path::new(os_str)
}

rust/hg-core/src/utils/mod.rs

  • This file was added.
pub mod files;
pub fn replace_slice<T>(buf: &mut [T], from: &[T], to: &[T])
where
T: Clone + PartialEq,
{
if buf.len() < from.len() || from.len() != to.len() {
return;
}
for i in 0..=buf.len() - from.len() {
if buf[i..].starts_with(from) {
buf[i..(i + from.len())].clone_from_slice(to);
}
}
}
pub trait SliceExt {
fn trim(&self) -> &Self;
fn trim_end(&self) -> &Self;
}
fn is_not_whitespace(c: &u8) -> bool {
!(*c as char).is_whitespace()
}
impl SliceExt for [u8] {
fn trim(&self) -> &[u8] {
if let Some(first) = self.iter().position(is_not_whitespace) {
if let Some(last) = self.iter().rposition(is_not_whitespace) {
&self[first..last + 1]
} else {
unreachable!();
}
} else {
&[]
}
}
fn trim_end(&self) -> &[u8] {
if let Some(last) = self.iter().rposition(is_not_whitespace) {
&self[..last + 1]
} else {
&[]
}
}
}

rust/hg-cpython/src/filepatterns.rs

// filepatterns.rs // filepatterns.rs
// //
// Copyright 2019, Georges Racinet <gracinet@anybox.fr>, // Copyright 2019, Georges Racinet <gracinet@anybox.fr>,
// Raphaël Gomès <rgomes@octobus.net> // 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.
//! Bindings for the `hg::filepatterns` module provided by the //! Bindings for the `hg::filepatterns` module provided by the
//! `hg-core` crate. From Python, this will be seen as `rustext.filepatterns` //! `hg-core` crate. From Python, this will be seen as `rustext.filepatterns`
//! and can be used as replacement for the the pure `filepatterns` Python module. //! and can be used as replacement for the the pure `filepatterns` Python module.
//! //!
use cpython::{ use cpython::{
exc, PyDict, PyErr, PyModule, PyResult, PyString, PyTuple, Python, PyBytes, PyDict, PyModule, PyObject, PyResult, PyTuple, Python, ToPyObject,
ToPyObject,
};
use hg::{build_single_regex, read_pattern_file, PatternTuple};
use exceptions::{
PatternError,
PatternFileError,
}; };
use exceptions::{PatternError, PatternFileError};
use hg::{build_single_regex, read_pattern_file, LineNumber, PatternTuple};
/// Rust does not like functions with different return signatures. /// Rust does not like functions with different return signatures.
/// The 3-tuple version is always returned by the hg-core function, /// The 3-tuple version is always returned by the hg-core function,
/// the (potential) conversion is handled at this level since it is not likely /// the (potential) conversion is handled at this level since it is not likely
/// to have any measurable impact on performance. /// to have any measurable impact on performance.
/// ///
/// The Python implementation passes a function reference for `warn` instead /// The Python implementation passes a function reference for `warn` instead
/// of a boolean that is used to emit warnings while parsing. The Rust /// of a boolean that is used to emit warnings while parsing. The Rust
/// implementation chooses to accumulate the warnings and propagate them to /// implementation chooses to accumulate the warnings and propagate them to
/// Python upon completion. See the `readpatternfile` function in `match.py` /// Python upon completion. See the `readpatternfile` function in `match.py`
/// for more details. /// for more details.
fn read_pattern_file_wrapper( fn read_pattern_file_wrapper(
py: Python, py: Python,
file_path: String, file_path: PyObject,
warn: bool, warn: bool,
source_info: bool, source_info: bool,
) -> PyResult<PyTuple> { ) -> PyResult<PyTuple> {
match read_pattern_file(file_path, warn) { match read_pattern_file(file_path.extract::<PyBytes>(py)?.data(py), warn) {
Ok((patterns, warnings)) => { Ok((patterns, warnings)) => {
if source_info { if source_info {
return Ok((patterns, warnings).to_py_object(py)); let itemgetter = |x: &PatternTuple| {
(PyBytes::new(py, &x.0), x.1, PyBytes::new(py, &x.2))
};
let results: Vec<(PyBytes, LineNumber, PyBytes)> =
patterns.iter().map(itemgetter).collect();
return Ok((results, warnings).to_py_object(py));
} }
let itemgetter = |x: &PatternTuple| x.0.to_py_object(py); let itemgetter = |x: &PatternTuple| PyBytes::new(py, &x.0);
let results: Vec<PyString> = let results: Vec<PyBytes> =
patterns.iter().map(itemgetter).collect(); patterns.iter().map(itemgetter).collect();
Ok((results, warnings).to_py_object(py)) Ok((results, warnings).to_py_object(py))
} }
Err(e) => Err(PatternFileError::pynew(py, e)), Err(e) => Err(PatternFileError::pynew(py, e)),
} }
} }
fn build_single_regex_wrapper( fn build_single_regex_wrapper(
py: Python, py: Python,
kind: String, kind: PyObject,
pat: String, pat: PyObject,
globsuffix: String, globsuffix: PyObject,
) -> PyResult<PyString> { ) -> PyResult<PyBytes> {
match build_single_regex( match build_single_regex(
kind.as_ref(), kind.extract::<PyBytes>(py)?.data(py),
pat.as_bytes(), pat.extract::<PyBytes>(py)?.data(py),
globsuffix.as_bytes(), globsuffix.extract::<PyBytes>(py)?.data(py),
) { ) {
Ok(regex) => match String::from_utf8(regex) { Ok(regex) => Ok(PyBytes::new(py, &regex)),
Ok(regex) => Ok(regex.to_py_object(py)),
Err(e) => Err(PyErr::new::<exc::UnicodeDecodeError, _>(
py,
e.to_string(),
)),
},
Err(e) => Err(PatternError::pynew(py, e)), Err(e) => Err(PatternError::pynew(py, e)),
} }
} }
pub fn init_module(py: Python, package: &str) -> PyResult<PyModule> { pub fn init_module(py: Python, package: &str) -> PyResult<PyModule> {
let dotted_name = &format!("{}.filepatterns", package); let dotted_name = &format!("{}.filepatterns", package);
let m = PyModule::new(py, dotted_name)?; let m = PyModule::new(py, dotted_name)?;
m.add(py, "__package__", package)?; m.add(py, "__package__", package)?;
m.add( m.add(
py, py,
"__doc__", "__doc__",
"Patterns files parsing - Rust implementation", "Patterns files parsing - Rust implementation",
)?; )?;
m.add( m.add(
py, py,
"build_single_regex", "build_single_regex",
py_fn!( py_fn!(
py, py,
build_single_regex_wrapper( build_single_regex_wrapper(
kind: String, kind: PyObject,
pat: String, pat: PyObject,
globsuffix: String globsuffix: PyObject
) )
), ),
)?; )?;
m.add( m.add(
py, py,
"read_pattern_file", "read_pattern_file",
py_fn!( py_fn!(
py, py,
read_pattern_file_wrapper( read_pattern_file_wrapper(
file_path: String, file_path: PyObject,
warn: bool, warn: bool,
source_info: bool source_info: bool
) )
), ),
)?; )?;
m.add(py, "PatternError", py.get_type::<PatternError>())?;
let sys = PyModule::import(py, "sys")?; let sys = PyModule::import(py, "sys")?;
let sys_modules: PyDict = sys.get(py, "modules")?.extract(py)?; let sys_modules: PyDict = sys.get(py, "modules")?.extract(py)?;
sys_modules.set_item(py, dotted_name, &m)?; sys_modules.set_item(py, dotted_name, &m)?;
Ok(m) Ok(m)
} }