On Sat, Aug 29, 2015 at 07:43:33PM +0200, Ingo Feinerer wrote:
> Some comments (with my changes attached to this posting):
>
> - COMMENT: Better use "Gibbs" than "gibbs" as it is a last name
> - COMMENT: I prefer the wording from the upstream homepage
> - Reduced/fixed WANTLIB
> - "MODULES= fortran" defines MODFORTRAN_BUILD_DEPENDS so no need to ask
> for lang/g77. Moreover, g77 is the default for MODFORTRAN_COMPILER.
> See "man port-modules" for details.
> - Add comment why USE_LIBTOOL = gnu is needed (which should normally be
> avoided).
> - Use YACC Makefile variable (automatically passed to GNU configure).
> See "man bsd.port.mk".
> - Use SEPARATE_BUILD = Yes
> - Do not pass CFLAGS and CPPFLAGS as the build appears to work out of the box
> - Pass ${CONFIGURE_SHARED} to CONFIGURE_ARGS (as we have SHARED_LIBS)
> - Add devel/flex build dependency and enforce its use (ac_cv_prog_LEX).
> The build fails with flex from base. Did it work for you?
>
> Open issues:
>
> - Would you like to take MAINTAINERship?
> - The port directory is "jags" whereas the package name is "JAGS". I do
> not know if this might cause problems and is good practice.
> - PFRAG.shared could probably be eliminated by moving corresponding
> lines to PLIST. make plist however creates PFRAG.shared (whereas often
> it is capable of this "folding in"), so I guess that is fine.
Two more thing to consider:
- When running configure:
checking for ltdl.h... no
checking where to find libltdl headers... -I${top_srcdir}/libltdl
checking where to find libltdl library...
${top_build_prefix}libltdl/libltdlc.la
This can be addressed with
--with-ltdl-include=${LOCALBASE}/include \
--with-ltdl-lib=${LOCALBASE}/lib
in CONFIGURE_ARGS. But I am unsure what we need to set for
--[en|dis]able-ltdl-install, --with[out]-included-ltdl as we use
USE_LIBTOOL = gnu. Moreover, devel/libtool,-ltdl might be necessary in
LIB_DEPENDS.
- There are PDF manuals available (in doc/manual/). Especially
jags_user_manual.pdf looks useful (whereas jags_developer_manual.pdf
and jags_installation_manual probably do not need to be shipped).
However, this needs a BUILD_DEPENDS on print/texlive/base,
ALL_TARGET = all docs, and a custom post-install to include the PDF
(as I did not find a working Makefile target for installing the PDFs).
