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

[Alexandre Janniaux]

Hi,

You can ignore, the answer from Zhao Zhili is clearer and easier
to read. :)

Regards,
--
Alexandre Janniaux
Videolabs

On Mon, Nov 02, 2020 at 05:22:35PM +0100, Alexandre Janniaux wrote:
> Hi,
>
> On Mon, Nov 02, 2020 at 03:45:39PM +0100, Steve Lhomme 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 ?
>
> We cannot use ninja generator if the contrib rules are expecting
> to build with make, or the reverse.
>
> > >   #
> > >   # 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 ?
>
> As mentioned above, the actual user is not the final user but
> the contrib rules, so environment overriding doesn't really
> make sense here except if the contrib rules are ready to deal
> with it. I guess contrib's rules expecting a different generator
> can directly suggest it to override the environment one?
>
> > > +		-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.
>
> It would be used by the call to $(CMAKE), wouldn't it?
>
> Regards,
> --
> Alexandre Janniaux
> Videolabs
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel