Hi Brian,
On 08/10/18 06:20, Brian Callahan wrote:
>
>
> On 8/10/2018 12:05 AM, Alessandro DE LAURENZIS wrote:
>> Hi Brian,
>>
>> [...]
>>> Could you explain why ports-gcc is needed? Is there a reason that
>>> gcc-4.9.4 would be preferred over clang-6.0.0?
>>> Usually, if you have a COMPILER line, you have a comment above it
>>> with a reason why. I don't know if it's true for this port, but
>>> something like:
>>> # C++11
>>> COMPILER = base-clang ports-gcc
>>>
>>> is usually what is wanted here.
>>
>> Actually, when compiled with base-clang, I was observing some
>> segmentation faults during tests (the first in test/fsm no. 43).
>>
>> After the upgrade to a more recent snapshot (I'm currently using the
>> one dated 4th Aug), the test suite completes without errors even using
>> clang. So, I agree with your suggestion.
>>
>
> Regardless, if you have a COMPILER line, you should leave a comment
> saying why. There's no way someone would know there were problems unless
> you tell us.
>
>> Please also note that we need a pre-build "conditional", since the
>> "config-" target in upstream Makefile is different for clang
>> (config-clang) and gcc (config-gcc-4.8); I defined the variable
>> CONFIG_TARGET and set it after the ".include <bsd.port.mk>", according
>> to the value of CHOSEN_COMPILER:
>>
>> [... snip ...]
>> pre-build:
>> ${SUBST_CMD} ${WRKSRC}/kernel/yosys.cc
>> @cd ${WRKBUILD} && exec ${MAKE_PROGRAM} config-$(CONFIG_TARGET)
>>
>> [...]
>>
>> .include <bsd.port.mk>
>>
>> .if ${CHOSEN_COMPILER} == "base-clang"
>> CONFIG_TARGET = clang
>> .else
>> CONFIG_TARGET = gcc-4.8
>> .endif
>> [... snip ...]
>>
>> Please confirm that this is acceptable (or suggest any wiser
>> alternatives).
>>
>
> It's late, so I'll have to revisit this bit in the morning, but there
> are examples where we simply just pick one single config target (I think
> in my stuff I always pick clang and massage it to also work with gcc).
> I'll find you a more concrete example of this tomorrow.
>
>>
>> [...]
>>> Some other notes:
>>>
>>> As I understand Makefile.template, you should prefer the GH_*
>>> variables for this port, since everything is auto-generated anyway.
>>> It will also let you get rid of your VERSION variable, since the GH_*
>>> variables will take care of it.
>>
>> I was following the comment in Makefile.template:
>>
>> [... snip ...]
>> #
>> # github:
>> # /releases/ -> preferred. ignore GH_*, just use MASTER_SITES and
>> DISTNAME.
>> # /archive/ -> GH_ACCOUNT and GH_PROJECT, plus either GH_TAGNAME or
>> GH_COMMIT.
>> #
>> [... snip ...]
>>
>> (in this case we have a release). I see a bunch of other ports doing
>> similar things (e.g., games/minetest, textproc/py-rdflib, ...).
>>
>
> Huh? You don't have a release. Your MASTER_SITES line clearly has
> /archive/ in it. Github is needlessly confusing, and it is possible that
> comment could be worded better because I can see where the confusion
> comes from.
>
>> Should I use GH_TAGNAME instead?
>>
>
> In this case, yes.
Got it. Please see the attached tarball.
Waiting for your further feedback.
>
>>> Your LDFLAGS line might be better located as a part of your
>>> MAKE_FLAGS line.
>>
>> Done.
>>
>> Updated tarball enclosed.
>>
>
> Thanks. I'll take a closer look at this tomorrow. But at a quick glance,
> if we decide to keep it, that conditional at the very bottom is going to
> have to be pulled up above pre-build.
I'm not a make expert at all, but it seems I can't:
[... snip ...]
*** Parse error in /usr/ports/mystuff/cad/yosys: Malformed conditional
(${CHOSEN_COMPILER} == "base-clang") (Makefile:54)
*** Parse error: Need an operator in '"base-clang"' (Makefile:54)
[... snip ...]
I found x11/vlc where a similar conditional is done after the .include
--
Alessandro DE LAURENZIS
[mailto:just22@atlantide.t28.net]
Web: http://www.atlantide.t28.net
LinkedIn: http://it.linkedin.com/in/delaurenzis
No comments:
Post a Comment