[vlc-devel] [PATCH] package/win32: use VLC_DEPLOYMENT_TARGET_CFLAGS environment variable to pass CFLAGS

[Marvin Scholz]

On 5 Nov 2019, at 16:22, Steve Lhomme wrote:

> On 2019-11-05 16:16, Marvin Scholz wrote:
>> On 5 Nov 2019, at 15:58, Steve Lhomme wrote:
>>
>>> So that Meson doesn't try to pick them.
>>>
>>> Similar to 5c76f3478cb038a550de21f6267d57c55c963391 on macos.
>>> ---
>>>   extras/package/win32/build.sh | 42 
>>> +++++++++++++++++++++++++++++++++++
>>>   1 file changed, 42 insertions(+)
>>>
>>> diff --git a/extras/package/win32/build.sh 
>>> b/extras/package/win32/build.sh
>>> index 9607fcfe058..069ffef01f8 100755
>>> --- a/extras/package/win32/build.sh
>>> +++ b/extras/package/win32/build.sh
>>> @@ -106,6 +106,43 @@ else
>>>       COMPILING_WITH_CLANG=0
>>>   fi
>>>
>>> +# Write config.mak for contribs
>>> +# Globals:
>>> +#   VLC_DEPLOYMENT_TARGET_CFLAGS
>>> +#   VLC_DEPLOYMENT_TARGET_CXXFLAGS
>>> +#   VLC_DEPLOYMENT_TARGET_LDFLAGS
>>
>> Nowhere in the script are any of these ever set, as
>> far as I can tell.
>
> They are coming from the enviroment (see title), so they don't collide 
> with the CFLAGS that Meson will try to use (incorrectly).

Ah I understand the intention now.

>
>>> +write_config_mak()
>>> +{
>>> +    # Flags to be used for C-like compilers (C, C++, Obj-C)
>>> +    local clike_flags="$VLC_DEPLOYMENT_TARGET_CFLAGS"
>>> +
>>
>> The local keyword is not supported by plain sh, only bash and
>> similar shells, but this script is has #!/bin/sh
>
> OK, I don't really need this step anyway.
>
>>> +    # local vlc_cppflags="-arch $ARCH"
>>
>> This can be removed, I guess?
>
> Probably.
>
>>> +    local vlc_cflags="$clike_flags"
>>> +    local vlc_cxxflags="$VLC_DEPLOYMENT_TARGET_CXXFLAGS 
>>> $clike_flags"
>>> +    local vlc_objcflags="$clike_flags"
>>
>> This is probably unnecessary and can be removed?
>> I doubt we will ever add Objective-C code for Windows.
>>
>>> +
>>> +    local vlc_ldflags="$VLC_DEPLOYMENT_TARGET_LDFLAGS"
>>> +
>>> +    echo "Creating makefile..."
>>> +    test -e config.mak && unlink config.mak
>>> +    exec 3>config.mak || return $?
>>> +
>>> +    printf '# This file was automatically generated!\n\n' >&3
>>> +    # printf '%s := %s\n' "CPPFLAGS" "${vlc_cppflags}" >&3
>>
>> Why keep this commented out if its unused?
>
> In case it needs to be brought back.

IMO it would be clearer to just remove them and re-add when needed.
Or at least add a comment about that so that it is clear why
those are commented-out.

>
>>> +    printf '%s := %s\n' "CFLAGS" "${vlc_cflags}" >&3
>>> +    printf '%s := %s\n' "CXXFLAGS" "${vlc_cxxflags}" >&3
>>> +    printf '%s := %s\n' "OBJCFLAGS" "${vlc_objcflags}" >&3
>>> +    printf '%s := %s\n' "LDFLAGS" "${vlc_ldflags}" >&3
>>> +    # printf '%s := %s\n' "CC" "clang" >&3
>>> +    # printf '%s := %s\n' "CPP" "clang -E" >&3
>>> +    # printf '%s := %s\n' "CXX" "clang++" >&3
>>> +    # printf '%s := %s\n' "OBJC" "clang" >&3
>>> +    # printf '%s := %s\n' "LD" "ld" >&3
>>> +    # printf '%s := %s\n' "AR" "ar" >&3
>>> +    # printf '%s := %s\n' "STRIP" "strip" >&3
>>> +    # printf '%s := %s\n' "RANLIB" "ranlib" >&3
>>
>> Same as above remark.
>>
>>> +}
>>> +
>>>   info "Building extra tools"
>>>   mkdir -p extras/tools
>>>   cd extras/tools
>>> @@ -147,6 +184,11 @@ fi
>>>   if [ "$RELEASE" != "yes" ]; then
>>>        CONTRIBFLAGS="$CONTRIBFLAGS --disable-optim"
>>>   fi
>>> +
>>> +# Write config.mak with flags for the build and compiler overrides
>>> +# Set flag to error on partial availability
>>
>> This comment does not match the following Werror type.
>>
>>> +write_config_mak "-Werror=incompatible-pointer-types"
>>
>> Nowhere in write_config_mak it seems $1 is used, so
>> the argument is completely ignored?
>
> You're right I cleaned a bit too much.
>
>>> +
>>>   ${SCRIPT_PATH}/../../../contrib/bootstrap --host=$TRIPLET 
>>> $CONTRIBFLAGS
>>>
>>>   # Rebuild the contribs or use the prebuilt ones
>>> -- 
>>> 2.17.1
>>>
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel