Hi Tim --
On 10/23/17 21:16, trondd wrote:
On Sun, October 22, 2017 10:42 pm, Brian Callahan wrote:
I didn't really know how to go about talking it out, so here's a new
tarball with the complete port with my fixes. I'm compiling the no_x11
FLAVOR now. The SDL2 version didn't work for me (it launched a windows
but then had to be kill -9'd), but it's impossible for me to tell if it
is indeed broken or if it is my super old and shitty netbook that just
refuses to play nicely with it.
~Brian
I'll talk then. :) I don't see any problems with your fixes, but I have
questions and want to understand them. Thanks for taking the time to
review.
BUILD_DEPENDS= devel/astyle \
I saw your comments in the update to astyle. I had ignored astyle because
I didn't think it was important for the port to care about upstream style
conformity. I don't know that it'll ever be an issue as upstream does
'experimental' builds off master for linux, windows and OSX so I think
style issues will be caught. But since we're building from development
source, would we want a style issue to fail the build and stop an update
to the port?
The build moves on even if astyle fails. I'm not wedded to keeping it as
a BDEP. You can remove the astyle lines from cdda's Makefile if you like.
MAKE_FLAGS= ... CXX="${CXX}"
pre-configure:
sed -i -e 's,-Os,${CXXFLAGS},g'
-e 's,$${LOCALBASE},${LOCALBASE},g' \
-e 's,-Werror,,g' ${WRKSRC}/Makefile
I'm guessing defining CXX, replacing '-Os' and removing '-Werror' are to
allow the pkg tools to control the build flags. Or something more
specific about -O and -W?
Correct. We want the pkg tools to control optimization flags.
As for -Werror, that's a flag used for testing, not for building
releases (unless you're damn sure you tested every possible combination
of every possible compiler that can build your software. And even
then...). Best to always get rid of it.
Substituting LOCALBASE here I'm lost on. I don't see where that was
targeting in the Makefile. Anyway, why wouldn't it get substituted when
make was run?
Indeed you're right. It's a leftover from my trying something else. The
pre-configure routine can be reduced to:
pre-configure:
sed -i -e 's,-Os,${CXXFLAGS},g' -e 's,-Werror,,g'
${WRKSRC}/Makefile
~Brian
