-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix build errors with multiple Lua libraries #2718
base: v3/master
Are you sure you want to change the base?
Fix build errors with multiple Lua libraries #2718
Conversation
@victorhora Could you take a look at this? |
@martinhsv Would you be able to review this? |
Hello @stanhu , The only question that comes to mind initially is about where you have placed the two new items in LUA_POSSIBLE_LIB_NAMES. I guess I'm wondering aloud if it might be better for the two luajit names to be at the end of the list instead. That might make the order in which things are looked for more straightforward (I.e. all regular lua options first, then luajit) |
048a591
to
f514740
Compare
@martinhsv I moved them to the end. It's been a while since I looked at this, so I don't remember if there was a reason why I put them before the standard |
From #1909 (comment), I think I intended to the |
This commit fixes two items: 1. Previously `configure` would search for Lua in known paths, but this could cause the Makefile to use inconsistent header and library paths. For example, if luajit were installed in `/usr/local/include` but lua 5.1 were installed in `/usr/lib/liblua-5.1.so`, the build would attempt to use the luajit headers but link against the lua 5.1 library. To fix this, we switch the order of the search: * First attempt to find an installed LUA library with `pkg-config`. * If we cannot find a library that way, fall back to the known-path scan. This is actually what is already documented in `LUA_POSSIBLE_PATHS`. 2. Add luajit back into `LUA_POSSIBLE_LIB_NAMES`. This was added in 0ac23a4 but quietly reverted in fe98ce4. The changes in the first item also ensure the `CFLAGS` are set properly for luajit. This should fix the issues raised in owasp-modsecurity#1909.
f514740
to
9acd950
Compare
This commit fixes two items:
configure
would search for Lua in known paths, butthis could cause the Makefile to use inconsistent header and library
paths. For example, if luajit were installed in
/usr/local/include
but lua 5.1 were installed in
/usr/lib/liblua-5.1.so
, the buildwould attempt to use the luajit headers but link against the lua 5.1
library.
To fix this, we switch the order of the search:
pkg-config
.scan.
This is actually what is already documented in
LUA_POSSIBLE_PATHS
.Note that
PKG_CONFIG_PATH
can be specified inconfigure
to look in the right path:LUA_POSSIBLE_LIB_NAMES
. This was added in0ac23a4 but quietly reverted in fe98ce4. The changes in the first
item also ensure the
CFLAGS
are set properly for luajit.This should fix the issues raised in #1909.