Created attachment 148374 [details] shar file with new port The attached patch adds a new port: graphics/code-eli. code-eli is a library in the form of several templatized C++ classes that provide several features for graphics based software (bezier surfaces, piecewise curves...) This port is part of my ongoing effort to include in the ports tree the new dependencies of the upcoming version of cad/openvsp which suffered a major overhaul. I marked it as broken if release < 10.0 because I was unable to make it compile with neither gcc49 nor clang33 in anything < 10.0. It compiles without problems in 10.0 with the stock clang version though. Attached is the shar file with the new port. The other attached file is the poudriere log for testing in 10.0-RELEASE-amd64
Created attachment 148375 [details] poudriere log on 10.0-RELEASE amd64 Poudriere log for the new port in 10.0 amd64 (host is 10.0-RELEASE amd64)
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 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 3) this line: XWRKSRC= ${WRKDIR}/${GH_ACCOUNT}-${GH_PROJECT}-${GH_COMMIT} I suspect using GITHUB already properly defines WRKSRC -- please check 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 5) PORTREVISION=1 A new port should never define PORTREVISION. I should be "0" right? 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. 7) PORTEXAMPLES You can also define "PORTEXAMPLES= AirfoilFitExample VSPPodExample test" and remove these from the pkg-plist too if you want. 8) USES don't define USES+= on the first instance, it should be "USES= cmake:outsource"
Created attachment 148893 [details] poudriere log on 10.0-RELEASE amd64 version 2 of the log.
(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!
Created attachment 148894 [details] shar file with new port version 2 of the shar file of the port.
Great job! It looks like you addressed all my concerns on the first attempt and anticipated my request for a new poudriere log, so you've left me with nothing more to complain about. :) I'm promoting this PR to the "patch-ready" pool now.
I missed a couple -- 1) you are using ${CP} to copy, you need to be using INSTALL macros (e.g. ${INSTALL_DATA}, ${INSTALL_SCRIPT}, ${INSTALL_PROGRAM} 2) You need to respect 80-column limits and wrap with "\" if possible 3) You need to surround compound commands in (), e.g. "(cd /here && do-something)" I'm cleaning this up, review after it's done.
A commit references this bug: Author: marino Date: Tue Nov 4 18:36:43 UTC 2014 New revision: 372161 URL: https://svnweb.freebsd.org/changeset/ports/372161 Log: Add new port graphics/code-eli PR: 194406 Submitted by: Fernando Apesteguia This is a collection of C++ libraries that provides a variety of functionalities. Eigen3 is needed for most of the components to work since all of the vector and matrix math is done using Eigen3. CPPTest is used to perform unit testing on the components. Changes: head/graphics/Makefile head/graphics/code-eli/ head/graphics/code-eli/Makefile head/graphics/code-eli/distinfo head/graphics/code-eli/pkg-descr head/graphics/code-eli/pkg-plist
okay, I ended up spending a lot of time on this. - The doxygen code was broken. You had it showing twice, once in OPTIONS framework and once always. When I removed it from "always", the broken DOCS option was reveal. - I changed the way eigen was specified to make it shorter - I created all the directories at once - I combined commands into one when possible (install, strip, REINPLACE_CMD) - I did some aesthetic rearrangement - I used xargs instead of -exec from ${FIND} etc. Please check out all my changes, but you have your port!
P.S. The pkg-descr still doesn't really say what this port does. Is there no way to describe the "various functionalities" more specifically so somebody can determine if this port helps them?
(In reply to John Marino from comment #10) > P.S. The pkg-descr still doesn't really say what this port does. Is there > no way to describe the "various functionalities" more specifically so > somebody can determine if this port helps them? First of all thanks for the work! I came up with this for the pkg-descr: This is a collection of C++ libraries that provides a variety of functionalities for geometries (bezier curves, splines, etc). Eigen3 is needed for most of the components to work since all of the vector and matrix math is done using Eigen3. CPPTest is used to perform unit testing on the components. WWW: https://github.com/ddmarshall/Code-Eli Should I file another PR?
(In reply to fernando.apesteguia from comment #11) > (In reply to John Marino from comment #10) > > P.S. The pkg-descr still doesn't really say what this port does. Is there > > no way to describe the "various functionalities" more specifically so > > somebody can determine if this port helps them? > > First of all thanks for the work! > > I came up with this for the pkg-descr: > > This is a collection of C++ libraries that provides a variety of > functionalities for geometries (bezier curves, splines, etc). > Eigen3 is needed for most of the components to work since all of > the vector and matrix math is done using Eigen3. CPPTest is used to > perform unit testing on the components. > > WWW: https://github.com/ddmarshall/Code-Eli > > Should I file another PR? And I tested the port on my machine and looks good. It is now properly recognized by the next-to-be version of openvsp as an installed dependency.
(In reply to fernando.apesteguia from comment #11) > Should I file another PR? No need, I fixed it on the spot, thanks. http://www.freshports.org/graphics/code-eli/