S-S-X / mineunit

Minetest core / engine libraries for regression tests
Other
10 stars 6 forks source link

Hello World trouble #71

Open SwissalpS opened 2 years ago

SwissalpS commented 2 years ago

Following the steps in readme.md, I've installed mineunit, changed to my mod dir and have run:

$ /home/x/.luarocks/bin/mineunit --demo
.
.
Demo tests installed to spec directory, you can now execute mineunit
$ /home/x/.luarocks/bin/mineunit
/usr/bin/lua: /home/x/.luarocks/lib/luarocks/rocks-5.4/mineunit/scm-4/bin/mineunit:96: global 'setfenv' is not callable (a nil value)
stack traceback:
    /home/x/.luarocks/lib/luarocks/rocks-5.4/mineunit/scm-4/bin/mineunit:96: in local 'read_mineunit_conf'
    /home/x/.luarocks/lib/luarocks/rocks-5.4/mineunit/scm-4/bin/mineunit:180: in main chunk
    [C]: in ?

OS: Fedora 34, if that matters.

Edit: there were no error messages on install. I ran it again to make sure. Edit2: Lua 5.4.3

SwissalpS commented 2 years ago

Maybe related: https://github.com/luarocks/luarocks/issues/68

S-S-X commented 2 years ago

Only tested with Lua 5.1 and LuaJIT, I guess it would be good to do things in a way that at least basic stuff (mineunit core functionality excluding game engine core) works with later Lua versions.

Relevant piece of code, reading mineunit configuration file similar way also happens again later in init.lua. This first conf reader is dirty hack just for luacov excludes configuration: https://github.com/S-S-X/mineunit/blob/7c60f7760b87ce7e0a7cd8736930061a12e43093/bin/mineunit#L91-L104

Should probably take a look at alternative ways to read configuration, either something that works with all Lua versions or checking Lua version and selecting conf reader method based on that.

S-S-X commented 2 years ago

Btw, I think you can skip this by deleting mineunit.conf. That should skip both mineunit.conf config file readers, no idea if something else will fail with Lua 5.4...

S-S-X commented 2 years ago

Basically few options for fix, pick one:

I think first option (work with all Lua versions) would be best if possible without a lot of duplicate code / conditional code based on Lua verison.

SwissalpS commented 2 years ago

Btw, I think you can skip this by deleting mineunit.conf. That should skip both mineunit.conf config file readers, no idea if something else will fail with Lua 5.4...

nope, same error after deleting mineunit.conf from demo directory

S-S-X commented 2 years ago

Tested a bit and there seems to be other problems with dependencies too and also many other Lua API changes that breaks compatibility.

It might be better to simply disallow using anything other than Lua 5.1 or LuaJIT 2.1 because full compatibility cannot be added without removing, rewriting and implementing a lot Lua core functionality, it would probably be possible with Lua debug API but would basically be new Lua API implemented in Lua through monkey patching and for sure not really worth it.

Somewhat Fedora specific stuff

Some compatibility with different operating systems would still be nice, I've not used fedora a lot myself but they seem to offer Lua 5.1 package.

Luarocks works fine with multiple Lua versions but probably would need to build that from sources, did not spot prebuilt fedora packages for other Lua versions (instead it seems like they're just updating used Lua version for luarocks package).

More generic, not Fedora specific

One option would be to release prebuilt Docker container, that would also allow bit more efficient automation within isolated execution environment. Setting up Docker and containers however is not always that straightforward when compared to installing and using application directly on host system.

@SwissalpS what do you think about it? What would be best thing from your PoV? Any other suggestions how it could possibly be handled nicely?

SwissalpS commented 2 years ago

patching Lua sounds like a lot of work for not much benifit.

a podman~podder~/docker image and documentation to the workflow would be far simpler and also eliminate a lot of other bugs that could creep in depending on OS used.

podman~podder~/docker guarantees faster development in other areas too, as it gives us a common known base.

Edit: the only downside to pods is the memory overhead. Which is one of the reasons I've been trying to work without it in the first place. I often work on rather low end machines :/ But I could spread out the load by installing pod on separate machine.

S-S-X commented 2 years ago

Maybe should also add simple instructions for debootstrap / febootstrap + schroot as it is fairly simple and lightweight setup, easier to setup and use compared to actual containers.

SwissalpS commented 2 years ago

72 Helped a lot :)

Maybe add a dockerHowTo.md and short note about chcon -R -t container_file_t $HOME/minetest/mods as this may throw of newbies too. Not sure if it's needed, but mentioning that docker can be replaced with podman on systems that use it over docker. At least if there is a dedicated docker/debootstrap-How-To.md, there would be space for that. Some repos put such information in the readme.md, but I think a link to other readme would be cleaner.

BTW: I did not run restorecon and the context labels have stuck so far over reboot.

SwissalpS commented 2 years ago

Maybe a install.md containing several variations of step-by-step guides.