[FFmpeg-devel] [PATCH] build: check for perl presence and compile POD pages only in that case

[Clément Bœsch]

On Fri, Aug 10, 2012 at 12:00:30AM +0200, Stefano Sabatini wrote:
> On date Thursday 2012-08-09 23:48:09 +0200, Clément Bœsch encoded:
> > On Thu, Aug 09, 2012 at 11:39:44PM +0200, Stefano Sabatini wrote:
> > > perl is required by the texi2pod.pl script. Should avoid FATE failure on
> > > Haiku, where perl is apparently not available.
> > > ---
> > >  configure    |    5 ++++-
> > >  doc/Makefile |   10 ++++++++--
> > >  2 files changed, 12 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/configure b/configure
> > > index 6c4047f..6dc983f 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -1285,6 +1285,7 @@ HAVE_LIST="
> > >      mmap
> > >      nanosleep
> > >      PeekNamedPipe
> > > +    perl
> > >      pod2man
> > >      poll_h
> > >      posix_memalign
> > > @@ -1838,7 +1839,7 @@ ffprobe_deps="avcodec avformat"
> > >  ffserver_deps="avformat ffm_muxer fork rtp_protocol rtsp_demuxer"
> > >  ffserver_extralibs='$ldl'
> > >  
> > > -doc_deps_any="texi2html makeinfo pod2man"
> > > +doc_deps_any="texi2html makeinfo perl pod2man"
> > >  
> > >  # tests
> > >  colormatrix1_test_deps="colormatrix_filter"
> > > @@ -3515,6 +3516,7 @@ enabled sdl && add_cflags $sdl_cflags && add_extralibs $sdl_libs
> > >  
> > >  texi2html --help 2> /dev/null | grep -q 'init-file' && enable texi2html || disable texi2html
> > >  makeinfo --version > /dev/null 2>&1 && enable makeinfo  || disable makeinfo
> > > +perl --version > /dev/null 2>&1 && enable perl || disable perl
> > >  pod2man --help > /dev/null 2>&1 && enable pod2man || disable pod2man
> > >  
> > >  check_header linux/fb.h
> > > @@ -3857,6 +3859,7 @@ echo "openssl enabled           ${openssl-no}"
> > >  echo "zlib enabled              ${zlib-no}"
> > >  echo "bzlib enabled             ${bzlib-no}"
> > >  echo "texi2html enabled         ${texi2html-no}"
> > > +echo "perl enabled              ${perl-no}"
> > >  echo "pod2man enabled           ${pod2man-no}"
> > >  echo "makeinfo enabled          ${makeinfo-no}"
> > >  test -n "$random_seed" &&
> > > diff --git a/doc/Makefile b/doc/Makefile
> > > index 845f5f7..16c13fc 100644
> > > --- a/doc/Makefile
> > > +++ b/doc/Makefile
> > > @@ -11,9 +11,15 @@ HTMLPAGES   = $(PROGS-yes:%=doc/%.html)                                 \
> > >  
> > >  TXTPAGES    = doc/fate.txt                                              \
> > >  
> > > +ifdef HAVE_PERL
> > > +ifdef HAVE_POD2MAN
> > > +HAVE_MANPAGES=yes
> > > +endif
> > > +endif
> > >  
> > 
> > Can't this be nicer?
> 
> Sure.
>  
> > Certainly something like adding "manpages" in the CONFIG_LIST, and add a
> > manpages_deps or something like that. Which might BTW add some automatic
> > magic such as --disable-manpages configure switch or something.
> > 
> > I'm not familiar with the build system but what you propose looks wrong.
> 
> I'm just following the current logic, with no attempt at improving it,
> and it fixes the dependency problem from what I see.
> 

The problem I see here is that afaict HAVE_* switches are for system caps;
a HAVE_MANPAGES makes imo no sense ("does your system have manpages or
no?"). IMO you should have manpages generated depending various builtin
(and additionally user configuration): "do you have everything for
generating manpages? great let's generate manpage, assuming you want them
[aka no --disable-manpages]".

> If you add --enable-manpages then you'll have to add enable-htmlpages
> and possibly --enable-podpages and review the current logic, which I'm
> not willing to do.

That manpage switch wouldn't need to be "documented" (aka in the
configure's help) in the first place; it just looks like to be a more
correct way to deal with the dependency. But this is based on a real quick
look, so I may be wrong.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120810/8857cc96/attachment.asc>