Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 01 Dec 2015 21:53:14 +0100
From:      Jan Beich <jbeich@FreeBSD.org>
To:        Kurt Jaeger <pi@FreeBSD.org>
Cc:        Michael Danilov <mike.d.ft402@gmail.com>, ports-committers@freebsd.org, svn-ports-all@freebsd.org, svn-ports-head@freebsd.org
Subject:   Re: 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
Message-ID:  <4mg1-how5-wny@vfemail.net>
In-Reply-To: <201512011850.tB1IowUw051478@repo.freebsd.org> (Kurt Jaeger's message of "Tue, 1 Dec 2015 18:50:58 %2B0000 (UTC)")
References:  <201512011850.tB1IowUw051478@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
--=-=-=
Content-Type: text/plain

Kurt Jaeger <pi@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.

--=-=-=
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQF8BAEBCgBmBQJWXgi7XxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w
ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXREQjQ0MzY3NEM3RDIzNTc4NkUxNDkyQ0VF
NEM3Nzg4MzQ3OURCRERCAAoJEOTHeINHnb3bzF4IAKW5qR8s1Iz4pvWadI5bIgmj
ji1JbKiQWHrnCnkfcZAwvNCbi32ZcPfY/O1YuCQutc9HWNq9xTPnXJ1imjSZWFxB
nE8lZcopVkL0x82sQUjLqYdWN5D2ARe/44TY+U4CBgkELb1nnKMscrSFLLZVFju4
62uMfrpCU78eXW/C09RAfwYePkyFPT+N8vdXuhIGZs4LnV7KywsDYNnq7e11+B/a
rgMQ71xWasbOysVWUGxN3uUthq6pY+ViQazT4F9bpL7jpfpsyz6P6Xlso4z/k6rQ
hw7W8zFoHmFp8JtlUVZPvo/FP8R5Fw9fmyaiVrPNl4psUTPAfopnXLI/yc7rL0M=
=9nU+
-----END PGP SIGNATURE-----
--=-=-=--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4mg1-how5-wny>