Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 27 Jan 2015 07:11:57 +0000
From:      Alexey Dokuchaev <danfe@FreeBSD.org>
To:        "Vanilla I. Shu" <vanilla@FreeBSD.org>
Cc:        svn-ports-head@freebsd.org, svn-ports-all@freebsd.org, ports-committers@freebsd.org
Subject:   Re: svn commit: r377975 - in head/devel: . godot godot/files
Message-ID:  <20150127071157.GA77865@FreeBSD.org>
In-Reply-To: <201501270345.t0R3jNnT067230@svn.freebsd.org>
References:  <201501270345.t0R3jNnT067230@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Jan 27, 2015 at 03:45:23AM +0000, Vanilla I. Shu wrote:
> New Revision: 377975
> URL: https://svnweb.freebsd.org/changeset/ports/377975
> QAT: https://qat.redports.org/buildarchive/r377975/
> 
> [...]
> +GH_PROJECT=	godot

Redundant line, GH_PROJECT is set to PORTNAME by default.

> +USES=		scons pkgconfig compiler

Please keep USES values sorted (ascending).

> +USE_OPENSSL=	yes
> +# uses pkg-config to find ssl - pkg-config only finds port version
> +WITH_OPENSSL_PORT=	yes

These three lines make little sense and generally look like a hack.  How
hard is to patch it so it won't require WITH_OPENSSL_PORT?

> +MAKE_ARGS+=	platform=x11

Bogus usage of +=.

> +OPTIONS_DEFAULT=	EXAMPLES TOOLS

EXAMPLES is already included in default options set.

> +TOOLS_DESC=	Include development tools (IDE)
> +TOOLS_MAKE_ARGS_ON=	tools=yes
> +TOOLS_MAKE_ARGS_OFF=	tools=no target=release

This reads like "if built without tools, target != release (debug?)".

> +.if ${ARCH}==amd64 || ${ARCH}==powerpc64 || ${ARCH}==sparc64 || ${ARCH}==ia64

Why not just check for ${ARCH:M*64} if all these end in `64'?  There are
lots of examples in the ports for inspiration:

  $ find . -name Makefile | xargs grep 'ARCH:.*64'

> +BITSUF=	.64
> +.else
> +BITSUF=	.32
> +.endif

Maybe you would be able to come up with smart BITSUF=	.${...} assigment
instead of .if/.else/.endif. :)

> +post-patch:
> +	@${REINPLACE_CMD} -e 's|custom_build|${OPSYS}_Ports_build|' ${WRKSRC}/methods.py

Overly long line needs wrapping.

> +do-install:
> +	@cd ${WRKSRC}/bin && ${INSTALL_PROGRAM} godot.x11${BINSUFFIX} \
> +		${STAGEDIR}/${PREFIX}/bin/godot.x11${BINSUFFIX}

${INSTALL_PROGRAM} ${WRKSRC}/bin/godot.x11${BINSUFFIX} ... is shorter,
easier to read, and $cwd-agnostic.

> +	@${LN} ${STAGEDIR}/${PREFIX}/bin/godot.x11${BINSUFFIX} \
> +		${STAGEDIR}/${PREFIX}/bin/godot

Bogus symlink target (should be just "godot.x11${BINSUFFIX}").

./danfe



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150127071157.GA77865>