Deprecation of the Lua 5.1 engine and NodeMCU capability enhancements

[TerryE]

Related Issue

• Unified loading and saving code to RAM and LFS #2917 Unified loading and saving code to RAM and LFS
• Concerned about moving parts of modules into Lua #3128 Concerned about moving parts of modules into Lua
• WIP: Rewrite sntp in Lua with only a little C #2819 Rewrite sntp in Lua with only a little C

Background

Currently we can only embed C modules in the firmware binary. @nwf has been developing the concept of hybrid C+Lua modules. @pjsg has raised a concern in #3128 about this increasing the barriers to entry and RAM use for new developers who are unfamiliar with the added complexity of LFS. Both @nwf and @HHHartmann have discussed the concepts for a firmware-embedded LFS partition which would allow use to package C, Lua and C/Lua hybrid modules automatically at build time. The more I consider this, the more I support this general concept as this would allow use to increase the out-of-box capability for new developers.

The rub is that there are technical reasons why this would involve quite a bit of Lua VM runtime changes in the case of the Lua51 VM -- changes which I have already made to the Lua53 VM, which brings us to the policy issue.

When do we deprecate the Lua 5.1 runtime?

At the moment make LUA=53 builds a Lua 5.3 based firmware image and the default make builds a Lua 5.1 image. Both work, but the Lua 5.3 engine is cleaner, faster, and more RAM-efficient. I have already re-implemented the dump/undump LC/IMG file format for Lua53 to facilitate new features such as Nathaniel and Gregor propose.

Back-porting new Lua 5.3 stuff into Lua 5.1 takes effort and introduces extra maintenance overheads. My view is that we need to move to a policy of only offering new VM features within the Lua 5.3 engine and we treat the Lua 5.1 engine as offering continuity options for legacy support. I feel that we need an active consensus on this and this is why I am opening this issue to seek this policy change.

[nwf]

So, certainly this master drop is going to be a 5.1-based release. Why don't we, immediately post drop, kick the default over to 5.3 on dev and see if anyone screams? Assuming nobody does, we can do the next master drop in this configuration, with 5.3 the default and 5.1 lingering. We can deprecate but not remove any C modules superseded by Lua+C modules for that release. Assuming, again, still no problems emerge with 5.3, the master drop after that (so two clicks forward) can remove 5.1 and any superseded modules.

[TerryE]

So, certainly this master drop is going to be a 5.1-based release.

They way that I would phrase this is that the default build is 5.1 based. Post drop, anyone can opt to build the 5.1 version. What I would prefer is for some of the committers and advanced users to use a 5.3 build in anger to see if we are generally comfortable with it and address any issues found before we switch the default to Lua 5.3. This being said, I see no material stopper in doing this within the next month.

But other than this small qualification, we seem to be on the same page.

[vsky279]

@marcelstoer Marcel, is there a way to build 5.3 using your docker image?

[TerryE]

@vsky279, if you look at build-esp8266, then you will see that this has an environment variable BUILD_MAKE_TARGETS. You can use this to pass parameters to the make command. See the ROOT Makefile for details, but "LUA=53" will build the firmware with the Lua 5.3 runtime.

@nwf @marcelstoer @HHHartmann et al. I've been pondering this issue of when to cut the default over to Lua 5.3 and on reflection, I feel that we need a similar decision process to one we follow for dev to master drop, and we need consensus that we have completed the major items tracked on #3077.

[pjsg]

@TerryE I just tried the lua53 build, and I got lots of errors. I would guess that something is up with the dependency checking/generation. It got a lot better once I did a make clean

modules/.output/eagle/debug/lib/libmodules.a(rtctime.o): In function `do_sleep_opt':
/opt/nodemcu-firmware/app/modules/rtctime.c:91: undefined reference to `lua_tonumber'
modules/.output/eagle/debug/lib/libmodules.a(rtctime.o): In function `rtc_time_unix_unitcycles':
/opt/nodemcu-firmware/app/modules/rtctime.c:91: undefined reference to `lua_tonumber'
modules/.output/eagle/debug/lib/libmodules.a(rtctime.o): In function `rtc_time_check_magic':
/opt/nodemcu-firmware/app/modules/rtctime.c:91: undefined reference to `lua_tonumber'
modules/.output/eagle/debug/lib/libmodules.a(ws2812.o): In function `ws2812_buffer_set':
/opt/nodemcu-firmware/app/modules/ws2812.c:222: undefined reference to `lua_tonumber'
modules/.output/eagle/debug/lib/libmodules.a(ws2812.o): In function `ws2812_buffer_tostring':
/opt/nodemcu-firmware/app/modules/ws2812.c:222: undefined reference to `luaL_prepbuffer'
/opt/nodemcu-firmware/app/modules/ws2812.c:222: undefined reference to `luaL_prepbuffer'
/opt/nodemcu-firmware/app/modules/ws2812.c:222: undefined reference to `luaL_prepbuffer'
/opt/nodemcu-firmware/app/modules/ws2812.c:222: undefined reference to `luaL_prepbuffer'
/opt/nodemcu-firmware/app/modules/ws2812.c:222: undefined reference to `luaL_prepbuffer'

[TerryE]

@pjsg,

Yup, you do need to do a make clean between switching from 51 to 53 and v.v. This is because one version paths in lua.h etc. from app/lua and the other from app/lua53 but these are source compatible but not binary compatible. So both have a lua_tonumber entry but in 5.3, this is a define calling lua_tonumber() so there is no function lua_tonumber; ditto luaL_prepbuffer: function in 5.1, define in 5.3

Did the make work after doing the clean?

[vsky279]

Thanks @TerryE, it works for me 👍. I get

 build 2020-06-06 19:15 powered by Lua 5.3.5 on SDK 3.0.1-dev(fce080e)

Docker how-to

My command to compile Lua NodeMCU is

docker run --rm -ti -v //d/Data/Sources/GitHub/nodemcu-firmware:/opt/nodemcu-firmware marcelstoer/nodemcu-build build

I needed to run

docker run --rm -ti -v //d/Data/Sources/GitHub/nodemcu-firmware:/opt/nodemcu-firmware marcelstoer/nodemcu-build bash

In bash I edited /opt/build-esp8266 (vi /opt/build-esp8266) and added

BUILD_MAKE_TARGETS="LUA=53"

after BRANCH="$(git rev-parse --abbrev-ref HEAD | sed -r 's/[\/\\]+/_/g')" line.
Then make clean and build gets me to the Lua 5.3 NodeMCU.

docker run --rm -ti -e "BUILD_MAKE_TARGETS=LUA=53 clean all" -v $(pwd):/opt/nodemcu-firmware marcelstoer/nodemcu-build build

[pjsg]

It worked for me (after I cleaned up some code that I'm working on). Thanks.

[marcelstoer]

<OT>
@vsky279 it's actually much simpler. As Terry pointed out BUILD_MAKE_TARGETS is an environment variable. As such you can pass it to the docker run command (-> docs)

docker run --rm -ti -e "BUILD_MAKE_TARGETS=LUA=53 clean all" -v $(pwd):/opt/nodemcu-firmware marcelstoer/nodemcu-build build
</OT>

[vsky279]

@marcelstoer Ah, excuse my ignorance. I've edited my previous comment not to mislead anyone. Thanks for help.

[TerryE]

@vsky279, Lukas documenting this is all on our TODO list. You should not apologise for not knowing something that we've yet to explain. 😄

[HHHartmann]

They way that I would phrase this is that the default build is 5.1 based. Post drop, anyone can opt to build the 5.1

@TerryE its not quite true that anyone can opt in to build a 5.1 Version. The cloud builder does not offer it and so for that level of users there is no way to go back.

I feel that we should start announcing the switch to 5.1 and especially the loss of the integer version which comes with the 5.3 version.

vsky279 is already starting to remove support for the int version.

So we should update the comment in the user_config.h next to #define LUA_NUMBER_INTEGRAL strongly discouraging the use of an INTEGRAL version especially for new projects. Same for all references in the documentation.

A runtime deprecation warning on startup of integer version and on each interface which differs from the float version.

Apart from that I also see no reason to stick with 5.1 for much longer. But some of the more involved should first see if the 5.3 version is as stable already or if there are things missing.

[TerryE]

@TerryE its not quite true that anyone can opt in to build a 5.1 Version. The cloud builder does not offer it ...

This one is up to @marcelstoer , but surely we can fix this one with essentially an extra checkbox on the configuration screen.

especially the loss of the integer version which comes with the 5.3 version.

I think that this underlines we need some form of "What is different" topic in our documentation. Yes, we don't have a separate Integer build, but that's because the Lua VM does all integer computation using integer subtype numbers. There are some breaks here but in that the / operator in general yields a float result and you need to use the '//' operator for integer divide.

[marcelstoer]

its not quite true that anyone can opt in to build a 5.1 Version. The cloud builder does not offer it and so for that level of users there is no way to go back.

I have doubts that it would be sensible to expose the cloud builder audience to that sort of decision. It's too easy for them to take the wrong turn which would result in broken applications. We have over 550 users who watch this repository. That should be a large enough group of potential testers, no?

[TerryE]

I have doubts that it would be sensible to expose the cloud builder audience to that sort of decision.

I feel that we agree. As I said above:

What I would prefer is for some of the committers and advanced users to use a 5.3 build in anger to see if we are generally comfortable with it and address any issues found before we switch the default to Lua 5.3.

We should have a grace period of 1 month or more where we get advanced users using and testing Lua53, before we even introduce the option or default of Lua 5.3 to the general users, or even need to have this discussion, really.

[TerryE]

Lua53 is stable and out-performs Lua51. I am not going to be doing anything more with the old version. I think that we should switch the default to Lua53 on our next release. @marcelstoer, comments?

[marcelstoer]

Absolutely, the sooner the better.