On Fri, Nov 15, 2019, at 03:30, Edd Barrett wrote:
>
> Here is my review of the old diff. You probably want to merge my changes
> with yours.
>
> On Thu, Nov 14, 2019 at 11:54:30AM -0800, Travis Cole wrote:
> > +pre-configure:
> > + @mkdir -p ${WRKBUILD}/build
> > + @(cd ${WRKBUILD}/build && \
> > + cmake -DWITH_LUA_ENGINE=Lua \
> > + -DLUA_LIBRARIES=${MODLUA_LIB} \
> > + -DLUA_INCLUDE_DIR=${MODLUA_INCL_DIR} -DBUILD_MODULE=OFF \
> > + -DLUA_BUILD_TYPE=System -DCMAKE_COLOR_MAKEFILE=OFF \
> > + -DCMAKE_INSTALL_PREFIX=${WRKBUILD}/deps ${WRKDIR}/${LUV} && \
> > + ${MAKE_PROGRAM} install)
>
> At the very least, this part of the build needs to honour CC, CXX,
> CFLAGS and CXXFLAGS.
Ok noted.
> At the moment it doesn't. You can verify this by adding something like:
> CFLAGS += -DXXX
>
> to the port Makefile, and adding `env VERBOSE=1` where you run make.
>
> This will print the compiler invocations, and you will see that the
> output doesn't contain our (otherwise conspicuous) -DXXX flag.
>
> If we look in ports/devel/cmake/cmake.port.mk, we see the cmake
> invocation is as follows:
I'd been trying to figure out where that stuff was defined. Now I know. Thanks!
> ---8<---
> MODCMAKE_configure= cd ${WRKBUILD} && ${SETENV} \
> CC="${CC}" CFLAGS="${CFLAGS}" \
> CXX="${CXX}" CXXFLAGS="${CXXFLAGS}" \
> ${CONFIGURE_ENV} ${LOCALBASE}/bin/cmake \
> -DCMAKE_SKIP_INSTALL_ALL_DEPENDENCY=ON \
> -G ${_MODCMAKE_GEN} ${CONFIGURE_ARGS} ${WRKSRC}
> --->8---
>
> Ideally, we'd just invoke ${MODCMAKE_configure} having set any extra configure
> flags needed for luv, but we can't do that without also adding flags to the
> main build, so I *think* we probably want to copy as much of the above cmake
> invocation as possible, and put a comment saying that this fragment needs to
> be
> kept close to cmake.port.mk.
I worry a bit that /usr/ports/devel/cmake/cmake.port.mk will drift without us
and we might miss changes except when it's time to update the neovim port again.
But I don't have a better solution.
> Ideally we'd also use `-G ${_MODCMAKE_GEN}` and
> ${MODCMAKE_INSTALL_TARGET} to invoke either make or ninja automatically,
> but it makes assumptions about the build dir, so we can't.
>
> I wouldn't bother prefixing commands in make targets with @ myself.
Ok noted.
> I've posted below an updated diff, but are you sure it isn't easier to invoke
> the neovim deps builder, just having downloaded the luv distfile on its
> behalf?
So if I understand the above correctly, we'd still have to define the extra
stuff from
/usr/ports/devel/cmake/cmake.port.mk because I'm calling cmake directly in that
pre-configure. Is that right?
I'm also running into at least two issues using the neovim deps builder:
1. They download the luv tar from the github archive URL. This download does
not include
the lua-compat53 headers, which are required for the build (and that don't
exist in ports).
So they also download the lua-compat53 tar, also from an archive URL.
The luv tar that I'm downloading comes from the github download URL and
includes a bundled
lua-compat53, so it skips having to deal with that. This could be patched away,
but...
2. When I use the neovim deps builder it always wants to pick up Lua5.3, which
then
means the final build doesn't link. That's the issue you saw previously. I
haven't
figured out why this is happening.
> (edit: remember the diff is based on your older diff)
>
> I fell we are close to having this working.
I feel like it's close too.
Are you sure you sent the right diff? It looks like one of the ones I'd sent
previously, but I don't notice
any changes.
If you could resend, or point out what I missed, I can merge it with my latest
diff.
Thanks!
