Re: [PATCH] net/samba4 4.3.6 update, zfsacl option, winbindd smf, man pages option

[Thomas Klausner <wiz%NetBSD.org@localhost>]

Hi John!

Thanks for your patches, most of these look fine. The patch to
docs-xml/wscript_build should be sent upstream, can you please do
that?

Some suggestions:

> +$NetBSD$
> +
> +Include docbook-xsl xml catalog location for SmartOS
> +
> +--- buildtools/wafsamba/wafsamba.py.orig	2015-07-21 09:47:48.000000000 +0000
> ++++ buildtools/wafsamba/wafsamba.py
> +@@ -892,7 +892,7 @@ def SAMBAMANPAGES(bld, manpages, extra_s
> +     bld.env.SAMBA_EXPAND_XSL = bld.srcnode.abspath() + '/docs-xml/xslt/expand-sambadoc.xsl'
> +     bld.env.SAMBA_MAN_XSL = bld.srcnode.abspath() + '/docs-xml/xslt/man.xsl'
> +     bld.env.SAMBA_CATALOG = bld.srcnode.abspath() + '/bin/default/docs-xml/build/catalog.xml'
> +-    bld.env.SAMBA_CATALOGS = 'file:///etc/xml/catalog file:///usr/local/share/xml/catalog file://' + bld.env.SAMBA_CATALOG
> ++    bld.env.SAMBA_CATALOGS = 'file:///etc/xml/catalog file:///usr/local/share/xml/catalog file:///opt/local/share/xml/catalog file://' + bld.env.SAMBA_CATALOG
> + 
> +     for m in manpages.split():
> +         source = m + '.xml'
> -- 
> 2.6.4
> 

The installation prefix for pkgsrc is configurable, so it would be
good to find a solution that uses PREFIX passed in from the
environment; or if that's not so easy, add a SUBST_* block to the
pkgsrc Makefile that replaces e.g. /usr/local/share/xml/catalog with
${PREFIX}/share/xml/catalog.

> +GENERATE_PLIST+= \
> +	cd ${DESTDIR}${PREFIX} && \
> +		${FIND} man \( -type f -print \) | ${SORT} -du;

I don't like GENERATE_PLIST since you never know what you'll get. If a
man page wouldn't translate on one system but exist on all others,
you'd never notice.

Please do it either as you did with the module (${PLIST.man}) or add a
separate PLIST.man file and add it to PLIST_SRC (e.g. PLIST_SRC=PLIST
PLIST.man in the block you add to options.mk).

Cheers,
 Thomas