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

CI: fix the build for LuaJIT and OpenResty on Windows #453

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

luau-project
Copy link
Contributor

@luau-project luau-project commented Mar 5, 2025

Description

Fixed the CI build of luasocket for LuaJIT and OpenResty on Windows. Now, CI is working correctly on all scenarios.

Changes

  1. Currently, most MinGW-w64 providers (like the one used by GitHub) ship a C compiler targeting the Universal C Run Time (UCRT). However, the current LuaRocks version (3.11.1) used by most GitHub Actions incorrectly detects the MSVCRT of LuaJIT and OpenResty on Windows. Thus, MSVCRT was set explicitly to UCRTBASE;

Note

This LuaRocks issue was already fixed by luarocks/luarocks@99c7267. However, it was merged after LuaRocks 3.11.1 release.

  1. LuaRocks is unable to detect the library directory of OpenResty on Windows. Thus, LUA_LIBDIR was set explicitly to the binary directory of Lua for OpenResty on Windows.

Copy link
Member

@alerque alerque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't these values be upstreamed into the GH Actions being used here? (Either @leafo's original or the pseudo-official forks in @luarocks, I'm not sure which one will stay as the canonical upstream.) I appreciate the fix and we could stuff it into this project if need be, but it seems to make more sense to just have it fixed in the action.

@luau-project
Copy link
Contributor Author

Shouldn't these values be upstreamed into the GH Actions being used here?

I'll try to fix it upstream.

Note

Since the upstream merge can take months to happen, I think that incorporating these changes now would benefit everyone submitting PRs to luasocket to get their code well tested on each platform / interpreter. Then, once I get a definitive response upstream, I get back here to downstream the results.

@alerque
Copy link
Member

alerque commented Mar 6, 2025

You might get a faster response upstream than you expect. In any event one way we can fix it here for this project is using your PR branch as the action source for these runs, then switching back to the main repo when it has a release that includes the PR.

@luau-project
Copy link
Contributor Author

You might get a faster response upstream than you expect. In any event one way we can fix it here for this project is using your PR branch as the action source for these runs, then switching back to the main repo when it has a release that includes the PR.

Oh, this seems an interesting solution. I'm investigating this MSVCRT a little deeper before to propose a solution to the maintainers of GH Actions for LuaRocks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants