develooper Front page | perl.perl5.porters | Postings from June 2011

Re: [PATCH] export store_cop_label

Thread Previous | Thread Next
From:
Joshua ben Jore
Date:
June 6, 2011 08:05
Subject:
Re: [PATCH] export store_cop_label
Message ID:
BANLkTikyLTxJGr6Dv2hpFJHkZ18vdG++gCKx85iO-SnshcptGg@mail.gmail.com
On Sun, Jun 5, 2011 at 4:16 PM, Reini Urban <rurban@x-ray.at> wrote:
> 2011/5/30 Nicholas Clark <nick@ccl4.org>:
>> On Mon, May 30, 2011 at 03:34:05PM +0200, Reini Urban wrote:
>>> 2011/5/30 Joshua ben Jore <twists@gmail.com>:
>>> > On Sat, May 28, 2011 at 11:40 PM, Konovalov, Vadim (Vadim)** CTR **
>>> > <vadim.konovalov@alcatel-lucent.com> wrote:
>>> >>> From: Konovalov, Vadim
>>> >>> > From: Jesse Vincent
>>> >>> > Based on a conversation with a few porters yesterday, it
>>> >>> > sounds like there's some
>>> >>> > desire for a more thought-out, comprehensive API here and
>>> >>> > that that is currently in Nick's backlog.
>>> >>
>>> >> Is it possible to elaborate once more please, what kind of API support
>>> >> is missing in Reini patch?
>>> >
>>> > The intention is to have a useful, properly granular API we can
>>> > support for a really long time. We'd like to come up with the API that
>>> > we actually intend to be publicly consumed. The existing function
>>> > under question wasn't developed with those goals in mind.
>>>
>>> This particular function was used in B::C and B::Bytecode since 1995.
>>
>> That's a neat trick, given the function only dates from 2008.
>>
>>
>> To be fair, yes, the functionality was in use - see below for what *that*
>> implies.
>>
>>> Since Linux/BSD/cygwin does not care about properly exported/not exported
>>> symbols nobody cares about Windows breakage.
>>
>> That's not true either.
>>
>>
>> To my mind the correct fix is to properly not export symbols on Linux too,
>> which I believe is possible, at least in some cases. I think Rafl had a
>> look at this at one point, but we didn't find a way to make it work.
>> I hope we find time to look at it again.
>>
>>> B::Generate also misses some exported functions from the very beginning on,
>>> for which I wrote the export patches some time ago. They were also
>>> warnocked resp. blocked by Nick. It's not my intent to bug p5p again
>>> and again. Three times is enough for me.
>>
>> The consistent position is that we're prepared to add functionality as a
>> proper API if it's documented and tested. IIRC you've consistently stated
>> that the functions don't belong to the API, and should just be exported for
>> the private use of the compiler, so you refused to write tests.
>>
>> If you ignore the feedback, what do you expect?
>
> I expect to have the limitations being fixed in time.
> "In time" means to the next major release, not blocking two major releases.
> Currently 5.12 and 5.14 are broken for Windows users at least.
>
>> Secondly, as far as B::Generate goes, several the functions it "wants"
>> exported are for routines in that could never have worked. Routines in op.c
>> assume (or assert) that IN_PERL_RUNTIME is false, yet the XS wrappers called
>> it directly. Requesting changes for code that couldn't even work *with those
>> changes* is stupid, because it shows that that requester hasn't tested the
>> change locally.
>
> Excuse me, B::Generate has a test suite. Which works on Linux and such.
> And it is used in useful modules. I see that you don't find it useful and you
> claim that they will not work at all. Maybe someone else sees the usefulness
> and will fix it.
>
> For the records: optree manipulation should be done at compile-time of course.
> Every module I know of uses it at compile-time.
> Currently it also works at run-time on non-threaded code without DEBUGGING.
>
>> As far as the patch for this particular function, the timing was unfortunate.
>> The patch was sent after Jesse had decreed "regressions only", and this
>> *isn't* a regression from 5.12.0. I don't think it's a regression from 5.10.0
>
> It's a regression as it worked until 5.10 and does not work since 5.12.
> Technically, it was part of the public API before (via a macro in the header),
> and was then moved to a private function, thus effectively breaking Windows.
>
>> Lack of bug reports is a reasonable indicator that this isn't an important
>> bug to the vast majority of Perl's userbase. Why should we bend the
>> maintenance policy for something relatively unimportant?
>
> Pardon me? "Bending the maintenance policy" is ridiculous.
> People want it to be fixed.
> What is the timeframe for the outstanding fixes?
> Will the fixes be applied to the next minor releases or not?
>
> I know of two B::Generate bugs being ignored by p5p and two compiler bugs
> being ignored by p5p for ages - one to two major release cycles.
> Windows users certainly lost faith.
>
>> Looking at the proposed patch is on my (mental) list of things to do, but
>> not at the top, and as the policy means that it's not valid as a candidate
>> for 5.14.1. They are the priority for my free time.
>
> We know that you failed to address these issues for two last major releases.
> I propose that someone else will fix these issues.
>
> I posted patches to be applied. If you don't like them p5p can fix in
> any way you see fit.
>
>>> Since these modules are used in production, the maintainer tried to
>>> support it, but
>>> CORE still refuses to support it, I can only think of keeping
>>> maintaining private versions
>>> of perl. Which I do for myself.
>>
>> We're not prepared to bend over and give you what you demand. We've offered
>> you a route, which you refuse to take.
>
> "You" proposed that route, not "we".
> I'm not demanding that "you" fix these issues, I don't care who will fix it.
> But they should be fixed.
> I offered help, but it was not accepted so far.
>
>> Ad-hoc use of any function that seemed useful, rather that working to create
>> and update a well-behaved API, has got the perl core into the maintenance
>> hell that it currently is. We need to stop making those mistakes.
>>
>> So, to this functionality, the "see below" above. I infer that when the core
>> saved memory by changing how labels are stored *in 2008*, then the compiler
>> code was changed to use a function *without checking whether it was part of
>> the API*.
>
> Wrong. It was part of the API before, and when it was removed I
> complained immediately.
>
>> Only years later is a request made for that function to be in the
>> API, on the basis that "I'm already using it". That's not the way to create
>> a sane API.
>
> Wrong. It's now "years later" because you blocked it for years.
> You are also blocking other bugfixes because you don't like it or have no time.
> Who can step up now?
>
>> You're welcome to maintain a fork - git should make this easy.
>>
>> You're also welcome to keep implying that I have a vendetta against the
>> compiler. For reference:
>>
>> Jarkko committed the API embedding and exporting changes back in 2002 into
>> maint-5.8 between 5.8.0 and 5.8.1, and Hugo merged it to blead in 2003
>>    http://rt.perl.org/rt3/Ticket/Display.html?id=8610
>>    http://perl5.git.perl.org/perl.git/commit/de37762f3a9822aa
>>
>> Rafael removed the compiler from core in 2006
>>   http://perl5.git.perl.org/perl.git/commit/de1254415ffeb03b
>>
>> Nicholas Clark
>
> Interesting, it was Rafael not you. I certainly remembered it differently.
> http://www.xray.mpe.mpg.de/cgi-bin/w3glimpse2html/perl5-porters/2006-09/msg00230.html?58#mfs
>
> I'm wondering why he was able to remove it without any discussion and
> with such a bold statement. Ok, p5p was not able to support it
> anymore.
> But PAR is by far no comparable solution. PAR makes perl scripts
> slower not faster.

I believe that was me, several years ago:
http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2006-07/msg00684.html.
It seems the code was removed from core but still depended on
non-public APIs. I think probably it's almost impossible to write
anything in the B:: namespace without depending on non-public APIs of
some sort.

Josh

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