Skip to content

fix(build): clean up system_lua option - #239

Open
robbielyman wants to merge 2 commits into
mainfrom
push-sovkyvnkmwuq
Open

robbielyman wants to merge 2 commits into
mainfrom
push-sovkyvnkmwuq

Conversation

@robbielyman

Copy link
Copy Markdown
Collaborator

This change removes -Dsystem_lua in favor of -fsys=system_lua. In order to find Lua, the user should pass --search-prefix {dir}, where Lua is installed in {dir}/lib. In the happy case that lua.h is present as {dir}/include/lua.h, no further action is needed. Otherwise (for example if lua.h is {dir}/include/lua/lua.h), the user should pass the path to the directory containing lua.h as an argument to -Dadditional_system_headers.

If -fsys=system_lua is present, the default for -Dshared is now true. Also, rather than panicking the configure stage, I added addFail steps.

@robbielyman

Copy link
Copy Markdown
Collaborator Author

Since this change is hard to test with CI, here is how I tested it: I have Lua 5.5 installed via homebrew at /opt/homebrew/lib/liblua5.5.dylib and, e.g. /opt/homebrew/include/lua/lua.h. I removed pkg-config (probably there are less extreme ways to test, but I was lazy) and compiled Ziglua as

zig build test -fsys=system_lua --search-prefix /opt/homebrew -Dadditional_system_headers=/opt/homebrew/include/lua

This change therefore does not remove additional_system_headers, since I figure a nonzero number of system installs will put lua.h in a funny spot. Interestingly, attempting to cover for that by changing #include "lua.h" to #include "lua/lua.h" didn't work.

Comment thread build.zig Resolved
Comment thread build.zig Resolved
@robbielyman

This comment was marked as outdated.

This change removes `-Dsystem_lua` in favor of `-fsys=system_lua`. In
order to find Lua, the user should pass `--search-prefix {dir}`, where Lua
is installed in `{dir}/lib`. In the happy case that `lua.h` is present as
`{dir}/include/lua.h`, no further action is needed. Otherwise (for example if
`lua.h` is `{dir}/include/lua/lua.h`), the user should pass the path to the
directory containing `lua.h` as an argument to `-Dadditional_system_headers`.

If `-fsys=system_lua` is present, the default for `-Dshared` is now `true`.
Also, rather than panicking the configure stage, I added `addFail` steps.
@natecraddock

Copy link
Copy Markdown
Owner

Just tested this on Linux and it works well!

~/dev/sandbox/ziglua-test $ zig build -Doptimize=small
~/dev/sandbox/ziglua-test $ ldd zig-out/bin/test
	linux-vdso.so.1 (0x00007ffeb58a9000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007e7c65ded000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007e7c65a00000)
	/lib64/ld-linux-x86-64.so.2 (0x00007e7c65eec000)
~/dev/sandbox/ziglua-test $ ll zig-out/bin/test
-rwxrwxr-x 1 nathan nathan 342K Oct 30 23:05 zig-out/bin/test*

~/dev/sandbox/ziglua-test $ zig build -fsys=lua -Doptimize=small
~/dev/sandbox/ziglua-test $ ldd zig-out/bin/test
	linux-vdso.so.1 (0x00007ffeaa4e6000)
	liblua5.4.so.0 => /lib/x86_64-linux-gnu/liblua5.4.so.0 (0x0000720787220000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x0000720787000000)
	/lib64/ld-linux-x86-64.so.2 (0x0000720787277000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x0000720786f17000)
~/dev/sandbox/ziglua-test $ ll zig-out/bin/test
-rwxrwxr-x 1 nathan nathan 136K Oct 30 23:05 zig-out/bin/test*

This branch has not been deployed

No deployments
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