This is an archive of the discontinued Mercurial Phabricator instance.

rust-matchers: add function to generate a regex matcher function
ClosedPublic

Authored by Alphare on Jan 17 2020, 5:41 AM.

Download Raw Diff

Details

Reviewers

pulkit
martinvonz
durin42

Group Reviewers

hg-reviewers

Commits

rHG52d40f8fb82d: rust-matchers: add function to generate a regex matcher function

Summary

This function will be used to help build the upcoming IncludeMatcher. While
Re2 is still used and behind a feature flag, this function returns an error
meant for fallback in the default case.

Diff Detail

Repository

rHG Mercurial

Branch

default

Lint

No Linters Available

Unit

No Unit Test Coverage

Event Timeline

Alphare created this revision.Jan 17 2020, 5:41 AM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptJan 17 2020, 5:41 AM

Herald added subscribers: mercurial-devel, kevincox, durin42. · View Herald Transcript

Alphare added a child revision: D7923: rust-matchers: add functions to get roots, dirs and parents from patterns.Jan 17 2020, 5:41 AM

pulkit accepted this revision.Feb 7 2020, 4:12 PM

This revision is now accepted and ready to land.Feb 7 2020, 4:12 PM

This one failed to apply on latest default, needs rebase.

Alphare edited parent revisions, added: D7914: rust-matchers: implement `visit_children_set` for `FileMatcher`; removed: D7921: rust-dirs-multiset: improve temporary error message.Feb 10 2020, 10:47 AM

martinvonz requested changes to this revision.Feb 10 2020, 7:48 PM

martinvonz added a subscriber: martinvonz.

martinvonz added inline comments.

rust/hg-core/src/matchers.rs
224	Hmm, I don't like to replicate this into Rust. I argued for a long time with Boris over a year ago that we should see if we can remove it from Python. He said they (Octobus, I think) would look into that if I would just queue the workaround for the time being. Could you see if you can simplify the Python code first instead? See https://patchwork.mercurial-scm.org/patch/36755/ for discussion.

This revision now requires changes to proceed.Feb 10 2020, 7:48 PM

Alphare added inline comments.Feb 11 2020, 5:09 AM

rust/hg-core/src/matchers.rs
224	I am having a little trouble reading the patchwork thread, but I gather that you want to get rid of the preventive splitting of patterns and just let the regex engine handle it on its own? Was this measure taken because of a bug in Python's `re` or because its exceptions were too coarse/unusable? I'll look into the behavior of Re2, in that case.

Alphare updated this revision to Diff 20154.Feb 11 2020, 5:54 AM

Alphare added inline comments.Feb 11 2020, 5:58 AM

rust/hg-core/src/matchers.rs
224	I've looked into the behavior of Re2. It will return an error if the DFA runs out of memory, which seems perfectly reasonable. I will simplify the Rust code, however I feel like you're better suited than I am to fix the Python side of things, since I don't really understand the ins-and-outs of the problem.

Alphare updated this revision to Diff 20160.Feb 11 2020, 6:13 AM

I think this looks good, but I want Martin to ack that his concern is adequately resolved.

In D7922#121720, @durin42 wrote:

I think this looks good, but I want Martin to ack that his concern is adequately resolved.

Yes, looks good to me now. Thanks.

rust/hg-core/src/matchers.rs
224	Thanks for fixing the Rust side.

This revision is now accepted and ready to land.Feb 28 2020, 12:38 PM

Alphare edited parent revisions, added: D8224: rust-format: cleanup ancestors.rs to make rustfmt happy; removed: D7914: rust-matchers: implement `visit_children_set` for `FileMatcher`.Mar 5 2020, 3:33 AM

@martinvonz this (and next few patches) seems accepted by you and Augie. Should I go ahead and push it or I missed some discussion on IRC about it?

In D7922#123057, @pulkit wrote:

@martinvonz this (and next few patches) seems accepted by you and Augie. Should I go ahead and push it or I missed some discussion on IRC about it?

You can push it. I wanted to finish the cleanup to the regex grouping that I mentioned earlier, but that can be done separately. But that's the only reason I haven't queued this patch anyway.

In D7922#123193, @martinvonz wrote:

In D7922#123057, @pulkit wrote:

@martinvonz this (and next few patches) seems accepted by you and Augie. Should I go ahead and push it or I missed some discussion on IRC about it?

You can push it. I wanted to finish the cleanup to the regex grouping that I mentioned earlier, but that can be done separately. But that's the only reason I haven't queued this patch anyway.

You mean cleanup on the Python side or on the rust side? IIUC, the rust side is fine now.

In D7922#123207, @pulkit wrote:

In D7922#123193, @martinvonz wrote:

In D7922#123057, @pulkit wrote:

@martinvonz this (and next few patches) seems accepted by you and Augie. Should I go ahead and push it or I missed some discussion on IRC about it?

You can push it. I wanted to finish the cleanup to the regex grouping that I mentioned earlier, but that can be done separately. But that's the only reason I haven't queued this patch anyway.

You mean cleanup on the Python side or on the rust side? IIUC, the rust side is fine now.

On the Python side.

Alphare added a commit: rHG52d40f8fb82d: rust-matchers: add function to generate a regex matcher function.Mar 11 2020, 12:22 PM

Closed by commit rHG52d40f8fb82d: rust-matchers: add function to generate a regex matcher function (authored by Alphare). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			rust/hg-core/src/lib.rs (7 lines)
M			rust/hg-core/src/matchers.rs (29 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	19401		Jan 17 2020, 5:41 AM	★	★
Diff 2	20154		Feb 11 2020, 5:54 AM	★	★
Diff 3	20160		Feb 11 2020, 6:13 AM	★	★
Diff 4	20712	rHG52d40f8fb82d64f424b04a71e23ac3456830f9c3	Jan 17 2020, 5:29 AM	★	★

Commit	Parents	Author	Summary	Date
f907e36f642f	69d573303bb8	Raphaël Gomès		Jan 17 2020, 5:29 AM

Status	Author	Revision
Closed	Alphare	D8253 hg-core: add function timing information
Closed	Alphare	D8252 rust: add logging utils
Closed	Alphare	D8251 rust-status: traverse working directory in parallel
Closed	Alphare	D8250 rust-status: wrap `stat_dmap_entries` to ease profiling
Closed	Alphare	D8249 rust-status: refactor handling of unknown files
Closed	Alphare	D8215 rust-status: move to recursive traversal to prepare for parallel traversal
Closed	Alphare	D8213 rust-pathauditor: use interior mutability for use in multi-threaded contexts
Closed	Alphare	D8212 rust-pathauditor: actually populate the `audited_dirs` cache
Closed	Alphare	D8211 rust-pathauditor: actually split Windows shortname aliases at `~`
Closed	Alphare	D8226 debuginstall: add entry about re2 Rust bindings when applicable
Closed	Alphare	D8225 rust-cpython: add `debug` module to expose debug information to Python
Closed	Alphare	D7931 rust-status: use bare hg status fastpath from Python
Closed	Alphare	D7930 rust-status: update rust-cpython bridge to account for the changes in core
Closed	Alphare	D7929 rust-status: add bare `hg status` support in hg-core
Closed	Alphare	D7928 rust-status: add function for sequential traversal of the working directory
Closed	Alphare	D8088 rust-status: add missing variants to `Dispatch` enum
Closed	Alphare	D8087 rust-status: rename `StatusResult` to `DirstateStatus`
Closed	Alphare	D8086 rust-status: refactor options into a `StatusOptions` struct
Closed	Alphare	D7927 rust-status: add util for listing a directory
Closed	Alphare	D7926 rust-dependencies: update rayon
Closed	Alphare	D7925 rust-matchers: add `IgnoreMatcher`
Closed	Alphare	D7924 rust-matchers: add `build_regex_match` function
Closed	Alphare	D7923 rust-matchers: add functions to get roots, dirs and parents from patterns
Closed	Alphare	D7922 rust-matchers: add function to generate a regex matcher function
Closed	Alphare	D8224 rust-format: cleanup ancestors.rs to make rustfmt happy

Diff 19401

rust/hg-core/src/lib.rs

	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			/// Needed a pattern that can be turned into a regex but got one that can't
	/// This should only happen through programmer error.			/// This should only happen through programmer error.
	NonRegexPattern(IgnorePattern),			NonRegexPattern(IgnorePattern),
				/// This is temporary, see `re2/mod.rs`.
				/// This will cause a fallback to Python.
				Re2NotInstalled,
	}			}

	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) => {			PatternError::NonRegexPattern(pattern) => {
	format!("'{:?}' cannot be turned into a regex", pattern)			format!("'{:?}' cannot be turned into a regex", pattern)
	}			}
				PatternError::Re2NotInstalled => {
				"Re2 is not installed, cannot use regex functionality."
				.to_string()
				}
	}			}
	}			}
	}			}

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

rust/hg-core/src/matchers.rs

	// matchers.rs			// matchers.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.

	//! Structs and types for matching files and directories.			//! Structs and types for matching files and directories.

	use crate::{utils::hg_path::HgPath, DirsMultiset, DirstateMapError};			#[cfg(feature = "with-re2")]
				use crate::re2::Re2;
				use crate::{
				filepatterns::PatternResult, utils::hg_path::HgPath, DirsMultiset,
				DirstateMapError, PatternError,
				};
	use std::collections::HashSet;			use std::collections::HashSet;
	use std::iter::FromIterator;			use std::iter::FromIterator;
	use std::ops::Deref;			use std::ops::Deref;

	#[derive(Debug, PartialEq)]			#[derive(Debug, PartialEq)]
	pub enum VisitChildrenSet<'a> {			pub enum VisitChildrenSet<'a> {
	/// Don't visit anything			/// Don't visit anything
	Empty,			Empty,
	}			}
	fn matches_everything(&self) -> bool {			fn matches_everything(&self) -> bool {
	false			false
	}			}
	fn is_exact(&self) -> bool {			fn is_exact(&self) -> bool {
	true			true
	}			}
	}			}

				const MAX_RE_SIZE: usize = 20000;
				martinvonzUnsubmitted Not Done Hmm, I don't like to replicate this into Rust. I argued for a long time with Boris over a year ago that we should see if we can remove it from Python. He said they (Octobus, I think) would look into that if I would just queue the workaround for the time being. Could you see if you can simplify the Python code first instead? See https://patchwork.mercurial-scm.org/patch/36755/ for discussion. martinvonz: Hmm, I don't like to replicate this into Rust. I argued for a long time with Boris over a year…
				AlphareAuthorUnsubmitted Done I am having a little trouble reading the patchwork thread, but I gather that you want to get rid of the preventive splitting of patterns and just let the regex engine handle it on its own? Was this measure taken because of a bug in Python's `re` or because its exceptions were too coarse/unusable? I'll look into the behavior of Re2, in that case. Alphare: I am having a little trouble reading the patchwork thread, but I gather that you want to get…
				AlphareAuthorUnsubmitted Done I've looked into the behavior of Re2. It will return an error if the DFA runs out of memory, which seems perfectly reasonable. I will simplify the Rust code, however I feel like you're better suited than I am to fix the Python side of things, since I don't really understand the ins-and-outs of the problem. Alphare: I've looked into the behavior of Re2. It will return an error if the DFA runs out of memory…
				martinvonzUnsubmitted Not Done Thanks for fixing the Rust side. martinvonz: Thanks for fixing the Rust side.

				#[cfg(feature = "with-re2")]
				/// Returns a function that matches an `HgPath` against the given regex
				/// pattern.
				///
				/// This can fail when the pattern is invalid or not supported by the
				/// underlying engine `Re2`, for instance anything with back-references.
				fn re_matcher(
				pattern: &[u8],
				) -> PatternResult<impl Fn(&HgPath) -> bool + Sync> {
				let regex = Re2::new(pattern);
				let regex = regex.map_err(\|e\| PatternError::UnsupportedSyntax(e))?;
				Ok(move \|path: &HgPath\| regex.is_match(path.as_bytes()))
				}

				#[cfg(not(feature = "with-re2"))]
				fn re_matcher(_: &[u8]) -> PatternResult<Box<dyn Fn(&HgPath) -> bool + Sync>> {
				Err(PatternError::Re2NotInstalled)
				}

	#[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 test_filematcher_visit_children_set() {			fn test_filematcher_visit_children_set() {
	// Visitchildrenset			// Visitchildrenset