GNOME Bugzilla – Bug 759808
regex test fails with system PCRE 8.38: different error for (?(?<ab))
Last modified: 2016-01-14 14:00:26 UTC
PCRE 8.38 was recently uploaded to Debian. We compile GLib to use the system PCRE library, as advocated on Bug #740573. This caused the regex test to start failing on our continuous integration server: https://ci.debian.net/data/packages/unstable/amd64/g/glib2.0/20151222_193725.autopkgtest.log.gz > ERROR:/build/glib2.0-ocmJ1Y/glib2.0-2.46.2/./glib/tests/regex.c:129: > test_new_fail: assertion failed (error == (g-regex-error-quark, 142)): > Error while compiling regular expression (?(?<ab)) at char 3: assertion > expected after (?( (g-regex-error-quark, 128) (?(?<ab)) is a syntactically invalid variation of the regular expression fragment (?(?<ab>)foo), which matches foo if a subpattern named "ab" has previously matched, or the empty string otherwise. With older PCRE, the error was "missing subpattern name terminator", complaining that <ab> was missing its > terminator. The new PCRE emits the error code "assertion expected" instead: I'm not sure where it thinks the subpattern name appeared. The syntactically invalid regex (?P<sub>foo)\g<sub (note the missing > at the end), which was not previously tested, now raises "missing subpattern name terminator", where it used to raise "missing back-reference". I think it's reasonable to consider these to be a GLib bug (being too specific in its expectations) rather than a PCRE bug (changing its diagnostic for an invalid pattern), as long as the invalid patterns are still diagnosed as invalid. However, if the GLib maintainers disagree, we should talk to the PCRE maintainer about changing these errors back. Patch on the way when I've tested it with (2.46, 2.47) x (system PCRE, embedded PCRE).
(In reply to Simon McVittie from comment #0) > With older PCRE, the error was "missing subpattern name terminator", > complaining that <ab> was missing its > terminator. The new PCRE emits the > error code "assertion expected" instead This appears to be deliberate: the tests' expected input/output changed in <http://vcs.pcre.org/pcre?view=revision&revision=1539>.
Created attachment 317821 [details] [review] regex test: expect ASSERTION_EXPECTED for /(?(?<ab))/ with PCRE 8.38 PCRE 8.38 changed the parsing of this invalid regex. It still fails, but with a different error (since PCRE r1539, <http://vcs.pcre.org/pcre?view=revision&revision=1539>). The regex /(?P<sub>foo)\g<sub/ used to raise MISSING_BACK_REFERENCE but now raises MISSING_SUBPATTERN_NAME_TERMINATOR, so we can still have a test for the latter. --- Please consider for both 2.46 and master; the same patch works for both.
Review of attachment 317821 [details] [review]: I think it's reasonable. I'd also add paragraph in the GLib release notes mentioning this.
(In reply to Emmanuele Bassi (:ebassi) from comment #3) > I think it's reasonable. I'd also add paragraph in the GLib release notes > mentioning this. A paragraph mentioning what? I haven't changed GRegex's behaviour at all - all I've changed is a regression test. Upgrading PCRE from 8.37 to 8.38 changes GRegex's behaviour, but that's orthogonal to the GLib version, so it doesn't seem useful to mention that in the release notes for a particular version.
(In reply to Simon McVittie from comment #4) > A paragraph mentioning what? I haven't changed GRegex's behaviour at all - > all I've changed is a regression test. Perhaps you were mixing this up with my patch on Bug #740573, where you were certainly right to ask for a release note? It would also be correct to add a release note if we updated the bundled PCRE, but I haven't done that. I'll apply this without additional release notes if my patch on Bug #740573 is approved, since it blocks that one.
(In reply to Simon McVittie from comment #5) > (In reply to Simon McVittie from comment #4) > > A paragraph mentioning what? I haven't changed GRegex's behaviour at all - > > all I've changed is a regression test. > > Perhaps you were mixing this up with my patch on Bug #740573, where you were > certainly right to ask for a release note? Yes, I was. Feel free to disregard my request; the patch is still a-c_n.
Comment on attachment 317821 [details] [review] regex test: expect ASSERTION_EXPECTED for /(?(?<ab))/ with PCRE 8.38 855594c4 (2.47.5) and d5f1de1a (2.46.3)