Page MenuHomeFreeBSD

[new port] textproc/RediSearch: Full-Text and Secondary Index engine over Redis
ClosedPublic

Authored by osa on Apr 25 2019, 10:03 PM.

Details

Summary

RediSearch is a source available Full-Text and Secondary Index engine over Redis, developed by Redis Labs.

Redisearch implements a search engine on top of Redis, but unlike other Redis search libraries, it does not use internal data structures like sorted sets.

This also enables more advanced features, like exact phrase matching and numeric filtering for text queries, that are not possible or efficient with traditional Redis search approaches.

WWW: https://oss.redislabs.com/redisearch/

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

linimon retitled this revision from RediSearch - new port to [new port] textproc/RediSearch: Full-Text and Secondary Index engine over Redis.Apr 26 2019, 1:08 AM
tobik added inline comments.
redisearch/Makefile
17–18 ↗(On Diff #56672)

Wrong place in the Makefile. See 15. Order of Variables in Port Makefiles.

More specifically 15.8. USES and USE_x.

20 ↗(On Diff #56672)

Why insource,noninja? It seems to build fine without it.

24–25 ↗(On Diff #56672)

Why the custom do-build? It seems to build fine without it.

28 ↗(On Diff #56672)

${INSTALL_LIB} ${INSTALL_WRKSRC}/redisearch.so ${STAGEDIR}${PREFIX}/lib

${INSTALL_WRKSRC} only needed if not using cmake:insource.

osa marked 2 inline comments as done.Apr 29 2019, 3:36 AM
osa added inline comments.
redisearch/Makefile
17–18 ↗(On Diff #56672)

Fixed.

20 ↗(On Diff #56672)

USES=cmake requires the additional port ninja needs to be installed.
Since port builds without ninja I see no reason to install additional depedences.

24–25 ↗(On Diff #56672)

According to https://oss.redislabs.com/redisearch/Quick_Start.html there are two ways to build the application:

  1. mkdir build && cd build && cmake ... && make
  2. make

So, I've choosen the second one.

redisearch/Makefile
24–25 ↗(On Diff #56672)

No, you are doing the first one, because USES=cmake does in essence mkdir build && cd build && cmake .. first (with USES-cmake:insource more like cmake .) in the configure phase.

It also takes care of calling make in the build phase if you do not override do-build. Here the custom do-build is missing passing MAKE_ARGS, MAKE_ENV, MAKE_FLAGS breaking building in parallel etc. All for no benefit that I can see.

mat requested changes to this revision.Apr 30 2019, 3:35 PM
mat added inline comments.
redisearch/Makefile
20 ↗(On Diff #56672)

The only reason to use noninja is if the port does not build with ninja. If the port builds with it, please do not set noninja.

This revision now requires changes to proceed.Apr 30 2019, 3:35 PM
0mp added inline comments.
redisearch/Makefile
4 ↗(On Diff #56672)

Is it necessary to call it like this? I am not sure if we have a policy for that, but a lowercase redisearch would at least match other package managers (https://repology.org/project/redisearch/versions).

10 ↗(On Diff #56672)

s/redis/Redis/

osa marked 6 inline comments as done.

This redisearch/Makefile update contains all suggestions and comments

Makefile
17–19 ↗(On Diff #66938)

Wrong place in the Makefile, it should happen after USES. See Chapter 15. Order of Variables in Port Makefiles.

Makefile
5 ↗(On Diff #66938)

Unless PORTVERSION is necessary (which I don't think is the case here), I'd use DISTVERSION here.

17–19 ↗(On Diff #66938)

@osa, try using portclippy and portfmt. They are very nice to work with and usually their suggestions are correct. As a result you might get a better understanding of the standard order of variables in our makefiles. :)

osa marked 3 inline comments as done.
This revision is now accepted and ready to land.Feb 25 2020, 2:00 PM