[vlc-devel] [PATCH] contrib: don't use standard cmake variable name CMAKE_GENERATOR

[Zhao Zhili]

Hi Steve,

The commit message is not clear enough. There are two issues the patch try to resolve:

First, the conflict setting/using of CMAKE_GENERATOR. If user set CMAKE_GENERATOR as Ninja, cmake $CMAKE_GENERATOR is unexpected.

Second, since we always call make after cmake configure, generator other than makefile doesn't work. So we have to reset/ignore the CMAKE_GENERATOR environment variable.

I tried to make generator other than makefile work by run build via cmake wrapper, like
$(cmake) --build . --target install.

Since we put too much thing in the $(cmake) variable, it doesn't work. I think it could be fixed by add $(cmake_prog) as plain cmake wrapper.

CMAKE_GENERATOR is being reset to empty on most platform, because there are multiple makefile variant, I'm not sure does "Unix Makefiles" works on every platform.

Some explanations was inlined.

> On Nov 2, 2020, at 10:45 PM, Steve Lhomme <robux4 at ycbcr.xyz> wrote:
> 
> Hi,
> 
> On 2020-11-02 15:18, Zhao Zhili wrote:
>> Fix build error when user have CMAKE_GENERATOR as environment variable,
>> like CMAKE_GENERATOR=Ninja.
>> ---
>>  contrib/bootstrap          | 2 +-
>>  contrib/src/main.mak       | 5 +++--
>>  contrib/src/soxr/rules.mak | 2 +-
>>  3 files changed, 5 insertions(+), 4 deletions(-)
>> diff --git a/contrib/bootstrap b/contrib/bootstrap
>> index 5ed5f6f25d..4da86af994 100755
>> --- a/contrib/bootstrap
>> +++ b/contrib/bootstrap
>> @@ -256,7 +256,7 @@ test -z "$GNUV3" || add_make_enabled "GNUV3"
>>  test -z "$AD_CLAUSES" || add_make_enabled "AD_CLAUSES"
>>  test -z "$WITH_OPTIMIZATION" || add_make_enabled "WITH_OPTIMIZATION"
>>  test -z "$ENABLE_PDB" || add_make_enabled "ENABLE_PDB"
>> -test "`uname -o 2>/dev/null`" != "Msys" || add_make "CMAKE_GENERATOR := -G \"MSYS Makefiles\""
>> +test "`uname -o 2>/dev/null`" != "Msys" || add_make "CONTRIB_CMAKE_GENERATOR := MSYS Makefiles"
> 
> We only set this value when building for Msys. Is Ninja expected to work in that case ? Or whatever else is set in the environment ?
> 
>>  #
>>  # Checks
>> diff --git a/contrib/src/main.mak b/contrib/src/main.mak
>> index aa273ef89c..7b395cd13f 100644
>> --- a/contrib/src/main.mak
>> +++ b/contrib/src/main.mak
>> @@ -392,8 +392,9 @@ AUTORECONF = autoreconf
>>  endif
>>  RECONF = mkdir -p -- $(PREFIX)/share/aclocal && \
>>  	cd $< && $(AUTORECONF) -fiv $(ACLOCAL_AMFLAGS)
>> -CMAKE = cmake . -DCMAKE_TOOLCHAIN_FILE=$(abspath toolchain.cmake) \
>> -		-DCMAKE_INSTALL_PREFIX=$(PREFIX) $(CMAKE_GENERATOR) \
>> +CMAKE = CMAKE_GENERATOR=$(CONTRIB_CMAKE_GENERATOR) cmake . \
> 
> Here you force the value to CONTRIB_CMAKE_GENERATOR which 99.9% of the time is never set, effectively overriding the environment preferred value ?

It is intended to reset CMAKE_GENERATOR.

> 
>> +		-DCMAKE_TOOLCHAIN_FILE=$(abspath toolchain.cmake) \
>> +		-DCMAKE_INSTALL_PREFIX=$(PREFIX) \
>>  		-DBUILD_SHARED_LIBS:BOOL=OFF
>>  ifdef HAVE_WIN32
>>  CMAKE += -DCMAKE_DEBUG_POSTFIX:STRING=
>> diff --git a/contrib/src/soxr/rules.mak b/contrib/src/soxr/rules.mak
>> index 3981271743..c68b1e40f0 100644
>> --- a/contrib/src/soxr/rules.mak
>> +++ b/contrib/src/soxr/rules.mak
>> @@ -41,6 +41,6 @@ endif
>>  		-DWITH_LSR_BINDINGS=OFF \
>>  		-DWITH_OPENMP=OFF \
>>  		-DWITH_AVFFT=ON \
>> -		-Wno-dev $(CMAKE_GENERATOR)
>> +		-Wno-dev
> 
> And here you don't use the generator at all, for example the Msys one we wanted.

cmake is called via $(CMAKE), so it got the generator setting.

> 
>>  	cd $< && $(MAKE) install
>>  	touch $@
>> -- 
>> 2.28.0
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel <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 <https://mailman.videolan.org/listinfo/vlc-devel>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20201102/afcdad2d/attachment.html>