Re: [LTP] [PATCH 1/1] configure.ac: Fix summary for disabled metadata

[Petr Vorel <pvorel@suse.cz>]

Hi Cyril,

> Hi!
> > Previously with --disable-metadata output didn't mention that metadata
> > are disabled and printed config which was not used. Now:

> > $ ./configure --disable-metadata
> > ...
> > METADATA
> > metadata disabled

> > $ ./configure
> > ...
> > METADATA
> > metadata generator: asciidoctor
> > HTML metadata: yes
> > PDF metadata: no

> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> >  configure.ac | 7 +++++++
> >  1 file changed, 7 insertions(+)

> > diff --git a/configure.ac b/configure.ac
> > index 3c56d19224..5b9e3c1781 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -404,7 +404,14 @@ libtirpc: ${have_libtirpc:-no}
> >  glibc SUN-RPC: ${have_rpc_glibc:-no}

> >  METADATA
> > +EOF
> > +
> > +if test "x$enable_metadata" = xyes; then
> > +cat << EOF
> >  metadata generator: $with_metadata_generator
> >  HTML metadata: $with_metadata_html
> >  PDF metadata: $with_metadata_pdf

> Don't we stil have the same problem with "$enable_metadata_html" and
> "$enable_metadata_pdf" ?
Well, this patch was about configure output. I wouldn't print them at all if
metadata disabled. They're not used anyway in the code if metadata disabled.

> Also looking at m4/ltp-docparse.m4 shouldn't we just skip the
> autodetection if metadata are disabled and exit with all three wariables
> set to no?
Well, but that's how it is implemented:
$ ./configure --disable-metadata
...
configure: metadata generation disabled

And variables in config.log are:
METADATA_GENERATOR=''
WITH_METADATA='no'
WITH_METADATA_HTML='no'
WITH_METADATA_PDF='no'

It's a bit hard to write m4 macros when there is no return :).
It's hard to not fallback into many if/else levels.
I should have put some comments into the code.

Also, currently it's implemented that that we can disable both HTML and
PDF as: --disable-metadata-html --disable-metadata-pdf, maybe we could rethink
it.

> I think that we should rethink what the flags really do, I guess that
> for instance it would make sense for the $enable_metadata=no to just set
> both $enable_metadata_html and $enable_metadata_pdf to no and the rest
> of the m4/ltp-docparse.m4 should just check the later two.
Rethink the flags meaning or a functionality?

And how about flags names? What name would you suggest?
--disable-doc, --with-html-doc, --with-pdf-doc ?
Because we now have metadata (i.e. JSON output) mandatory.

I wonder what is needed to be fixed now and what's better to postpone after the
release?

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp