search

premake / premake-xcode

BSD 3-Clause "New" or "Revised" License
9 stars 15 forks source link

Correct usage of deprecated magic-variable in xcode.newid #19

Closed jeremyong closed 9 years ago

jeremyong commented 9 years ago

The arg variable was used in Lua 5.0 to automatically capture variadic arguments into a table. However, this was since deprecated, resulting in the pairs function attempting to consume nil and subsequently crashing.

jeremyong commented 9 years ago

I wasn't sure how to run the test suite so if you could advise on that, I would appreciate it. Furthermore, I'm not sure how the Lua code actually gets embedded into the final executable and what controls what version of the Lua runtime is bundled. Either way, this fix addresses the recent build for me locally when creating a simple project on OSX for Xcode4.

samsinsane commented 9 years ago

To run locally, you will need to run premake test from the root of the premake-core directory. The premake5.lua file contains the action that kicks off the test suite. Similarly, if you want to embed the Lua scripts, you can run premake embed, which will generate the scripts.c file for compilation. I haven't worked much with the modules that are included in premake-core, but I believe that if you make changes to the scripts, you can bypass the embed step.

@starkos this PR LGTM, but I'm unsure how this crash managed to make it through the test cases (maybe a couple of tests are required?). It looks like this (premake/premake-core#244) PR from core caused the issue with the undefines in luaconf.h.

jeremyong commented 9 years ago

I believe the reason the test suite passed is because the newid function is monkey patched. Thanks for the explanation for how to run the test suite. I'll test it when I get a chance.

jeremyong commented 9 years ago

See https://github.com/premake/premake-xcode/blob/master/tests/test_xcode4_project.lua#L20

starkos commented 9 years ago

LGTM

jeremyong commented 9 years ago

Did you want me to run the test suite before merging? Looking at the code, it's unlikely to have any effect on the test result. If you wanted to make the test more robust, you could store the canonical xcode.newid and have the monkey patched xcode.newid call the old one first before doing anything

samsinsane commented 9 years ago

All good, merging. Thanks! :)