ctdb/wscript: Avoid installing pre-built man pages which does not exist during make

[Martin Schwenke]

On Tue, 10 Nov 2015 07:49:15 +0100, Michael Adam <obnox at samba.org>
wrote:

> On 2015-11-10 at 17:39 +1100, Martin Schwenke wrote:
> > On Tue, 10 Nov 2015 07:33:26 +0100, Michael Adam <obnox at samba.org>
> > wrote:
> >   
> > > On 2015-11-10 at 15:21 +1100, Martin Schwenke wrote:  
>  [...]  
>  [...]  
>  [...]  
> > >  [...]    
>  [...]  
>  [...]  
> > > 
> > > Apparently we had not thought about the tarball
> > > possibly containing pre-built manpages. :-)
> > > We just wanted to fix the non-tarball build.
> > >   
>  [...]  
> > > 
> > > No offense meant. Your protest is completely justified.
> > >   
>  [...]  
> > > 
> > > Your patch is definitely the right way to solve it.
> > > Just one comment -- see below.
> > >   
>  [...]  
> > > 
> > > 
> > > I think it is too subtle and a little misleading to
> > > have the xml-generation of the manpages conditionalized
> > > by 'bld.env.ctdb_prebuilt_manpages is None'.
> > > 
> > > It is much more obvious to keep the line
> > > 
> > > "if 'XSLTPROC_MANPAGES' in bld.env and bld.env['XSLTPROC_MANPAGES']:"  
> > 
> > That's a matter of opinion, but I can do that...  :-)  
> 
> Taste, even.
> 
> > I just though that setting a clear condition in configure and then
> > using it in build would be cleanest.  
> 
> 100% agreed, but if that condition says 'there are no pre-built
> manpages' insted of 'we want to (and can) do xml processing',
> then this is what I find confusing. Especially when the
> creation of this condition is up in configure.

Sure, I don't mind vague implicit stuff... but annoys other
people...  :-)

> > > You could then even put the loop for finding of the pre-installed
> > > manpages into the else branch of that condition.  
> > 
> > That's what I tried first.  Then when you run "make clean" it prints
> > messages about whether it could find the pre-built manpages.  Cleaner
> > to put it in configure so it doesn't get run in unexpected contexts...  
> 
> Ah, that's a good argument. Sold.
> 
> what about s/th like this:
> 
> +    if 'XSLTPROC_MANPAGES' in conf.env and conf.env['XSLTPROC_MANPAGES']:
> +        conf.env.generate_manpages = True
> +    else:
> +        conf.env.generate_manpages = False
> +
> +        # --> don't even need to have this in else <--
> +        Logs.info("xsltproc unavailable, checking for pre-built manpages")
> +        conf.env.ctdb_prebuilt_manpages = []
> +        for m in manpages:
> +            if os.path.exists(os.path.join("doc", m)):
> +                Logs.info("  %s: yes" % (m))
> +                conf.env.ctdb_prebuilt_manpages.append(m)
> +            else:
> +                Logs.info("  %s: no" % (m))
>  
>  def build(bld):
>      if bld.env.standalone_ctdb:
> @@ -493,12 +504,12 @@ def build(bld):
>                              target=x,
>                              rule='sed %s ${SRC} > ${TGT}' % (sed_cmdline))
>  
> -    if 'XSLTPROC_MANPAGES' in bld.env and bld.env['XSLTPROC_MANPAGES']:
> +    if bld.env.generate_manpages:
>          bld.MANPAGES('''onnode.1 ctdbd_wrapper.1 ctdbd.conf.5
>                          ctdb.7 ctdb-statistics.7 ctdb-tunables.7''',
>                        True)
>      else:
> -        for m in manpages:
> +        for m in bld.env.ctdb_prebuilt_manpages:
>              bld.SAMBA_GENERATOR(m,
>                                  source=os.path.join("doc", m),
>                                  target=m,  

Sold!  Want to push it?  :-)

peace & happiness,
martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 173 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20151110/acd0e8d0/attachment.sig>