[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: perl ithreads support: why hardcode at configure time ?
From: |
Stefano Lattarini |
Subject: |
Re: perl ithreads support: why hardcode at configure time ? |
Date: |
Fri, 11 Jan 2013 18:21:24 +0100 |
On 01/11/2013 06:11 PM, Mike Frysinger wrote:
> On Friday 11 January 2013 04:08:26 Stefano Lattarini wrote:
>> On 01/11/2013 05:07 AM, Mike Frysinger wrote:
>>> i can't imagine this is a big runtime penalty, so why does configure
>>> check for the perl's thread settings and then hardcode that in the
>>> generated automake ?
>>
>> I don't know, I wasn't around when the change was introduced. You'll have
>> to dig out the archives; if we don't find any compelling reason for having
>> the check at configure time rather than runtime, I agree your change is a
>> nice simplification ...
>
> i couldn't find references to ithreads in the automake or automake-patches
> archives
>
I only found an explanation of why "require ...; import ..." was used instead
of a simpler "use ...", but as for you, I was unable to find out a rationale
for having threading support checked at configure time. So I say we apply
your proposed change, and reconsider it only if something breaks or someone
complains.
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -77,32 +77,6 @@ installed, select the one Automake should use using
>>> ./configure PERL=/path/to/perl])
>>> }
>>> -# We require ithreads support, and version 5.7.2 for CLONE.
>>
>> Here, the comments say (and the code agree) that we should support
>> ithreads only from perl 5.7.2 onwards ...
>
> i thought that meant something else. oh well.
>
>>> +use Config;
>>> +our $perl_threads = $Config{useithreads};
>>
>> ... but there is no such check here. And yes, so far we still support
>> perl 5.6 so far (albeit we might want to start requiring 5.8 in Automake
>> 1.14, but that's for another thread).
>
> i'm not a perl hacker, so i don't know how to do that. but a grep shows that
> automake is already doing that sort of thing in the tests,
>
Oh, good point, I missed that.
> so i just copied that:
>
> +our $perl_threads = 0;
> +if (eval { require 5.007_002; }) # for CLONE support
> + {
> + use Config;
> + $perl_threads = $Config{useithreads};
> + }
>
Seems the sane thing to do, yes.
>> Also, might I ask you to format your patches with "git am"? That will make
>> it super-easy for me to apply them.
>
> i'm familiar with git. i expected more push back on this, so i didn't write
> a
> proper commit as this was a RFC with a PoC patch inline. no point in wasting
> time doing a write up (or the god awful time waste known as the GNU ChangeLog)
> if the patch is going to get rejected.
>
Sensible approach. But I'm not going to reject the patch ;-) Unless
it causes some failure of course, but it appears by now that that will
be unlikely.
Thanks,
Stefano