develooper Front page | perl.perl5.porters | Postings from May 2017

Re: [perl #128420] Changes in regex recursion in 5.24

Thread Previous | Thread Next
From:
Steve Hay via perl5-porters
Date:
May 3, 2017 08:19
Subject:
Re: [perl #128420] Changes in regex recursion in 5.24
Message ID:
CADED=K5NSMaHpzavChmnvWy8fjK0K_-pvOd1cRuLMM2tqSU4Uw@mail.gmail.com
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.


>
> 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.


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


>
>> 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?


>
> 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/"

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About