nodemcu-firmware icon indicating copy to clipboard operation
nodemcu-firmware copied to clipboard

node.LFS.reload should detect before reboot if the LFS image given is from 5.1 luac.cross

Open mk-pmb opened this issue 3 years ago • 9 comments

Expected behavior

  • cd app/lua/luac_cross && make LUA=53 clean all either builds a useful executable or refuses with a link to this issue to explain why we don't support i686. (Update: The command was wrong. The LUA=53 had no effect at the time, and built with LUA 5.1 regardless.)
    • I'd accept "nobody uses 32 bit CPUs anyway" if someone is bold enough to make that claim.
  • node.LFS.reload() detects before reboot that the reboot would fail, and thus, would not reboot at all.

Actual behavior

When I create an LFS image with luac.cross compiled from commit b4c148e (current dev branch) with LUA=53 on Ubuntu on an x86_64 machine, it works as expected. (Update: Because it was from a build of the entire project, thus using the luac from lua53. At the time I didn't know there was a difference, and thus paid no attention.)

When I try the same (Update: same luac command, but with luac.cross compiled using the errornous command from above) on an i686 machine, it goes through all the motions with no sign that anything is wrong, and not even the pass 1 check of node.LFS.reload complains. Only after the MCU reboot it fails with (null): invalid header in precompiled chunk.

Test code

-- -*- coding: UTF-8, tab-width: 2 -*-
return function () print('Hello.') end
( cd app/lua/luac_cross && make LUA=53 clean all
) && sudo mv -v luac.cross /usr/local/bin/luac-for-nodemcu
luac-for-nodemcu -f -o hello.$(uname -m).lfs hello.lua

NodeMCU startup banner

See UART LUA session log.

Hardware

LoLin v3 ESP8266

mk-pmb avatar Sep 01 '20 20:09 mk-pmb

If you look at the code, then you will see that we support both 32-bit and 64-bit hosts. I understand your report that reload isn't working for an LFS image built on a 32-bit host, but at worst this is a bug and not some sort of deep-state conspiracy, so please avoid this type of wording. You are free to use sarcasm when you are paying for my time; it really doesn't help when I am giving it pro bono.

PS. Your first link is broken.

TerryE avatar Sep 02 '20 15:09 TerryE

OK, I have just tried an image build on my i5 laptop and an RPi4 running 32-bit rasbian compiling _init.lua into an LFS image. They are identical, so we haven't got fundamental error here. You will need to fix the link before I can do more. I might have some old 32-bit VM images that I can spin up, but I can't see any reason for the output from a 32-bit ARM compiled version and a 32-bit x86 compiled version to be different.

TerryE avatar Sep 02 '20 16:09 TerryE

On reflection saying as there isn't an error in RPi variants and I hope to have the next feature release within the week, I will check that for 32-bit x86 working.

TerryE avatar Sep 02 '20 16:09 TerryE

Sorry if this came accross the wrong way, it wasn't meant against you or anyone in person. The wording "sneakily" was intended to be funny, I guess it wasn't after all. Sorry again. The 32bit remark was to clarify that it's not overly important to me to continue the 32bit support. Lots of projects (e.g. Ubuntu) have chosen to drop 32bit versions lately, so considering the scarcity of C devs here, I considered the possibility that, if this bug could be too cumbersome, we might follow them. (Not saying we should though.)

No idea why the Wayback Machine deleted the file. I fixed the first link to point to the live version instead.

mk-pmb avatar Sep 03 '20 00:09 mk-pmb

By accident I discovered that there is a luac in the lua53 directory as well, its subdirectory called "host". At first I couldn't find it because I had only searched for files with "cross" in the name. That one works perfectly. So I guess the correct fix for the first part is to make the 5.1 luac Makefile refuse to build if the "LUA" variable is not empty, with a hint where the 5.3 luac is. I'll prepare a PR for that.

Edit: This also explains why it worked on the 64bit machine. There I used the default luac.cross which of course was the one from lua53. I didn't pay attention to that detail because at the time I had assumed there's only one luac.cross.

mk-pmb avatar Sep 03 '20 03:09 mk-pmb

The makes are a bit cludgy when you swap versions. you really need to make LUA=51 clear before doing the make LUA=53 and v.v.

TerryE avatar Sep 06 '20 01:09 TerryE

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Sep 07 '21 06:09 stale[bot]

I still want this.

mk-pmb avatar Sep 08 '21 20:09 mk-pmb

OK, let's not close this.

TerryE avatar Sep 26 '21 19:09 TerryE