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

[Marvin Scholz]

On 17 Dec 2019, at 13:00, Steve Lhomme wrote:

> On 2019-12-17 12:10, Marvin Scholz wrote:
>>
>>
>> On 17 Dec 2019, at 8:36, Steve Lhomme wrote:
>>
>>> So that Meson doesn't try to pick them.
>>>
>>> When cross-compiling Meson uses the environment variables to test 
>>> the native
>>> compiler (even if it will never be used). Passing clang options 
>>> meant to
>>> produce PDB files will be rejected by the host gcc compiler and 
>>> Meson will
>>> refuse to compile (with the cross compiler).
>>>
>>> Similar to 5c76f3478cb038a550de21f6267d57c55c963391 on macos.
>>> ---
>>>  extras/package/win32/build.sh | 37 
>>> ++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 36 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/extras/package/win32/build.sh 
>>> b/extras/package/win32/build.sh
>>> index 9607fcfe058..6b19f0b629e 100755
>>> --- a/extras/package/win32/build.sh
>>> +++ b/extras/package/win32/build.sh
>>> @@ -106,6 +106,34 @@ else
>>>      COMPILING_WITH_CLANG=0
>>>  fi
>>>
>>> +# Write config.mak for contribs
>>> +# Globals:
>>> +#   VLC_EXTRA_CFLAGS
>>> +#   VLC_EXTRA_CXXFLAGS
>>> +#   VLC_EXTRA_LDFLAGS
>>> +write_config_mak()
>>> +{
>>> +    # Flags to be used for C-like compilers (C, C++, Obj-C)
>>> +    local clike_flags="$VLC_EXTRA_CFLAGS"
>>> +
>>> +    # local vlc_cppflags="-arch $ARCH"
>> Just remove this?
>>
>>> +    local vlc_cflags="$clike_flags"
>>> +    local vlc_cxxflags="$VLC_EXTRA_CXXFLAGS $clike_flags"
>>> +    local vlc_objcflags="$clike_flags"
>> This can be removed.
>>
>>> +
>>> +    local vlc_ldflags="$VLC_EXTRA_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' "CFLAGS" "${vlc_cflags}" >&3
>>> +    printf '%s := %s\n' "CXXFLAGS" "${vlc_cxxflags}" >&3
>>> +    printf '%s := %s\n' "OBJCFLAGS" "${vlc_objcflags}" >&3
>> This can be removed as well.
>>
>>> +    printf '%s := %s\n' "LDFLAGS" "${vlc_ldflags}" >&3
>>> +}
>>> +
>>
>> You probably dont want to set the clfags to cxxflags either, as the 
>> reason this
>> was done in the iOS script is that the variable I used there only 
>> contains
>> flags that I know work for C, CXX and OBJC and its provided by the 
>> script, no
>> external means.
>> So the whole thing can be greatly simplified to this:
>>
>> write_config_mak()
>> {
>>      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' "CFLAGS" "${VLC_EXTRA_CFLAGS}" >&3
>>      printf '%s := %s\n' "CXXFLAGS" "${VLC_EXTRA_CXXFLAGS}" >&3
>>      printf '%s := %s\n' "LDFLAGS" "${VLC_EXTRA_LDFLAGS}" >&3
>> }
>
> By the way, does it make sense to write empty values in config.mak if 
> the environment contains no VLC_EXTRA_CFLAGS ? There might be some 
> CFLAGS in the environment that will go through with Meson. So there's 
> no reason to force people to use VLC_EXTRA_xxx if they don't have to.

You might want to add ${CFLAGS} there, yeah.

>
>>>  info "Building extra tools"
>>>  mkdir -p extras/tools
>>>  cd extras/tools
>>> @@ -147,6 +175,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
>>> +write_config_mak
>>> +
>>>  ${SCRIPT_PATH}/../../../contrib/bootstrap --host=$TRIPLET 
>>> $CONTRIBFLAGS
>>>
>>>  # Rebuild the contribs or use the prebuilt ones
>>> @@ -190,7 +223,9 @@ if [ ! -z "$WITH_PDB" ]; then
>>>      CONFIGFLAGS="$CONFIGFLAGS --enable-pdb"
>>>  fi
>>>
>>> -${SCRIPT_PATH}/configure.sh --host=$TRIPLET 
>>> --with-contrib=../contrib/$TRIPLET $CONFIGFLAGS
>>> +${SCRIPT_PATH}/configure.sh --host=$TRIPLET 
>>> --with-contrib=../contrib/$TRIPLET $CONFIGFLAGS \
>>> +    CFLAGS="$VLC_EXTRA_CFLAGS  
>>> -Werror=incompatible-pointer-types 
>>> -Werror=missing-field-initializers" \
>>> +    CXXFLAGS="$VLC_EXTRA_CXXFLAGS  
>>> -Werror=incompatible-pointer-types 
>>> -Werror=missing-field-initializers"
>>>
>>
>> This does not behave like it should.
>> For contribs you set CXXFLAGS to VLC_EXTRA_CXXFLAGS and 
>> VLC_EXTRA_CFLAGS, while here
>> you only set it to VLC_EXTRA_CXXFLAGS.
>> And VLC_EXTRA_LDFLAGS are not set at all here.
>>
>> Please either consistently do the one thing or the other, but dont 
>> differ from what
>> you do for contribs vs. configure, as thats really quite confusing.
>
> OK
>
>> So you probably just should do:
>>
>>    vlc_error_flags="-Werror=incompatible-pointer-types 
>> -Werror=missing-field-initializers"
>>
>>    CFLAGS="$VLC_EXTRA_CFLAGS $vlc_error_flags"
>>    CXXFLAGS="$VLC_EXTRA_CXXFLAGS $vlc_error_flags"
>>    LDFLAGS="$VLC_EXTRA_LDFLAGS"
>
> Yes There's still the question of not using the env CFLAGS if they are 
> set. In my case I will not set them but other uses may use some, 
> including forcing defines to build for a specific Windows target like 
> -D_WIN32_WINNT=0x0602.
>
> I'll try to add it in addition to VLC_EXTRA_xxx
>
>>>  info "Compiling"
>>>  make -j$JOBS
>>> -- 
>>> 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