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

[Steve Lhomme]

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.

>>  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