include dir removed from build artifacts

[mrcjkb]

In version 548.0.0, the include directory was removed from the build artifacts: 41e9c03

What's the reason for this change?
We've been using lua-src-rs in the lux package manager to be able to compile Lua libraries.
With the latest release (which has this change), it looks like we can no longer use it.

[khvzak]

In the beginning of mlua, it used include_dir to get constants using a small C program. This did not work well with cross-compilation so few years ago I switched to hardcoded constants (they are standard in 99% of cases and custom Lua builds with different constants is rather a big headache and not something I'm willing to support).
I did not touch lua-src for a while and only recently removed some legacy parts not needed for mlua.

I'm surprised that someone uses include_dir! But seems your use case is valid and I'm happy to return this back.
Do you need this only for Lua (this crate) or for luajit as well?

[mrcjkb]

I'm surprised that someone uses include_dir! But seems your use case is valid and I'm happy to return this back.

That would be awesome! Thanks 🙏

We are also hoping to eventually be able to package Lua binaries (e.g. for repl purposes). If that's outside the scope of lua-src, maybe we should maintain our own fork?
What do you think? Of course, we'd also be more than happy to contribute and help with maintenance 😀

Do you need this only for Lua (this crate) or for luajit as well?

Yes, we use the luajit-src crate too.

[khvzak]

I added Artifacts::include_dir() method back.

I also noticed that you use (also removed) Build::host method.
Could you confirm that you still need it? The cc crate says that this option is automatically set based on HOST environment variable.

For lua-src only target is important because it has special logic that depends on target.

maybe we should maintain our own fork?

If you use mlua (and seems you do), it uses lua-src, so any fork would not be compatible with it. With fork you'll end up using two different source packages.

We are also hoping to eventually be able to package Lua binaries

Could you provide more details?

[mrcjkb]

Thanks 🙏

I'm not sure if we need .host, I'll have to run it though our test suite. I think we do, because we don't use lua-src in a build script.

Could you provide more details?

In a lux or luarocks project, you can specify the lua version(s) your project is meant to be built for.
Currently, lux uses pkg-config to find an installed Lua library for the requested lua version.
If it can't find one, it falls back to using lua-src or luajit-src to build it.
This happens at runtime, which is why I don't think a fork would conflict with mlua - and we probably shouldn't be using something that panics on failure.

lux-cli also has two commands: run and lua. The former runs an entrypoint (a lua script) and the latter drops you into a lua repl. In both cases, lux sets LUA_PATH and LUA_CPATH to include your project dependencies or installed packages for you.
This command currently requires a lua installation that was detected with pkg-config and has a bin directory containing the lua executable.
If it can't find a lua binary for the requested lua version, it'll error.

The problem we've encountered is that most users only have Lua 5.4 installed, as that's what their distro's package manager defaults to. So if they want to run/repl for a project that needs Lua 5.1, they currently have to install it manually.
This is a pain, especially for macOS/homebrew and Windows users.
So we are considering falling back to building the bin/lua binary for the requested lua version from source.

[khvzak]

I'm about to release v548.1.0 which should solve most (if not all) of the issues.
I added more options to the build configuration. Setting host is not required if target is set.
Also you can try try_build method that returns an error instead of panicking.

So if they want to run/repl for a project that needs Lua 5.1, they currently have to install it manually.
This is a pain, especially for macOS/homebrew and Windows users.
So we are considering falling back to building the bin/lua binary for the requested lua version from source.

I'm happy to review a PR to add this functionality. This probably will be more complicated than just building a lua.c as you may want to have shared Lua libraries to support loading C modules (especially on Windows).

[mrcjkb]

Thanks again 🙏

I'm happy to review a PR to add this functionality. This probably will be more complicated than just building a lua.c as you may want to have shared Lua libraries to support loading C modules (especially on Windows).

If you're open to that, I'll look into it.
Thanks for the pointers.

[mrcjkb]

Hmm something seems to be broken when using lua-src v548.1.0 on Windows/msvc.

I'm getting a bunch of linker errors that I don't get with v547.0.0:

LNK2001: unresolved external symbol lua_pushstring\r\ned4e32051b93abbf-bitflags.o : error LNK2001: unresolved external symbol lua_pushboolean\r\ned4e32051b93abbf-core.o : error LNK2001: unresolved external symbol lua_pushboolean\r\ned4e32051b93abbf-time.o : error LNK2001: unresolved external symbol lua_pushboolean\r\ned4e32051b93abbf-environment.o : error LNK2001: unresolved external symbol 

Any idea what that could be caused by?
I haven't tried switching to try_build yet.

[mrcjkb]

@khvzak could the linker error on Windows potentially be caused by the change where all .c sources are added:

.add_files_by_ext(&source_dir, "c")

It looks like this is the only remaining logic change in 41e9c03 and I think some of the sources should only added for certain Lua versions:

       match version {
            Lua51 => {}
            Lua52 => {
                config
                    .file(source_dir.join("lbitlib.c"))
                    .file(source_dir.join("lcorolib.c"))
                    .file(source_dir.join("lctype.c"));
            }
            Lua53 => {
                config
                    .file(source_dir.join("lbitlib.c"))
                    .file(source_dir.join("lcorolib.c"))
                    .file(source_dir.join("lctype.c"))
                    .file(source_dir.join("lutf8lib.c"));
            }
            Lua54 => {
                config
                    .file(source_dir.join("lcorolib.c"))
                    .file(source_dir.join("lctype.c"))
                    .file(source_dir.join("lutf8lib.c"));
            }
        }

[khvzak]

add_files_by_ext should add all files found in a directory for specific Lua version (previously the list of files was provided manually).

I cannot reproduce the issue yet, even in mlua in CI, the linking process works fine using x86_64-pc-windows-msvc target.

When you compile in standalone mode (not in build script), how do you link the Lua library?

[mrcjkb]

I think I've found the cause. The lib is now added to build_dir and the includes are added to a subdirectory.
MSVC doesn't seem to like that.