Skip to content

feat: Add Lua 5.3 compatibility and fix script argument handling#5

Closed
ItsMeForLua wants to merge 1 commit intoBanceDev:mainfrom
ItsMeForLua:fix-compat-and-args
Closed

feat: Add Lua 5.3 compatibility and fix script argument handling#5
ItsMeForLua wants to merge 1 commit intoBanceDev:mainfrom
ItsMeForLua:fix-compat-and-args

Conversation

@ItsMeForLua
Copy link
Contributor

Key Changes:

Lua 5.3 Compatibility: The lua-compat-5.3 library is now included and compiled into the project, allowing the use of functions like table.unpack, math.log(x, base), etc.

Argument Handling Fix: I refactored the non-interactive script runner in main() to correctly parse argc and argv. This ensures that arguments passed to a script from the command line (e.g., lush test.lua my_arg) are now properly available inside the script's args table.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why rename the file instead of using the .sh extension

local lua_inc_path = "/usr/include"
local lua_lib_path = "/usr/lib"
links({ "lua5.4" })

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what there an issue with how the linking/including was handled that would make these changes needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there was definitely an issue with how the linking was handled. The original approach using local lua_lib_path = "/usr/lib" was problematic for a few reasons:

The Main Problem:
On most Linux distributions, Lua libraries are installed with versioned names to support multiple Lua versions simultaneously. Instead of a generic liblua.so, you'll typically find files like:

  • liblua5.4.so.5.4.6 (full version)
  • liblua5.4.so.5.4 (minor version symlink)
  • liblua5.4.so (major version symlink)

The generic liblua.so symlink often doesn't exist at all, or if it does, it may point to an unexpected version (like an older Lua 5.1 for backwards compatibility).

So What Was Happening?:
When the build system tried to link using the path-based approach, it was likely doing something equivalent to -L/usr/lib -llua, which tells the linker to look for liblua.so in /usr/lib. This would fail with errors like:

/usr/bin/ld: cannot find -llua: No such file or directory

By switching to links({ "lua5.4" }), I explicitly tell the linker to look for liblua5.4.so, which is the actual filename that exists on the system.
So this,

  • Targets the specific Lua version I want (5.4)
  • Avoids ambiguity about which Lua version to use
  • Works consistently across different distributions
  • Follows the standard naming convention used by package managers

Additional Context:
This is why many professional build systems use pkg-config to handle this automatically:

pkg-config --libs lua5.4    # Returns: -llua5.4

The include path (/usr/include) didn't need changes because header files are typically installed in a predictable location and the compiler was finding them correctly.

The original linking approach assumed a generic liblua.so would exist, but modern package management practices use versioned library names. My fix explicitly targets the correct versioned library, eliminating the ambiguity and build failures.


Also, for some additional context here:
I usually use multiple wsl2 distros, but mostly archlinux.
when I was working on this, I moved from ubuntu LTS wsl2, to archlinux wsl2, so I was able to spot some fragile points in the make system.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I apologize for not covering that in my pull description. I actually forgot I even made that change. I need to start keeping tabs in a notebook or something of what changes I made and where, for Open Source projects.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this diff looks like you have some kind of auto formatter on your editor that is leading to lots of staged changes on lines that don't need them. I'd turn off your formatter so that the diff is easier to read.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll be able to properly review the changes you made if the diff gets cleaned up, its just a lot to try and find the actual changes in this rn.

Copy link
Contributor Author

@ItsMeForLua ItsMeForLua Jul 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes sir, I'll pull a clean git clone of your repo, and then I'll surgically edit. sorry about that.


Also, it seems the workflow build and test check failed, so, if you don't mind, I can add:

- name: Initialize submodules
  run: git submodule update --init --recursive

To the workflow YAML file(?) Since, the build logic all checks out(I spent 11 hours on this), the problem is just that github CI starts with a clean submodules directory, meaning, it cant spot the compat library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants