[Bug 194406] [new port] graphics/code-eli

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=194406

--- Comment #4 from fernando.apesteguia at gmail.com ---
(In reply to John Marino from comment #2)
> The log looks good, but I see several issues (or possible issues) with the
> shar:
> 
> 1) You used "@" on all your commands in the do-install target.  You are only
> allowed to mask mkdir commands

Done. I'll remember that.

> 
> 2) the clang stuff is hacked
> X.if ${OSVERSION} < 1000000
> XBUILD_DEPENDS+=	clang33:${PORTSDIR}/lang/clang33
> XCC=             clang33
> XCXX=            clang++33
> XCPP=            clang-cpp33
> X.endif
> You should not checking OSVERSION, you should be using USES=compiler:args. 
> Even if you did use OSVERSION (you can't) you need to prefix it with
> ${OPSYS} == FreeBSD since OSVERSION only applies to FreeBSD
> 

I was unaware of the compiler:args option and I saw the hack in another port...
:)

> 3) this line:
> XWRKSRC=		${WRKDIR}/${GH_ACCOUNT}-${GH_PROJECT}-${GH_COMMIT}
> I suspect using GITHUB already properly defines WRKSRC -- please check
> 

Right. Done.

> 4) these lines:
> XBUILD_DEPENDS+=
> ${LOCALBASE}/include/eigen3/Eigen/src/Core/Array.h:${PORTSDIR}/math/eigen3
> XBUILD_DEPENDS+=	${LOCALBASE}/bin/doxygen:${PORTSDIR}/devel/doxygen
> 
> change to: 
> XBUILD_DEPENDS=
> ${LOCALBASE}/include/eigen3/Eigen/src/Core/Array.h:${PORTSDIR}/math/eigen3 \
> 		${LOCALBASE}/bin/doxygen:${PORTSDIR}/devel/doxygen


Done. I thought multilines are not friendly with out makefiles in the port's
collection.

> 
> 5) PORTREVISION=1
> A new port should never define PORTREVISION.  I should be "0" right?
> 

Done. Sorry, I used other of my ports as template and I missed this.

> 6) PORTDOCS
> Personally I'd like to see you define "PORTDOCS= html latex" in the makefile
> and remove EVERY SINGLE PORTDOCS line from the pkg-plist.  You can also
> remove the check for PORT_OPTIONS:MDOCS and just install them
> unconditionally.  (the port will ignore them if the DOCS option is unset). 
> But you can also leave it if the copying of the html and latex is heavy.

Done. The pkg-plist is much shorter now.

I'd rather leave the conditional though. If I remove the option, I will try to
cd into a directory that doesn't exist yet...

> 
> 7) PORTEXAMPLES
> You can also define "PORTEXAMPLES= AirfoilFitExample VSPPodExample test" and
> remove these from the pkg-plist too if you want.

Done.

> 
> 8) USES
> don't define USES+= on the first instance, it should be "USES=
> cmake:outsource"

Done.

I run the port with port test and it works fine with the 4 combinations of the
options.

I attach new shar file(v2) and a new poudriere log(v2) for 10.0-RELEASE-amd64.

Thanks for the mentoring!

-- 
You are receiving this mail because:
You are the assignee for the bug.