Add LUA 5.4 support to VLC

Patch applied to version 3.0.11.1

Added attachment:

0001-lua-Make-scripts-compatible-to-lua-5.4.patch

VLC 4.0？

Hello. Thanks for your patch, I'll take a look at it, although I can only check with lua 5.2. I was bemused because of librivox.lua and magnatune.lua that use the simplexml module; indeed these scripts aren't compiled or distributed, but I don't see why excluding them from the update. Also, how does this work for share/lua/http/custom.lua, did you test?

A more troublesome concern is that this seems to break calling scripts. There are third-party out-of-tree scripts out there (for example on https://addons.videolan.org/), some of which might be using the in-tree modules in the old way. Is it possible to remove the no-longer-supported module() calls while maintaining compatibility with any current require that doesn't store its result into a local table? We might want to stay conservative about this.

I did test my patch with LUA 5.2 and LUA 5.4. The scope of the tests was:

Checking that http lua server was working, open a web browser and check that web interface was OK
Tested that the android app named VLCRemote was still working (was broken before this fix)

I did not really understand how share/lua/http/custom.lua was "working", this is the only piece of code that I did not fully understand (How gettext is exported ?). But I did test that with my change that custom.lua was still working: If for example gettext was modified, the modification is taken into account...

I did not test without storing the require result into a local table. But if you say that without this change this does not work (so with a basic require "simplexml", instead of local simplexml = require("simplexml"), I think we are screwed.

I am not an expert (at all) in LUA, but the only solution that I see, is to still call the module function but conditionally on the LUA version

#25041 (closed) marked as duplicate

Replying to [comment:3 Benjamin ROBIN]:

I did not test without storing the require result into a local table. But if you say that without this change this does not work (so with a basic require "simplexml", instead of local simplexml = require("simplexml"), I think we are screwed.

I'm not saying that :) I'm wondering too. I've looked at it briefly, but I'm a bit confused by the results.

I am not an expert (at all) in LUA, but the only solution that I see, is to still call the module function but conditionally on the LUA version

That could certainly be part of a solution, yes.

I checked, we do not need to take into account LUA version prior 5.2 (this is way too old, and nobody is going to compile a new VLC version with a very old LUA version)

LUA 5.2 version does the same thing that LUA version 5.3, so I analyze the difference between LUA version 5.2 and 5.4. Here the result:

= lua module =	= lua main file =	= lua 5.2 =	= lua 5.4 =	= Result =
module	require	OK	KO	m-r.5x.txt
module	local require	OK	KO	m-l.5x.txt
return obj	require	KO	KO	r-r.5x.txt
return obj	local require	OK	OK	r-l.5x.txt
module + return obj	require	OK	KO	mr-r.5x.txt
module + return obj	local require	OK	KO	mr-l.5x.txt
module (if < 5.4) + return obj	require	OK	KO	m2r-r.5x.txt
module (if < 5.4) + return obj	local require	OK	OK	m2r-l.5x.txt

See results files in testlua.zip

So in summary:

For version 5.2:
- module must be called if the lua script that require this module only use require "mod" instead of local mod = require("mod")
  
  So for third-party/legacy LUA script, the LUA version must be checked, see my example in the testlua.zip for a factorization if we do not want to duplicate the condition if _VERSION == "Lua 5.1" or _VERSION == "Lua 5.2" or _VERSION == "Lua 5.3" then for every module...
- The module returning the object with all "public" functions is fine
For version 5.4:
- The only valid way is to return the object with all "public" functions and to use local mod = require("mod")

Raw outputs of the difference between LUA version 5.2 and 5.4

Added attachment:

testlua.zip

I found a pattern that works in any condition and this is quite simple. In the module, remove the module call, and at the end add something like

testmodule = { testfunc = testfunc }
return testmodule

This is working because:

All global variable declared inside a module are global to whole LUA program.
This imply that all functions and "global" variable inside a LUA script should be declared local
And now I understand why custom.lua is "working", the gettext is declared globally, so this function will be available everywhere.
So this is the same logic that with a C program, just need to replace static by local
So most of the LUA scripts should be cleanup (add local almost everywhere, but this is quite risky to do

Nice, okay. The documentation page states:

The way it works is it creates a new table for the module, stores it in the global named by the first argument to module, and sets it as the environment for the chunk, so if you create a global variable, it gets stored in the module table.

So removing the module call goes hand in hand with making all following definitions local. I guess we need a patch that does that and adds your trick at the end for all modules, and a second optional patch to convert all in-tree module requires to the new, cleaner way receiving the module into a local table; I think both changes are really independent from each other.

If you want to write patches for this, you can also send them to the mailing list if you want, where they can get a wider review: https://mailman.videolan.org/listinfo/vlc-devel

I am going in vacation, and I am quite busy... So I do not think I will have time to prepare a clean patch soon.

Also there are a lot of difference between the current master branch and the 3.X branch. So I think this require 2 group of patches (one for master and one for 3.X)

What is the current strategy since master has diverged quite a lot of 3.X ?

Lua modules haven't diverged a lot. Ideally, I would fix 5.4 compatibility in master, and then leave 3.0 alone since the change is not really a bugfix and has potential for breaking things. So it's more of a packaging and release management concern.

If I undertand correctly, module() is still supported by lua 5.3, right? When I look at https://www.lua.org/versions.html , it doesn't look like they really maintain older versions; but then it's apparently not a problem to still run VLC with a 5 years old lua version, so I'm not certain that it would be an issue to keep requiring lua 5.3 with VLC 3.0 for a while more.

mentioned in issue #25041 (closed)

added Component::LUA run-time label

mentioned in merge request !867 (merged)

mentioned in issue #16775 (closed)

closed with merge request !867 (merged)

mentioned in merge request !2557 (merged)

added Status::fixed label

changed milestone to %4.0

Add LUA 5.4 support to VLC

Child items ...

Activity