ctdb/wscript: Avoid installing pre-built man pages which does not exist during make
Michael Adam
obnox at samba.org
Tue Nov 10 06:49:15 UTC 2015
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:
> > > 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... :-)
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.
> > 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,
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20151110/d4ceecab/signature.sig>
More information about the samba-technical
mailing list