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:jus...@atlantide.t28.net] Web: http://www.atlantide.t28.net LinkedIn: http://it.linkedin.com/in/delaurenzis
yosys.tar.gz
Description: application/gzip
