Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hello World trouble #71

Open
SwissalpS opened this issue Jan 27, 2022 · 10 comments · May be fixed by #72
Open

Hello World trouble #71

SwissalpS opened this issue Jan 27, 2022 · 10 comments · May be fixed by #72
Labels
bug Something isn't working

Comments

@SwissalpS
Copy link
Contributor

SwissalpS commented Jan 27, 2022 •
edited
Loading

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
Copy link
Contributor Author

Maybe related: luarocks/luarocks#68

@S-S-X
Copy link
Owner

S-S-X commented Jan 28, 2022 •
edited
Loading

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:

mineunit/bin/mineunit

Lines 91 to 104 in 7c60f77

local function read_mineunit_conf()
local configpath = "spec/mineunit.conf"
local configfile, err = loadfile(configpath)
if configfile then
local configenv = {}
setfenv(configfile, configenv)
configfile()
if configenv.exclude then
for _,v in ipairs(configenv.exclude) do
table.insert(args.luacov.exclude, v)
end
end
end
end

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
Copy link
Owner

S-S-X commented Jan 28, 2022

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 S-S-X added the bug Something isn't working label Jan 28, 2022
@S-S-X
Copy link
Owner

S-S-X commented Jan 28, 2022

Basically few options for fix, pick one:

  • Make it work with any Lua version starting from 5.1
    • Lua 5.1 or LuaJIT should still be used for main testing as those are used by game engine
  • Throw clear error if used Lua version is not Lua 5.1 or LuaJIT 2.1
    • Makes it harder to setup working environment on some operating systems
  • Create package that includes all required libraries and Lua
    • A lot more work, will fix everything.

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
Copy link
Contributor Author

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
Copy link
Owner

S-S-X commented Jan 29, 2022 •
edited
Loading

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?

  • Provide Docker container with everything needed to run mineunit
    • I'll probably do this anyway at some point for github integrations but if you think this is also helpful here then I might increase priority a bit.
  • Start adding "how to" instructions for different operating system documenting steps to install and use Luarocks with Lua 5.1
    • This solution wont be complete ever and would only cover systems where someone already did required things providing documentation for required steps.
  • Provide some Lua 5.2 / 5.3 / 5.4 support anyway with ALL CAPS warnings about using Lua version that does not match actual game engine.
    • Somewhat doable with downsides: Test result accuracy gets worse. Cannot use some functions in mods. Some game engine core functionality will probably be broken. Will be unsupported experimental feature and would only implement things for very basic Mineunit core functionality (excluding support for any mineunit modules and minetest engine modules).

@SwissalpS
Copy link
Contributor Author

SwissalpS commented Jan 29, 2022 •
edited
Loading

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

a podmanpodder/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.

podmanpodder/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 S-S-X linked a pull request Feb 8, 2022 that will close this issue
Draft
@S-S-X
Copy link
Owner

S-S-X commented Feb 8, 2022

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
Copy link
Contributor Author

#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
Copy link
Contributor Author

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

@S-S-X S-S-X added this to Mineunit Jan 11, 2024
@S-S-X S-S-X moved this to In review in Mineunit Feb 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
Mineunit
Status: In review
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add Dockerfile S-S-X/mineunit
2 participants
@SwissalpS @S-S-X