-
Notifications
You must be signed in to change notification settings - Fork 15
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
Windows support #14
base: master
Are you sure you want to change the base?
Windows support #14
Conversation
`luarocks path` on its own does not set the paths, it just displays the variables to be set. `luarocks config` is a more useful test as it prints the whole internal configuration.
Any progress here? |
@leafo You can see it running on my fork's CI here: https://github.com/hishamhm/gh-actions-luarocks/actions/runs/2758104388 |
So this PR could be merged? |
Up — can this be merged? Seems like it should address #17 and I'd need that. |
Up? |
@hishamhm Could you make a release on your fork? Then we could simply use your version for the moment. |
lrCpath = lrCpath.trim() | ||
|
||
if (luaPath != "") { | ||
core.exportVariable("LUA_PATH", ";;" + luaPath) | ||
if (lrPath != "") { | ||
core.exportVariable("LUA_PATH", ";;" + lrPath) | ||
} | ||
|
||
if (luaCpath != "") { | ||
core.exportVariable("LUA_CPATH", ";;" + luaCpath) | ||
if (lrCpath != "") { | ||
core.exportVariable("LUA_CPATH", ";;" + lrCpath) | ||
} |
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.
@hishamhm I've been using your fork, and this seems to generate a unix style path (with semicolons as separators, instead of colons).
> printenv
LUA_PATH=;;C:\Users\runneradmin\AppData\Roaming/luarocks/share/lua/5.1/?.lua;C:\Users\runneradmin\AppData\Roaming/luarocks/share/lua/5.1/?/init.lua
PATH=/c/Program Files/PowerShell/7:/c/Users/runneradmin/AppData/Roaming/luarocks/bin:/d/a/rocks-binaries/rocks-binaries/.luarocks/bin:/d/a/rocks-binaries/rocks-binaries/.lua/bin:/c/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/Common7/IDE/Extensions/Microsoft/IntelliCode/CLI:/c/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Tools/MSVC/14.29.30133/bin/HostX64/x64:/c/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/Common7/IDE/VC/VCPackages:/c/Program Files (x86)/Microsoft Visual
...
As a result (I think), build-time dependencies can't be found (e.g. when installing toml-edit
, which depends on luarocks-build-rust-mlua
).
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.
@mrcjkb did you solve this issue?
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.
No, not yet.
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.
@mrcjkb LUA_PATH uses ;
on both Windows and Unix, and PATH uses :
on Unix and ;
on Windows. I don't think the semicolons are the cause of your issue, but there were issues related to build dependency lookup that were fixed in 3.10.0.
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.
🙏 I can confirm it works when I add
with:
luarocksVersion: "3.10.0"
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.
Actually, no it doesn't. I was looking at the Linux builder.
The Windows builder still fails when trying to use the luarocks-build-rust-mlua
build backend:
Error: Failed initializing build back-end for build type 'rust-mlua': module 'luarocks.build.rust-mlua' not found:
No LuaRocks module found for luarocks.build.rust-mlua
https://github.com/nvim-neorocks/rocks-binaries/actions/runs/8168782585/job/22331528480#step:11:1256
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.
https://github.com/nvim-neorocks/rocks-binaries/actions/runs/8168782585/job/22331528480#step:11:1256
Ok, this looks like the same problem as the one @Tieske is facing in lunarmodules/luasystem#17
I spent all night digging that down yesterday. I'm going to push a "preview build" later today so we can test if I got it fixed. I'll ping back here when I have more info!
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.
@mrcjkb Please try again bumping the version number to 3.11.0
! I've uploaded a pre-release build but I haven't announced it yet. 🤫 😉
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.
That has fixed it! 🙏 🙏 🙏
🎉 🚀
@hishamhm , trying your fork off the tip of Edit: fixed by moving the Lua/Luarocks install steps after a repo checkout. |
@magneto538 no idea, sorry! it seems to be working for others, so I don't even know what to suggest... |
Great work on the windows support and thank you for your fork @hishamhm - everything seems to work perfectly except when I try to require Here is my workflow: on:
workflow_dispatch:
jobs:
test-luarocks-loader:
name: Test LuaRocks Loader
runs-on: windows-latest
steps:
- name: Setup Microsoft Visual C++ Developer Command Prompt
uses: ilammy/[email protected]
- name: Setup Lua
uses: leafo/[email protected]
- name: Setup LuaRocks
uses: hishamhm/gh-actions-luarocks@master
with:
luarocksVersion: 3.11.0
- name: Test require luarocks loader
run: lua -e "require('luarocks.loader');" And here is the output of the last step above:
I'm not really using the luarocks loader so it's not a critical issue, just noticed it and wanted to let you know. EDIT:Okay I just noticed that requiring my own library after running EDIT 2:Seems like I have found another issue. When running
or:
This could very likely be an issue with |
I have a problem related to packages not found too. My workflow requires the
Log from Environment setup step:
|
@magneto538 are you using @hishamhm s fork? That should fix that issue (see lunarmodules/luasystem#17 (comment)) unless you're hitting a new issue. |
@Tieske Yes, I am, as per the code snippet I've posted. Luarocks config output for completeness:
|
@hishamhm bump in case you missed it ^ |
@magneto538 as a workaround, try adding |
@hishamhm thanks. Now
|
@magneto538 Add |
@hishamhm no luck with that one:
|
@magneto538 he mistakenly gave you the code for Unix, here's how you do it on Windows CMD: luarocks path > "%temp%\_lrp.bat"
call "%temp%\_lrp.bat" && del "%temp%\_lrp.bat" (You can find this command by running Unfortunately I already tried this without success, but you might be luckier 🤞🏻 EDITI have started investigating my issue again and I'm pretty sure that the problem lies within my C module. Maybe I'm not exporting correctly or something is wrong with the EDIT 2The Windows build works in my VM, works when I download it from GitHub Actions, and works on my old Windows PC. It just doesn't work on the GitHub Actions Runner itself, which is a Window Server 2022... No idea. But probably not a bug in hishamhm's fork. |
@jonasgeiler thanks! Actually I did try switching to
So nope, not working for me either. |
@magneto538 GHA defaults to PowerShell as the shell, but you can set it up to switch to @jonasgeiler I originally also thought the dll was wrong, hence we added this: https://github.com/lunarmodules/luasystem/pull/17/files#diff-5c3fa597431eda03ac3339ae6bf7f05e1a50d6fc7333679ec38e21b337cb6721R66 |
@Tieske , I had already tried both PowerShell and cmd to no avail. I reckon I hadn't posted the cmd output here, so here we go: LFS still isn't being picked up inside the Lua script I'm running.
The above fails when executing
|
Thank you for showing me the workflow you use for luasystem! I tried adapting some of your steps and I got the whole thing working once I switched to Here is the commit that finally fixed my problem: Unfortunately I didn't get LuaJIT tests to work on Windows but as long as all the other versions work I think it's fine for now. |
The trick in |
Yeah I did try to skip the MSVC step and force MingW with |
Just added "semver" and ran `make vendor`.
@hishamhm It seems like you've commited the Edit: Wait leafo also added the |
Installs from the LuaRocks source package on Unix and uses the precompiled binaries on Windows.
CI: Test matrix runs it with Lua 5.1 (oldest supported), 5.4 (newest supported) and LuaJIT; being based on gh-actions-lua, it uses MSVC for PUC-Rio Lua runs and Mingw for the LuaJIT run. In principle this action should work with any combination of Lua version and supported Windows compiler, even though gh-actions-lua currently doesn't (it would be rather annoying to support the full Windows matrix on gh-actions-lua because the LuaJIT Makefile works out of the box with Mingw and my handwritten implementation for building PUC Lua targets MSVC and would have to be rewritten for Mingw).
Closes #13.