svn commit: r324179 - in head/devel: . acme

[Jason Helfman]

On Sun, Aug 4, 2013 at 9:35 AM, Alexey Dokuchaev <danfe at freebsd.org> wrote:

> On Sat, Aug 03, 2013 at 07:33:16AM +0000, Jason Helfman wrote:
> > New Revision: 324179
> > URL: http://svnweb.freebsd.org/changeset/ports/324179
> >
> > +BUILD_WRKSRC =       ${WRKSRC}/src
>
> Stray space before =.
>
> > +PLIST_FILES= bin/acme \
> > +             %%DATADIR%%/6502 \
> > +             %%DATADIR%%/6502/Help.txt \
> > +             %%DATADIR%%/6502/std.a \
> > +             %%DATADIR%%/65816 \
> > +             %%DATADIR%%/65816/Help.txt \
> > +             %%DATADIR%%/65816/std.a \
> > +             %%DATADIR%%/Own \
> > +             %%DATADIR%%/Own/Help.txt
> > +PLIST_DIRSTRY=       %%DATADIR%%/6502 \
> > +             %%DATADIR%%/65816 \
> > +             %%DATADIR%%/Own \
> > +             %%DATADIR%%
>
> Typically using PLIST_* makes sense for short (like, five or so) entries;
> in this case I would say that traditional pkg-plist should have been used.
>
> > +post-patch-script:
> > +.for i in CC CFLAGS
> > +     @${REINPLACE_CMD} 's|${i}.*= |${i} ?= |g' ${WRKSRC}/src/Makefile
> > +.endfor
>
> Did you try to pass CC="${CC}" CFLAGS="${CFLAGS}" as MAKE_ARGS instead?
>
> > +     ${INSTALL_PROGRAM} ${WRKSRC}/src/acme ${PREFIX}/bin/${PORTNAME}
>
> Trailing ${PORTNAME} is both unneeded and confusing here (it suggests
> that ${PORTNAME} != acme).
>
> > +.for dir in 6502 65816 Own
> > +     @${MKDIR} ${DATADIR}/${dir}
> > +.endfor
> > +     ${INSTALL_DATA} ${WRKSRC}/ACME_Lib/6502/* ${DATADIR}/6502
> > +     ${INSTALL_DATA} ${WRKSRC}/ACME_Lib/65816/* ${DATADIR}/65816
> > +     ${INSTALL_DATA} ${WRKSRC}/ACME_Lib/Own/* ${DATADIR}/Own
> > +
> > +.if ${PORT_OPTIONS:MDOCS}
> > +     ${MKDIR} ${DOCSDIR}
>
> This MKDIR call is unmuted, but previous and the next ones are.  This is
> inconsistent and thus does not look good.
>
> ./danfe
>
>
Thank you for your comments and suggestions. I have raised most of these
with the maintainer for approval.

-jgh

-- 
Jason Helfman          | FreeBSD Committer
jgh at FreeBSD.org     | http://people.freebsd.org/~jgh  | The Power to Serve