On 2013/06/20 12:31, Brian Callahan wrote:
> On 6/20/2013 11:57 AM, Florian Stinglmayr wrote:
> >Hi list,
> >
> >as my first OpenBSD port I ported the the_silver_searcher aka "ag". It
> >is a command line utility similar to grep to aid searching for text
> >within files. It has a focus on searching through source code, as it,
> >for example, ignores files listed in git and mercurial repository ignore
> >files.
> >
> >Comments and feedback highly appreciated and anticipated.
> >
> >Regards,
> >Florian
> >
>
> Comments:
> This might be my personal preference, but variables should be like this:
> VARIABLE <space> = <tab> whatever
> Makes reading easier.
>
> More formatting, I would look at
> ports/infrastructure/templates/Makefile.template and rearrange
> variables according to that. (i.e. put WANTLIB between
> PERMIT_PACKAGE_CDROM and MASTER_SITES)
>
> COMMENT should be: code searching tool, with a focus on speed (ag)
> so that people who only know the software as ag doing 'make search
> key=ag' will find it.
..
> PKGNAME = the_silver_searcher-${V}
I would actually go the other way, make PKGNAME be ag-$VERSION,
and put "the_silver_searcher" or similar in the COMMENT, this is a
very awkward PKGNAME for somebody to type..
> Be more precise with what license it is. License is Apache 2.0.
>
> Don't use DISTFILES since you're only pulling one item.
> Instead, try:
> DISTNAME = ${V}
this filename format (versionnumber.tar.gz) isn't acceptable directly
in the distfiles directory.
Florian, can you talk to upstream about making a real release tarball
with a proper generated configure script?
> Remove EXTRACT_SUFX, .tar.gz is the default.
>
> More personal preference, but put each DEPEND on their own line, like this:
> BUILD_DEPENDS = devel/autoconf/2.59 \
> devel/automake/1.9 \
> devel/pcre
> Make reading easier.
separate lines yes, but use
${MODGNU_AUTOCONF_DEPENDS}
${MODGNU_AUTOMAKE_DEPENDS}
(but only needed if it's impossible to get upstream to put a bit
more love into their code ;)
> Drop devel/gmake from BUILD_DEPENDS, USE_GMAKE=Yes implies that.
>
> Drop USE_GMAKE=Yes, you don't need it (and you weren't using it, as
> I'll explain later).
>
> Remove the second declaration of AUTOCONF_VERSION and
> AUTOMAKE_VERSION. The declaration in CONFIGURE_ENV is enough.
patch to remove the line in build.sh which runs configure, it is
running it with incorrect arguments for the ports tree.
use CONFIGURE_STYLE=gnu, get rid of CONFIGURE_SCRIPT, CONFIGURE_ENV.
do keep AUTOCONF_VERSION and AUTOMAKE_VERSION.
use a post-patch target to run autoconf/automake etc.:
cd ${WRKSRC} && AUTOCONF_VERSION=${AUTOCONF_VERSION}
AUTOMAKE_VERSION=${AUTOMAKE_VERSION} ./build.sh
as before, all this stuff is not needed if upstream will make a
proper release.
