Created attachment 181431 [details] update pocl to 0.14rc1 pocl 0.14 adds support for LLVM 3.9 and 4.0, and I've confirmed the build of rc1 with both. pocl also switched from autotools to CMake with this version, so there's a bit of churn in the Makefile. I hope I got it all right, please look closely to be sure. The default LLVM version is set to llvm40, but I have also made it follow MESA_LLVM_VER to allow users to set a version for all the graphics related (OpenGL/OpenCL) ports at once. I relaxed the ARCH restriction after verifying the build on i386. It looks like the ports should support some non-x86 architectures, but I do not have time right now to test building on any others. I may check PPC64 if time permits. I have not run the test suite as I do not have the CPU cycles to spare (need them for test builds, can't afford more hardware). The port now builds a selection of OCL kernels for distribution. Since the default is to build "native", I suspect the binary packages for previous releases were of limited use.
Maintainer informed via mail
Submitter is committer, cc maintainer
Created attachment 181686 [details] update pocl to 0.14 I have updated the diff now that 0.14 has been released.
Maintainer timeout
Comment on attachment 181686 [details] update pocl to 0.14 Mostly cosmetic nits: > LICENSE= GPLv3 > LICENSE_FILE= ${WRKSRC}/LICENSE The code only references MIT license. pocl-0.14 added tinyprintf.[ch] under LGPL21+ but it's not yet used. > OCL_ICD_VENDORS?= ${PREFIX}/etc/OpenCL/vendors Without .export (see make(1)) or via MAKE_ENV the line has no effect. Am I wrong? > WRKSRC= ${WRKDIR}/${PORTNAME}-${DISTVERSION} This line is redundant. See DISTNAME and WRKSRC defaults in Mk/bsd.port.mk. > PLIST_SUB= OPSYS=${OPSYS:tl} ARCH=${ARCH:C/amd64/x86_64/} As you don't use regex :S would work fine as well. > USE_GL= yes Nothing in the code uses <GL/glu.h>. Probably cruft or an obscure workaround. > USES= cmake gmake localbase ncurses pathfix pkgconfig libtool:keepla - USES=cmake obsoletes at least USES=gmake and USES=libtool - Explicit POCL_INSTALL_PKGCONFIG_DIR obsoletes USES=pathfix > # needed for the libltdl configure check > LDFLAGS+= -L${LOCALBASE}/lib Replace USES=localbase with USES=localbase:ldflags instead. > DEBUG_CMAKE_BOOL_ON= WITH_DEBUG - _BOOL_ON helper doesn't exist, see Mk/bsd.options.mk. Either use _BOOL or _ON. - WITH_DEBUG isn't used by pocl build system. Did you mean POCL_DEBUG_MESSAGES? > OCLBOOK_ALL_TARGET= test > > TEST_TARGET= test One of these is redundant but as https://github.com/freebsd/poudriere/pull/355 workaround it looks inconsistent. Maybe try the following: TEST_TARGET= test OPTIONS_DEFINE= TEST TEST_CMAKE_OFF= -DENABLE_TESTSUITES="" TEST_CMAKE_ON= -DENABLE_TESTSUITES="all" pre-install-TEST-on: do-test > @${REINPLACE_CMD} -e 's/AMD64/amd64/' -e 's/i\.86/i386/' \ Why do you need to replace i.86 ? . (dot) matches "any single character" according to re_format(7) while CMake docs say: if(<variable|string> MATCHES regex) True if the given string or variable's value matches the given regular expression.
(In reply to Jan Beich from comment #5) You're late to the party; this update is now the topic of D10508. While it would be better in general to do reviews in phabricator, the questions were already posted here and the maintainer can reply here if so inclined. >Mostly cosmetic nits: > > LICENSE= GPLv3 > LICENSE_FILE= ${WRKSRC}/LICENSE > >The code only references MIT license. pocl-0.14 added tinyprintf.[ch] under LGPL21+ but it's not yet used. I avoid touching LICENSE except when it's abundantly clear, e.g. none specified and there is only one, or the changelog indicates a re-licensing that can be easily verified. As there was one specified already, I did not question the maintainer's choice, but it does say MIT license on their website, so now wonder if there was a GPLv3 component that's since removed, so it should have been marked dual, or if that was a complete mistake. > > OCL_ICD_VENDORS?= ${PREFIX}/etc/OpenCL/vendors > >Without .export (see make(1)) or via MAKE_ENV the line has no effect. Am I wrong? You are probably correct. I do not see any reference to it, so it may have been cruft even before the switch from autotools to cmake. > > WRKSRC= ${WRKDIR}/${PORTNAME}-${DISTVERSION} > >This line is redundant. See DISTNAME and WRKSRC defaults in Mk/bsd.port.mk. Indeed, that is a relic from when the port version was 0.14-rc1. > > PLIST_SUB= OPSYS=${OPSYS:tl} ARCH=${ARCH:C/amd64/x86_64/} > >As you don't use regex :S would work fine as well. > Good to know; given sed syntax, I would not have guessed :S is the simpler, non-regex replacement. I copied that from elsewhere but do not recall where so I cannot go 'correct' that example now. > USE_GL= yes > >Nothing in the code uses <GL/glu.h>. Probably cruft or an obscure workaround. Perhaps cruft. I assumed one of the GL headers was needed but it appears not to be. > > USES= cmake gmake localbase ncurses pathfix pkgconfig libtool:keepla > >- USES=cmake obsoletes at least USES=gmake and USES=libtool >- Explicit POCL_INSTALL_PKGCONFIG_DIR obsoletes USES=pathfix libtool was already dropped in the review but I hadn't updated the patch here assuming nobody was looking since there had been no activity. INSTALL lists both GNU make and cmake under Requirements, so I assumed they really needed gmake and left it in there, but testing has confirmed the build does complete without gmake. > > # needed for the libltdl configure check > LDFLAGS+= -L${LOCALBASE}/lib > >Replace USES=localbase with USES=localbase:ldflags instead. > > DEBUG_CMAKE_BOOL_ON= WITH_DEBUG > >- _BOOL_ON helper doesn't exist, see Mk/bsd.options.mk. Either use _BOOL or _ON. >- WITH_DEBUG isn't used by pocl build system. Did you mean POCL_DEBUG_MESSAGES? WITH_DEBUG was a mistranslated relic, probably from outdated docs prior to release. POCL_DEBUG_MESSAGES is default ON and uses a runtime environment check so there is no need for that to be connected to the DEBUG switch, which works as-is. > > OCLBOOK_ALL_TARGET= test > > TEST_TARGET= test > >One of these is redundant but as https://github.com/freebsd/poudriere/pull/355 workaround it looks inconsistent. Maybe try the following: Most of the _TARGET stuff is obsolete. I had changed them to valid targets, but all doesn't need to be set to all. > > TEST_TARGET= test > > OPTIONS_DEFINE= TEST > > TEST_CMAKE_OFF= -DENABLE_TESTSUITES="" > TEST_CMAKE_ON= -DENABLE_TESTSUITES="all" > > pre-install-TEST-on: do-test The group of one did seem odd; a single TEST option makes more sense. > > @${REINPLACE_CMD} -e 's/AMD64/amd64/' -e 's/i\.86/i386/' \ > >Why do you need to replace i.86 ? . (dot) matches "any single character" according to re_format(7) while CMake docs say: > > if(<variable|string> MATCHES regex) > True if the given string or variable's value matches the given > regular expression. That seemed necessary for the release candidate so there may have been another instance that was the target of the replacement but I don't recall now.
Created attachment 182267 [details] update pocl to 0.14
A commit references this bug: Author: rezny Date: Fri May 12 17:08:27 UTC 2017 New revision: 440691 URL: https://svnweb.freebsd.org/changeset/ports/440691 Log: Update to 0.14 and switch to llvm40 by default PR: 218332 Reviewed by: jbeich Approved by: swills (mentor), maintainer (timeout) Differential Revision: https://reviews.freebsd.org/D10508 Changes: head/lang/pocl/Makefile head/lang/pocl/distinfo head/lang/pocl/files/patch-CMakeLists.txt head/lang/pocl/files/patch-Makefile.in head/lang/pocl/files/patch-config.h.in.cmake head/lang/pocl/files/patch-lib_CL_devices_cpuinfo.c head/lang/pocl/files/patch-lib_CL_pocl__binary.c head/lang/pocl/files/patch-scripts_Makefile.in head/lang/pocl/files/patch-tests_regression_test__issue__445.cpp head/lang/pocl/pkg-plist