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

Consider Vendoring in Older Dependencies. #366

Closed
ghost opened this issue Nov 19, 2021 · 3 comments · Fixed by #369
Closed

Consider Vendoring in Older Dependencies. #366

ghost opened this issue Nov 19, 2021 · 3 comments · Fixed by #369

Comments

@ghost
Copy link

ghost commented Nov 19, 2021

Currently this project and Wargus depend on some older libraries, namely lua5.1, tolua++(https://github.com/LuaDist/toluapp), and StackTrace(https://sourceforge.net/p/stacktrace/code/HEAD/tree/). tolua++ and lua5.1 currently have some distro specific patching done which potentially is causing some issues with running stratagus on Fedora. Outside of distros occasional patches there is almost no development activity that I can see happening. According to the Lua website(https://www.lua.org/versions.html) "The last release was Lua 5.1.5, released on 17 Feb 2012. There will be no further releases of Lua 5.1." Similarly tolua++ had its last commit on Feb 18, 2013, and StackTrace in 2009.

Since lua5.1, tolua++, and StackTrace are highly unlikely to change much in the future, and the patches are likely to be minimal and easily applied as needed, it might make sense to add a directory into the toplevel directory called third-party and include the most recent versions of those libraries from ubuntu/debian with all the most recent patches applied to ensure some consistency in the versions used.

Another slightly larger concern is being able to build stratagus in the future as debian(or any other linux distribution) isn't guaranteed to continue packaging these libraries forever.

If that seems reasonable, I can try working on getting that moving. Lua5.1 and StackTrace use make, if this proposal is worth pursuing I can work on converting those to use cmake(there should be existing solutions for lua5.1) and integrate all three as ExternalProjects in the CMakeLists.txt file.

If there are any other libraries for which this would seem reasonable, let me know.

@timfel
Copy link
Member

timfel commented Nov 20, 2021

Well, someone actually put in some work to get Stratagus to run with later lua versions. It should work on 5.2 and 5.3, I don't know about 5.4.

One problem is that Lua 5.2 and 5.3 have significantly worse performance than 5.1, it was actually noticeable in games.

Another problem is that tolua++ is only for lua 5.1. But I was thinking that we should just retire tolua++. We can generate the tolua.cpp file once and commit it, and then just delete the src/tolua folder and then invocation of tolua++ from the build scripts. This way we get rid of such an old dependency. We've been more and more just writing the lua bindings manually anyway, and I think that's the better option rather than rely on an old unmaintained project.

@timfel
Copy link
Member

timfel commented Nov 20, 2021

But yeah, I guess we could also vendor. If we do that, I would actually take the opportunity to switch to luajit, since I've seen good performance improvements with that.

@ghost
Copy link
Author

ghost commented Nov 20, 2021

It looks like Lua 5.4 is supposed to be faster than 5.3 by a decent amount based on (https://lwn.net/Articles/826134/). Switching to luajit would work best with removing the tolua++ dependency since when I linked tolua++ to luajit it errored out on the build.

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

Successfully merging a pull request may close this issue.

1 participant