[feature request] Allow building with shared lua #8153
Labels
Comments
|
I think this is related: #7569 |
|
I have no way to test a shared lua and that PR was ignored... |
|
@orbea There are lua packages in MinGW/MSYS2 toolchain. Is it enough for test? |
|
Testing it during the build is no problem for me, testing it during runtime is. I think its only used for the new cheevos implementation and I haven't used that before (Or the old cheevos). |
|
I rebased my branch and found it doesn't build with lua51, but it does with lua52. I'll look to see if lua53 builds as well tomorrow. https://github.com/orbea/RetroArch/commit/3ad7c92f786f0160e511cd0487ce022122ab31c7 If you want to test it you need |
|
|
Its not merged in the master, only on my branch. |
But: |
|
I'm not sure which package of those you want, I haven't gotten around to hooking up lua53 yet either. Lua upstream doesn't provide a This package is the one that currently builds here in Slackware linux and my branch. |
|
I think script should check lua 5.1 and 5.3 too... |
|
Lua 5.1 is apparently not supported and won't build. I just haven't gotten to trying lua 5.3 yet. |
@orbea I'm not see any info about supported Lua versions in https://github.com/RetroAchievements/rcheevos repo. |
|
Its using features that are not included with lua51, I spent sometime trying to improve |
|
@ofry or anyone else, can you provide me the contents of your available lua52 and lua53 packages? |
|
Good news, I think I have a good start, but it is still missing some details and needs testing. The bad news is I ran into a new roadblock, the new cheevos doesn't build anymore... |
|
@orbea I not see in MSYS2/MinGW lua52 package. About lua53: |
|
Thanks, I might have to give some consideration to the include directories which could be in multiple places for |
|
The new cheevos builds again thanks to @natinusala so here is an updated commit which successfully builds with a system lua53 or lua52 using the pkgconfig path. https://github.com/orbea/RetroArch/commit/f436a04100726e7b083e7d69d2c6d3b98366644e Remaining problems:
|
|
@ofry I think this should be close. https://github.com/orbea/RetroArch/commit/3df935bb5a59fcdcc9b375e278887df985956300 There are some finishing touches, but it would be good to test it during build and runtime now if you can help with that? It successfully builds with |
|
|
What does the command |
|
|
I see why it doesn't work now, but I am also not sure I see a better way.... How far do you get with? Here it returns: The idea was to return the base directory which would be I suppose the easiest would be to drop |
|
|
Huh, didn't expect that.... What the does command lua is a mess unfortunately. :\ |
|
|
Of course....this is a real mess....I suppose there is another platform which only ships |
|
To be honest I think this needs to be fixed in lua first or it will just break for every other platform... Every distro providing their own pkgconfig files and shared libraries is just maddening and not supportable, I think upstream suggests using a static lua like we already do even if I agree with you its not ideal... |
|
Another idea would be to also add support for lua51 in rcheevos which would make this a little easier, but I don't think I am up for that task... |
|
I made this feature request just for debugging purposes, because of #7832 built-in lua and mpv core conflicts. So even some hardcoded paths or trying to locate lua.h from pkg-config will be useful. |
|
I suspect this would build on your system. |
|
@orbea Please tell me where I should apply this patch? On top of your lua branch? Or? |
|
The current master. I made a PR to fix the lua include which is a general preexisting issue. |
|
On current master: |
|
You need to apply the patch against master. |
|
Fwiw the lua+mpv issue I think is a conflict in the lua versions RetroArch and libmpv are built against, the crash is clearly in the lua code and I suspect it would need to be fixed there... Or otherwise we need to find a way to avoid the situation. |
|
After applying your patch to current master, it compiles and runs fine. Could you make PR? |
|
@orbea About lua+mpv issue, I'm interested, could it compile and run fine with built-in lua, built-in ffmpeg and built-in mpv simultaneously? |
Ofc not, that patch only works for your system and will cause issues on several others...
The builtin mpv doesn't use the builtin ffmpeg, it uses libmpv which is presumably built against a system version of ffmpeg (Just like the ffmpeg core). There are a few issues that need to be addressed with lua + mpv.
As for the system lua this issue is essentially stuck and at one of two upstream issues need to be resolved before I might be able to make it work.
|
|
This PR in mpv upstream could add audio callback: Could anyone fix code style issues in this PR? |
|
@orbea Could crossplatform lua version check be done by this way? First, lua executable (with different versions) could be: Paths to these commands (and are it exist at all) we can get at least by: For version check we could parse output of this command: Examples: |
|
That is not a very clean solution and doesn't address problems like the header directories. Also fwiw we use the The current roadblock is in issue RetroAchievements/rcheevos#15. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
First and foremost consider this:
Description
Allow building with shared lua (instead of baked-in) for debugging purposes.
Expected behavior
Should be option in configure script
Actual behavior
Steps to reproduce the bug
Bisect Results
[Try to bisect and tell us when this started happening]
Version/Commit
You can find this information under Information/System Information
Environment information
The text was updated successfully, but these errors were encountered: