[cmake-developers] proposal of fix for FindLua

[ivan Ivanov]

Fixed, please review it again (patches are attached to #15756
<https://gitlab.kitware.com/cmake/cmake/issues/15756>) .
Erase variable from cache is necessary, I think (see comment at gitlab).
Ignoring of LUA_INCLUDE_DIR was my fault, fixed. For now I use this
variable as a recommendation, i.e. if there is no lua.h with compatible
version in this dir, search will be continue in another dirs and in case of
success this variable will be changed.

Sorry for delay.


2016-05-09 16:32 GMT+03:00 Brad King <brad.king at kitware.com>:

> On 05/06/2016 09:02 AM, ivan Ivanov wrote:
> > Fix for https://cmake.org/Bug/view.php?id=15756
>
> Thanks for working on this.
>
> Please split the patch to first perform refactoring like moving
> the version extraction into a helper function.  Then the actual
> logic change will be easier to see in the second commit.  Also
> please name helper functions as "_lua_...".  We might as well
> rename the set_lua_version_vars helper while we're at it.
>
> > +    unset(LUA_INCLUDE_PREFIX CACHE)
> > +    find_path(LUA_INCLUDE_PREFIX ${subdir}/lua.h
>
> This renames the cache entry from LUA_INCLUDE_DIR to
> LUA_INCLUDE_PREFIX and also stops re-using an already-found
> or user-provided value.  Neither of these follows CMake conventions.
> I don't follow why this change is needed.  Using version-specific
> paths in a normal find_path(LUA_INCLUDE_DIR ...) should be enough.
>
> Thanks,
> -Brad
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20160620/fcb0921c/attachment.html>