This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7910

rust-re2: add wrapper for calling Re2 from Rust
ClosedPublic

Authored by Alphare on Jan 16 2020, 8:54 AM.

Details

Reviewers
kevincox
Group Reviewers
hg-reviewers
Commits
rHGd8d4fa9a7f18: rust-re2: add wrapper for calling Re2 from Rust
rHGcdf3e49a0572: rust-re2: add wrapper for calling Re2 from Rust
Summary

This assumes that Re2 is installed following Google's guide. I am not sure
how we want to integrate it in the project, but I think a follow-up patch would
be more appropriate for such work.
As it stands, *not* having Re2 installed results in a compilation error, which
is a problem as it breaks install compatibility. Hence, this is gated behind
a non-default with-re2 compilation feature.

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, 8:54 AM
kevincox accepted this revision.Jan 16 2020, 10:41 AM

Does mercurial currently use RE2? If not then can you explain why we don't just use the rust regex crate?

kevincox requested changes to this revision.Jan 16 2020, 10:42 AM
kevincox added inline comments.
rust/hg-core/src/re2/rust_re2.cpp
23

This is never freed. Should we add a Drop implementation in rust?

This revision now requires changes to proceed.Jan 16 2020, 10:42 AM
durin42 added a comment.Jan 16 2020, 10:43 AM
In D7910#116208, @kevincox wrote:

Does mercurial currently use RE2? If not then can you explain why we don't just use the rust regex crate?

We have support for using re2 iff the correct Python bindings are installed. That said, I thought the rust regex crate would be compatible with re2? If we can't use that, we should document why so that future hackers (like me) don't try and simplify....

kevincox added a comment.Jan 16 2020, 11:06 AM

While I think the rust regex crate is heavily inspired by RE2 and uses a fairly compatible syntax I don't think there is a goal to be 100% compatible and I would be surprised if there weren't any differences. What I am trying to say is that if we are going to change engines we may as well change to regex. If we are using RE2 for backwards compatibility then this change makes sense.

Alphare marked an inline comment as done.Jan 16 2020, 4:50 PM
Alphare added a parent revision: D7909: rust-filepatterns: add support for `include` and `subinclude` patterns.
Alphare updated this revision to Diff 19386.
kevincox accepted this revision.Jan 17 2020, 10:08 AM
Alphare updated this revision to Diff 19546.Jan 24 2020, 4:57 AM
Alphare updated this revision to Diff 19938.Feb 6 2020, 10:14 AM
Alphare updated this revision to Diff 20040.Feb 10 2020, 10:46 AM
Alphare added a commit: rHGcdf3e49a0572: rust-re2: add wrapper for calling Re2 from Rust.Feb 10 2020, 6:30 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGcdf3e49a0572: rust-re2: add wrapper for calling Re2 from Rust (authored by Alphare). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 20080

rust/Cargo.lock

name = "c2-chacha" name = "c2-chacha"
version = "0.2.3" version = "0.2.3"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [ dependencies = [
"ppv-lite86 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)", "ppv-lite86 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)",
] ]
[[package]] [[package]]
name = "cc"
version = "1.0.50"
source = "registry+https://github.com/rust-lang/crates.io-index"
[[package]]
name = "cfg-if" name = "cfg-if"
version = "0.1.10" version = "0.1.10"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
[[package]] [[package]]
name = "cloudabi" name = "cloudabi"
version = "0.0.3" version = "0.0.3"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
version = "0.4.0" version = "0.4.0"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
[[package]] [[package]]
name = "hg-core" name = "hg-core"
version = "0.1.0" version = "0.1.0"
dependencies = [ dependencies = [
"byteorder 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
"cc 1.0.50 (registry+https://github.com/rust-lang/crates.io-index)",
"hex 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "hex 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
"lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.66 (registry+https://github.com/rust-lang/crates.io-index)",
"memchr 2.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "memchr 2.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
"pretty_assertions 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", "pretty_assertions 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)",
"rand 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)",
"rand_pcg 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "rand_pcg 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
"rayon 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "rayon 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
"regex 1.3.4 (registry+https://github.com/rust-lang/crates.io-index)", "regex 1.3.4 (registry+https://github.com/rust-lang/crates.io-index)",
"same-file 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)", "same-file 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)",
"tempfile 3.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "tempfile 3.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
[metadata] [metadata]
"checksum aho-corasick 0.7.8 (registry+https://github.com/rust-lang/crates.io-index)" = "743ad5a418686aad3b87fd14c43badd828cf26e214a00f92a384291cf22e1811" "checksum aho-corasick 0.7.8 (registry+https://github.com/rust-lang/crates.io-index)" = "743ad5a418686aad3b87fd14c43badd828cf26e214a00f92a384291cf22e1811"
"checksum ansi_term 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ee49baf6cb617b853aa8d93bf420db2383fab46d314482ca2803b40d5fde979b" "checksum ansi_term 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ee49baf6cb617b853aa8d93bf420db2383fab46d314482ca2803b40d5fde979b"
"checksum autocfg 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)" = "1d49d90015b3c36167a20fe2810c5cd875ad504b39cff3d4eae7977e6b7c1cb2" "checksum autocfg 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)" = "1d49d90015b3c36167a20fe2810c5cd875ad504b39cff3d4eae7977e6b7c1cb2"
"checksum autocfg 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f8aac770f1885fd7e387acedd76065302551364496e46b3dd00860b2f8359b9d" "checksum autocfg 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f8aac770f1885fd7e387acedd76065302551364496e46b3dd00860b2f8359b9d"
"checksum bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" "checksum bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693"
"checksum byteorder 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "a7c3dd8985a7111efc5c80b44e23ecdd8c007de8ade3b96595387e812b957cf5" "checksum byteorder 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "a7c3dd8985a7111efc5c80b44e23ecdd8c007de8ade3b96595387e812b957cf5"
"checksum c2-chacha 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)" = "214238caa1bf3a496ec3392968969cab8549f96ff30652c9e56885329315f6bb" "checksum c2-chacha 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)" = "214238caa1bf3a496ec3392968969cab8549f96ff30652c9e56885329315f6bb"
"checksum cc 1.0.50 (registry+https://github.com/rust-lang/crates.io-index)" = "95e28fa049fda1c330bcf9d723be7663a899c4679724b34c81e9f5a326aab8cd"
"checksum cfg-if 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)" = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" "checksum cfg-if 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)" = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822"
"checksum cloudabi 0.0.3 (registry+https://github.com/rust-lang/crates.io-index)" = "ddfc5b9aa5d4507acaf872de71051dfd0e309860e88966e1051e462a077aac4f" "checksum cloudabi 0.0.3 (registry+https://github.com/rust-lang/crates.io-index)" = "ddfc5b9aa5d4507acaf872de71051dfd0e309860e88966e1051e462a077aac4f"
"checksum cpython 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "bfaf3847ab963e40c4f6dd8d6be279bdf74007ae2413786a0dcbb28c52139a95" "checksum cpython 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "bfaf3847ab963e40c4f6dd8d6be279bdf74007ae2413786a0dcbb28c52139a95"
"checksum crossbeam-deque 0.7.2 (registry+https://github.com/rust-lang/crates.io-index)" = "c3aa945d63861bfe624b55d153a39684da1e8c0bc8fba932f7ee3a3c16cea3ca" "checksum crossbeam-deque 0.7.2 (registry+https://github.com/rust-lang/crates.io-index)" = "c3aa945d63861bfe624b55d153a39684da1e8c0bc8fba932f7ee3a3c16cea3ca"
"checksum crossbeam-epoch 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "5064ebdbf05ce3cb95e45c8b086f72263f4166b29b97f6baff7ef7fe047b55ac" "checksum crossbeam-epoch 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "5064ebdbf05ce3cb95e45c8b086f72263f4166b29b97f6baff7ef7fe047b55ac"
"checksum crossbeam-queue 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "c695eeca1e7173472a32221542ae469b3e9aac3a4fc81f7696bcad82029493db" "checksum crossbeam-queue 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "c695eeca1e7173472a32221542ae469b3e9aac3a4fc81f7696bcad82029493db"
"checksum crossbeam-utils 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ce446db02cdc3165b94ae73111e570793400d0794e46125cc4056c81cbb039f4" "checksum crossbeam-utils 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ce446db02cdc3165b94ae73111e570793400d0794e46125cc4056c81cbb039f4"
"checksum ctor 0.1.12 (registry+https://github.com/rust-lang/crates.io-index)" = "cd8ce37ad4184ab2ce004c33bf6379185d3b1c95801cab51026bd271bf68eedc" "checksum ctor 0.1.12 (registry+https://github.com/rust-lang/crates.io-index)" = "cd8ce37ad4184ab2ce004c33bf6379185d3b1c95801cab51026bd271bf68eedc"

rust/hg-core/Cargo.toml

[package] [package]
name = "hg-core" name = "hg-core"
version = "0.1.0" version = "0.1.0"
authors = ["Georges Racinet <gracinet@anybox.fr>"] authors = ["Georges Racinet <gracinet@anybox.fr>"]
description = "Mercurial pure Rust core library, with no assumption on Python bindings (FFI)" description = "Mercurial pure Rust core library, with no assumption on Python bindings (FFI)"
edition = "2018" edition = "2018"
build = "build.rs"
[lib] [lib]
name = "hg" name = "hg"
[dependencies] [dependencies]
byteorder = "1.3.1" byteorder = "1.3.1"
hex = "0.4.0" hex = "0.4.0"
lazy_static = "1.3.0" lazy_static = "1.3.0"
libc = { version = "0.2.66", optional = true }
memchr = "2.2.0" memchr = "2.2.0"
rand = "0.6.5" rand = "0.6.5"
rand_pcg = "0.1.1" rand_pcg = "0.1.1"
rayon = "1.3.0" rayon = "1.3.0"
regex = "1.1.0" regex = "1.1.0"
twox-hash = "1.5.0" twox-hash = "1.5.0"
same-file = "1.0.6" same-file = "1.0.6"
[dev-dependencies] [dev-dependencies]
tempfile = "3.1.0" tempfile = "3.1.0"
pretty_assertions = "0.6.1" pretty_assertions = "0.6.1"
No newline at end of file
[build-dependencies]
cc = { version = "1.0.48", optional = true }
[features]
default = []
with-re2 = ["cc", "libc"]

rust/hg-core/build.rs

  • This file was added.
// build.rs
//
// Copyright 2020 Raphaël Gomès <rgomes@octobus.net>
//
// This software may be used and distributed according to the terms of the
// GNU General Public License version 2 or any later version.
#[cfg(feature = "with-re2")]
use cc;
#[cfg(feature = "with-re2")]
fn compile_re2() {
cc::Build::new()
.cpp(true)
.flag("-std=c++11")
.file("src/re2/rust_re2.cpp")
.compile("librustre.a");
println!("cargo:rustc-link-lib=re2");
}
fn main() {
#[cfg(feature = "with-re2")]
compile_re2();
}

rust/hg-core/src/lib.rs

status::{status, StatusResult}, status::{status, StatusResult},
CopyMap, CopyMapIter, DirstateEntry, DirstateParents, EntryState, CopyMap, CopyMapIter, DirstateEntry, DirstateParents, EntryState,
StateMap, StateMapIter, StateMap, StateMapIter,
}; };
mod filepatterns; mod filepatterns;
pub mod matchers; pub mod matchers;
pub mod revlog; pub mod revlog;
pub use revlog::*; pub use revlog::*;
#[cfg(feature = "with-re2")]
pub mod re2;
pub mod utils; pub mod utils;
use crate::utils::hg_path::{HgPathBuf, HgPathError}; use crate::utils::hg_path::{HgPathBuf, HgPathError};
pub use filepatterns::{ pub use filepatterns::{
parse_pattern_syntax, read_pattern_file, IgnorePattern, parse_pattern_syntax, read_pattern_file, IgnorePattern,
PatternFileWarning, PatternSyntax, PatternFileWarning, PatternSyntax,
}; };
use std::collections::HashMap; use std::collections::HashMap;

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

  • This file was added.
/// re2 module
///
/// The Python implementation of Mercurial uses the Re2 regex engine when
/// possible and if the bindings are installed, falling back to Python's `re`
/// in case of unsupported syntax (Re2 is a non-backtracking engine).
///
/// Using it from Rust is not ideal. We need C++ bindings, a C++ compiler,
/// Re2 needs to be installed... why not just use the `regex` crate?
///
/// Using Re2 from the Rust implementation guarantees backwards compatibility.
/// We know it will work out of the box without needing to figure out the
/// subtle differences in syntax. For example, `regex` currently does not
/// support empty alternations (regex like `a||b`) which happens more often
/// than we might think. Old benchmarks also showed worse performance from
/// regex than with Re2, but the methodology and results were lost, so take
/// this with a grain of salt.
///
/// The idea is to use Re2 for now as a temporary phase and then investigate
/// how much work would be needed to use `regex`.
mod re2;
pub use re2::Re2;

rust/hg-core/src/re2/re2.rs

  • This file was added.
/*
re2.rs
Rust FFI bindings to Re2.
Copyright 2020 Valentin Gatien-Baron
This software may be used and distributed according to the terms of the
GNU General Public License version 2 or any later version.
*/
use libc::{c_int, c_void};
type Re2Ptr = *const c_void;
pub struct Re2(Re2Ptr);
/// `re2.h` says:
/// "An "RE2" object is safe for concurrent use by multiple threads."
unsafe impl Sync for Re2 {}
/// These bind to the C ABI in `rust_re2.cpp`.
extern "C" {
fn rust_re2_create(data: *const u8, len: usize) -> Re2Ptr;
fn rust_re2_destroy(re2: Re2Ptr);
fn rust_re2_ok(re2: Re2Ptr) -> bool;
fn rust_re2_error(
re2: Re2Ptr,
outdata: *mut *const u8,
outlen: *mut usize,
) -> bool;
fn rust_re2_match(
re2: Re2Ptr,
data: *const u8,
len: usize,
anchor: c_int,
) -> bool;
}
impl Re2 {
pub fn new(pattern: &[u8]) -> Result<Re2, String> {
unsafe {
let re2 = rust_re2_create(pattern.as_ptr(), pattern.len());
if rust_re2_ok(re2) {
Ok(Re2(re2))
} else {
let mut data: *const u8 = std::ptr::null();
let mut len: usize = 0;
rust_re2_error(re2, &mut data, &mut len);
Err(String::from_utf8_lossy(std::slice::from_raw_parts(
data, len,
))
.to_string())
}
}
}
pub fn is_match(&self, data: &[u8]) -> bool {
unsafe { rust_re2_match(self.0, data.as_ptr(), data.len(), 1) }
}
}
impl Drop for Re2 {
fn drop(&mut self) {
unsafe { rust_re2_destroy(self.0) }
}
}

rust/hg-core/src/re2/rust_re2.cpp

  • This file was added.
/*
rust_re2.cpp
C ABI export of Re2's C++ interface for Rust FFI.
Copyright 2020 Valentin Gatien-Baron
This software may be used and distributed according to the terms of the
GNU General Public License version 2 or any later version.
*/
#include <re2/re2.h>
using namespace re2;
extern "C" {
RE2* rust_re2_create(const char* data, size_t len) {
RE2::Options o;
o.set_encoding(RE2::Options::Encoding::EncodingLatin1);
o.set_log_errors(false);
o.set_max_mem(50000000);
return new RE2(StringPiece(data, len), o);
}
kevincoxUnsubmitted
Done

This is never freed. Should we add a Drop implementation in rust?

kevincox: This is never freed. Should we add a `Drop` implementation in rust?
void rust_re2_destroy(RE2* re) {
delete re;
}
bool rust_re2_ok(RE2* re) {
return re->ok();
}
void rust_re2_error(RE2* re, const char** outdata, size_t* outlen) {
const std::string& e = re->error();
*outdata = e.data();
*outlen = e.length();
}
bool rust_re2_match(RE2* re, char* data, size_t len, int ianchor) {
const StringPiece sp = StringPiece(data, len);
RE2::Anchor anchor =
ianchor == 0 ? RE2::Anchor::UNANCHORED :
(ianchor == 1 ? RE2::Anchor::ANCHOR_START :
RE2::Anchor::ANCHOR_BOTH);
return re->Match(sp, 0, len, anchor, NULL, 0);
}
}

rust/hg-cpython/Cargo.toml

[package] [package]
name = "hg-cpython" name = "hg-cpython"
version = "0.1.0" version = "0.1.0"
authors = ["Georges Racinet <gracinet@anybox.fr>"] authors = ["Georges Racinet <gracinet@anybox.fr>"]
edition = "2018" edition = "2018"
[lib] [lib]
name='rusthg' name='rusthg'
crate-type = ["cdylib"] crate-type = ["cdylib"]
[features] [features]
default = ["python27"] default = ["python27"]
with-re2 = ["hg-core/with-re2"]
# Features to build an extension module: # Features to build an extension module:
python27 = ["cpython/python27-sys", "cpython/extension-module-2-7"] python27 = ["cpython/python27-sys", "cpython/extension-module-2-7"]
python3 = ["cpython/python3-sys", "cpython/extension-module"] python3 = ["cpython/python3-sys", "cpython/extension-module"]
# Enable one of these features to build a test executable linked to libpython: # Enable one of these features to build a test executable linked to libpython:
# e.g. cargo test --no-default-features --features python27-bin # e.g. cargo test --no-default-features --features python27-bin
python27-bin = ["cpython/python27-sys"] python27-bin = ["cpython/python27-sys"]
python3-bin = ["cpython/python3-sys"] python3-bin = ["cpython/python3-sys"]
[dependencies] [dependencies]
hg-core = { path = "../hg-core" } hg-core = { path = "../hg-core"}
libc = '*' libc = '*'
[dependencies.cpython] [dependencies.cpython]
version = "0.4" version = "0.4"
default-features = false default-features = false

setup.py

# Unix only fix (os.path.expanduser not really reliable if # Unix only fix (os.path.expanduser not really reliable if
# HOME is shadowed like this) # HOME is shadowed like this)
import pwd import pwd
env['HOME'] = pwd.getpwuid(os.getuid()).pw_dir env['HOME'] = pwd.getpwuid(os.getuid()).pw_dir
cargocmd = ['cargo', 'rustc', '-vv', '--release'] cargocmd = ['cargo', 'rustc', '-vv', '--release']
feature_flags = []
if sys.version_info[0] == 3 and self.py3_features is not None: if sys.version_info[0] == 3 and self.py3_features is not None:
cargocmd.extend( feature_flags.append(self.py3_features)
('--features', self.py3_features, '--no-default-features') cargocmd.append('--no-default-features')
)
rust_features = env.get("HG_RUST_FEATURES")
if rust_features:
feature_flags.append(rust_features)
cargocmd.extend(('--features', " ".join(feature_flags)))
cargocmd.append('--') cargocmd.append('--')
if sys.platform == 'darwin': if sys.platform == 'darwin':
cargocmd.extend( cargocmd.extend(
("-C", "link-arg=-undefined", "-C", "link-arg=dynamic_lookup") ("-C", "link-arg=-undefined", "-C", "link-arg=dynamic_lookup")
) )
try: try:
subprocess.check_call(cargocmd, env=env, cwd=self.rustsrcdir) subprocess.check_call(cargocmd, env=env, cwd=self.rustsrcdir)
except OSError as exc: except OSError as exc: