[macports-ports] branch master updated: py-protobuf3: fix build on older systems

[Ryan Schmidt]

On May 25, 2018, at 11:04, Ken Cunningham wrote:

> On 2018-05-25, at 8:38 AM, Ryan Schmidt wrote:
> 
>> If this is supposed to help 10.6...
>> 
>> 
>>> +@@ -197,6 +197,8 @@
>>> +       v = float('.'.join(v.split('.')[:2]))
>>> +       if v >= 10.12:
>>> +         extra_compile_args.append('-std=c++11')
>>> ++        extra_compile_args.append('@@MACPORTS_STDLIB@@')
>>> ++        extra_compile_args.append('@@MACPORTS_EXTRAARG@@')
>> 
>> ...why is it inside an if statement that's for 10.12 and later?
> 
> 
> Yeah, their logic for the test must be busted.  My guess is that testing for 10.12 passes everything 10.1 and above,

Right.

> but I defer to someone who can read python. Can you fix that v.float () business for them?

I've never looked at this code before; I won't presume to know what the correct fix would be.


>> I guess it works because the if statement is erroneously treating the version number as a floating point number, in which case 10.6 is considered to be greater than 10.12.
>> 
>> So the question is: does this code always require C++11?
> 
> Probably. protobuf3-cpp seems to, and this port errors otherwise with the errors noted in the ticket.

Of course it errored: it was told to use C++11, while using a C++ standard library that didn't support C++11.

The question is: should the build have been told to use C++11 or not? I don't know this code so I can't answer that question.

I've asked the developers:

https://github.com/google/protobuf/issues/4683



>> If so, remove the if statement.
>> 
>> If it does not always require C++11, fix the if statement so that it correctly uses C++11 only on 10.12 and later.
> 
> I don't grok python very deeply, I"m afraid.
> 
> What I did fixes it for us -- but I agree there is something busted in that build.
> 
> I can revert the fixes if you like.

I guess you don't need to revert, but what we have now is C++11 being used on 10.12 and later, and on 10.9 and earlier, but not on 10.10 or 10.11, which is bonkers.