On 2/28/22 00:57, Omar Polo wrote:
>> * I'm taking the gamble that the
>> "slow machines can get a head of themselves and fail to link"
>> comment is not valid anymore, using MAKE_JOBS for the parallelBuild
>> parameter.
>
> I can't really comment on that. If it was set that way it was for a
> reason, but IIRC until we add the DPB_PROPERTIES parallel to the port it
> should still be built with one job on bulk machines, so...
I'm sure it was for a reason, but the reason existed 6 years ago with
version 0.11.2 ... I think it's worth checking if it's still around.
> The point is not really to blindly use "-O2" instead of "-O3", but to
> respect the CFLAGS from /etc/mk.conf (which are "-O2 -pipe" by default.)
> The attached patch does that using SUBST_CMD to set the correct CFLAGS
> in pre-configure. However this has the consequence (advantage?) that
> all the executables built with nim will probably inherit the CFLAGS that
> were set at build time, don't know if it's unwanted or not. There's
> also the issue that now the "speed" and "size" cflags are actually the
> same ^^
I don't think we can get the nim compiler to universally respect CFLAGS
from the shell environment. Having said that, the users who absolutely
want -O3 back or enforce any other compiler flag, can later change their
/etc/nim/nim.cfg
For the same reason however I now think it would be better not to touch
the size optimization. It's not used in building the compiler, so it
doesn't make a difference in the port, but it basically disables the
whole optimization option.
> I've added some comments to the patches, the only thing I don't
> understand is why comment useFork (I admit I haven't looked into what it
> does)
It uses fork instead of posix_spawn ... both OpenBSD and FreeBSD remove
that line in their ports, though. NetBSD keeps it as it is.
It was added by upstream 8 years ago, and the comment refers to
posix_spawn being new to BSDs at the time.
> I've also taken the chance to do some cosmethic changes to the makefile:
>
> - drop unused variable VERSION > - no need to ${INSTALL_PROGRAM_DIR} ${PREFIX}/bin, it's already there
> - group some common patterns in a .for loop
>
> Thoughts?
Other than my point about the size optimization options in nim.cfg, this
looks good to me!
No comments:
Post a Comment