Skip to content

Conversation

@mentatbot
Copy link

@mentatbot mentatbot bot commented Apr 14, 2025

This PR modifies the approach to use Homebrew-installed Luau instead of the internal headers. Key changes:

  1. Modified GitHub workflow to install Luau via Homebrew and configure paths correctly
  2. Simplified lfs.c to use standard includes without compatibility layer
  3. Updated CMake configuration to find and use Homebrew Luau

This approach should solve previous compilation issues while making the code simpler and more maintainable.


🤖 See my steps and cost here

  • Wake on any new activity.

This PR modifies the approach to use Homebrew-installed Luau instead of the internal headers. Key changes:

1. Modified GitHub workflow to install Luau via Homebrew and configure paths correctly
2. Simplified lfs.c to use standard includes without compatibility layer
3. Updated CMake configuration to find and use Homebrew Luau

This approach should solve previous compilation issues while making the code simpler and more maintainable.
@mentatbot mentatbot bot requested a review from stayaway245 April 14, 2025 05:37
@mentatbot
Copy link
Author

mentatbot bot commented Apr 14, 2025

You are out of Mentat credits; I won't iterate on CI failure. You can view and refresh your credits here.

- Changed includes in lfs.c to use standard Lua headers (lua.h, lauxlib.h, lualib.h)
- Added better library detection in GitHub workflow to find actual Luau library path
- Enhanced CMake Lua finder to try multiple library names and paths
- Added diagnostic output to help troubleshoot the build process

These changes should fix the 'luau.h' file not found error and ensure the build can correctly locate the Homebrew Luau installation regardless of its exact structure.
@mentatbot
Copy link
Author

mentatbot bot commented Apr 14, 2025

I've updated the PR with fixes for the build failure. The main changes:

  1. Fixed the include statements in lfs.c:

    • Changed from <luau.h> to standard Lua headers: <lua.h>, <lauxlib.h>, <lualib.h>
    • It appears Homebrew Luau uses standard Lua naming conventions for its headers
  2. Added better library detection in the GitHub workflow:

    • Now inspects the actual Homebrew Luau installation structure
    • Uses find to locate the actual library file (*.dylib or *.a)
    • Provides detailed diagnostic output to help troubleshoot
  3. Enhanced the CMake FindLua.cmake module:

    • Tries multiple library names (luau, lua, liblua, libluau)
    • Uses GLOB to find any .dylib or .a file in the lib directory
    • Provides multiple fallbacks for different potential library paths

These changes should fix the 'file not found' error and make the build process more robust by handling variations in how Homebrew might install Luau.

@mentatbot
Copy link
Author

mentatbot bot commented Apr 14, 2025

You are out of Mentat credits; I won't iterate on CI failure. You can view and refresh your credits here.

This approach solves the header file not found errors by:

1. Using internal Luau headers from source/cpp/luau instead of trying to find Homebrew headers
2. Adding multiple include paths to compiler flags to ensure all necessary headers are found
3. Still using the Homebrew Luau library for linking
4. Adding compatibility defines for better Lua 5.1 API compatibility

This should fix the build errors while keeping the advantages of using Homebrew Luau.
@mentatbot
Copy link
Author

mentatbot bot commented Apr 14, 2025

I've changed the approach to solve the build issues. After analyzing the failures, it seems the Homebrew Luau headers might not be properly detected or installed in the standard include paths.

New approach:

  1. Use internal headers, Homebrew library - Use the Luau headers from the project's source tree (source/cpp/luau/) while still using the Homebrew Luau library for linking
  2. Multiple include paths - Added several include paths to the compiler flags to ensure all headers can be found
  3. Added compatibility definition - Added LUA_COMPAT_5_1=1 to maintain compatibility with Lua 5.1 APIs

This hybrid approach gives us the best of both worlds:

  • We know the headers will be found since they're directly in the repo
  • We still benefit from the Homebrew Luau library installation
  • We don't need the complex compatibility layer from the original implementation

This should be more robust and reliable for the build process while achieving the original goal of moving away from a complex custom compatibility layer.

@stayaway245 stayaway245 merged commit 7c51738 into main Apr 14, 2025
1 check failed
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.

1 participant