Re: [Libreoffice-commits] .: 2 commits - configure.in

On Tuesday 17 of April 2012, Tomáš Chvátal wrote:
> Dne 17.4.2012 10:11, Lubos Lunak napsal(a):
> > On Tuesday 17 of April 2012, TomXXXX ChvXXtal wrote:
> > >      --enable-ext-languagetool requires --with-java now bit more sanely.
...
> > > AC_MSG_CHECKING([for LanguageTool extension integration])
> > > -if test "x$enable_ext_languagetool" = "xyes" -a
> > > "x$enable_extension_integration" != "xno"; then +if test
> > > "x$enable_ext_languagetool" = "xyes" -a "x$enable_extension_integration"
> > > != "xno" -a "x$with_java" != "xno"; then BUILD_TYPE="$BUILD_TYPE
> > > LANGUAGETOOL" SCPDEFS="$SCPDEFS -DWITH_EXTENSION_LANGUAGETOOL"
> > >       AC_MSG_RESULT([yes])
> >
> >   How is this sane? Somebody passes --enable-ext-languagetool and
> > configure will simply ignore it (even without giving a warning, not that
> > it would change that much)?
>
> It prints NO on the line so you can see it. But the extensions are 
> enabled by default so you DIE on the configure because user decided to
> disable java.
>
> So if you run:
> ./configure --disable-java --enable-ext-languagetool it prints
> Checking whether to build languagetool... no

 That's really what I'm going to look for if I explicitly passed YES. 
Especially given that you have also removed the part from --help explaining 
it.

> or:
> ./configure --disable-java
> Checking whether to build languagetool... no

 This is acceptable, but not the case above. If what you wanted to fix was 
just the default, then please fix just the default.

> Same as most of the other modules does.

./configure --enable-dbgutil --with-system-cppunit
...
configure: error: --with-system-cppunit conflicts with DBG_UTIL build

> The commit from Stephan simply stopped your build, and frankly even if
> you have messy configure options that conflict out the worst it should
> do is some warning and then disable one of the conflicting features.

 No, why? If somebody explicitly asks for it, they want it, not some (not so) 
clever second-guessing disabling it (almost) silently.

> Also check the extension enabler in configure.in that is directly above
> this one, it has same condition as I now added.

 So it is broken too.

-- 
 Lubos Lunak
 l.lunak@suse.cz