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

[Martin Schwenke]

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:
> > On Mon, 9 Nov 2015 15:52:27 +0100, Michael Adam <obnox at samba.org> wrote:
> >   
> > > On 2015-11-09 at 18:13 +0530, Anoop C S wrote:  
> >   
>  [...]  
> > > 
> > > Reviewed-by: me.
> > > 
> > > The reverted patch reads 'Install pre-built manpages ...'
> > > But there are no pre-built manpages any more since
> > > 
> > > 56e1d523a538bbd2434a37415605fb6a23d39706
> > > "doc: Do not keep the built version of manpages in version control"
> > > (Oct 22, 2012)  
> > 
> > There are pre-built manpages in a tarball.  I don't think a revert here
> > is the right thing to do.  The original commit fixed a bug where the
> > pre-built manpages for a tarball do not get installed.  It introduced a
> > new bug that isn't very hard to fix.  
> 
> Apparently we had not thought about the tarball
> possibly containing pre-built manpages. :-)
> We just wanted to fix the non-tarball build.
> 
> > Perhaps I'm being precious because it is my commit being reverted?  
> 
> No offense meant. Your protest is completely justified.
> 
> > However, what about the attached patch instead?  It even handles the
> > case where someone has done something strange and a subset of the
> > pre-built manpages are available.  :-)  
> 
> Your patch is definitely the right way to solve it.
> Just one comment -- see below.
> 
> > From f9606c8ac2fc5881f14848973fd7167fc35b55cd Mon Sep 17 00:00:00 2001
> > From: Martin Schwenke <martin at meltin.net>
> > Date: Tue, 10 Nov 2015 10:14:56 +1100
> > Subject: [PATCH] ctdb-build: Don't try to install unavailable prebuilt
> >  manpages
> > 
> > Commit 3ddd35142ab86de431d00f93de2fb6a2b371317d tries to
> > unconditionally install pre-built manpages if xsltproc is unavailable.
> > However, these only exist in a tarball, not in a git repo.  That can
> > make the installation fail.
> > 
> > If xsltproc does not exist then check for each pre-built manpage at
> > configure time.  This should cover all the possible cases.
> > 
> > Signed-off-by: Martin Schwenke <martin at meltin.net>
> > ---
> >  ctdb/wscript | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/ctdb/wscript b/ctdb/wscript
> > index b720748..38023d7 100755
> > --- a/ctdb/wscript
> > +++ b/ctdb/wscript
> > @@ -227,6 +227,17 @@ def configure(conf):
> >          conf.DEFINE('SAMBA_UTIL_CORE_ONLY', 1, add_to_cflags=True)
> >          conf.SAMBA_CONFIG_H()
> >  
> > +    if 'XSLTPROC_MANPAGES' in conf.env and conf.env['XSLTPROC_MANPAGES']:
> > +        conf.env.ctdb_prebuilt_manpages = None
> > +    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.ctdb_prebuilt_manpages is None:
> >          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,  
> 
> 
> 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...  :-)

I just though that setting a clear condition in configure and then
using it in build would be cleanest.

> 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...

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/d0d1ba2e/attachment-0001.sig>