search

draios / sysdig

Linux system exploration and troubleshooting tool with first class support for containers
http://www.sysdig.com/
Other
7.78k stars 725 forks source link

Lua conflict using external luajit #203

Closed g5pw closed 10 years ago

g5pw commented 10 years ago

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 commented 10 years ago

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

gianlucaborello commented 10 years ago

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 commented 10 years ago

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

gianlucaborello commented 10 years ago

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

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

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

4) 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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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ž