svn commit: r402777 - in head/audio: . festvox-cmu_us_awb_arctic festvox-cmu_us_bdl_arctic festvox-cmu_us_clb_arctic festvox-cmu_us_jmk_arctic festvox-cmu_us_rms_arctic festvox-cmu_us_slt_arctic

[Jan Beich]

Kurt Jaeger <pi at FreeBSD.org> writes:

> Added: head/audio/festvox-cmu_us_awb_arctic/Makefile
[...]
> +COMMENT=	English male voice for festival
[...]
> Added: head/audio/festvox-cmu_us_awb_arctic/pkg-descr
[...]
> @@ -0,0 +1,4 @@
> +English male voice for festival.
> +(voice_cmu_us_awb_arctic_clunits)

Too generic. How is it different from the other English male voices?

> +WWW: http://www.speech.cs.cmu.edu/

That page is is full of unrelated links. WWW should be about a specific
voice or common for all CMU ARCTIC ones e.g.,

http://www.festvox.org/cmu_arctic/dbs_awb.html

> Added: head/audio/festvox-cmu_us_awb_arctic/pkg-message
[...]
> +===============================================================================
> +For correct operation, please add cmu_us_awb_arctic_clunits
> +after "(defvar default-voice-priority-list"
> +in ${PREFIX}/share/festival/lib/voices.scm
> +===============================================================================

Do you expect package users to know what ${PREFIX} means? Also, consider
using SUB_FILES=pkg-message to avoid duplication among slaves.

https://www.freebsd.org/doc/en/books/porters-handbook/using-sub-files.html

> Added: head/audio/festvox-cmu_us_bdl_arctic/Makefile
[...]
> @@ -0,0 +1,15 @@
> +# Created by: ?

Fill in the blank field or remove the line.

> +DESCR=	${.CURDIR}/pkg-descr
> +DISTINFO_FILE=	${.CURDIR}/distinfo
> +PLIST=	${.CURDIR}/pkg-plist
> +PKGMESSAGE=	${.CURDIR}/pkg-message

If you need to override them in every slave it may make sense to define
better defaults in master port instead.

> Added: head/audio/festvox-cmu_us_slt_arctic/Makefile
[...]
> +MASTERDIR=	${.CURDIR}/../festvox-cmu_us_slt_arctic
[...]
> +.include "${MASTERDIR}/Makefile.common"

This makes the port both master *and* slave. Converting a few variables
into ?= assignments would have sufficed.

$ make -V SLAVE_PORT
yes

or see https://freshports.org/audio/festvox-cmu_us_slt_arctic/

> Added: head/audio/festvox-cmu_us_slt_arctic/Makefile.common
[...]
> +RUN_DEPENDS=	festival:${PORTSDIR}/audio/festival \
> +	${PREFIX}/share/festival/lib/dicts/cmu/cmulex.scm:${PORTSDIR}/audio/festlex-cmu \
> +	${PREFIX}/share/festival/lib/dicts/wsj.wp39.poslexR:${PORTSDIR}/audio/festlex-poslex

Dependencies are always under LOCALBASE. The difference with PREFIX is
underdocumented in ports(7) manpage and Porter's Handbook. To check try

  $ poudriere testport -P ...

> +LICENSE=	cmu
> +LICENSE_NAME=	cmu license

CMU is abbreviation of three words with omitted dots, so it should be
in uppercase. Besides, the license itself reads like MIT.

https://fedoraproject.org/wiki/Licensing:MIT#Festival_variant

> +LICENSE_FILE=	${WRKSRC}/COPYING
> +LICENSE_PERMS=	dist-mirror pkg-mirror auto-accept

Why did you remove dist-sell pkg-sell? COPYING says:

  This voice is free for use for any purpose (commercial or otherwise)
  subject to the pretty light restrictions detailed below.

> +do-install:
> +	${MKDIR} ${STAGEDIR}${FHOME}
> +	cd ${WRKSRC} && ${COPYTREE_SHARE} . ${STAGEDIR}${FHOME}

No need for MKDIR before COPYTREE_* as cpio(1) is invoked with -d.
Try the following if you don't believe

  $ find -d /usr/include | cpio -dumpl /nonexistent

> +.for dir in ${NODIRS}
> +	${RMDIR} ${STAGEDIR}${FHOME}/${dir}
> +.endfor

This can be simplified to

  ${FIND} ${STAGEDIR}${FHOME} -type d -empty -delete

> +	( cd ${STAGEDIR}${PREFIX} && ${FIND} . -type f ) | ${SORT} | \
> +	${SED} -e 's|^\.\/||' >> ${TMPPLIST}

The following would have been more compact

  DATADIR=	${FHOME}
  PORTDATA=	*

FHOME variable can also be eliminated.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 602 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-ports-all/attachments/20151201/b288160c/attachment.bin>