Re: Neovim update and new libluv port

Hi,

Thanks for looking at this.

On Sun, Sep 29, 2019 at 01:16:13PM -0700, Travis Cole wrote:
> * A new port of libluv:
> - Neovim now depends on libluv
> - (See https://github.com/neovim/neovim/wiki/Following-HEAD) so I had to
> create a new port which I've attached.

Let's start with this ^

Should the package name be "libluv" or "luv"? Upstream seems to call it "luv".


> PKGVER = 1.30.1.1

Since that's only used once, you might aswell inline it. Or even better,
to the replacement using make's regex support:

PKGNAME = libluv-${VER:S/-/./g}


> BUILD_DEPENDS = devel/cmake \
> devel/libuv
>
> LIB_DEPENDS = devel/libuv

If libuv is a LIB_DEPEND, I don't think you need it as a BUILD_DEPEND as well.


> CONFIGURE_ARGS = -DWITH_SHARED_LIBUV=ON \
> -DWITH_LUA_ENGINE=Lua \
> -DLUA_BUILD_TYPE=System \
> -DBUILD_MODULE=OFF \
> -DBUILD_SHARED_LIBS=ON \

Indent is odd here. I'd make all the `-D`s line up.


In Makefile and pkg/DESCR we need to use the right capitalisation for Lua and LuaJIT:
- s/luajit/LuaJIT/g
- s/lua/Lua/g


`make port-lib-depends-check` says:
---8<---
Extra: pthread.26
--->8---

So you probably want to kill that from your WANTLIB.


I notice that the source tarball bundles some dependencies in `deps/` and uses them once during the build:
---8<---
===> Building for libluv-1.30.1.1
[1/3] /usr/local/pobj/libluv-1.30.1.1/bin/cc -DLUA_USE_DLOPEN -Dluv_EXPORTS -I/usr/local/include -I/usr/local/include/lua-5.1 -I/usr/local/pobj/libluv-1.30.1.1/luv-1.30.1-1/deps/lua-compat-5.3 -O2 -pipe -g -DNDEBUG -fPIC -MD -MT CMakeFile
s/luv.dir/src/luv.c.o -MF CMakeFiles/luv.dir/src/luv.c.o.d -o CMakeFiles/luv.dir/src/luv.c.o -c /usr/local/pobj/liblu
v-1.30.1.1/luv-1.30.1-1/src/luv.c
--->8---

So it's using `deps/lua-compat-5.3`. I notice we have a port devel/lua-compat53
already. It'd be best if you can make your port use it. Perhaps we should `rm
-rf` the `deps/` directory in `pre-build:` so as to avoid accidental usage of
bundled deps?


And finally:
---8<---
fremen$ /usr/ports/infrastructure/bin/portcheck
trailing whitespace in Makefile
--->8---


Other than these, this looks good. If you mail me a revised diff, I'll start
looking for an OK.

--
Best Regards
Edd Barrett

http://www.theunixzoo.co.uk