On 2018/08/10 08:41, Alessandro DE LAURENZIS wrote:
> 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
Save trouble, it's not needed. These config options don't do anything useful.
Still building but here's a diff on top of your last tar which fixes a few
little issues (one of which was failure to fetch the tarball from MASTER_SITES
:)
diff --git a/Makefile b/Makefile
index 06e33f6..e596518 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,12 @@
# $OpenBSD$
COMMENT = framework for Verilog RTL synthesis
+
+GH_ACCOUNT = YosysHQ
+GH_PROJECT = yosys
+GH_TAGNAME = yosys-0.7
+DISTNAME = ${GH_TAGNAME}
+
CATEGORIES = cad
HOMEPAGE = http://www.clifford.at/yosys/
@@ -9,35 +15,30 @@ MAINTAINER = Alessandro De Laurenzis
<jus...@atlantide.t28.net>
# ISC (yosys), MIT (MiniSat)
PERMIT_PACKAGE_CDROM = Yes
-WANTLIB += ${COMPILER_LIBCXX} c curses m readline tcl85 ffi
-
-GH_ACCOUNT = YosysHQ
-GH_PROJECT = yosys
-GH_TAGNAME = 0.7
+WANTLIB += ${COMPILER_LIBCXX} ${MODTCL_WANTLIB} c curses m readline ffi
# C++11
COMPILER = base-clang ports-gcc
-MODULES = lang/tcl lang/python
+MODULES = lang/python \
+ lang/tcl
MODPY_VERSION = ${MODPY_DEFAULT_VERSION_3}
+CONFIGURE_STYLE = none
-BUILD_DEPENDS = ${MODTCL_BUILD_DEPENDS} \
- devel/bison \
+BUILD_DEPENDS = devel/bison \
shells/bash
-RUN_DEPENDS = ${MODTCL_RUN_DEPENDS} \
+RUN_DEPENDS = cad/abc \
math/graphviz \
- cad/abc \
shells/bash
-LIB_DEPENDS = devel/libffi
+LIB_DEPENDS = ${MODTCL_LIB_DEPENDS} \
+ devel/libffi
-TEST_DEPENDS = lang/iverilog \
- cad/abc \
+TEST_DEPENDS = cad/abc \
+ lang/iverilog \
shells/bash
-CONFIGURE_STYLE = none
-
USE_GMAKE = Yes
MAKE_FLAGS = CXX="${CXX}" LD="${CXX} -L${LOCALBASE}/lib"
@@ -49,20 +50,12 @@ TEST_ENV = MAKE=${MAKE_PROGRAM}
FAKE_FLAGS = PREFIX="${TRUEPREFIX}"
-WRKDIST = ${WRKDIR}/yosys-yosys-${GH_TAGNAME}
-
-pre-build:
- ${SUBST_CMD} ${WRKSRC}/kernel/yosys.cc
- @cd ${WRKBUILD} && exec ${MAKE_PROGRAM} config-$(CONFIG_TARGET)
+do-configure:
+ @${SUBST_CMD} ${WRKSRC}/kernel/yosys.cc
+ @cd ${WRKBUILD} && exec ${MAKE_PROGRAM} config-gcc
post-install:
${MODPY_BIN} ${MODPY_LIBDIR}/compileall.py \
${PREFIX}/share/yosys/python3
.include <bsd.port.mk>
-
-.if ${CHOSEN_COMPILER} == "base-clang"
-CONFIG_TARGET = clang
-.else
-CONFIG_TARGET = gcc-4.8
-.endif
