Skip Menu |
Report information
Id: 128420
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: jg [at] jgsoft.com
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: unknown
Perl Version: (no value)
Fixed In: 5.25.1



To: perlbug [...] perl.org
Date: Fri, 17 Jun 2016 08:25:42 +0700
From: Jan Goyvaerts <jg [...] jgsoft.com>
Subject: Changes in regex recursion in 5.24
Download (untitled) / with headers
text/plain 1.5k
Hi. While preparing RegexBuddy to support Perl 5.24 I discovered some changes in the regex engine that I find hard to explain, and thus might be bugs. In particular, regex recursion works differently, and I can't see the logic behind the new behavior. It looks like backtracking into the recursion isn't working correctly. Here are some sample regexes along with the subject strings and the matches they produce in 5.22 and 5.24. re: /aa$|a(?R)a|a/ subject: "aaa" 5.22: "aaa" 5.24: "aa" (the first two, as if the second alternative in the regex matched without the recursion) re: /(?:\1|a)([bcd])\1(?:(?R)|e)\1/ subject: "abbaccaddedcb" 5.22: "abbaccaddedcb" 5.24: "added" re: /(.)\W*+(?R)\W*+\1|\W*+.\W*+/i subject: "A man, a plan, a canal: Panama" 5.22: "A man, a plan, a canal: Panama" 5.24: "A " I also found that infinite recursion is no longer an error but is treated as match failure. Perhaps the above differences between 5.22 and 5.24 are unintended side effects of attempting to fix any issues with infinite recursion. The perlbug page in the docs tells me you want the output of perl -V so I have attached that. I've attached a sample program that demonstrates the matches. X marks the spot. Perl 5.22 matches the strings entirely: 5.022002 X X X Perl 5.24 finds partial matches: 5.024000 Xa abbaccXcb Xman, a plan, a canal: Panama Please let me know whether the Perl developers consider this to be a bug. If not, I'd like to understand the logic behind the behavior so that we can make RegexBuddy emulate it. Kind regards, Jan Goyvaerts -- http://www.just-great-software.com
Download perl524.txt
text/plain 2.7k
Summary of my perl5 (revision 5 version 24 subversion 0) configuration: Platform: osname=MSWin32, osvers=6.3, archname=MSWin32-x64-multi-thread uname='Win32 strawberry-perl 5.24.0.1 #1 Tue May 10 21:30:49 2016 x64' config_args='undef' hint=recommended, useposix=true, d_sigaction=undef useithreads=define, usemultiplicity=define use64bitint=define, use64bitall=undef, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='gcc', ccflags =' -s -O2 -DWIN32 -DWIN64 -DCONSERVATIVE -DPERL_TEXTMODE_SCRIPTS -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -fwrapv -fno-strict-aliasing -mms-bitfields', optimize='-s -O2', cppflags='-DWIN32' ccversion='', gccversion='4.9.2', gccosandvers='' intsize=4, longsize=4, ptrsize=8, doublesize=8, byteorder=12345678, doublekind=3 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16, longdblkind=3 ivtype='long long', ivsize=8, nvtype='double', nvsize=8, Off_t='long long', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='g++', ldflags ='-s -L"C:\strawberry\perl\lib\CORE" -L"C:\strawberry\c\lib"' libpth=C:\strawberry\c\lib C:\strawberry\c\x86_64-w64-mingw32\lib C:\strawberry\c\lib\gcc\x86_64-w64-mingw32\4.9.2 libs=-lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32 perllibs=-lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32 libc=, so=dll, useshrplib=true, libperl=libperl524.a gnulibc_version='' Dynamic Linking: dlsrc=dl_win32.xs, dlext=xs.dll, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags='-mdll -s -L"C:\strawberry\perl\lib\CORE" -L"C:\strawberry\c\lib"' Characteristics of this binary (from libperl): Compile-time options: HAS_TIMES HAVE_INTERP_INTERN MULTIPLICITY PERLIO_LAYERS PERL_COPY_ON_WRITE PERL_DONT_CREATE_GVSV PERL_HASH_FUNC_ONE_AT_A_TIME_HARD PERL_IMPLICIT_CONTEXT PERL_IMPLICIT_SYS PERL_MALLOC_WRAP PERL_PRESERVE_IVUV USE_64_BIT_INT USE_ITHREADS USE_LARGE_FILES USE_LOCALE USE_LOCALE_COLLATE USE_LOCALE_CTYPE USE_LOCALE_NUMERIC USE_LOCALE_TIME USE_PERLIO USE_PERL_ATOF Built under MSWin32 Compiled at May 10 2016 21:42:01 @INC: G:/Dev/Perl524/site/lib G:/Dev/Perl524/vendor/lib G:/Dev/Perl524/lib .
Download perl522.txt
text/plain 2.6k
Summary of my perl5 (revision 5 version 22 subversion 2) configuration: Platform: osname=MSWin32, osvers=6.3, archname=MSWin32-x64-multi-thread uname='Win32 strawberry-perl 5.22.2.1 #1 Sat Apr 30 17:18:06 2016 x64' config_args='undef' hint=recommended, useposix=true, d_sigaction=undef useithreads=define, usemultiplicity=define use64bitint=define, use64bitall=undef, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='gcc', ccflags =' -s -O2 -DWIN32 -DWIN64 -DCONSERVATIVE -DPERL_TEXTMODE_SCRIPTS -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -fwrapv -fno-strict-aliasing -mms-bitfields', optimize='-s -O2', cppflags='-DWIN32' ccversion='', gccversion='4.9.2', gccosandvers='' intsize=4, longsize=4, ptrsize=8, doublesize=8, byteorder=12345678, doublekind=3 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16, longdblkind=3 ivtype='long long', ivsize=8, nvtype='double', nvsize=8, Off_t='long long', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='g++', ldflags ='-s -L"C:\strawberry\perl\lib\CORE" -L"C:\strawberry\c\lib"' libpth=C:\strawberry\c\lib C:\strawberry\c\x86_64-w64-mingw32\lib C:\strawberry\c\lib\gcc\x86_64-w64-mingw32\4.9.2 libs=-lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32 perllibs=-lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32 libc=, so=dll, useshrplib=true, libperl=libperl522.a gnulibc_version='' Dynamic Linking: dlsrc=dl_win32.xs, dlext=xs.dll, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags='-mdll -s -L"C:\strawberry\perl\lib\CORE" -L"C:\strawberry\c\lib"' Characteristics of this binary (from libperl): Compile-time options: HAS_TIMES HAVE_INTERP_INTERN MULTIPLICITY PERLIO_LAYERS PERL_DONT_CREATE_GVSV PERL_HASH_FUNC_ONE_AT_A_TIME_HARD PERL_IMPLICIT_CONTEXT PERL_IMPLICIT_SYS PERL_MALLOC_WRAP PERL_NEW_COPY_ON_WRITE PERL_PRESERVE_IVUV USE_64_BIT_INT USE_ITHREADS USE_LARGE_FILES USE_LOCALE USE_LOCALE_COLLATE USE_LOCALE_CTYPE USE_LOCALE_NUMERIC USE_LOCALE_TIME USE_PERLIO USE_PERL_ATOF Built under MSWin32 Compiled at Apr 30 2016 17:23:36 @INC: G:/Dev/Perl522/site/lib G:/Dev/Perl522/vendor/lib G:/Dev/Perl522/lib .
Download replace.pl
text/x-perl 301b
print "$]\n"; $subject = "aaa"; $subject =~ s/aa$|a(?R)a|a/X/; print "$subject\n"; $subject = "abbaccaddedcb"; $subject =~ s/(?:\1|a)([bcd])\1(?:(?R)|e)\1/X/; print "$subject\n"; $subject = "A man, a plan, a canal: Panama"; $subject =~ s/(.)\W*+(?R)\W*+\1|\W*+.\W*+/X/i; print "$subject\n";
RT-Send-CC: perl5-porters [...] perl.org
A few bisects later... This was "broken" in: d5a00e4af6b155495be31a35728b8fef8e671ebe is the first bad commit commit d5a00e4af6b155495be31a35728b8fef8e671ebe Author: Yves Orton <demerphq@gmail.com> Date: Sat Mar 5 22:04:28 2016 +0100 Unify GOSTART and GOSUB GOSTART is a special case of GOSUB, we can remove a lot of offset twiddling, and other special casing by unifying them, at pretty much no cost. GOSUB has 2 arguments, ARG() and ARG2L(), which are interpreted as a U32 and an I32 respectively. ARG() holds the "parno" we will recurse into. ARG2L() holds a signed offset to the relevant start node for the recursion. Prior to this patch the argument to GOSUB would always be >=, and unlike other parts of our logic we would not use 0 to represent "start/end" of pattern, as GOSTART would be used for "recurse to beginning of pattern", after this patch we use 0 to represent "start/end", and a lot of complexity "goes away" along with GOSTART regops. :040000 040000 98c201349ff073a1e14744c0471ef3d9ab5fb584 a2df6849db8b2292fe1340f916f941c0e3c0f80f M pod :100644 100644 f8f4b91df368dfbec28f74e030de78fc4b260095 b789ca30c71b1698e2b7c16abdfc6a88fcd960d2 M regcomp.c :100644 100644 c08888e8f81a6cfcc848bb0c422c12574a22fe0f f16197f891b2aed9b705edfa7b85b63657db6a1c M regcomp.h :100644 100644 8f9861ab6be65be243f9a02f3feb17d1dc05dc18 ac679552703103ac6f6137d5eb1cf838a0c3787b M regcomp.sym :100644 100644 66dc245ddc68fb3c81a9e14ca3ea99310d249175 5e188fdc77e43e6a2f2bf578db31b329306dbdc8 M regexec.c :100644 100644 ff44df27e7dfa6d64d451687e5641588ab310796 02258fe176d72cc9e6aede142bdeadb507d53072 M regexp.h :100644 100644 f27abe0c7c4d43a4d17bcd05a031a671cd6e1ba0 f820c5684e6967f591ec0b0f276aaf4f65d911bb M regnodes.h bisect run success That took 725 seconds. And it was "fixed" in: da7cf1cc7cedc01f35ceb6724e8260c3b0ee0d12 is the first bad commit commit da7cf1cc7cedc01f35ceb6724e8260c3b0ee0d12 Author: Yves Orton <demerphq@gmail.com> Date: Tue May 10 09:44:31 2016 +0200 fix #128109 - do not move RExC_open_parens[0] in reginsert In d5a00e4af6b155495be31a35728b8fef8e671ebe I merged GOSUB and GOSTART, part of which involved making RExC_open_parens[0] refer to the start of the pattern, and RExC_close_parens[0] referring to the end of the pattern. This tripped up in reginsert in a subtle way, the start of the pattern cannot and should not move in reginsert(). Unlike a paren that might be at the start of the pattern which should move when something is inserted in front of it, the start is a fixed point and should never move. This patches fixes this up, and adds an assert to check that reginsert() is not called once study_chunk() starts, as reginsert() does not adjust RExC_recurse. This was noticed by hv while debugging [perl #128085], thanks hugo! :100644 100644 c6823b51e82c55b39104ebb58d52a1a86d540a9e e6b352b2b796001d8501bf373b3fdf46956c9799 M regcomp.c bisect run success That took 429 seconds. This should perhaps be included in 5.24.1?
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 153b
Patch adds two of the requestor's testcases that I used for the bisect to re_tests for regression testing. Confirmed passing on blead, failing on 5.24.0.
Subject: 0001-perl-128420-Add-tests-for-regex-recursion.patch
From 6965891ac125072bbd890010ea752a27b34b585f Mon Sep 17 00:00:00 2001 From: Dan Collins <dcollinsn@gmail.com> Date: Fri, 17 Jun 2016 19:40:57 -0400 Subject: [PATCH] [perl #128420] Add tests for regex recursion d5a00e4af introduced a bug in reginsert that was fixed by da7cf1cc7, originally documented in [perl #128109]. This patch adds two regression tests for the testcase reported by Jan Goyvaerts in [perl #128420]. --- t/re/re_tests | 2 ++ 1 file changed, 2 insertions(+) diff --git a/t/re/re_tests b/t/re/re_tests index 34ac94a..7e8522d 100644 --- a/t/re/re_tests +++ b/t/re/re_tests @@ -1966,6 +1966,8 @@ ab(?#Comment){2}c abbc y $& abbc .{1}?? - c - Nested quantifiers .{1}?+ - c - Nested quantifiers (?:.||)(?|)000000000@ 000000000@ y $& 000000000@ # [perl #126405] +aa$|a(?R)a|a aaa y $& aaa # [perl 128420] recursive matches +(?:\1|a)([bcd])\1(?:(?R)|e)\1 abbaccaddedcb y $& abbaccaddedcb # [perl 128420] recursive match with backreferences # Keep these lines at the end of the file # vim: softtabstop=0 noexpandtab -- 2.8.1
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 124b
Thanks, applied as ec5bd2262bb4e28f0dc6a0a3edb9b1f1b5befa2f I'm marking this ticket as a 5.24.1 blocker -- Karl Williamson
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
From: demerphq <demerphq [...] gmail.com>
To: Perl5 Porteros <perl5-porters [...] perl.org>
Date: Sun, 19 Jun 2016 08:48:24 +0200
Download (untitled) / with headers
text/plain 2.2k
On 17 June 2016 at 03:25, Jan Goyvaerts <perlbug-followup@perl.org> wrote: Show quoted text
> # New Ticket Created by Jan Goyvaerts > # Please include the string: [perl #128420] > # in the subject line of all future correspondence about this issue. > # <URL: https://rt.perl.org/Ticket/Display.html?id=128420 > > > > Hi. > > While preparing RegexBuddy to support Perl 5.24 I discovered some > changes in the regex engine that I find hard to explain, and thus might > be bugs. In particular, regex recursion works differently, and I can't > see the logic behind the new behavior. It looks like backtracking into > the recursion isn't working correctly. > > Here are some sample regexes along with the subject strings and the > matches they produce in 5.22 and 5.24. > > re: /aa$|a(?R)a|a/ > subject: "aaa" > 5.22: "aaa" > 5.24: "aa" (the first two, as if the second alternative in the regex > matched without the recursion) > > re: /(?:\1|a)([bcd])\1(?:(?R)|e)\1/ > subject: "abbaccaddedcb" > 5.22: "abbaccaddedcb" > 5.24: "added" > > re: /(.)\W*+(?R)\W*+\1|\W*+.\W*+/i > subject: "A man, a plan, a canal: Panama" > 5.22: "A man, a plan, a canal: Panama" > 5.24: "A " > > I also found that infinite recursion is no longer an error but is > treated as match failure. Perhaps the above differences between 5.22 > and 5.24 are unintended side effects of attempting to fix any issues > with infinite recursion.
Could very likely be. We tried to detect infinite recursion better than we used to, and its possible that something broke as a side effect. Show quoted text
> > The perlbug page in the docs tells me you want the output of perl -V so > I have attached that. > > I've attached a sample program that demonstrates the matches. X marks > the spot. Perl 5.22 matches the strings entirely: > > 5.022002 > X > X > X > > Perl 5.24 finds partial matches: > > 5.024000 > Xa > abbaccXcb > Xman, a plan, a canal: Panama > > Please let me know whether the Perl developers consider this to be a > bug. If not, I'd like to understand the logic behind the behavior so > that we can make RegexBuddy emulate
I will review this. If at all possible please test out blead as well. There was some strangeness about the 5.24 release, and its possible a patch that should have been included was not. If so it would be fixed in 5.24.1 and in blead. cheers, Yves
To: Perl5 Porteros <perl5-porters [...] perl.org>
From: demerphq <demerphq [...] gmail.com>
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
Date: Sun, 19 Jun 2016 08:56:28 +0200
Download (untitled) / with headers
text/plain 2.7k
On 19 June 2016 at 08:48, demerphq <demerphq@gmail.com> wrote: Show quoted text
> On 17 June 2016 at 03:25, Jan Goyvaerts <perlbug-followup@perl.org> wrote:
>> # New Ticket Created by Jan Goyvaerts >> # Please include the string: [perl #128420] >> # in the subject line of all future correspondence about this issue. >> # <URL: https://rt.perl.org/Ticket/Display.html?id=128420 > >> >> >> Hi. >> >> While preparing RegexBuddy to support Perl 5.24 I discovered some >> changes in the regex engine that I find hard to explain, and thus might >> be bugs. In particular, regex recursion works differently, and I can't >> see the logic behind the new behavior. It looks like backtracking into >> the recursion isn't working correctly. >> >> Here are some sample regexes along with the subject strings and the >> matches they produce in 5.22 and 5.24. >> >> re: /aa$|a(?R)a|a/ >> subject: "aaa" >> 5.22: "aaa" >> 5.24: "aa" (the first two, as if the second alternative in the regex >> matched without the recursion) >> >> re: /(?:\1|a)([bcd])\1(?:(?R)|e)\1/ >> subject: "abbaccaddedcb" >> 5.22: "abbaccaddedcb" >> 5.24: "added" >> >> re: /(.)\W*+(?R)\W*+\1|\W*+.\W*+/i >> subject: "A man, a plan, a canal: Panama" >> 5.22: "A man, a plan, a canal: Panama" >> 5.24: "A " >> >> I also found that infinite recursion is no longer an error but is >> treated as match failure. Perhaps the above differences between 5.22 >> and 5.24 are unintended side effects of attempting to fix any issues >> with infinite recursion.
> > Could very likely be. We tried to detect infinite recursion better > than we used to, and its possible that something broke as a side > effect. >
>> >> The perlbug page in the docs tells me you want the output of perl -V so >> I have attached that. >> >> I've attached a sample program that demonstrates the matches. X marks >> the spot. Perl 5.22 matches the strings entirely: >> >> 5.022002 >> X >> X >> X >> >> Perl 5.24 finds partial matches: >> >> 5.024000 >> Xa >> abbaccXcb >> Xman, a plan, a canal: Panama >> >> Please let me know whether the Perl developers consider this to be a >> bug. If not, I'd like to understand the logic behind the behavior so >> that we can make RegexBuddy emulate
> > I will review this. If at all possible please test out blead as well. > > There was some strangeness about the 5.24 release, and its possible a > patch that should have been included was not. If so it would be fixed > in 5.24.1 and in blead.
Yeah, this is a regression in 5.24 that should be fixed in 5.24.1 (once we definitively find the patch(es) responsible), but we should add your test cases to our test suite to ensure that this does not happen again. Like one way or another this is my fault. My apologies for any inconvenience. Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
Date: Mon, 10 Apr 2017 07:27:53 +0700
To: perlbug-followup [...] perl.org
From: Jan Goyvaerts <jg [...] jgsoft.com>
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
Download (untitled) / with headers
text/plain 304b
yves orton via RT wrote: Show quoted text
> Yeah, this is a regression in 5.24 that should be fixed in 5.24.1
I get the exact same behavior in 5.24.1 as in 5.24.0. Is this still scheduled to be fixed in 5.24 or will the fix have to wait for 5.26? Kind regards, Jan Goyvaerts -- http://www.just-great-software.com
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
To: Jan Goyvaerts <jg [...] jgsoft.com>, perlbug-followup [...] perl.org
Date: Tue, 11 Apr 2017 12:09:50 +0200
From: Sawyer X <xsawyerx [...] gmail.com>
Download (untitled) / with headers
text/plain 437b
At this point, I think 5.26.1 and 5.24.3 might be better for this. On 04/10/2017 02:27 AM, Jan Goyvaerts wrote: Show quoted text
> yves orton via RT wrote: >
>> Yeah, this is a regression in 5.24 that should be fixed in 5.24.1
> > I get the exact same behavior in 5.24.1 as in 5.24.0. Is this still > scheduled to be fixed in 5.24 or will the fix have to wait for 5.26? > > Kind regards, > Jan Goyvaerts > > -- > http://www.just-great-software.com
Date: Tue, 11 Apr 2017 12:18:27 +0200
CC: Jan Goyvaerts <jg [...] jgsoft.com>, Perl RT Bug Tracker <perlbug-followup [...] perl.org>
From: demerphq <demerphq [...] gmail.com>
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
To: Sawyer X <xsawyerx [...] gmail.com>
Download (untitled) / with headers
text/plain 530b
On 11 April 2017 at 12:09, Sawyer X <xsawyerx@gmail.com> wrote: Show quoted text
> At this point, I think 5.26.1 and 5.24.3 might be better for this. > > > On 04/10/2017 02:27 AM, Jan Goyvaerts wrote:
>> yves orton via RT wrote: >>
>>> Yeah, this is a regression in 5.24 that should be fixed in 5.24.1
>> >> I get the exact same behavior in 5.24.1 as in 5.24.0. Is this still >> scheduled to be fixed in 5.24 or will the fix have to wait for 5.26?
You want me to backport the patch? Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
CC: Sawyer X <xsawyerx [...] gmail.com>, Perl RT Bug Tracker <perlbug-followup [...] perl.org>
Date: Tue, 11 Apr 2017 17:36:54 +0700
To: demerphq <demerphq [...] gmail.com>
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
From: Jan Goyvaerts <jg [...] jgsoft.com>
Download (untitled) / with headers
text/plain 1.3k
demerphq wrote: Show quoted text
>> On 04/10/2017 02:27 AM, Jan Goyvaerts wrote:
>>> yves orton via RT wrote: >>>
>>>> Yeah, this is a regression in 5.24 that should be fixed in 5.24.1
>>> I get the exact same behavior in 5.24.1 as in 5.24.0. Is this still >>> scheduled to be fixed in 5.24 or will the fix have to wait for 5.26?
> > You want me to backport the patch?
You can do whatever you think is best for the Perl community. I just want to know what the plan is. I'm the developer of RegexBuddy which emulates the regex features in many applications and programming languages, including Perl 5.8 through 5.22. Basically, I need to decide whether to spend the time to make RegexBuddy emulate the buggy behavior in Perl 5.24 (or determine the situations in which the bug occurs and flag those as errors). I've delayed supporting Perl 5.24 because I was under the impression that it was going to be fixed in Perl 5.24.1 which would render the effort of emulating it unnecessary. Another change I've noticed in Perl 5.24.0 and 5.24.1 compared with previous versions is that [[:AlPhA:]] and [[:w:]] (and seemingly every other single letter class) are no longer errors, but also doesn't seem to match anything (or at least nothing obvious that I could figure out). Kind regards, Jan Goyvaerts -- http://www.just-great-software.com
From: demerphq <demerphq [...] gmail.com>
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
To: Jan Goyvaerts <jg [...] jgsoft.com>
CC: Sawyer X <xsawyerx [...] gmail.com>, Perl RT Bug Tracker <perlbug-followup [...] perl.org>, karl williamson <public [...] khwilliamson.com>
Date: Tue, 11 Apr 2017 13:02:26 +0200
Download (untitled) / with headers
text/plain 1.7k
On 11 April 2017 at 12:36, Jan Goyvaerts <jg@jgsoft.com> wrote: Show quoted text
> demerphq wrote: >
>>> On 04/10/2017 02:27 AM, Jan Goyvaerts wrote:
>>>> >>>> yves orton via RT wrote: >>>>
>>>>> Yeah, this is a regression in 5.24 that should be fixed in 5.24.1
>>>> >>>> I get the exact same behavior in 5.24.1 as in 5.24.0. Is this still >>>> scheduled to be fixed in 5.24 or will the fix have to wait for 5.26?
>> >> >> You want me to backport the patch?
> > > You can do whatever you think is best for the Perl community. I just want > to know what the plan is.
That was aimed at Sawyer X. :-) I will put together a patch for this. Show quoted text
> I'm the developer of RegexBuddy which emulates the regex features in many > applications and programming languages, including Perl 5.8 through 5.22. > Basically, I need to decide whether to spend the time to make RegexBuddy > emulate the buggy behavior in Perl 5.24 (or determine the situations in > which the bug occurs and flag those as errors). I've delayed supporting > Perl 5.24 because I was under the impression that it was going to be fixed > in Perl 5.24.1 which would render the effort of emulating it unnecessary.
I was under the impression that the patch was already applied to the Perl 5.24 branch. This thread bobbing up again is the first I've thought about this for ages. Very for sorry for any trouble this has caused you. Show quoted text
> Another change I've noticed in Perl 5.24.0 and 5.24.1 compared with previous > versions is that [[:AlPhA:]] and [[:w:]] (and seemingly every other single > letter class) are no longer errors, but also doesn't seem to match anything > (or at least nothing obvious that I could figure out).
If you notice things like that could file a new ticket please? I am sure Karl will want to know. cheers, Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
To: demerphq <demerphq [...] gmail.com>, Jan Goyvaerts <jg [...] jgsoft.com>
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
From: Karl Williamson <public [...] khwilliamson.com>
Date: Tue, 11 Apr 2017 10:01:18 -0600
CC: Sawyer X <xsawyerx [...] gmail.com>, Perl RT Bug Tracker <perlbug-followup [...] perl.org>
Download (untitled) / with headers
text/plain 1.4k
On 04/11/2017 05:02 AM, demerphq wrote: Show quoted text
>> On 11 April 2017 at 12:36, Jan Goyvaerts <jg@jgsoft.com> wrote: >> Another change I've noticed in Perl 5.24.0 and 5.24.1 compared with previous >> versions is that [[:AlPhA:]] and [[:w:]] (and seemingly every other single >> letter class) are no longer errors, but also doesn't seem to match anything >> (or at least nothing obvious that I could figure out).
> > If you notice things like that could file a new ticket please? I am > sure Karl will want to know. >
These are now legal. The first one warns if warnings are enabled, as they always should be: Assuming NOT a POSIX class since the name must be all lowercase letters in regex; marked by <-- HERE in m/[[:AlPhA:] <-- HERE ]/ at -e line 1. The single letter constructs, which you assume are meant to be Posix classes, don't warn, as none of them are close enough to a legal class name that they could have been intended to be one. That could be changed if there was evidence that people think [[:w:]] might mean something. Maybe this is legal in other languages? Perl uses the names in the Posix standard, plus a couple of extensions. If you want to see what a pattern compiles into, and hence what it matches, use re qw(Debug COMPILE); qr/pattern/; no re; That shows that [[:w:]] matches a string that contains a ']' immediately preceded by one of '[', ':', or 'w'. It's impossible to figure out for sure what was the programmer's intent.
Date: Wed, 12 Apr 2017 13:02:50 +0700
CC: demerphq <demerphq [...] gmail.com>, Sawyer X <xsawyerx [...] gmail.com>, Perl RT Bug Tracker <perlbug-followup [...] perl.org>
From: Jan Goyvaerts <jg [...] jgsoft.com>
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
To: Karl Williamson <public [...] khwilliamson.com>
Download (untitled) / with headers
text/plain 1.1k
Karl Williamson wrote: Show quoted text
> The single letter constructs, which you assume are meant to be Posix > classes, don't warn, as none of them are close enough to a legal class > name that they could have been intended to be one.
Dinkumware's implementation of std::regex (and possibly other implementations) support [:w:], [:s], and [:d:] as shorthands for [:word:], [:space:], and [:digit:]. Boost::regex additionally supports [:v:] and [:h:] to match vertical and horizontal whitespace. Show quoted text
> If you want to see what a pattern compiles into, and hence what it
matches, Thanks for the tip. Show quoted text
> That shows that [[:w:]] matches a string that contains a ']' > immediately preceded by one of '[', ':', or 'w'. > > It's impossible to figure out for sure what was the programmer's intent.
If the programmer intended to add a literal : to the character class then a single : would suffice. Anyway, Perl 5.24's behavior is fine. It's just that the change took me by surprise. It certainly doesn't break any existing regexes when porting code from 5.22 to 5.24. Kind regards, Jan Goyvaerts -- http://www.just-great-software.com
From: demerphq <demerphq [...] gmail.com>
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
To: Jan Goyvaerts <jg [...] jgsoft.com>
Date: Wed, 12 Apr 2017 09:38:56 +0200
CC: Karl Williamson <public [...] khwilliamson.com>, Sawyer X <xsawyerx [...] gmail.com>, Perl RT Bug Tracker <perlbug-followup [...] perl.org>
Download (untitled) / with headers
text/plain 1.3k
On 12 April 2017 at 08:02, Jan Goyvaerts <jg@jgsoft.com> wrote: Show quoted text
> Karl Williamson wrote: >
>> The single letter constructs, which you assume are meant to be Posix >> classes, don't warn, as none of them are close enough to a legal class >> name that they could have been intended to be one.
> > Dinkumware's implementation of std::regex (and possibly other > implementations) support [:w:], [:s], and [:d:] as shorthands for [:word:], > [:space:], and [:digit:].
Do you know why? Do they not support \w \s and \d? Show quoted text
> Boost::regex additionally supports [:v:] and [:h:] to match vertical and > horizontal whitespace.
I dont think we handle POSIX charclasses very cleanly. We warn about misplaced "^" negation, but we dont warn about mistyped POSIX names, which means we have not properly reserved the POSIX namespace for our own use, and we let people typo themselves into incorrect matches. IMO silently treating a typoed POSIX charclass as its constituent characters is an insidious thing to do. 1. I think in the next release we should produce a deprecation warning when we see anything that is POSIX-char-class-like but which is invalid. 2. Then in a release or so afterwards we could make them errors. 3. Then in a release or so after that we could add support for these short forms if we wanted. IMO, even if we dont do step 3 we should step 1 and 2. Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
CC: Karl Williamson <public [...] khwilliamson.com>, Sawyer X <xsawyerx [...] gmail.com>, Perl RT Bug Tracker <perlbug-followup [...] perl.org>
Date: Wed, 12 Apr 2017 16:30:17 +0700
From: Jan Goyvaerts <jg [...] jgsoft.com>
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
To: demerphq <demerphq [...] gmail.com>
Download (untitled) / with headers
text/plain 1.1k
demerphq wrote: Show quoted text
>> Dinkumware's implementation of std::regex (and possibly other >> implementations) support [:w:], [:s], and [:d:] as shorthands for [:word:], >> [:space:], and [:digit:].
> > Do you know why? Do they not support \w \s and \d?
std::regex supports \w, \s, and \d when using its "ECMAScript" grammar but not when using the basic, extended, grep, egrep, or awk grammars. boost::regex supports \w, \s, \d, and \h in all grammars. It does not support \v as a shorthand. So [[:v:]] is the only concise way of matching vertical whitespace in Boost. Show quoted text
> 1. I think in the next release we should produce a deprecation warning > when we see anything that is POSIX-char-class-like but which is > invalid. > > 2. Then in a release or so afterwards we could make them errors. > > 3. Then in a release or so after that we could add support for these > short forms if we wanted.
Sounds like a good plan to me. Anyway, RegexBuddy will emulate whatever you guys decide. Another inconsistency you have right now is that Perl 5.24 does throw errors about unknown POSIX classes for [[:a-z:]] and [[:a!z:]] but not for [[:az:]] or [[:A-Z:]]. Kind regards, Jan Goyvaerts -- http://www.just-great-software.com
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
CC: Karl Williamson <public [...] khwilliamson.com>, Sawyer X <xsawyerx [...] gmail.com>, Perl RT Bug Tracker <perlbug-followup [...] perl.org>
Date: Mon, 01 May 2017 08:13:12 +0700
From: Jan Goyvaerts <jg [...] jgsoft.com>
To: demerphq <demerphq [...] gmail.com>
Download (untitled) / with headers
text/plain 299b
Hi. Running this under Strawberry Perl 5.24.1 if ("aqzaqzaqz" =~ /(a(?R)z|q)*/) { print "$&\n"; } else { print "fail\n"; } causes the Perl interpreter to crash. In Perl 5.22 it correctly matches the whole subject string. Kind regards, Jan Goyvaerts -- http://www.just-great-software.com
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
CC: perl5-porters [...] perl.org
From: Zefram <zefram [...] fysh.org>
To: Jan Goyvaerts <jg [...] jgsoft.com>
Date: Mon, 1 May 2017 02:51:16 +0100
Jan Goyvaerts wrote: Show quoted text
>if ("aqzaqzaqz" =~ /(a(?R)z|q)*/) {
This has been fixed in blead. -zefram
From: Dave Mitchell <davem [...] iabyn.com>
To: Zefram <zefram [...] fysh.org>
CC: Jan Goyvaerts <jg [...] jgsoft.com>, perl5-porters [...] perl.org
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
Date: Tue, 2 May 2017 09:28:45 +0100
Download (untitled) / with headers
text/plain 461b
On Mon, May 01, 2017 at 02:51:16AM +0100, Zefram wrote: Show quoted text
> Jan Goyvaerts wrote:
> >if ("aqzaqzaqz" =~ /(a(?R)z|q)*/) {
> > This has been fixed in blead.
in particular, v5.25.0-36-gda7cf1c: fix #128109 - do not move RExC_open_parens[0] in reginsert -- Wesley Crusher gets beaten up by his classmates for being a smarmy git, and consequently has a go at making some friends of his own age for a change. -- Things That Never Happen in "Star Trek" #18
CC: Zefram <zefram [...] fysh.org>, Jan Goyvaerts <jg [...] jgsoft.com>, perl5-porters [...] perl.org
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
From: ilmari [...] ilmari.org (Dagfinn Ilmari Mannsåker)
To: Dave Mitchell <davem [...] iabyn.com>
Date: Tue, 02 May 2017 16:39:21 +0100
Download (untitled) / with headers
text/plain 635b
Dave Mitchell <davem@iabyn.com> writes: Show quoted text
> On Mon, May 01, 2017 at 02:51:16AM +0100, Zefram wrote:
>> Jan Goyvaerts wrote:
>> >if ("aqzaqzaqz" =~ /(a(?R)z|q)*/) {
>> >> This has been fixed in blead.
> > in particular, v5.25.0-36-gda7cf1c: > > fix #128109 - do not move RExC_open_parens[0] in reginsert
That commit, and v5.25.1-196-gec5bd2262b, which adds a test for it, cherry-pick cleanly to maint-5.24. Seeing as it's a crasher and regression from 5.22, it's a candidate for backporting according to perlpolicy. -- "A disappointingly low fraction of the human race is, at any given time, on fire." - Stig Sandbeck Mathisen
Date: Tue, 2 May 2017 19:38:00 +0200
CC: Dave Mitchell <davem [...] iabyn.com>, Zefram <zefram [...] fysh.org>, Jan Goyvaerts <jg [...] jgsoft.com>, Perl5 Porteros <perl5-porters [...] perl.org>
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
From: demerphq <demerphq [...] gmail.com>
To: Dagfinn Ilmari Mannsåker <ilmari [...] ilmari.org>
On 2 May 2017 at 17:39, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote: Show quoted text
> Dave Mitchell <davem@iabyn.com> writes: >
>> On Mon, May 01, 2017 at 02:51:16AM +0100, Zefram wrote:
>>> Jan Goyvaerts wrote:
>>> >if ("aqzaqzaqz" =~ /(a(?R)z|q)*/) {
>>> >>> This has been fixed in blead.
>> >> in particular, v5.25.0-36-gda7cf1c: >> >> fix #128109 - do not move RExC_open_parens[0] in reginsert
> > That commit, and v5.25.1-196-gec5bd2262b, which adds a test for it, > cherry-pick cleanly to maint-5.24. Seeing as it's a crasher and > regression from 5.22, it's a candidate for backporting according to > perlpolicy.
I just pushed 78b60e1 Add tests for regex recursion 4539ae3 fix #128109 - do not move RExC_open_parens[0] in reginsert 5edefd5 fix #128085 - SIGSEGV in S_regmatch with S_study_chunk: Assertion "!frame" failed. to maint-5.24 -- I took the liberty of including 5edefd5 which was part of how I found out about #128109 These patches are equivalent to (respectively): ec5bd2262b da7cf1c 1bda7a749022793a1a7ec1719cdd3d5b12feba7a Cheers, Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
CC: Sawyer X <xsawyerx [...] gmail.com>, Perl RT Bug Tracker <perlbug-followup [...] perl.org>
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
From: demerphq <demerphq [...] gmail.com>
To: Jan Goyvaerts <jg [...] jgsoft.com>
Date: Tue, 2 May 2017 21:24:20 +0200
Download (untitled) / with headers
text/plain 1.2k
On 11 April 2017 at 12:36, Jan Goyvaerts <jg@jgsoft.com> wrote: Show quoted text
> demerphq wrote: >
>>> On 04/10/2017 02:27 AM, Jan Goyvaerts wrote:
>>>> >>>> yves orton via RT wrote: >>>>
>>>>> Yeah, this is a regression in 5.24 that should be fixed in 5.24.1
>>>> >>>> I get the exact same behavior in 5.24.1 as in 5.24.0. Is this still >>>> scheduled to be fixed in 5.24 or will the fix have to wait for 5.26?
>> >> >> You want me to backport the patch?
> > > You can do whatever you think is best for the Perl community. I just want > to know what the plan is. > > I'm the developer of RegexBuddy which emulates the regex features in many > applications and programming languages, including Perl 5.8 through 5.22. > Basically, I need to decide whether to spend the time to make RegexBuddy > emulate the buggy behavior in Perl 5.24 (or determine the situations in > which the bug occurs and flag those as errors).
Rereading this thread I realized I did not comment on this, so i will now. I would not emulate the buggy behavior. Show quoted text
> I've delayed supporting > Perl 5.24 because I was under the impression that it was going to be fixed > in Perl 5.24.1 which would render the effort of emulating it unnecessary.
The patches are now pushed to the maint branch, so the next release of 5.24 will have the fix in it. Yves
From: Steve Hay via perl5-porters <perl5-porters [...] perl.org>
To: demerphq <demerphq [...] gmail.com>
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
CC: Dagfinn Ilmari Mannsåker <ilmari [...] ilmari.org>, Dave Mitchell <davem [...] iabyn.com>, Zefram <zefram [...] fysh.org>, Jan Goyvaerts <jg [...] jgsoft.com>, Perl5 Porteros <perl5-porters [...] perl.org>
Date: Tue, 2 May 2017 21:49:23 +0100
Download (untitled) / with headers
text/plain 1.5k
On 2 May 2017 at 18:38, demerphq <demerphq@gmail.com> wrote: Show quoted text
> On 2 May 2017 at 17:39, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:
>> Dave Mitchell <davem@iabyn.com> writes: >>
>>> On Mon, May 01, 2017 at 02:51:16AM +0100, Zefram wrote:
>>>> Jan Goyvaerts wrote:
>>>> >if ("aqzaqzaqz" =~ /(a(?R)z|q)*/) {
>>>> >>>> This has been fixed in blead.
>>> >>> in particular, v5.25.0-36-gda7cf1c: >>> >>> fix #128109 - do not move RExC_open_parens[0] in reginsert
>> >> That commit, and v5.25.1-196-gec5bd2262b, which adds a test for it, >> cherry-pick cleanly to maint-5.24. Seeing as it's a crasher and >> regression from 5.22, it's a candidate for backporting according to >> perlpolicy.
> > I just pushed > > 78b60e1 Add tests for regex recursion > 4539ae3 fix #128109 - do not move RExC_open_parens[0] in reginsert > 5edefd5 fix #128085 - SIGSEGV in S_regmatch with S_study_chunk: > Assertion "!frame" failed. > > to maint-5.24 -- I took the liberty of including 5edefd5 which was > part of how I found out about #128109 >
I don't think this should have been done right now. maint-5.22 and maint-5.24 are both still in code freeze pending the release of 5.22.4 and 5.24.2, which are intended only to contain the last @INC fixes (and possibly some other security fixes, as per the previous releases on those branches). There are many, many other bug fixes (crashes and otherwise) that could be backported, but haven't been yet. They were deliberately being left for 5.24.3, once the last of the @INC fixes was out of the way.
Date: Wed, 3 May 2017 08:56:47 +0200
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
CC: Dagfinn Ilmari Mannsåker <ilmari [...] ilmari.org>, Dave Mitchell <davem [...] iabyn.com>, Zefram <zefram [...] fysh.org>, Jan Goyvaerts <jg [...] jgsoft.com>, Perl5 Porteros <perl5-porters [...] perl.org>
From: demerphq <demerphq [...] gmail.com>
To: Steve Hay <steve.m.hay [...] googlemail.com>
Download (untitled) / with headers
text/plain 2.9k
On 2 May 2017 at 22:49, Steve Hay <steve.m.hay@googlemail.com> wrote: Show quoted text
> On 2 May 2017 at 18:38, demerphq <demerphq@gmail.com> wrote:
>> On 2 May 2017 at 17:39, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:
>>> Dave Mitchell <davem@iabyn.com> writes: >>>
>>>> On Mon, May 01, 2017 at 02:51:16AM +0100, Zefram wrote:
>>>>> Jan Goyvaerts wrote:
>>>>> >if ("aqzaqzaqz" =~ /(a(?R)z|q)*/) {
>>>>> >>>>> This has been fixed in blead.
>>>> >>>> in particular, v5.25.0-36-gda7cf1c: >>>> >>>> fix #128109 - do not move RExC_open_parens[0] in reginsert
>>> >>> That commit, and v5.25.1-196-gec5bd2262b, which adds a test for it, >>> cherry-pick cleanly to maint-5.24. Seeing as it's a crasher and >>> regression from 5.22, it's a candidate for backporting according to >>> perlpolicy.
>> >> I just pushed >> >> 78b60e1 Add tests for regex recursion >> 4539ae3 fix #128109 - do not move RExC_open_parens[0] in reginsert >> 5edefd5 fix #128085 - SIGSEGV in S_regmatch with S_study_chunk: >> Assertion "!frame" failed. >> >> to maint-5.24 -- I took the liberty of including 5edefd5 which was >> part of how I found out about #128109 >>
> > I don't think this should have been done right now. maint-5.22 and > maint-5.24 are both still in code freeze pending the release of 5.22.4 > and 5.24.2, which are intended only to contain the last @INC fixes > (and possibly some other security fixes, as per the previous releases > on those branches). > > There are many, many other bug fixes (crashes and otherwise) that > could be backported, but haven't been yet. They were deliberately > being left for 5.24.3, once the last of the @INC fixes was out of the > way.
Sigh. Honestly I give up. Our release process is broken. If it weren't for the fact that we are going to change these policies that I would insert a long rant here about how expecting our developers to keep track of all their patches, which branches they have and have not been merged and which they should be merged to in some distant point in time is not a good way manage things[1]. Just like distributing swords in lakes in the middle of the night not a sound way to form a government. :-) Anyway, bulk reverted in 9ecaf4dba7ccf61301f3a0ca342810f57060fbd6 sorry for yet more revert turbulence. cheers, Yves [1] So here is the *short* rant: I know there are some on this list that one way or another do not find these types of things a burden. But processes should work for everybody, not just the meticulous and careful, and they should be robust to things like unexpected life-changes. This patch was written in a period when I was dealing with the death of my father and a very sick mother. Making sure the patch was backported at the right time so that it didn't violate a code freeze was simply not a priority for me, and as our release procedures and policies do not provide for any solution other than "remember yourself", no-one else was going to do it either. This to me says that the policies and procedures are broken by design. -- perl -Mre=debug -e "/just|another|perl|hacker/"
Date: Wed, 3 May 2017 08:25:31 +0100
CC: Dagfinn Ilmari Mannsåker <ilmari [...] ilmari.org>, Dave Mitchell <davem [...] iabyn.com>, Zefram <zefram [...] fysh.org>, Jan Goyvaerts <jg [...] jgsoft.com>, Perl5 Porteros <perl5-porters [...] perl.org>
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
To: demerphq <demerphq [...] gmail.com>
From: Steve Hay via perl5-porters <perl5-porters [...] perl.org>
Download (untitled) / with headers
text/plain 4.2k
On 3 May 2017 at 07:56, demerphq <demerphq@gmail.com> wrote: Show quoted text
> On 2 May 2017 at 22:49, Steve Hay <steve.m.hay@googlemail.com> wrote:
>> On 2 May 2017 at 18:38, demerphq <demerphq@gmail.com> wrote:
>>> On 2 May 2017 at 17:39, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:
>>>> Dave Mitchell <davem@iabyn.com> writes: >>>>
>>>>> On Mon, May 01, 2017 at 02:51:16AM +0100, Zefram wrote:
>>>>>> Jan Goyvaerts wrote:
>>>>>> >if ("aqzaqzaqz" =~ /(a(?R)z|q)*/) {
>>>>>> >>>>>> This has been fixed in blead.
>>>>> >>>>> in particular, v5.25.0-36-gda7cf1c: >>>>> >>>>> fix #128109 - do not move RExC_open_parens[0] in reginsert
>>>> >>>> That commit, and v5.25.1-196-gec5bd2262b, which adds a test for it, >>>> cherry-pick cleanly to maint-5.24. Seeing as it's a crasher and >>>> regression from 5.22, it's a candidate for backporting according to >>>> perlpolicy.
>>> >>> I just pushed >>> >>> 78b60e1 Add tests for regex recursion >>> 4539ae3 fix #128109 - do not move RExC_open_parens[0] in reginsert >>> 5edefd5 fix #128085 - SIGSEGV in S_regmatch with S_study_chunk: >>> Assertion "!frame" failed. >>> >>> to maint-5.24 -- I took the liberty of including 5edefd5 which was >>> part of how I found out about #128109 >>>
>> >> I don't think this should have been done right now. maint-5.22 and >> maint-5.24 are both still in code freeze pending the release of 5.22.4 >> and 5.24.2, which are intended only to contain the last @INC fixes >> (and possibly some other security fixes, as per the previous releases >> on those branches). >> >> There are many, many other bug fixes (crashes and otherwise) that >> could be backported, but haven't been yet. They were deliberately >> being left for 5.24.3, once the last of the @INC fixes was out of the >> way.
> > Sigh. Honestly I give up. Our release process is broken. If it weren't > for the fact that we are going to change these policies that I would > insert a long rant here about how expecting our developers to keep > track of all their patches, which branches they have and have not been > merged and which they should be merged to in some distant point in > time is not a good way manage things[1]. Just like distributing swords > in lakes in the middle of the night not a sound way to form a > government. :-) > > Anyway, bulk reverted in 9ecaf4dba7ccf61301f3a0ca342810f57060fbd6 > sorry for yet more revert turbulence.
Thanks. Show quoted text
> > cheers, > Yves > [1] So here is the *short* rant: I know there are some on this list > that one way or another do not find these types of things a burden. > But processes should work for everybody, not just the meticulous and > careful, and they should be robust to things like unexpected > life-changes. This patch was written in a period when I was dealing > with the death of my father and a very sick mother. Making sure the > patch was backported at the right time so that it didn't violate a > code freeze was simply not a priority for me, and as our release > procedures and policies do not provide for any solution other than > "remember yourself", no-one else was going to do it either. This to me > says that the policies and procedures are broken by design. >
I wasn't aware that the discussions over changes in branch management were intended to cover maint releases too. Regarding remembering that patches get backported, I think the existing process is very simple, actually: Just add the patch to the relevant voting file in the maint-votes branch and the maint release manager will pick it up at the approrpiate point in time. That's the whole point of having a maint release manager isn't it? -- so that individual developers don't have to remember all these things themselves. If ever a patch is non-trivial to backport then I will seek assistance when I come to cherry-pick it, which is something that I've done on several occasions in the past. Also, patches are supposed to get three votes before being backported -- something else that I didn't see happen on this occasion. Again, the voting file takes care of that perfectly simply. It's not obvious to me that this process needs to change since it doesn't suffer from the "blockage" issue that blead has during a long code freeze: There is no active development being done on maint branches anyway (virtually by definition).
Date: Wed, 3 May 2017 10:00:28 +0200
CC: Dagfinn Ilmari Mannsåker <ilmari [...] ilmari.org>, Dave Mitchell <davem [...] iabyn.com>, Zefram <zefram [...] fysh.org>, Jan Goyvaerts <jg [...] jgsoft.com>, Perl5 Porteros <perl5-porters [...] perl.org>
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
From: demerphq <demerphq [...] gmail.com>
To: Steve Hay <steve.m.hay [...] googlemail.com>
Download (untitled) / with headers
text/plain 6.7k
On 3 May 2017 at 09:25, Steve Hay <steve.m.hay@googlemail.com> wrote: Show quoted text
> On 3 May 2017 at 07:56, demerphq <demerphq@gmail.com> wrote:
>> On 2 May 2017 at 22:49, Steve Hay <steve.m.hay@googlemail.com> wrote:
>>> On 2 May 2017 at 18:38, demerphq <demerphq@gmail.com> wrote:
>>>> On 2 May 2017 at 17:39, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:
>>>>> Dave Mitchell <davem@iabyn.com> writes: >>>>>
>>>>>> On Mon, May 01, 2017 at 02:51:16AM +0100, Zefram wrote:
>>>>>>> Jan Goyvaerts wrote:
>>>>>>> >if ("aqzaqzaqz" =~ /(a(?R)z|q)*/) {
>>>>>>> >>>>>>> This has been fixed in blead.
>>>>>> >>>>>> in particular, v5.25.0-36-gda7cf1c: >>>>>> >>>>>> fix #128109 - do not move RExC_open_parens[0] in reginsert
>>>>> >>>>> That commit, and v5.25.1-196-gec5bd2262b, which adds a test for it, >>>>> cherry-pick cleanly to maint-5.24. Seeing as it's a crasher and >>>>> regression from 5.22, it's a candidate for backporting according to >>>>> perlpolicy.
>>>> >>>> I just pushed >>>> >>>> 78b60e1 Add tests for regex recursion >>>> 4539ae3 fix #128109 - do not move RExC_open_parens[0] in reginsert >>>> 5edefd5 fix #128085 - SIGSEGV in S_regmatch with S_study_chunk: >>>> Assertion "!frame" failed. >>>> >>>> to maint-5.24 -- I took the liberty of including 5edefd5 which was >>>> part of how I found out about #128109 >>>>
>>> >>> I don't think this should have been done right now. maint-5.22 and >>> maint-5.24 are both still in code freeze pending the release of 5.22.4 >>> and 5.24.2, which are intended only to contain the last @INC fixes >>> (and possibly some other security fixes, as per the previous releases >>> on those branches). >>> >>> There are many, many other bug fixes (crashes and otherwise) that >>> could be backported, but haven't been yet. They were deliberately >>> being left for 5.24.3, once the last of the @INC fixes was out of the >>> way.
>> >> Sigh. Honestly I give up. Our release process is broken. If it weren't >> for the fact that we are going to change these policies that I would >> insert a long rant here about how expecting our developers to keep >> track of all their patches, which branches they have and have not been >> merged and which they should be merged to in some distant point in >> time is not a good way manage things[1]. Just like distributing swords >> in lakes in the middle of the night not a sound way to form a >> government. :-) >> >> Anyway, bulk reverted in 9ecaf4dba7ccf61301f3a0ca342810f57060fbd6 >> sorry for yet more revert turbulence.
> > Thanks. > >
>> >> cheers, >> Yves >> [1] So here is the *short* rant: I know there are some on this list >> that one way or another do not find these types of things a burden. >> But processes should work for everybody, not just the meticulous and >> careful, and they should be robust to things like unexpected >> life-changes. This patch was written in a period when I was dealing >> with the death of my father and a very sick mother. Making sure the >> patch was backported at the right time so that it didn't violate a >> code freeze was simply not a priority for me, and as our release >> procedures and policies do not provide for any solution other than >> "remember yourself", no-one else was going to do it either. This to me >> says that the policies and procedures are broken by design. >>
> > I wasn't aware that the discussions over changes in branch management > were intended to cover maint releases too.
I had no direct thoughts on this, but when i wrote that mail I was thinking that they kinda flow together. However some of what you say below makes me reconsider. Show quoted text
> Regarding remembering that patches get backported, I think the > existing process is very simple, actually: Just add the patch to the > relevant voting file in the maint-votes branch and the maint release > manager will pick it up at the approrpiate point in time. That's the > whole point of having a maint release manager isn't it? -- so that > individual developers don't have to remember all these things > themselves. If ever a patch is non-trivial to backport then I will > seek assistance when I come to cherry-pick it, which is something that > I've done on several occasions in the past.
Ok, I think I missed some of these details being announced. However I see no mention of a maint-votes branch in the docs, at least not the docs for 5.24. Also this maint-votes branch is pretty clunky. Why don't we just put a file in blead? Then when you commit your patch you can do a follow up patch to the file. We can exclude the file from the build process when we do a release. Anyway, if there is a file I can edit for these things then obviously some of what I complained about has been addressed already. Show quoted text
> Also, patches are supposed to get three votes before being backported > -- something else that I didn't see happen on this occasion. Again, > the voting file takes care of that perfectly simply.
I was under the impression that there were, at least for two of the three patches. For the latter I interpreted the approval for the other a bit loosely and included it too, as they were all part of the same meta-bug, if not specific bug-report. Show quoted text
> It's not obvious to me that this process needs to change since it > doesn't suffer from the "blockage" issue that blead has during a long > code freeze: There is no active development being done on maint > branches anyway (virtually by definition).
No. I withdraw that. Albeit clunkier than I would like there is a procedure to fire-and-forget, so the problem I complained about is solved and I just didn't know it fully. I apologise for the noise, and for not getting with the program quicker. On the other hand, I then think we should put push blocks on these branches and lock them down to the person who is doing the branch management. I was just trying to "take care of business" without realizing I was breaking the rules. IMO technical provisions to prevent that kind of thing are a good idea. If I had pushed and gotten back a message back telling me what to do I think things would work smoother. I mean even if everybody knows the policy, people make mistakes, etc. I could imagine something like: ########################################################################### Push refused - You are not the branch maintainer. What you probably want to do is push your commits to a temporary branch and then update maint-votes branch, which you can do with something like: git push origin $branch:backports/$branch-TOPIC perl Porting/add-backport-patch TOPIC $sha1 $sha2 $sha3 ... ########################################################################## And then we write add-backport-patch to do whatever vote file git ker-fsckery necessary to add the patches to the vote file. (And insulate anybody doing this from however we store the file. This whole dangling branch thing is cute, but annoying.) Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
Date: Wed, 3 May 2017 09:19:03 +0100
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
CC: Dagfinn Ilmari Mannsåker <ilmari [...] ilmari.org>, Dave Mitchell <davem [...] iabyn.com>, Zefram <zefram [...] fysh.org>, Jan Goyvaerts <jg [...] jgsoft.com>, Perl5 Porteros <perl5-porters [...] perl.org>
To: demerphq <demerphq [...] gmail.com>
From: Steve Hay via perl5-porters <perl5-porters [...] perl.org>
Download (untitled) / with headers
text/plain 8.3k
On 3 May 2017 at 09:00, demerphq <demerphq@gmail.com> wrote: Show quoted text
> On 3 May 2017 at 09:25, Steve Hay <steve.m.hay@googlemail.com> wrote:
>> On 3 May 2017 at 07:56, demerphq <demerphq@gmail.com> wrote:
>>> On 2 May 2017 at 22:49, Steve Hay <steve.m.hay@googlemail.com> wrote:
>>>> On 2 May 2017 at 18:38, demerphq <demerphq@gmail.com> wrote:
>>>>> On 2 May 2017 at 17:39, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:
>>>>>> Dave Mitchell <davem@iabyn.com> writes: >>>>>>
>>>>>>> On Mon, May 01, 2017 at 02:51:16AM +0100, Zefram wrote:
>>>>>>>> Jan Goyvaerts wrote:
>>>>>>>> >if ("aqzaqzaqz" =~ /(a(?R)z|q)*/) {
>>>>>>>> >>>>>>>> This has been fixed in blead.
>>>>>>> >>>>>>> in particular, v5.25.0-36-gda7cf1c: >>>>>>> >>>>>>> fix #128109 - do not move RExC_open_parens[0] in reginsert
>>>>>> >>>>>> That commit, and v5.25.1-196-gec5bd2262b, which adds a test for it, >>>>>> cherry-pick cleanly to maint-5.24. Seeing as it's a crasher and >>>>>> regression from 5.22, it's a candidate for backporting according to >>>>>> perlpolicy.
>>>>> >>>>> I just pushed >>>>> >>>>> 78b60e1 Add tests for regex recursion >>>>> 4539ae3 fix #128109 - do not move RExC_open_parens[0] in reginsert >>>>> 5edefd5 fix #128085 - SIGSEGV in S_regmatch with S_study_chunk: >>>>> Assertion "!frame" failed. >>>>> >>>>> to maint-5.24 -- I took the liberty of including 5edefd5 which was >>>>> part of how I found out about #128109 >>>>>
>>>> >>>> I don't think this should have been done right now. maint-5.22 and >>>> maint-5.24 are both still in code freeze pending the release of 5.22.4 >>>> and 5.24.2, which are intended only to contain the last @INC fixes >>>> (and possibly some other security fixes, as per the previous releases >>>> on those branches). >>>> >>>> There are many, many other bug fixes (crashes and otherwise) that >>>> could be backported, but haven't been yet. They were deliberately >>>> being left for 5.24.3, once the last of the @INC fixes was out of the >>>> way.
>>> >>> Sigh. Honestly I give up. Our release process is broken. If it weren't >>> for the fact that we are going to change these policies that I would >>> insert a long rant here about how expecting our developers to keep >>> track of all their patches, which branches they have and have not been >>> merged and which they should be merged to in some distant point in >>> time is not a good way manage things[1]. Just like distributing swords >>> in lakes in the middle of the night not a sound way to form a >>> government. :-) >>> >>> Anyway, bulk reverted in 9ecaf4dba7ccf61301f3a0ca342810f57060fbd6 >>> sorry for yet more revert turbulence.
>> >> Thanks. >> >>
>>> >>> cheers, >>> Yves >>> [1] So here is the *short* rant: I know there are some on this list >>> that one way or another do not find these types of things a burden. >>> But processes should work for everybody, not just the meticulous and >>> careful, and they should be robust to things like unexpected >>> life-changes. This patch was written in a period when I was dealing >>> with the death of my father and a very sick mother. Making sure the >>> patch was backported at the right time so that it didn't violate a >>> code freeze was simply not a priority for me, and as our release >>> procedures and policies do not provide for any solution other than >>> "remember yourself", no-one else was going to do it either. This to me >>> says that the policies and procedures are broken by design. >>>
>> >> I wasn't aware that the discussions over changes in branch management >> were intended to cover maint releases too.
> > I had no direct thoughts on this, but when i wrote that mail I was > thinking that they kinda flow together. However some of what you say > below makes me reconsider. >
>> Regarding remembering that patches get backported, I think the >> existing process is very simple, actually: Just add the patch to the >> relevant voting file in the maint-votes branch and the maint release >> manager will pick it up at the approrpiate point in time. That's the >> whole point of having a maint release manager isn't it? -- so that >> individual developers don't have to remember all these things >> themselves. If ever a patch is non-trivial to backport then I will >> seek assistance when I come to cherry-pick it, which is something that >> I've done on several occasions in the past.
> > Ok, I think I missed some of these details being announced. However I > see no mention of a maint-votes branch in the docs, at least not the > docs for 5.24.
You're right - there is no mention of it in the docs (even in blead), which amazes me. We've been using this process for some time now, and documenting it after some trial period has evidently slipped through the cracks. Sorry about that. I will try to fix that, though perhaps it should wait until we've made any changes in light of your proposals below. Show quoted text
> > Also this maint-votes branch is pretty clunky. Why don't we just put a > file in blead? Then when you commit your patch you can do a follow up > patch to the file. We can exclude the file from the build process when > we do a release.
I think this may have been discussed at the time the voting files were introduced, and there were some objections to having a file in blead because the votes being cast would then cause noise on the blead commit history. Show quoted text
> > Anyway, if there is a file I can edit for these things then obviously > some of what I complained about has been addressed already. >
>> Also, patches are supposed to get three votes before being backported >> -- something else that I didn't see happen on this occasion. Again, >> the voting file takes care of that perfectly simply.
> > I was under the impression that there were, at least for two of the > three patches. For the latter I interpreted the approval for the other > a bit loosely and included it too, as they were all part of the same > meta-bug, if not specific bug-report.
Sorry if I missed that. This is why having a file *somewhere* to keep track of the votes is definitely a good idea (IMO). Show quoted text
>
>> It's not obvious to me that this process needs to change since it >> doesn't suffer from the "blockage" issue that blead has during a long >> code freeze: There is no active development being done on maint >> branches anyway (virtually by definition).
> > No. I withdraw that. Albeit clunkier than I would like there is a > procedure to fire-and-forget, so the problem I complained about is > solved and I just didn't know it fully. I apologise for the noise, and > for not getting with the program quicker. > > On the other hand, I then think we should put push blocks on these > branches and lock them down to the person who is doing the branch > management. I was just trying to "take care of business" without > realizing I was breaking the rules. IMO technical provisions to > prevent that kind of thing are a good idea.
I think I would welcome the idea of push blocks (on the proposed blead release branch too) during code freeze because that's when it becomes most important to stop accidental commits, but I'm not sure they'd be necessary the rest of the time. Other committers have pushed to maint branches in the past. As long as it's well ahead of a release and follows the voting rules then I have no problem with that (it saves me a bit of work!). Would it be easy for the branch manager to add/remove these push blocks, or is it a more complicated thing to do that (e.g.) Dennis would have to do, and once it's done it's done? Show quoted text
> > If I had pushed and gotten back a message back telling me what to do I > think things would work smoother. I mean even if everybody knows the > policy, people make mistakes, etc. I could imagine something like: > > ########################################################################### > Push refused - You are not the branch maintainer. What you probably > want to do is push your commits to a temporary branch and then update > maint-votes branch, which you can do with something like: > > git push origin $branch:backports/$branch-TOPIC > perl Porting/add-backport-patch TOPIC $sha1 $sha2 $sha3 ... > ########################################################################## > > And then we write add-backport-patch to do whatever vote file git > ker-fsckery necessary to add the patches to the vote file. (And > insulate anybody doing this from however we store the file. This whole > dangling branch thing is cute, but annoying.) > > Yves > > -- > perl -Mre=debug -e "/just|another|perl|hacker/"
Date: Wed, 3 May 2017 11:21:59 +0200
From: demerphq <demerphq [...] gmail.com>
To: Steve Hay <steve.m.hay [...] googlemail.com>
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
CC: Dagfinn Ilmari Mannsåker <ilmari [...] ilmari.org>, Dave Mitchell <davem [...] iabyn.com>, Zefram <zefram [...] fysh.org>, Jan Goyvaerts <jg [...] jgsoft.com>, Perl5 Porteros <perl5-porters [...] perl.org>
Download (untitled) / with headers
text/plain 4.5k
[using * to mark my comments since apparently gmail html to text is broken for quoting] On 3 May 2017 at 10:19, Steve Hay <steve.m.hay@googlemail.com> wrote: Show quoted text
> On 3 May 2017 at 09:00, demerphq <demerphq@gmail.com> wrote:
>> On 3 May 2017 at 09:25, Steve Hay <steve.m.hay@googlemail.com> wrote:
>>> On 3 May 2017 at 07:56, demerphq <demerphq@gmail.com> wrote:
>>>> On 2 May 2017 at 22:49, Steve Hay <steve.m.hay@googlemail.com> wrote:
.... Show quoted text
> You're right - there is no mention of it in the docs (even in blead), > which amazes me. We've been using this process for some time now, and > documenting it after some trial period has evidently slipped through > the cracks. Sorry about that. I will try to fix that, though perhaps > it should wait until we've made any changes in light of your proposals > below.
* Ok, cool. Show quoted text
>
>> >> Also this maint-votes branch is pretty clunky. Why don't we just put a >> file in blead? Then when you commit your patch you can do a follow up >> patch to the file. We can exclude the file from the build process when >> we do a release.
> > I think this may have been discussed at the time the voting files were > introduced, and there were some objections to having a file in blead > because the votes being cast would then cause noise on the blead > commit history. >
* I see. Hrm. Thinking about it more if there was a tool that managed it then it doesnt matter where it lives, and having the separate branch is at least "fair" in that it is as easy, or hard to use, regardless what your workflow is. * I guess I have to write a tool. :-) Show quoted text
>> >> Anyway, if there is a file I can edit for these things then obviously >> some of what I complained about has been addressed already. >>
>>> Also, patches are supposed to get three votes before being backported >>> -- something else that I didn't see happen on this occasion. Again, >>> the voting file takes care of that perfectly simply.
>> >> I was under the impression that there were, at least for two of the >> three patches. For the latter I interpreted the approval for the other >> a bit loosely and included it too, as they were all part of the same >> meta-bug, if not specific bug-report.
> > Sorry if I missed that. This is why having a file *somewhere* to keep > track of the votes is definitely a good idea (IMO).
* Yes. The issues of votes requires somewhere to store and track them. Show quoted text
>
>>
>>> It's not obvious to me that this process needs to change since it >>> doesn't suffer from the "blockage" issue that blead has during a long >>> code freeze: There is no active development being done on maint >>> branches anyway (virtually by definition).
>> >> No. I withdraw that. Albeit clunkier than I would like there is a >> procedure to fire-and-forget, so the problem I complained about is >> solved and I just didn't know it fully. I apologise for the noise, and >> for not getting with the program quicker. >> >> On the other hand, I then think we should put push blocks on these >> branches and lock them down to the person who is doing the branch >> management. I was just trying to "take care of business" without >> realizing I was breaking the rules. IMO technical provisions to >> prevent that kind of thing are a good idea.
> > I think I would welcome the idea of push blocks (on the proposed blead > release branch too) during code freeze because that's when it becomes > most important to stop accidental commits, but I'm not sure they'd be > necessary the rest of the time. Other committers have pushed to maint > branches in the past. As long as it's well ahead of a release and > follows the voting rules then I have no problem with that (it saves me > a bit of work!). Would it be easy for the branch manager to add/remove > these push blocks, or is it a more complicated thing to do that (e.g.) > Dennis would have to do, and once it's done it's done?
* this is all me here We could do that, but it would require some finagling. Perhaps better would be to require a signed-off-by signature in the commit message. Maybe something as simple as each commit requires at least three lines of the form vote: demerphq vote: Steve Hay vote: Dave Mitchell then the push hook can validate that, and if it is missing give a suitable message about how to mange all of this. If we have tools to manage the vote branch we could even automate this process. (IIRC There is a more formal git concept of signed-off-by but i dont know all the details or how suitable it would be for us, I will dig and see, although if someone knows feel free to educate us.) cheers, Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
To: demerphq <demerphq [...] gmail.com>
From: "Craig A. Berry" <craig.a.berry [...] gmail.com>
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
CC: Steve Hay <steve.m.hay [...] googlemail.com>, Dagfinn Ilmari Mannsåker <ilmari [...] ilmari.org>, Dave Mitchell <davem [...] iabyn.com>, Zefram <zefram [...] fysh.org>, Jan Goyvaerts <jg [...] jgsoft.com>, Perl5 Porteros <perl5-porters [...] perl.org>
Date: Thu, 4 May 2017 16:41:33 -0500
Download (untitled) / with headers
text/plain 2.4k
On Wed, May 3, 2017 at 4:21 AM, demerphq <demerphq@gmail.com> wrote: Show quoted text
> [using * to mark my comments since apparently gmail html to text is > broken for quoting] > On 3 May 2017 at 10:19, Steve Hay <steve.m.hay@googlemail.com> wrote:
Show quoted text
>> I think I would welcome the idea of push blocks (on the proposed blead >> release branch too) during code freeze because that's when it becomes >> most important to stop accidental commits, but I'm not sure they'd be >> necessary the rest of the time. Other committers have pushed to maint >> branches in the past. As long as it's well ahead of a release and >> follows the voting rules then I have no problem with that (it saves me >> a bit of work!). Would it be easy for the branch manager to add/remove >> these push blocks, or is it a more complicated thing to do that (e.g.) >> Dennis would have to do, and once it's done it's done?
> > * this is all me here > > We could do that, but it would require some finagling. Perhaps better > would be to require a signed-off-by signature in the commit message. > Maybe something as simple as each commit requires at least three lines > of the form > > vote: demerphq > vote: Steve Hay > vote: Dave Mitchell > > then the push hook can validate that, and if it is missing give a > suitable message about how to mange all of this. If we have tools to > manage the vote branch we could even automate this process.
I think what's missing in this discussion is the degree to which the base.pm business has completely gummed up the works of the maint releases and that's really what got in your way. Not saying that it could be otherwise, but it has meant a very long code freeze in maint which as far as I can remember has never happened before. In any normal year I don't think there would have been a problem applying what you did to maint. If it were missing a vote we could have scrounged another vote more easily than reverting. Or agreed that neither of the other two people on the planet qualified to review your work on the regex engine was available to vote Subject, of course, to Steve's decision on a case-by-case basis, which would probably come with a reminder about any parts of the maint policy that had been overlooked.. All of which is to say, heavy-duty automation that enforces the maint policy in a rigid way and is hard to turn on and off seems like overkill to me. Even with the many-month-long code freeze what happened to you is unique as far as I know, and if we let such lengthy freezes become the norm we've got bigger problems.
Date: Fri, 5 May 2017 09:44:01 +0200
CC: Perl5 Porteros <perl5-porters [...] perl.org>, Jan Goyvaerts <jg [...] jgsoft.com>, Steve Hay <steve.m.hay [...] googlemail.com>, Zefram <zefram [...] fysh.org>, Dagfinn Ilmari Mannsåker <ilmari [...] ilmari.org>, Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
To: "Craig A. Berry" <craig.a.berry [...] gmail.com>
From: demerphq <demerphq [...] gmail.com>
Download (untitled) / with headers
text/plain 3.2k


On 4 May 2017 23:41, "Craig A. Berry" <craig.a.berry@gmail.com> wrote:
Show quoted text
On Wed, May 3, 2017 at 4:21 AM, demerphq <demerphq@gmail.com> wrote:
> [using * to mark my comments since apparently gmail html to text is
> broken for quoting]
> On 3 May 2017 at 10:19, Steve Hay <steve.m.hay@googlemail.com> wrote:


>> I think I would welcome the idea of push blocks (on the proposed blead
>> release branch too) during code freeze because that's when it becomes
>> most important to stop accidental commits, but I'm not sure they'd be
>> necessary the rest of the time. Other committers have pushed to maint
>> branches in the past. As long as it's well ahead of a release and
>> follows the voting rules then I have no problem with that (it saves me
>> a bit of work!). Would it be easy for the branch manager to add/remove
>> these push blocks, or is it a more complicated thing to do that (e.g.)
>> Dennis would have to do, and once it's done it's done?
>
> * this is all me here
>
> We could do that, but it would require some finagling. Perhaps better
> would be to require a signed-off-by signature in the commit message.
> Maybe something as simple as each commit requires at least three lines
> of the form
>
> vote: demerphq
> vote: Steve Hay
> vote: Dave Mitchell
>
> then the push hook can validate that, and if it is missing give a
> suitable message about how to mange all of this. If we have tools to
> manage the vote branch we could even automate this process.

I think what's missing in this discussion is the degree to which the
base.pm business has completely gummed up the works of the maint
releases and that's really what got in your way.  Not saying that it
could be otherwise, but it has meant a very long code freeze in maint
which as far as I can remember has never happened before.  In any
normal year I don't think there would have been a problem applying
what you did to maint. If it were missing a vote we could have
scrounged another vote more easily than reverting.  Or agreed that
neither of the other two people on the planet qualified to review your
work on the regex engine was available to vote  Subject, of course, to
Steve's decision on a case-by-case basis, which would probably come
with a reminder about any parts of the maint policy that had been
overlooked.

Thanks for saying this.

Show quoted text
All of which is to say, heavy-duty automation that enforces the maint
policy in a rigid way and is hard to turn on and off seems like
overkill to me.  Even with the many-month-long code freeze what
happened to you is unique as far as I know, and if we let such lengthy
freezes become the norm we've got bigger problems.

When I was in high school I did a short course in a sheet metal and machine shop. During that course I got to use all sorts of crazy tools like machines that could fold sheet metal like it was paper, or cutting devices that could take your finger off before you even felt the pain. It taught me a powerful respect for technical safety measures which has stuck with me ever since.  So I personally don't see requiring vote lines in maint commit message as a huge burden. 

Having said that you have a point about not making policy from a black Swan event.

We can ruminate on this for a bit and revisit it the next time someone makes the same mistakes I did. 

Cheers 
Yves
Subject: Re: [perl #128420] Changes in regex recursion in 5.24
To: demerphq <demerphq [...] gmail.com>
CC: Dagfinn Ilmari Mannsåker <ilmari [...] ilmari.org>, Dave Mitchell <davem [...] iabyn.com>, Zefram <zefram [...] fysh.org>, Jan Goyvaerts <jg [...] jgsoft.com>, Perl5 Porteros <perl5-porters [...] perl.org>
From: Steve Hay via perl5-porters <perl5-porters [...] perl.org>
Date: Thu, 20 Jul 2017 14:13:33 +0100
Download (untitled) / with headers
text/plain 5.1k
On 3 May 2017 at 09:19, Steve Hay <steve.m.hay@googlemail.com> wrote: Show quoted text
> On 3 May 2017 at 09:00, demerphq <demerphq@gmail.com> wrote:
>> On 3 May 2017 at 09:25, Steve Hay <steve.m.hay@googlemail.com> wrote:
>>> On 3 May 2017 at 07:56, demerphq <demerphq@gmail.com> wrote:
>>>> On 2 May 2017 at 22:49, Steve Hay <steve.m.hay@googlemail.com> wrote:
>>>>> On 2 May 2017 at 18:38, demerphq <demerphq@gmail.com> wrote:
>>>>>> On 2 May 2017 at 17:39, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:
>>>>>>> Dave Mitchell <davem@iabyn.com> writes: >>>>>>>
>>>>>>>> On Mon, May 01, 2017 at 02:51:16AM +0100, Zefram wrote:
>>>>>>>>> Jan Goyvaerts wrote:
>>>>>>>>> >if ("aqzaqzaqz" =~ /(a(?R)z|q)*/) {
>>>>>>>>> >>>>>>>>> This has been fixed in blead.
>>>>>>>> >>>>>>>> in particular, v5.25.0-36-gda7cf1c: >>>>>>>> >>>>>>>> fix #128109 - do not move RExC_open_parens[0] in reginsert
>>>>>>> >>>>>>> That commit, and v5.25.1-196-gec5bd2262b, which adds a test for it, >>>>>>> cherry-pick cleanly to maint-5.24. Seeing as it's a crasher and >>>>>>> regression from 5.22, it's a candidate for backporting according to >>>>>>> perlpolicy.
>>>>>> >>>>>> I just pushed >>>>>> >>>>>> 78b60e1 Add tests for regex recursion >>>>>> 4539ae3 fix #128109 - do not move RExC_open_parens[0] in reginsert >>>>>> 5edefd5 fix #128085 - SIGSEGV in S_regmatch with S_study_chunk: >>>>>> Assertion "!frame" failed. >>>>>> >>>>>> to maint-5.24 -- I took the liberty of including 5edefd5 which was >>>>>> part of how I found out about #128109 >>>>>>
>>>>> >>>>> I don't think this should have been done right now. maint-5.22 and >>>>> maint-5.24 are both still in code freeze pending the release of 5.22.4 >>>>> and 5.24.2, which are intended only to contain the last @INC fixes >>>>> (and possibly some other security fixes, as per the previous releases >>>>> on those branches). >>>>> >>>>> There are many, many other bug fixes (crashes and otherwise) that >>>>> could be backported, but haven't been yet. They were deliberately >>>>> being left for 5.24.3, once the last of the @INC fixes was out of the >>>>> way.
>>>> >>>> Sigh. Honestly I give up. Our release process is broken. If it weren't >>>> for the fact that we are going to change these policies that I would >>>> insert a long rant here about how expecting our developers to keep >>>> track of all their patches, which branches they have and have not been >>>> merged and which they should be merged to in some distant point in >>>> time is not a good way manage things[1]. Just like distributing swords >>>> in lakes in the middle of the night not a sound way to form a >>>> government. :-) >>>> >>>> Anyway, bulk reverted in 9ecaf4dba7ccf61301f3a0ca342810f57060fbd6 >>>> sorry for yet more revert turbulence.
>>> >>> Thanks. >>> >>>
>>>> >>>> cheers, >>>> Yves >>>> [1] So here is the *short* rant: I know there are some on this list >>>> that one way or another do not find these types of things a burden. >>>> But processes should work for everybody, not just the meticulous and >>>> careful, and they should be robust to things like unexpected >>>> life-changes. This patch was written in a period when I was dealing >>>> with the death of my father and a very sick mother. Making sure the >>>> patch was backported at the right time so that it didn't violate a >>>> code freeze was simply not a priority for me, and as our release >>>> procedures and policies do not provide for any solution other than >>>> "remember yourself", no-one else was going to do it either. This to me >>>> says that the policies and procedures are broken by design. >>>>
>>> >>> I wasn't aware that the discussions over changes in branch management >>> were intended to cover maint releases too.
>> >> I had no direct thoughts on this, but when i wrote that mail I was >> thinking that they kinda flow together. However some of what you say >> below makes me reconsider. >>
>>> Regarding remembering that patches get backported, I think the >>> existing process is very simple, actually: Just add the patch to the >>> relevant voting file in the maint-votes branch and the maint release >>> manager will pick it up at the approrpiate point in time. That's the >>> whole point of having a maint release manager isn't it? -- so that >>> individual developers don't have to remember all these things >>> themselves. If ever a patch is non-trivial to backport then I will >>> seek assistance when I come to cherry-pick it, which is something that >>> I've done on several occasions in the past.
>> >> Ok, I think I missed some of these details being announced. However I >> see no mention of a maint-votes branch in the docs, at least not the >> docs for 5.24.
> > You're right - there is no mention of it in the docs (even in blead), > which amazes me. We've been using this process for some time now, and > documenting it after some trial period has evidently slipped through > the cracks. Sorry about that. I will try to fix that, though perhaps > it should wait until we've made any changes in light of your proposals > below. >
I've now documented the current process in commit 29f85661b824c66e6666be35c0d0da874dccd7b1. Hopefully this will avert similar confusion in the future. If not then we may need to reconsider things.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org