[vlc-devel] [PATCH 4/4] contrib: move macOS compilation flags to the script where they belong and generalize Darwin OS environment manipulations

[Felix Paul Kühne]

Hi David,

Thanks for your feedback!

> On 25. Jan 2018, at 19:07, David Fuhrmann <david.fuhrmann at gmail.com> wrote:
> 
> Hi Felix,
> 
>> Am 24.01.2018 um 23:57 schrieb Felix Paul Kühne <fkuehne at videolan.org>:
>> 
>> From: Felix Paul Kühne <felix at feepk.net>
>> 
>> ---
>> contrib/src/main.mak           | 23 ++++++++++-------------
>> extras/package/macosx/build.sh | 14 +++++++++++---
>> 2 files changed, 21 insertions(+), 16 deletions(-)
>> 
>> diff --git a/contrib/src/main.mak b/contrib/src/main.mak
>> index 0398a5ad1a..49b4627110 100644
>> --- a/contrib/src/main.mak
>> +++ b/contrib/src/main.mak
>> @@ -112,11 +112,16 @@ CXX := clang++
>> endif
>> endif
>> 
>> +ifdef HAVE_DARWIN_OS
>> +CC=xcrun clang
>> +CXX=xcrun clang++
>> +AR=xcrun ar
>> +LD=xcrun ld
>> +STRIP=xcrun strip
>> +RANLIB=xcrun ranlib
>> +endif
> 
> Please do not add those lines, I specifically removed this for macOS a couple of months ago.

Great, even better!

> 
>> ifdef HAVE_MACOSX
>> -MIN_OSX_VERSION=10.7
>> -EXTRA_CFLAGS += -isysroot $(MACOSX_SDK) -mmacosx-version-min=$(MIN_OSX_VERSION) -DMACOSX_DEPLOYMENT_TARGET=$(MIN_OSX_VERSION)
> 
> Would the compilation succeed if you remove this and do not compile with build.sh? If yes, then this looks fine.

Yes, but the resulting binary will probably only work on the OS version you compiled on.

> 
>> -EXTRA_CXXFLAGS += -stdlib=libc++
> 
> I am not sure about that one, AFAIK we do not really support linking with libstdc++ anymore. Seeing how many problems we already had with mixed compilations, I would prefer if we can keep this in here, hardcoded (analogous to configure.ac).

Sure, but then, for all Darwin OS flavors.

> 
>> -ifdef HAVE_IOS
>> +ifdef HAVE_DARWIN_OS
>> AUTORECONF = AUTOPOINT=true autoreconf
> 
> Is this really needed? So far I did not see any problems without this option.

Yes, this is needed, if you compile contribs without gettext.

Best regards,

Felix