Skip to content
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

Lua conflict using external luajit #203

Closed
g5pw opened this issue Jul 8, 2014 · 15 comments
Closed

Lua conflict using external luajit #203

g5pw opened this issue Jul 8, 2014 · 15 comments

Comments

@g5pw
Copy link

g5pw commented Jul 8, 2014

Maintainer of sysdig on MacPorts here. We tend to use system libraries when available, so I noticed a bug: when building libsinsp, the compiler somehow picks up standard lua headers, which don't have the #defines for the old lua features (like lua_open, for example) and errors out.

I tried selecting the luajit include dir with -isystem, so it has precedence, but no dice. I finally patched userspace/libsinsp/chisel.cpp by including luajit headers with the full absolute path. Another fix would be using the includes like this: #include <luajit-2.0/lua.h>. If you decide that, you can even drop the -I directive to specifically include the luajit include directory.

@g5pw
Copy link
Author

g5pw commented Jul 8, 2014

I decided to implement the latter, commited the update in r121800.

@gianlucaborello
Copy link
Contributor

I see.

Do you have a quick way to replicate this, maybe on a Linux host?
Otherwise I can try to reproduce it myself by installing both Lua and LuaJIT and see how it behaves.

If I'm able to experience the issue, I might find some way to directly solve the issue within cmake, rather than the code.

Thanks.

@g5pw
Copy link
Author

g5pw commented Jul 8, 2014

I'm afraid the only way to replicate it would be to install both Lua and LuaJIT, yes.

@gianlucaborello
Copy link
Contributor

I really have a hard time reproducing the issue. This is what I did, on a Debian box.

  1. Installed libluajit-5.1-dev liblua5.1-0-dev liblua5.2-dev liblua50-dev, so at the end I have all these Lua headers in the system:

/usr/include/lua50/lua.h
/usr/include/lua5.1/lua.h
/usr/include/luajit-2.0/lua.h
/usr/include/lua5.2/lua.h

  1. Compiled sysdig with "cmake ..".
    The compiler picked lua.h from the build directory, which is expected

  2. Compiled sysdig with "cmake -DUSE_BUNDLED_LUAJIT=OFF .."
    The compiler picked /usr/include/luajit-2.0/lua.h, which is expected

  3. Uninstalled the system's libluajit-5.1-dev and compiled sysdig with "cmake -DUSE_BUNDLED_LUAJIT=OFF .."
    The compiler picked /usr/include/lua5.1/lua.h, which is expected

I can't seem to replicate your problem. Am I missing something?

@g5pw
Copy link
Author

g5pw commented Jul 9, 2014

At 4), how could cmake pick /usr/include/lua5.1/lua.h? Isn't that header provided by libluajit-5.1-dev?

Anyway, the conflict actually happenes with lua 5.2, and that is because (interestingly enough) lua header files are installed directly under /opt/local/include (and cmake has -DCMAKE_INSTALL_PREFIX=/opt/local and -DCMAKE_SYSTEM_PREFIX_PATH=/opt/local;/usr).

I don't maintain the lua package, so I don't know if it's ok to install headers under /opt/local/include without subdirs (and that could be the real issue here).

@gianlucaborello
Copy link
Contributor

On Wed, Jul 9, 2014 at 10:51 AM, Aljaž Srebrnič notifications@github.com
wrote:

At 4), how could cmake pick /usr/include/lua5.1/lua.h? Isn't that header
provided by libluajit-5.1-dev?

As I said in 4), "Uninstalled the system's libluajit-5.1-dev..."

Anyway, the conflict actually happenes with lua 5.2, and that is because
(interestingly enough) lua header files are installed directly under
/opt/local/include (and cmake has -DCMAKE_INSTALL_PREFIX=/opt/local and
-DCMAKE_SYSTEM_PREFIX_PATH=/opt/local;/usr).

I don't maintain the lua package, so I don't know if it's ok to install
headers under /opt/local/include without subdirs (and that could be the
real issue here).

Yeah, it sounds like it's wrong to put lua.h under /opt/local/include, it
should be correctly put in a directory name like the Linux distributions
do. In fact, even cmake relies on that, because if I just install lua5.2 on
my system, and then compile with "cmake -DUSE_BUNDLED_LUAJIT=OFF ..", then
cmake refuses to run with:

-- Could NOT find Lua51 (missing: LUA_LIBRARIES LUA_INCLUDE_DIR)

Anyway, if I manually move /usr/include/lua5.2/lua.h to /usr/include/lua.h,
I still can't replicate the issue, the compiler still picks the right
luajit header from the embedded directory.

@g5pw
Copy link
Author

g5pw commented Jul 9, 2014

On 09/lug/2014, at 11:26, Gianluca Borello notifications@github.com wrote:

At 4), how could cmake pick /usr/include/lua5.1/lua.h? Isn't that header
provided by libluajit-5.1-dev?

As I said in 4), "Uninstalled the system's libluajit-5.1-dev…"

Ah, yes, sorry.

Anyway, the conflict actually happenes with lua 5.2, and that is because
(interestingly enough) lua header files are installed directly under
/opt/local/include (and cmake has -DCMAKE_INSTALL_PREFIX=/opt/local and
-DCMAKE_SYSTEM_PREFIX_PATH=/opt/local;/usr).

I don't maintain the lua package, so I don't know if it's ok to install
headers under /opt/local/include without subdirs (and that could be the
real issue here).

Yeah, it sounds like it's wrong to put lua.h under /opt/local/include, it
should be correctly put in a directory name like the Linux distributions
do. In fact, even cmake relies on that, because if I just install lua5.2 on
my system, and then compile with "cmake -DUSE_BUNDLED_LUAJIT=OFF ..", then
cmake refuses to run with:

-- Could NOT find Lua51 (missing: LUA_LIBRARIES LUA_INCLUDE_DIR)

Anyway, if I manually move /usr/include/lua5.2/lua.h to /usr/include/lua.h,
I still can't replicate the issue, the compiler still picks the right
luajit header from the embedded directory.

By embedded you mean the one included in sysdig? Even with -DUSE_BUNDLED_LUAJIT=OFF?

@gianlucaborello
Copy link
Contributor

On Wed, Jul 9, 2014 at 11:59 AM, Aljaž Srebrnič notifications@github.com
wrote:

By embedded you mean the one included in sysdig? Even with
-DUSE_BUNDLED_LUAJIT=OFF?

If I just install lua5.2 on the system and then compile with "cmake ..", it
will use the embedded one (included in sysdig).

If, instead, I use -DUSE_BUNDLED_LUAJIT=OFF, cmake refuses to run (with the
message "Could NOT find Lua51") because it can't find any suitable
lua/luajit 5.1 on the system, so it behaves as expected.

@g5pw
Copy link
Author

g5pw commented Jul 9, 2014

To reproduce this bug, you should install the latest luajit and lua, move lua header files into /usr/include and build with -DUSE_BUNDLED_LUAJIT=OFF.

Cmake will find the system luajit, but due to the fact that -I/usr/include comes before -I/usr/include/luajit-2.0 in invocation, the compiler will include /usr/include/lua.h instead of /usr/include/luajit-2.0/lua.h
See the following snippet (default prefix at MacPorts is /opt/local, cmake will use that)

/usr/bin/clang++   -DPLATFORM_NAME=\"Darwin\" -pipe -Os -arch x86_64 -stdlib=libc++  -Wall -ggdb --std=c++0x -O3 -fno-strict-aliasing -DNDEBUG -arch x86_64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk -mmacosx-version-min=10.9 -I/opt/local/var/macports/build/_opt_local_var_macports_sources_svn.macports.org_dports_sysutils_sysdig/sysdig/work/sysdig-0.1.85/userspace/libsinsp/. -I/opt/local/var/macports/build/_opt_local_var_macports_sources_svn.macports.org_dports_sysutils_sysdig/sysdig/work/sysdig-0.1.85/userspace/libsinsp/../../common -I/opt/local/var/macports/build/_opt_local_var_macports_sources_svn.macports.org_dports_sysutils_sysdig/sysdig/work/sysdig-0.1.85/userspace/libsinsp/../libscap -I/opt/local/include -I/opt/local/include/luajit-2.0    -o CMakeFiles/sinsp.dir/chisel.cpp.o -c /opt/local/var/macports/build/_opt_local_var_macports_sources_svn.macports.org_dports_sysutils_sysdig/sysdig/work/sysdig-0.1.85/userspace/libsinsp/chisel.cpp

In the meantime, I'll start a discussion as to why are lua header files installed directly in include/.

@gianlucaborello
Copy link
Contributor

My command line is different than yours. I switched to OSX and, by doing exactly what you said and then cmake -DCMAKE_BUILD_TYPE=Release -DUSE_BUNDLED_LUAJIT=OFF ..

The clang line becomes:

/usr/bin/c++   -DPLATFORM_NAME=\"Darwin\" -Wall -ggdb --std=c++0x -O3 -fno-strict-aliasing -DNDEBUG -I/Users/gianluca/src/sysdig/userspace/libsinsp/. -I/Users/gianluca/src/sysdig/userspace/libsinsp/../../common -I/Users/gianluca/src/sysdig/userspace/libsinsp/../libscap -I/Users/gianluca/src/sysdig/userspace/libsinsp/third-party/jsoncpp -I/opt/local/include/luajit-2.0    -o CMakeFiles/sinsp.dir/chisel.cpp.o -c /Users/gianluca/src/sysdig/userspace/libsinsp/chisel.cpp

As you can see, there's no -I/opt/local/include.

How are you calling cmake? And what's his environment?

@g5pw
Copy link
Author

g5pw commented Jul 9, 2014

The complete cmake invocation is:

/opt/local/bin/cmake -DCMAKE_INSTALL_PREFIX=/opt/local -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_COLOR_MAKEFILE=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_BUILD_WITH_INSTALL_RPATH=ON -DCMAKE_INSTALL_RPATH=/opt/local/lib -DCMAKE_INSTALL_NAME_DIR=/opt/local/lib -DCMAKE_SYSTEM_PREFIX_PATH="/opt/local;/usr" -DCMAKE_MODULE_PATH=/opt/local/share/cmake/modules -DCMAKE_FIND_FRAMEWORK=LAST -Wno-dev -DUSE_BUNDLED_LUAJIT=OFF -DUSE_BUNDLED_JSONCPP=OFF -DUSE_BUNDLED_ZLIB=OFF /opt/local/var/macports/build/_opt_local_var_macports_sources_svn.macports.org_dports_sysutils_sysdig/sysdig/work/sysdig-0.1.85 -DCMAKE_C_FLAGS_RELEASE="-DNDEBUG" -DCMAKE_CXX_FLAGS_RELEASE="-DNDEBUG" -DCMAKE_OSX_ARCHITECTURES="x86_64" -DCMAKE_OSX_SYSROOT="/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk" -DCMAKE_OSX_DEPLOYMENT_TARGET="10.9"

I'm pretty sure the -I/opt/local/include comes from -DCMAKE_SYSTEM_PREFIX_PATH="/opt/local;/usr". We use that because all the software from MacPorts gets installed in /opt/local (by default).

@gianlucaborello
Copy link
Contributor

I still can't replicate it (I'm on Mavericks):

cmake -DCMAKE_INSTALL_PREFIX=/opt/local -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_COLOR_MAKEFILE=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_BUILD_WITH_INSTALL_RPATH=ON -DCMAKE_INSTALL_RPATH=/opt/local/lib -DCMAKE_INSTALL_NAME_DIR=/opt/local/lib -DCMAKE_SYSTEM_PREFIX_PATH="/opt/local;/usr" -DCMAKE_MODULE_PATH=/opt/local/share/cmake/modules -DCMAKE_FIND_FRAMEWORK=LAST -Wno-dev -DUSE_BUNDLED_LUAJIT=OFF -DCMAKE_OSX_DEPLOYMENT_TARGET="10.9" ..

Does:

/usr/bin/c++   -DPLATFORM_NAME=\"Darwin\" -Wall -ggdb --std=c++0x -O3 -fno-strict-aliasing -DNDEBUG -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk -mmacosx-version-min=10.9 -I/Users/gianluca/src/sysdig/userspace/libsinsp/. -I/Users/gianluca/src/sysdig/userspace/libsinsp/../../common -I/Users/gianluca/src/sysdig/userspace/libsinsp/../libscap -I/Users/gianluca/src/sysdig/userspace/libsinsp/third-party/jsoncpp -I/opt/local/include/luajit-2.0    -o CMakeFiles/sinsp.dir/chisel.cpp.o -c /Users/gianluca/src/sysdig/userspace/libsinsp/chisel.cpp

Still no /opt/local/include

@g5pw
Copy link
Author

g5pw commented Jul 14, 2014

That's odd. What about if you try to use system zlib and jsoncpp? Since their include files are in /opt/local/include, maybe that's why cmake adds the -I/opt/local/include

@gianlucaborello
Copy link
Contributor

Yes, if you happen to have everything mixed in /opt/local/include, then it can create that issue.

Honestly, yours still look like a weird setup and what I'd do is put lua/luajit in separate /opt/local/include/lua* directories. Otherwise, I can't see a trivial fix (e.g. a possible one would be to have cmake pass a macro to gcc like -DLUA_INCLUDE_DIR=/custom/lua/path and use that one inside the #include <lua.h> statement).

And I wouldn't do the include #include <luajit-2.0/lua.h> thing because we "unofficially" support building against a standard Lua5.1 as well, we don't strictly require LuaJIT.

@g5pw
Copy link
Author

g5pw commented Jul 14, 2014

Well, I will for now patch the files to use #include <luajit-2.0/lua.h> until the lua maintainer sorts out where to install lua headers.

Grazie,
Aljaž

@g5pw g5pw closed this as completed Jul 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants