On Tue, Sep 22, 2009 at 09:40:34AM +0200, Sumit Bose wrote:
> On Tue, Sep 22, 2009 at 09:32:22AM +0200, Sumit Bose wrote:
>> On Mon, Sep 21, 2009 at 04:47:14PM -0400, Stephen Gallagher wrote:
>>> On 09/21/2009 04:35 PM, Stephen Gallagher wrote:
>>>> Older python versions (such as that used in RHEL5) do not have a
>>>> python-config executable to report CFLAGS and LIBS. In order to
>>>> support such versions of python, we will duplicate the logic that
>>>> python-config would have performed directly in our configure
>>>> script
>>>>
>>>>
>>>> The original patch that enabled the python support broke the RHEL5
>>>> builds. This patch will fix it. Please review promptly.
>>>>
>>>
>>> It would be easier to review promptly if I'd actually attached the
patch...
>>
>> yes, it was easier this way :-)
>>
>> ACK
>>
>> Btw. I have seen a couple for compiler warnings not related to this
>> patch but to the python bindings in general.
>>
>
> ah, sorry, I have forgotten a minor comment. Please consider adding a
> comment to python.m4 why python-config is not used here. Because in a
> couple of month someone who does not read the commit logs carefully
> might be tempted to 'optimize' this section with the help of
> python-config.
>
after a discussion with Jakub it turns out that some of the warnings I
see are related to this patch, so I revoke my ACK and say
NACK
Please change sysconfig.get_config_var('CFLAGS') to
sysconfig.get_config_var('BASECFLAGS') because it fits much better to
the original python-config --includes. CFLAGS corresponds to
python-config --cflags which includes all the compiler options used to
compile python.
bye,
Sumit
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
New patch attached.
I have added the requested comment and changed to using BASECFLAGS
instead of CFLAGS.
--
Stephen Gallagher
RHCE 804006346421761
Looking to carve out IT costs?