-
Notifications
You must be signed in to change notification settings - Fork 635
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
feature: MSYS2 support on src/makefile #435
base: master
Are you sure you want to change the base?
Conversation
@luau-project I think better msys2 support is great! Did you test with other Lua versions than 5.1, in particular 5.4? A GitHub workflow would be great too, since I think most contributors to LuaSocket do not have a Windows machine with msys2 set up. I have not used it in a long time and just noticed they provide an official GitHub Action now. |
Hello @catwell . I tested it on every 64 bit environment on MSYS2 (but not the ARM one). I even submitted a pull request on MSYS2 msys2/MINGW-packages#21344 to add luasocket on their repositories based on this makefile changes. As you can see on the MSYS2 PR, it is building fine for every 64 bit env. I didn't test on 32 bit envs though, because I do not have such a machine. However, I'm still confident it does. |
Just a small notice: on MSYS2, the available versions are current Lua (5.4), Lua 5.3 and Lua 5.1. There is no Lua 5.2. |
Hello @catwell . Based on Should I push this workflow? (I'm supposing you have write access to the repository). Just in case, I'll write the workflow here. I'm waiting your signal to push it: name: Build on MSYS2
on:
push:
branches:
- master
pull_request:
jobs:
build:
name: Test ${{ matrix.Lua.version }} from MSYS2 package mingw-w64-${{ matrix.MSYS2.env }}-${{ matrix.Lua.msys2_pkg_name }}
runs-on: windows-latest
strategy:
matrix:
Lua:
# For future updates:
# the fields 'msys2_pkg_name' and 'msys2_lua_exe'
# in the matrix below are always 'lua'
# for the current Lua version .
- { version: '5.4', msys2_pkg_name: 'lua', msys2_lua_exe: 'lua' }
- { version: '5.3', msys2_pkg_name: 'lua53', msys2_lua_exe: 'lua5.3' }
# At the moment, Lua 5.2 is not on MSYS2 repositories.
- { version: '5.1', msys2_pkg_name: 'lua51', msys2_lua_exe: 'lua5.1' }
- { version: '5.1', msys2_pkg_name: 'luajit', msys2_lua_exe: 'luajit' }
MSYS2:
- { sys: ucrt64, env: ucrt-x86_64 }
- { sys: mingw64, env: x86_64 }
- { sys: mingw32, env: i686 }
- { sys: clang64, env: clang-x86_64 }
- { sys: clang32, env: clang-i686 }
defaults:
run:
shell: msys2 {0}
env:
LUA_EXE: /${{ matrix.MSYS2.sys }}/bin/${{ matrix.Lua.msys2_lua_exe }}
steps:
- uses: msys2/setup-msys2@v2
name: Setup MSYS2
with:
msystem: ${{ matrix.MSYS2.sys }}
install: |
base-devel
git
mingw-w64-${{ matrix.MSYS2.env }}-cc
mingw-w64-${{ matrix.MSYS2.env }}-${{ matrix.Lua.msys2_pkg_name }}
- name: Checkout
uses: actions/checkout@v4
- name: Build
if: ${{ !contains(matrix.Lua.msys2_pkg_name, 'luajit') }}
run: |
make -C src \
PLAT=msys2${{ matrix.MSYS2.sys }} \
LUAV=${{ matrix.Lua.version }} \
DEBUG=DEBUG \
all
- name: Build with luajit
if: ${{ contains(matrix.Lua.msys2_pkg_name, 'luajit') }}
run: |
make -C src \
PLAT=msys2${{ matrix.MSYS2.sys }} \
LUAV=${{ matrix.Lua.version }} \
DEBUG=DEBUG \
"MYCFLAGS=$(pkgconf.exe --cflags lua${{ matrix.Lua.version }})" \
all
- name: Install
run: |
make -C src \
PLAT=msys2${{ matrix.MSYS2.sys }} \
LUAV=${{ matrix.Lua.version }} \
DEBUG=DEBUG \
install
- name: Run regression tests
run: |
cd test
${{ env.LUA_EXE }} hello.lua
${{ env.LUA_EXE }} testsrvr.lua > /dev/null &
${{ env.LUA_EXE }} testclnt.lua
${{ env.LUA_EXE }} stufftest.lua
${{ env.LUA_EXE }} excepttest.lua
${{ env.LUA_EXE }} test_bind.lua
${{ env.LUA_EXE }} test_getaddrinfo.lua
${{ env.LUA_EXE }} ltn12test.lua
${{ env.LUA_EXE }} mimetest.lua
${{ env.LUA_EXE }} urltest.lua
${{ env.LUA_EXE }} test_socket_error.lua
kill %1 |
I do have write access to the repository and I don't see a problem with you pushing the workflow. I will probably wait for feedback from @Tieske or @alerque to merge but I can run the workflow in the meantime. By the way, is your nickname in any way related to @luau-lang? |
cda5e78
to
b28f6ce
Compare
Pushed it.
Oh, no. When I created this gh acc, I didn't even know the roblox luau. |
Sorry, it took me some time to run the workflows because the new one didn't appear in the list so I was wondering what I had to do... It turns out when you allow the run they still get picked up and run anyway! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the nitpick about where to put the CI bits this looks fine to me. I'll me taking for word that the Windows side of things is setup properly because I have no relevant experience or local testing opportunities.
.github/workflows/msys2.yml
Outdated
pull_request: | ||
|
||
jobs: | ||
build: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add these builds as a new job under the existing Build workflow instead of putting them in a different workflow file? Conceptually what is being tested here seems to be a pretty good match for what is is there. I'm not sure if it makes sens to merge them as part of a single matrix for one job or keep it as two jobs with their own matrices but at least being part of the same workflow makes more sense to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
b28f6ce
to
433caf5
Compare
Thanks for the contribution. This should become available via the dev (scm) rockspec soon. I don't know when the next release will be, honestly I haven't assessed where this project is at or what it needs before the next tag in a while and I'm kinda busy at the moment. If anybody has input on that ("the SCM version works fine, just send it", "suggest cleaning up X first", "X and Y PRs should land first) I'd be happy to hear input on another issues. |
Description
The changes in the
src/makefile
allows to build luasocket on MSYS2 shells with the compiler toolchain for each environment:mingw32
mingw64
ucrt64
clang32
clang64
clangarm64
How to test the changes
Initial setup of tools (only once)
ucrt64
on Windows start menu to open a MSYS2 shell for the ucrt64 environment listed above;/tmp
directory and clone my branchTest
Note
Lua C modules get installed at
/ucrt64/lib/lua/5.1
and .lua files at/ucrt64/share/lua/5.1
for the ucrt64 environment, in case you want to remove them.Extra
If you guys want, I can contribute a Github workflow to test luasocket on MSYS2 tools.