diff --git a/Cargo.lock b/Cargo.lock index deac2220..064adbca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -388,16 +388,6 @@ version = "0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dc0b364ead1874514c8c2855ab558056ebfeb775653e7ae45ff72f28f8f3166c" -[[package]] -name = "bstr" -version = "1.12.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "63044e1ae8e69f3b5a92c736ca6269b8d12fa7efe39bf34ddb06d102cf0e2cab" -dependencies = [ - "memchr", - "serde", -] - [[package]] name = "bumpalo" version = "3.20.2" @@ -1139,19 +1129,6 @@ version = "0.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e4eba85ea1d0a966a983acd07deee566e67395d2d96b6fb39e62b5a833f1eb0b" -[[package]] -name = "globset" -version = "0.4.20" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "07c34a9410465b45bd9787443bc7370f37735bad04b0f0cd57ff1a3186c98988" -dependencies = [ - "aho-corasick", - "bstr", - "log", - "regex-automata", - "regex-syntax", -] - [[package]] name = "h2" version = "0.4.17" @@ -1768,13 +1745,13 @@ dependencies = [ "flate2", "getrandom 0.4.3", "glob", - "globset", "mergify-config", "mergify-core", "mergify-test-support", "opentelemetry-proto", "prost", "quick-xml", + "regex", "reqwest", "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 0ef1b793..99c89f2a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,7 +35,6 @@ dirs = "7" flate2 = "1" getrandom = "0.4" glob = "0.3" -globset = "0.4" iana-time-zone = "0.1" jsonschema = { version = "0.58", default-features = false } # OS credential store for the token `mergify auth login` mints. The diff --git a/crates/mergify-ci/Cargo.toml b/crates/mergify-ci/Cargo.toml index 562bc9eb..7ff0bb08 100644 --- a/crates/mergify-ci/Cargo.toml +++ b/crates/mergify-ci/Cargo.toml @@ -14,12 +14,12 @@ chrono = { workspace = true } flate2 = { workspace = true } getrandom = { workspace = true } glob = { workspace = true } -globset = { workspace = true } mergify-config = { path = "../mergify-config" } mergify-core = { path = "../mergify-core" } opentelemetry-proto = { workspace = true } prost = { workspace = true } quick-xml = { workspace = true } +regex = { workspace = true } reqwest = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } diff --git a/crates/mergify-ci/src/scopes_detect/changed_files.rs b/crates/mergify-ci/src/scopes_detect/changed_files.rs index 60ea049e..4983f25f 100644 --- a/crates/mergify-ci/src/scopes_detect/changed_files.rs +++ b/crates/mergify-ci/src/scopes_detect/changed_files.rs @@ -244,9 +244,8 @@ pub fn ensure_history( /// `caf\xe9.txt` and a Shift-JIS name, MRGFY-8289). An error would /// fail CI on a pull request the engine scopes without complaint, /// and no pattern in the UTF-8 YAML config can spell the raw bytes. -/// The name agrees with the engine's; how `?` and `[...]` match -/// its U+FFFD does not yet (globset matches bytes, the engine -/// characters, MRGFY-10066). +/// `?` and `[...]` match that U+FFFD as one character, as the +/// engine does. pub fn git_changed_files( repo_dir: Option<&Path>, base: &str, diff --git a/crates/mergify-ci/src/scopes_detect/engine_glob.rs b/crates/mergify-ci/src/scopes_detect/engine_glob.rs new file mode 100644 index 00000000..1c7f92e0 --- /dev/null +++ b/crates/mergify-ci/src/scopes_detect/engine_glob.rs @@ -0,0 +1,494 @@ +//! A port of the engine's scope-glob compiler. +//! +//! The engine compiles a scope glob in `mergify_engine/rules/globs.py`: +//! its own brace expansion, then Python's `glob.translate(recursive=True, +//! include_hidden=True, seps=["/", "\\"])` on every expansion, matched +//! from the start with the `regex` module and end-anchored with `\z`. +//! This module is that pipeline, step for step, emitting the regex for +//! Rust's `regex` crate instead of Python's. Each function names the +//! Python one it ports; keep them in step, and regenerate +//! `engine_glob_cases.json` against the engine when either side moves. +//! +//! What falls out of following the engine rather than a glob library: +//! +//! - `/` and `\` are both separators, in the pattern and in the path. +//! `\` escapes nothing but `{`, `}` and `,`. +//! - The pattern is split on separators before anything else, so a +//! `[...]` class never spans one: `x[a/b]y` is the literal `x[a`, +//! a separator, then the literal `b]y`. +//! - `[!x]` negates, `[^x]` is a class holding `^` and `x`, and an +//! unterminated `[` is a literal. +//! - `*` and `?` stay inside a segment; `**` crosses segments only when +//! it is a whole segment, and a lone `*` segment is never empty. +//! - `?` and classes match a character, not a byte. +//! +//! One thing is refused rather than ported. The `regex` module reads +//! `[:name:]` inside a set as a POSIX class, where `glob.translate` +//! meant literal characters. A glob class ending that way swallows its +//! own closing bracket, and the engine either rejects the pattern (an +//! unknown name) or matches something no documentation describes +//! (`[a[:alpha:]]/b` matches one character of a set holding every +//! letter, `]`, `[`, `/` and `\`, then `b`). Such a pattern is a +//! configuration error here, which is the useful answer for whoever +//! wrote it. The `cli` rows of `engine_glob_cases.json` pin it. + +use regex::Regex; +use regex::RegexBuilder; + +const SEPARATORS: [char; 2] = ['/', '\\']; + +/// What `\` escapes in brace expansion. Nothing else: `\` is also a +/// separator, and `src\**\*.py` has to keep meaning what it means. +const ESCAPABLE: [char; 3] = ['{', '}', ',']; + +/// The engine's `MAX_BRACE_EXPANSIONS`. +const MAX_BRACE_EXPANSIONS: usize = 128; + +/// The engine's `MAX_BRACE_NESTING`. +const MAX_BRACE_NESTING: usize = 32; + +const ANY_SEP: &str = r"[/\\]"; +const NOT_SEP: &str = r"[^/\\]"; + +/// Why a glob did not compile. +#[derive(Debug)] +pub enum CompileError { + /// The engine refuses the pattern too (or, for a POSIX class, reads + /// it as something else): the configuration's fault. + Invalid(String), + /// The translation did not compile: a bug in this port. + Regex(regex::Error), +} + +/// The `regex` module has no size limit and the crate's default (10 +/// MB) trips on a glob the engine accepts: 128 brace expansions of a +/// few dozen `?` each, every one a Unicode `[^/\\]` class. +const REGEX_SIZE_LIMIT: usize = 256 * 1024 * 1024; + +/// Compile `pattern` into a regex matching the paths the engine puts in +/// a scope for it. +pub fn compile(pattern: &str) -> Result { + let chars: Vec = pattern.chars().collect(); + let mut translated = Vec::new(); + for expansion in expand_braces(&chars).map_err(CompileError::Invalid)? { + translated.push(translate(&expansion).map_err(CompileError::Invalid)?); + } + // `glob.translate` wraps each expansion in `(?s:...)\z` and the + // engine joins them with `|` and matches from the start, so this + // is the same language. No translation holds a top-level `|`. + RegexBuilder::new(&format!(r"\A(?s:{})\z", translated.join("|"))) + .size_limit(REGEX_SIZE_LIMIT) + .build() + .map_err(CompileError::Regex) +} + +/// `globs.expand_braces`. +fn expand_braces(pattern: &[char]) -> Result, String> { + // Outside a group the expansion only stops at the end of the + // pattern, so the returned index carries nothing. + let (expansions, _) = expand_from(pattern, 0, 0, false)?; + Ok(expansions) +} + +/// `globs._expand_from`. +fn expand_from( + pattern: &[char], + mut index: usize, + depth: usize, + inside_group: bool, +) -> Result<(Vec, usize), String> { + let mut expansions = vec![String::new()]; + let mut literal = String::new(); + + while let Some(&c) = pattern.get(index) { + if c == '\\' + && let Some(&next) = pattern.get(index + 1) + && ESCAPABLE.contains(&next) + { + literal.push(next); + index += 2; + continue; + } + match c { + '[' => { + if let Some(end) = scan_character_class(pattern, index) { + literal.extend(&pattern[index..end]); + index = end; + continue; + } + } + '{' => { + if depth >= MAX_BRACE_NESTING { + return Err(format!( + "nests brace groups more than {MAX_BRACE_NESTING} deep" + )); + } + let (branches, next) = expand_group(pattern, index + 1, depth + 1)?; + expansions = combine(&expansions, &literal, &branches)?; + literal.clear(); + index = next; + continue; + } + '}' | ',' if inside_group => break, + '}' => return Err("unbalanced `}`: use `\\}` to match a literal brace".into()), + _ => {} + } + literal.push(c); + index += 1; + } + + Ok((combine(&expansions, &literal, &[String::new()])?, index)) +} + +/// `globs._expand_group`: `index` is just past the `{`. +fn expand_group( + pattern: &[char], + mut index: usize, + depth: usize, +) -> Result<(Vec, usize), String> { + let mut branches = Vec::new(); + loop { + let (branch, next) = expand_from(pattern, index, depth, true)?; + branches.extend(branch); + if branches.len() > MAX_BRACE_EXPANSIONS { + return Err(too_many_expansions()); + } + match pattern.get(next) { + None => return Err("unclosed `{`: use `\\{` to match a literal brace".into()), + Some('}') => { + // `_drop_empty_branches`: `foo{,.txt}` matches + // `foo.txt` but not `foo`, while `{}` and `{,}` keep + // one empty branch. + branches.retain(|b: &String| !b.is_empty()); + if branches.is_empty() { + branches.push(String::new()); + } + return Ok((branches, next + 1)); + } + // The `,` that ends this branch. + Some(_) => index = next + 1, + } + } +} + +/// `globs._scan_character_class`: the index just past the `[...]` +/// starting at `start`, or `None` when that `[` is a literal. +fn scan_character_class(pattern: &[char], start: usize) -> Option { + let mut index = start + 1; + if pattern.get(index) == Some(&'!') { + index += 1; + } + if pattern.get(index) == Some(&']') { + index += 1; + } + while let Some(&c) = pattern.get(index) { + if c == ']' { + return Some(index + 1); + } + if SEPARATORS.contains(&c) { + return None; + } + index += 1; + } + None +} + +/// `globs._combine`. +fn combine(prefixes: &[String], literal: &str, suffixes: &[String]) -> Result, String> { + if prefixes.len() * suffixes.len() > MAX_BRACE_EXPANSIONS { + return Err(too_many_expansions()); + } + Ok(prefixes + .iter() + .flat_map(|prefix| { + suffixes + .iter() + .map(move |suffix| format!("{prefix}{literal}{suffix}")) + }) + .collect()) +} + +fn too_many_expansions() -> String { + format!("expands to more than {MAX_BRACE_EXPANSIONS} alternatives") +} + +/// `glob.translate(pattern, recursive=True, include_hidden=True, +/// seps=["/", "\\"])`, without the `(?s:...)\z` wrapper. +fn translate(pattern: &str) -> Result { + let parts: Vec<&str> = pattern.split(SEPARATORS).collect(); + let last = parts.len() - 1; + let mut out = String::new(); + for (index, &part) in parts.iter().enumerate() { + match part { + "*" if index < last => { + out.push_str(NOT_SEP); + out.push('+'); + out.push_str(ANY_SEP); + } + "*" => { + out.push_str(NOT_SEP); + out.push('+'); + } + "**" if index < last => { + // Consecutive `**` segments collapse into the last one. + if parts[index + 1] != "**" { + out.push_str("(?:.+"); + out.push_str(ANY_SEP); + out.push_str(")?"); + } + } + "**" => out.push_str(".*"), + _ => { + translate_segment(part, &mut out)?; + if index < last { + out.push_str(ANY_SEP); + } + } + } + } + Ok(out) +} + +/// `fnmatch._translate(segment, star="[^/\\]*", question_mark="[^/\\]")`. +fn translate_segment(segment: &str, out: &mut String) -> Result<(), String> { + let pattern: Vec = segment.chars().collect(); + let n = pattern.len(); + let mut i = 0; + while i < n { + let c = pattern[i]; + i += 1; + match c { + '*' => { + out.push_str(NOT_SEP); + out.push('*'); + while i < n && pattern[i] == '*' { + i += 1; + } + } + '?' => out.push_str(NOT_SEP), + '[' => { + let mut j = i; + if j < n && pattern[j] == '!' { + j += 1; + } + if j < n && pattern[j] == ']' { + j += 1; + } + while j < n && pattern[j] != ']' { + j += 1; + } + if j >= n { + push_literal(out, '['); + } else { + push_class(out, &class_chunks(&pattern[i..j]))?; + i = j + 1; + } + } + c => push_literal(out, c), + } + } + Ok(()) +} + +/// The class body `fnmatch._translate` builds from `stuff`, the +/// characters between `[` and `]`, as chunks of literal characters +/// where each boundary between two chunks is a range operator: the +/// last character of one chunk to the first of the next. +fn class_chunks(stuff: &[char]) -> Vec> { + if !stuff.contains(&'-') { + return vec![stuff.to_vec()]; + } + let find_hyphen = |from: usize| (from..stuff.len()).find(|&k| stuff[k] == '-'); + let mut chunks: Vec> = Vec::new(); + let mut i = 0; + let mut k = if stuff[0] == '!' { 2 } else { 1 }; + while let Some(hyphen) = find_hyphen(k) { + chunks.push(stuff[i..hyphen].to_vec()); + i = hyphen + 1; + k = hyphen + 3; + } + let chunk = &stuff[i..]; + if !chunk.is_empty() { + chunks.push(chunk.to_vec()); + } else if let Some(last) = chunks.last_mut() { + last.push('-'); + } + // "Remove empty ranges -- invalid in RE.": a reversed range is + // dropped along with its two bounds. + for k in (1..chunks.len()).rev() { + if let (Some(&low), Some(&high)) = (chunks[k - 1].last(), chunks[k].first()) + && low > high + { + let tail = chunks.remove(k); + chunks[k - 1].pop(); + chunks[k - 1].extend(&tail[1..]); + } + } + chunks +} + +fn push_class(out: &mut String, chunks: &[Vec]) -> Result<(), String> { + let body: Vec = chunks.join(&'-'); + match body.as_slice() { + // An empty range never matches. + [] => { + out.push_str("[a&&b]"); + return Ok(()); + } + // A negated empty range matches any character, `/` included. + ['!'] => { + out.push('.'); + return Ok(()); + } + _ => {} + } + let negated = body[0] == '!'; + reject_posix_class(chunks, negated)?; + out.push('['); + if negated { + out.push('^'); + } + let mut first = true; + for (index, chunk) in chunks.iter().enumerate() { + if index > 0 { + out.push('-'); + } + for &c in chunk { + if first && negated { + // The `!` itself. + first = false; + continue; + } + first = false; + push_literal(out, c); + } + } + out.push(']'); + Ok(()) +} + +/// Refuse a class in which the `regex` module would read a POSIX +/// class (see the module doc). +fn reject_posix_class(chunks: &[Vec], negated: bool) -> Result<(), String> { + let body = python_class_body(chunks, negated); + let mut index = 0; + while index < body.len() { + match body[index] { + // An escape is one item, whatever it escapes. + '\\' => index += 2, + '[' if body.get(index + 1) == Some(&':') && is_posix_class(&body[index + 2..]) => { + return Err( + "`[:` inside a character class is read by the engine as a POSIX class, \ + not as literal characters" + .into(), + ); + } + _ => index += 1, + } + } + Ok(()) +} + +/// The set body `fnmatch._translate` emits for the class, between the +/// `[` and the `]`, escapes included: what the `regex` module parses. +fn python_class_body(chunks: &[Vec], negated: bool) -> Vec { + let mut body = Vec::new(); + for (index, chunk) in chunks.iter().enumerate() { + if index > 0 { + body.push('-'); + } + for &c in chunk { + // A segment holds no `\\`, so these are all the escapes: + // a literal `-` and the set operators. + if matches!(c, '-' | '&' | '~' | '|') { + body.push('\\'); + } + body.push(c); + } + } + if negated { + body[0] = '^'; + } else if matches!(body.first(), Some('^' | '[')) { + body.insert(0, '\\'); + } + body +} + +/// Whether `rest`, the set body after a `[:` up to the set's closing +/// `]`, parses as a POSIX class name: `regex`'s `parse_posix_class` +/// and `parse_property_name`. The name must run to the end of the body, +/// since the only `]` that can close it is the set's own. +fn is_posix_class(rest: &[char]) -> bool { + let name_part = |c: char| c.is_ascii_alphanumeric() || " &_-.".contains(c); + let value_part = |c: char| c.is_ascii_alphanumeric() || " &_-./".contains(c); + let mut index = usize::from(rest.first() == Some(&'^')); + while index < rest.len() && name_part(rest[index]) { + index += 1; + } + if matches!(rest.get(index), Some(':' | '=')) { + let start = index + 1; + let mut end = start; + while end < rest.len() && value_part(rest[end]) { + end += 1; + } + // A qualified name (`[:script=latin:]`) only when the value + // is not blank; otherwise the `:` or `=` is not consumed. + if rest[start..end].iter().any(|&c| c != ' ') { + index = end; + } + } + rest.get(index) == Some(&':') && index + 1 == rest.len() +} + +fn push_literal(out: &mut String, c: char) { + let mut buf = [0; 4]; + out.push_str(®ex::escape(c.encode_utf8(&mut buf))); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[derive(serde::Deserialize)] + struct Case { + pattern: String, + path: String, + engine: String, + /// Set where this module answers differently on purpose: only + /// `invalid`, for the POSIX classes the module doc describes. + cli: Option, + } + + #[test] + fn every_case_gets_the_engines_verdict() { + let cases: Vec = serde_json::from_str(include_str!("engine_glob_cases.json")) + .expect("engine_glob_cases.json parses"); + let failures: Vec = cases + .iter() + .filter_map(|case| { + let got = match compile(&case.pattern) { + Err(CompileError::Invalid(_)) => "invalid", + Err(CompileError::Regex(e)) => { + panic!("{:?} does not compile: {e}", case.pattern) + } + Ok(regex) if regex.is_match(&case.path) => "match", + Ok(_) => "miss", + }; + let want = case.cli.as_deref().unwrap_or(&case.engine); + (got != want).then(|| { + format!( + "{:?} vs {:?}: got {got}, want {want}", + case.pattern, case.path + ) + }) + }) + .collect(); + assert!( + failures.is_empty(), + "{} of {} cases differ:\n{}", + failures.len(), + cases.len(), + failures.join("\n") + ); + } +} diff --git a/crates/mergify-ci/src/scopes_detect/engine_glob_cases.json b/crates/mergify-ci/src/scopes_detect/engine_glob_cases.json new file mode 100644 index 00000000..71017264 --- /dev/null +++ b/crates/mergify-ci/src/scopes_detect/engine_glob_cases.json @@ -0,0 +1,96 @@ +[ + {"pattern": "src/*.py", "path": "src/main.py", "engine": "match"}, + {"pattern": "src/*.py", "path": "src/deep/nested.py", "engine": "miss"}, + {"pattern": "*.md", "path": "readme.md", "engine": "match"}, + {"pattern": "*.md", "path": "docs/readme.md", "engine": "miss"}, + {"pattern": ".github/workflows/*", "path": ".github/workflows/ci.yml", "engine": "match"}, + {"pattern": ".github/workflows/*", "path": ".github/workflows/nested/ci.yml", "engine": "miss"}, + {"pattern": "package*.json", "path": "package-lock.json", "engine": "match"}, + {"pattern": "package*.json", "path": "packages/ui/tsconfig.json", "engine": "miss"}, + {"pattern": "a?c", "path": "abc", "engine": "match"}, + {"pattern": "a?c", "path": "a/c", "engine": "miss"}, + {"pattern": "**/*.py", "path": "top.py", "engine": "match"}, + {"pattern": "**/*.py", "path": "a/b/c.py", "engine": "match"}, + {"pattern": "**/x", "path": "x", "engine": "match"}, + {"pattern": "**/*", "path": "top.py", "engine": "match"}, + {"pattern": "**/*", "path": "a/b/c/deep.py", "engine": "match"}, + {"pattern": "**/tests/**", "path": "a/b/tests/c/d.py", "engine": "match"}, + {"pattern": "src/**", "path": "src/a/b.py", "engine": "match"}, + {"pattern": "src/**", "path": "src", "engine": "miss"}, + {"pattern": "src/**/*.py", "path": "src/a.py", "engine": "match"}, + {"pattern": "src/**/*.py", "path": "src/a/b.py", "engine": "match"}, + {"pattern": "**/x", "path": "deep/nested/x", "engine": "match"}, + {"pattern": "**/**/x", "path": "a/x", "engine": "match"}, + {"pattern": "a**b", "path": "axxb", "engine": "match"}, + {"pattern": "a**b", "path": "ax/xb", "engine": "miss"}, + {"pattern": "*/x", "path": "/x", "engine": "miss"}, + {"pattern": "a*/x", "path": "a/x", "engine": "match"}, + {"pattern": ".github/*", "path": "../x", "engine": "miss"}, + {"pattern": ".*", "path": "...", "engine": "match"}, + {"pattern": "*.{md,rst}", "path": "readme.md", "engine": "match"}, + {"pattern": "*.{md,rst}", "path": "readme.rst", "engine": "match"}, + {"pattern": "{a,b}{a,b}{a,b}{a,b}{a,b}{a,b}{a,b}/????????????????????????????????????????????????????????????????????????????????", "path": "abbabba/????????????????????????????????????????????????????????????????????????????????", "engine": "match"}, + {"pattern": "*.{md,rst}", "path": "docs/readme.md", "engine": "miss"}, + {"pattern": "src/{a,b}**/x", "path": "src/a/x", "engine": "match"}, + {"pattern": "foo{,.txt}", "path": "foo", "engine": "miss"}, + {"pattern": "foo{,.txt}", "path": "foo.txt", "engine": "match"}, + {"pattern": "{}", "path": "x", "engine": "miss"}, + {"pattern": "a{b,{c,d}}e", "path": "ade", "engine": "match"}, + {"pattern": "\\{a,b\\}", "path": "{a,b}", "engine": "match"}, + {"pattern": "a\\,b", "path": "a,b", "engine": "match"}, + {"pattern": "[{]x", "path": "{x", "engine": "match"}, + {"pattern": "a{b", "path": "a{b", "engine": "invalid"}, + {"pattern": "a}b", "path": "a}b", "engine": "invalid"}, + {"pattern": "{a,b", "path": "a", "engine": "invalid"}, + {"pattern": "{a,b}{a,b}{a,b}{a,b}{a,b}{a,b}{a,b}{a,b}", "path": "aaaaaaaa", "engine": "invalid"}, + {"pattern": "{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{a}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}", "path": "a", "engine": "invalid"}, + {"pattern": "x[^a]y", "path": "x^y", "engine": "match"}, + {"pattern": "x[^a]y", "path": "xby", "engine": "miss"}, + {"pattern": "x[!a]y", "path": "xby", "engine": "match"}, + {"pattern": "x[!a]y", "path": "xay", "engine": "miss"}, + {"pattern": "x[a/b]y", "path": "x[a/b]y", "engine": "match"}, + {"pattern": "x[a/b]y", "path": "xay", "engine": "miss"}, + {"pattern": "x[a/b]y", "path": "x[a\\b]y", "engine": "match"}, + {"pattern": "a\\b", "path": "a/b", "engine": "match"}, + {"pattern": "a\\b", "path": "a\\b", "engine": "match"}, + {"pattern": "src\\**\\*.py", "path": "src/a/b.py", "engine": "match"}, + {"pattern": "a/b", "path": "a\\b", "engine": "match"}, + {"pattern": "a\\*", "path": "a/b", "engine": "match"}, + {"pattern": "[a-c]x", "path": "bx", "engine": "match"}, + {"pattern": "[a-c-e]", "path": "-", "engine": "match"}, + {"pattern": "[a-c-e]", "path": "d", "engine": "miss"}, + {"pattern": "[]a]", "path": "]", "engine": "match"}, + {"pattern": "[!]a]", "path": "b", "engine": "match"}, + {"pattern": "[a-]", "path": "-", "engine": "match"}, + {"pattern": "[-a]", "path": "-", "engine": "match"}, + {"pattern": "[z-a]", "path": "z", "engine": "miss"}, + {"pattern": "[!z-a]", "path": "/", "engine": "match"}, + {"pattern": "[!]", "path": "!", "engine": "miss"}, + {"pattern": "x[", "path": "x[", "engine": "match"}, + {"pattern": "x[]", "path": "x[]", "engine": "match"}, + {"pattern": "[a&&b]", "path": "&", "engine": "match"}, + {"pattern": "[a||b]", "path": "|", "engine": "match"}, + {"pattern": "[a~~b]", "path": "~", "engine": "match"}, + {"pattern": "caf?.txt", "path": "café.txt", "engine": "match"}, + {"pattern": "caf??.txt", "path": "café.txt", "engine": "miss"}, + {"pattern": "caf[é].txt", "path": "café.txt", "engine": "match"}, + {"pattern": "caf?.txt", "path": "caf�.txt", "engine": "match"}, + {"pattern": "caf[!é].txt", "path": "café.txt", "engine": "miss"}, + {"pattern": "caf[à-ê].txt", "path": "café.txt", "engine": "match"}, + {"pattern": "日本?.txt", "path": "日本語.txt", "engine": "match"}, + {"pattern": "café.txt", "path": "cafe.txt", "engine": "miss"}, + {"pattern": "a(b)+c$", "path": "a(b)+c$", "engine": "match"}, + {"pattern": "a.b", "path": "axb", "engine": "miss"}, + {"pattern": "#a b", "path": "#a b", "engine": "match"}, + {"pattern": "src/**", "path": "src/a\nb.txt", "engine": "match"}, + {"pattern": "a?b", "path": "a\nb", "engine": "match"}, + {"pattern": "[a[:alpha:]]", "path": "a", "engine": "invalid"}, + {"pattern": "[a[:alpha:]]/b", "path": "ab", "engine": "match", "cli": "invalid"}, + {"pattern": "[![:alpha:]]/b", "path": "1b", "engine": "match", "cli": "invalid"}, + {"pattern": "[a[:nope:]]", "path": "a", "engine": "invalid"}, + {"pattern": "[a[:,,!:]]", "path": ",]", "engine": "match"}, + {"pattern": "[[:alpha:]]", "path": "a]", "engine": "match"}, + {"pattern": "[a[:]x]", "path": "x", "engine": "miss"}, + {"pattern": "[a[=b=]]", "path": "=]", "engine": "match"}, + {"pattern": "[a[.b.]]", "path": ".]", "engine": "match"} +] diff --git a/crates/mergify-ci/src/scopes_detect/engine_glob_cases.py b/crates/mergify-ci/src/scopes_detect/engine_glob_cases.py new file mode 100644 index 00000000..5e26cca4 --- /dev/null +++ b/crates/mergify-ci/src/scopes_detect/engine_glob_cases.py @@ -0,0 +1,71 @@ +# /// script +# requires-python = "~=3.14.0" +# dependencies = ["regex==2026.9.10"] +# /// +"""Fill in the engine's verdict for every case in `engine_glob_cases.json`. + +The verdicts are what the engine says, not what anyone expects it to say: +this runs the engine's own `mergify_engine/rules/globs.py` on Python 3.14 +(the engine's version, which `glob.translate` comes from) and matches with +the `regex` module, the engine's judge of a pattern. Both pins follow +`engine/pyproject.toml`: bump them with it. Add a case with any +`engine` value, run this, and review the diff: + + uv run --no-project engine_glob_cases.py /engine/mergify_engine/rules/globs.py + +Only `globs.py` is loaded. Its `regexp_engine` import is stubbed with +`regex.compile`, which is the backtracking engine `regexp_engine` always +compiles and whose answer its RE2 path is proven to give. +""" + +import importlib.util +import json +import pathlib +import sys +import types + +import regex + +CASES = pathlib.Path(__file__).with_name("engine_glob_cases.json") + + +def load_globs(path: str) -> types.ModuleType: + regexp_engine = types.ModuleType("mergify_engine.rules.regexp_engine") + regexp_engine.compile_pattern = regex.compile # type: ignore[attr-defined] + rules = types.ModuleType("mergify_engine.rules") + rules.regexp_engine = regexp_engine # type: ignore[attr-defined] + sys.modules["mergify_engine"] = types.ModuleType("mergify_engine") + sys.modules["mergify_engine.rules"] = rules + sys.modules["mergify_engine.rules.regexp_engine"] = regexp_engine + spec = importlib.util.spec_from_file_location("mergify_engine.rules.globs", path) + assert spec is not None and spec.loader is not None + globs = importlib.util.module_from_spec(spec) + spec.loader.exec_module(globs) + return globs + + +def verdict(globs: types.ModuleType, pattern: str, path: str) -> str: + # `glob_patterns_to_regexes` is what a scope's `include` / `exclude` + # compiles through, and `match` is how `filter.glob_match` asks. + try: + compiled = globs.glob_patterns_to_regexes((pattern,)) + except (globs.InvalidGlobPatternError, regex.error): + return "invalid" + return "match" if any(rx.match(path) for rx in compiled) else "miss" + + +def dump(cases: list[dict[str, str]]) -> str: + lines = ",\n".join(f" {json.dumps(case, ensure_ascii=False)}" for case in cases) + return f"[\n{lines}\n]\n" + + +def main() -> None: + globs = load_globs(sys.argv[1]) + cases = json.loads(CASES.read_text(encoding="utf-8")) + for case in cases: + case["engine"] = verdict(globs, case["pattern"], case["path"]) + CASES.write_text(dump(cases), encoding="utf-8") + + +if __name__ == "__main__": + main() diff --git a/crates/mergify-ci/src/scopes_detect/matching.rs b/crates/mergify-ci/src/scopes_detect/matching.rs index 92113607..86706ace 100644 --- a/crates/mergify-ci/src/scopes_detect/matching.rs +++ b/crates/mergify-ci/src/scopes_detect/matching.rs @@ -1,33 +1,14 @@ //! Glob-pattern matching: file path → matching scopes. //! //! The engine derives a pull request's scopes from the very same -//! `scopes:` block, in `mergify_engine/rules/globs.py` (brace -//! expansion, then `glob.translate`, matched with the third-party -//! `regex` module's `.match()` — `glob.translate` end-anchors its -//! output with `\z`, so that is a full match), and it — not this -//! command — is what the merge queue acts on. So the -//! semantics here have to track that module rather than globset's -//! defaults, or one config means two things and `ci scopes` picks -//! CI jobs for a scope set the engine never derives: -//! -//! - `*` and `?` stop at `/` (`literal_separator(true)`). `src/*.py` -//! is `src`'s own Python files, not the whole subtree; only `**` -//! crosses directories. This is also what -//! `/configuration/data-types` documents. -//! - `{a,b}` alternation means the same on both sides — globset -//! compiles it inline, the engine expands it into one pattern per -//! branch (MRGFY-8359). -//! -//! Parity is exact for every ASCII path git can report as changed. -//! On a non-ASCII one, `?` and `[...]` differ: globset matches -//! bytes and the engine characters, so `caf?.txt` misses -//! `café.txt` here and hits it there (MRGFY-10066). The other -//! residual differences all need a path git never emits: a leading -//! `/`, a trailing `/`, an empty segment (`a//b`), or the empty -//! string. An unterminated `[` is the one place globset is stricter -//! — it rejects the pattern where the engine degrades it to a -//! literal — and erroring out on a malformed config is the side to -//! be on. +//! `scopes:` block, and it — not this command — is what the merge +//! queue acts on. So a glob here compiles as the engine compiles it +//! (`engine_glob`, a port of `mergify_engine/rules/globs.py`, which +//! documents the one pattern shape it refuses instead), or one config +//! means two things and `ci scopes` picks CI jobs for a +//! scope set the engine never derives. `engine_glob_cases.json` is the +//! proof: patterns and paths with the engine's own verdicts, which +//! this module must reproduce. //! //! One intentional behavior: a pattern with an empty `include` list //! follows the engine and matches every path (the scope's exclude @@ -37,18 +18,18 @@ use std::collections::BTreeMap; use std::collections::BTreeSet; -use globset::GlobBuilder; -use globset::GlobMatcher; use mergify_core::CliError; +use regex::Regex; use super::config::FileFilters; +use super::engine_glob; /// Pre-built matchers for one scope. #[derive(Debug)] pub struct ScopeMatcher { pub name: String, - include: Vec, - exclude: Vec, + include: Vec, + exclude: Vec, } impl ScopeMatcher { @@ -88,29 +69,20 @@ pub fn compile(filters: &BTreeMap) -> Result Result, CliError> { +fn compile_list(scope: &str, patterns: &[String]) -> Result, CliError> { patterns.iter().map(|pat| build_glob(scope, pat)).collect() } -fn build_glob(scope: &str, pattern: &str) -> Result { - // `literal_separator(true)` is the whole parity story for - // everything but `**`: it compiles `*` to `[^/]*` and `?` to - // `[^/]`, which is what `glob.translate` emits and what the - // data-types page documents. globset defaults it off, and that - // default is what used to let `*.md` swallow every `.md` in the - // tree. `case_insensitive(false)` is the default but stated for - // the record — file paths are case-sensitive on the platforms - // Mergify cares about. - GlobBuilder::new(pattern) - .literal_separator(true) - .case_insensitive(false) - .build() - .map(|g| g.compile_matcher()) - .map_err(|e| { - CliError::Configuration(format!( - "invalid glob {pattern:?} under scope {scope:?}: {e}" - )) - }) +fn build_glob(scope: &str, pattern: &str) -> Result { + engine_glob::compile(pattern).map_err(|e| match e { + engine_glob::CompileError::Invalid(reason) => CliError::Configuration(format!( + "invalid glob {pattern:?} under scope {scope:?}: {reason}" + )), + // Not the config's fault: the translation is ours. + engine_glob::CompileError::Regex(e) => { + CliError::wrap(format!("compile glob {pattern:?} under scope {scope:?}"), e) + } + }) } /// Result of routing a set of changed files through every scope @@ -159,45 +131,6 @@ mod tests { compile(&m).expect("globs compile") } - /// Assert a single `include` pattern's verdict on one path. - fn assert_matches(cases: &[(&str, &str, bool)]) { - for &(pattern, path, expected) in cases { - let ms = compile_one(&[pattern], &[]); - let hit = route([path], &ms).hit.contains("s"); - assert_eq!( - hit, expected, - "{pattern:?} vs {path:?}: expected match={expected}", - ); - } - } - - #[test] - fn single_star_and_question_mark_stop_at_a_separator() { - // MRGFY-8392. globset's default (`literal_separator(false)`) - // compiles `*` to `.*`, so `src/*.py` used to claim - // `src/deep/nested.py` — a file the engine, which compiles - // the same pattern to `src[/\\][^/\\]*\.py`, never puts in - // the scope. Each pattern below is paired with the path the - // two sides disagreed on and one they always agreed on, so - // the bound is pinned without pinning `*` shut entirely. - assert_matches(&[ - ("src/*.py", "src/deep/nested.py", false), - ("src/*.py", "src/main.py", true), - ("*.md", "docs/readme.md", false), - ("*.md", "readme.md", true), - ( - ".github/workflows/*", - ".github/workflows/nested/ci.yml", - false, - ), - (".github/workflows/*", ".github/workflows/ci.yml", true), - ("package*.json", "packages/ui/tsconfig.json", false), - ("package*.json", "package-lock.json", true), - ("a?c", "a/c", false), - ("a?c", "abc", true), - ]); - } - #[test] fn separator_bounded_star_in_exclude_does_not_drop_a_scope() { // The direction that actually skipped CI. With `*` crossing @@ -217,42 +150,6 @@ mod tests { assert!(res.hit.is_empty(), "unexpected hit: {:?}", res.hit); } - #[test] - fn double_star_is_the_only_wildcard_that_crosses_directories() { - // `**` keeps its recursive meaning, including the "zero - // segments" case (`**/x` matches a root-level `x`) that the - // engine's `(?:.+[/\\])?` prefix also allows. Without this, - // narrowing `*` could plausibly have been read as narrowing - // `**` too. - assert_matches(&[ - ("**/*.py", "a/b/c.py", true), - ("**/*.py", "top.py", true), - ("**/x", "x", true), - ("**/x", "deep/nested/x", true), - ("src/**", "src/a/b/c.py", true), - ("src/**/*.py", "src/a/b.py", true), - ("**/tests/**", "a/b/tests/c/d.py", true), - // `**/*` is `FileFilters`' default `include`, so every - // scope that only declares an `exclude` list rides on it. - // Narrowing the trailing `*` must not stop it catching a - // nested file, or those scopes would quietly go empty. - ("**/*", "a/b/c/deep.py", true), - ("**/*", "top.py", true), - ]); - } - - #[test] - fn brace_alternation_expands_within_the_separator_bound() { - // Braces are the other half of engine parity (MRGFY-8359), - // and they compose with the bound rather than escaping it: - // each branch is a separator-bounded `*` in its own right. - assert_matches(&[ - ("*.{md,rst}", "readme.md", true), - ("*.{md,rst}", "readme.rst", true), - ("*.{md,rst}", "docs/readme.md", false), - ]); - } - #[test] fn exclude_takes_precedence_over_include() { // File matches include but also matches exclude — must @@ -300,11 +197,10 @@ mod tests { #[test] fn invalid_glob_surfaces_configuration_error() { - // An obviously-bad pattern (unterminated bracket - // expression) should fail config validation rather than - // crash at match time. + // A pattern the engine refuses (an unclosed brace group) + // fails config validation rather than never matching. let mut m = BTreeMap::new(); - m.insert("s".to_string(), filters(&["[unterminated"], &[])); + m.insert("s".to_string(), filters(&["src/{a,b"], &[])); let err = compile(&m).unwrap_err(); assert!(matches!(err, CliError::Configuration(_))); assert!(err.to_string().contains("invalid glob"), "got {err}"); diff --git a/crates/mergify-ci/src/scopes_detect/mod.rs b/crates/mergify-ci/src/scopes_detect/mod.rs index b50cc82f..cd421fc8 100644 --- a/crates/mergify-ci/src/scopes_detect/mod.rs +++ b/crates/mergify-ci/src/scopes_detect/mod.rs @@ -26,6 +26,7 @@ pub mod changed_files; pub mod config; +mod engine_glob; pub mod matching; pub mod outputs;