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.

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

Fix stack manipulation in Premake's luaL_loadfilex override. #2243

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

tritao
Copy link
Contributor

@tritao tritao commented Sep 4, 2024

What does this PR do?

Allows the Lua base functionality of passing a custom environment to loadfile to work.

assert(pcall(loadfile("foo.lua", nil, env))

I believe this has been broken for like 7 or more years, since the upgrade to Lua 5.2.

  • Focus on a single fix or feature; remove any unrelated formatting or code changes
  • Add unit tests showing fix or feature works; all tests pass
  • Mention any related issues (put closes #XXXX in comment to auto-close issue when PR is merged)
  • Follow our coding conventions
  • Minimize the number of commits
  • Align documentation to your changes

You can now support Premake on our OpenCollective. Your contributions help us spend more time responding to requests like these!

@tritao tritao force-pushed the fix-stack-manip-loadfilex branch 2 times, most recently from 78690c2 to 3914c21 Compare September 4, 2024 23:32
@tritao tritao marked this pull request as ready for review September 4, 2024 23:35
tests/test_lua.lua Outdated Show resolved Hide resolved
An example of code that did not work:

```lua
local env = { ["foo"] = function(n) print(n) end }
assert(pcall(loadfile("foo.lua", nil, env))
```
@tritao
Copy link
Contributor Author

tritao commented Sep 16, 2024

Updated with a new test without a custom env, as requested.

@samsinsane samsinsane merged commit 2c8d0b6 into premake:master Sep 24, 2024
12 checks passed
noresources added a commit to depinxi/premake-core that referenced this pull request Nov 8, 2024
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.

3 participants