Bug 354772 - Cantor: hardcoded include of luajit-2.0/lua.hpp
Summary: Cantor: hardcoded include of luajit-2.0/lua.hpp
Status: RESOLVED FIXED
Alias: None
Product: cantor
Classification: Applications
Component: lua-backend (show other bugs)
Version: unspecified
Platform: Gentoo Packages Linux
: NOR normal
Target Milestone: ---
Assignee: Filipe Saraiva
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-03 06:47 UTC by Vadim A. Misbakh-Soloviov (mva)
Modified: 2017-02-15 18:24 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Vadim A. Misbakh-Soloviov (mva) 2015-11-03 06:47:15 UTC
Hi there!
Cantor-15.08.2 fails to build, on my system, where I have luajit-2.1 installed (for a couple of years already, btw).

Related part of buildlog is:

In file included from /var/tmp/portage/kde-apps/cantor-15.08.2/work/cantor-15.08.2/src/backends/lua/luabackend.cpp:22:0:
/var/tmp/portage/kde-apps/cantor-15.08.2/work/cantor-15.08.2/src/backends/lua/luasession.h:25:92: фатальная ошибка: luajit-2.0/lua.hpp: Нет такого файла или каталога
 #include <luajit-2.0/lua.hpp> // need the luajit-2.0 prefix to avoid conflicts with Lua 5.2


As you can see, it is hardcoded (!!!) luajit-2.0 prefix, which is very bad practice, because of things like that bug.
I think it will be right, to remove that prefix and let buildsystem (and/or user) to add needed -I/path/to the cflags.

Reproducible: Always
Comment 1 Filipe Saraiva 2016-05-26 19:51:30 UTC
Maybe can you to provide a patch for it?
This import is there since the first release, we never had problem with it (packagers never report it as a bug).
Comment 2 Lucas Hermann Negri 2016-06-06 13:17:14 UTC
Hi,

Mea culpa. After years (and a change in the mantainer), LuaJIT bumped its version number. I will fix this.
Comment 3 Vadim A. Misbakh-Soloviov (mva) 2017-02-11 10:15:06 UTC
(In reply to Lucas Hermann Negri from comment #2)
> Hi,
> 
> Mea culpa. After years (and a change in the mantainer), LuaJIT bumped its
> version number. I will fix this.

Hi, Lukas. Unfortunately, it is still 2.0 in the cantor (and I still forced to locally patch it on every KF upgrade) :'(

I'd suggest to just include lua.hpp there, but append $(pkg-config --cflags luajit) to C{,XX}FLAGS. It will be proper fix.

Should I provide a patch for that fix, or you'll be fine to fix it directly in the code?
Comment 4 Filipe Saraiva 2017-02-11 11:04:31 UTC
(In reply to Vadim A. Misbakh-Soloviov (mva) from comment #3)
> I'd suggest to just include lua.hpp there, but append $(pkg-config --cflags
> luajit) to C{,XX}FLAGS. It will be proper fix.
> 
> Should I provide a patch for that fix, or you'll be fine to fix it directly
> in the code?

Please Vadim, submit a patch for it so we can test and review your suggestion.

Create a diff here https://phabricator.kde.org/differential/diff/create/ If you don't have a KDE Identity account, create one in https://identity.kde.org/index.php?r=registration/index and after it login to Phabricator and submit the patch.
Comment 5 Vadim A. Misbakh-Soloviov (mva) 2017-02-12 02:13:01 UTC
https://phabricator.kde.org/differential/diff/11245/
Comment 6 Filipe Saraiva 2017-02-15 18:24:20 UTC
Git commit 0b6f7a5721e779dc6f67bc22d5b2511d010f19da by Filipe Saraiva, on behalf of Vadim A. Misbakh-Soloviov.
Committed on 15/02/2017 at 18:21.
Pushed by filipesaraiva into branch 'master'.

Fix the hardcoded include for lua backend

Currently Cantor has a hardcoded include to get lua.hpp library.
This patch fix the search to include different versions of lua
library. The hardcoded solution is no more necessary.
Differential Revision: https://phabricator.kde.org/D4588

M  +10   -11   cmake/FindLuaJIT.cmake
M  +1    -1    src/backends/lua/luaexpression.cpp
M  +1    -1    src/backends/lua/luahelper.cpp
M  +1    -1    src/backends/lua/luasession.h

https://commits.kde.org/cantor/0b6f7a5721e779dc6f67bc22d5b2511d010f19da