Friday, June 26, 2020

Re: [NEW] textproc/lucene++

On Fri, Jun 26, 2020 at 11:02:42AM +0200, Omar Polo wrote:
>
> Landry Breuil <landry@openbsd.org> writes:
>
> > On Thu, Jun 25, 2020 at 06:08:56PM +0200, Omar Polo wrote:
> >>
> >> Hello there!
> >>
> >> Attached the port for Lucene++, a port of the java lucene library. It's
> >> required by newer version of devel/poedit.
> >>
> >> The patches against the three CMakefiles are to disable cotire, as it
> >> breaks the build. The patch against VariantUtils.h is to support newer
> >> version of boost. These patches are heavily inspired (read: copied)
> >> from the freebsd port.
> >>
> >> I'm not sure about the two patches against the .pc files, because by
> >> default it generates something like
> >>
> >> Libs: -L/usr/local//usr/local/lib
> >>
> >> which is broken. Comments appreciated.
> >
> > i had a look, and its indeed a bit strange - your fix makes sense but
> > why not consistently using LIB_DESTINATION in the patches ?
>
> I'm not entirely sure I've understand correctly what you mean, but I've
> changed that. I've killed the libdir variable and set
>
> Libs: -L@LIBDESTINATION@ -l${lib}

another way that implies less patching, as you already patch
CMakeLists.txt for cotire:

@@ -85,7 +80,7 @@ include(TestCXXAcceptsFlag)
include(GNUInstallDirs)

set(LIB_DESTINATION
- "${CMAKE_INSTALL_FULL_LIBDIR}" CACHE STRING "Define lib output directory name"
+ "lib" CACHE STRING "Define lib output directory name"
)

if(ENABLE_CYCLIC_CHECK)

with that chunk added to patch-CMakeLists.txt, no need to patch the .cmake
files to fiddle with prefix/LIB_DESTINATION, and the resulting .pc look
correct.

> I've added a comment in the patches, do you think it's clear enough?
> I'm not too sure about the wording:
>
> disable cotire as it's deprecated and breaks the build with
> newer libc++

that's great, thanks for the details - your comment is fine.

> I totally missed that part of the readme and stopped at 'No test were
> found!!!'. The new tarball enables tests and add devel/gtests as
> BUILD_DEPENDS, since that binary is generated during the build.
>
> It seems that now it picks up gtest:
>
> $ ldd lucene++-tester | grep libgtest
> 000003b213cf7000 000003b213d53000 rlib 0 1 0 /usr/local/lib/libgtest.so.1.0

good!

> unfortunately, after a while it core dumps:
>
> ...
> [----------] 102 tests from IndexWriterTest
> [ RUN ] IndexWriterTest.testDocCount
> [ OK ] IndexWriterTest.testDocCount (7 ms)
> [ RUN ] IndexWriterTest.testAddIndexOnDiskFull
> Segmentation fault (core dumped)

Tests are still running here, dunno if they will fail, but it's always good to
have them at least available/running.

besides the patching bit that could be simplified as i proposed, the rest of
the port looks good to import to me.

Landry

No comments:

Post a Comment