chore(ci): macos-latest + windows-latest #17
Conversation
Tieske
force-pushed
the
ci-win
branch
9 times, most recently
from
7e39efd to
00cceff
Compare
I couldn't track down which scenarios cause this, but it has happened on Windows: See: lunarmodules/luasystem#17 See: https://github.com/lunarmodules/luasystem/actions/runs/7907096563/job/21583369125?pr=17
|
@Tieske no idea what's causing this error, unfortunately, but I've added a mitigation to the crash that should hopefully provide more diagnostics if it happens again: luarocks/luarocks@8d0df02 It's probably about time I push a new LuaRocks release, too... |
|
The Windows dependency actually built this time, but the Windows bindir wasn't added to the PATH... 🤔 |
|
which component is responsible for adding the BinDir? I saw the luarocks action does set the path, but somehow doesn't work. The error is a PowerShell one, so seems the commands are executed in PowerShell. Does the action update the correct paths? |
|
this looks correct; https://github.com/hishamhm/gh-actions-luarocks/blob/master/main.js#L121 |
Yeah, this was supposed to be the one to do it. I have no idea why this didn't work |
|
Can you work around it by explicitly setting the PATH in your CI script? It would be interesting to know if that at least works... (If that doesn't work, then it's a sign of something more deeply broken in the luarocks installation...) |
|
So I added some debugging, but it (the busted executable) seems to be gone.... Here's the log: https://github.com/lunarmodules/luasystem/actions/runs/8161889298/job/22311635078?pr=17#step:6:13 Here's an edited version with commands and results:
|
|
hmmm... this entry |
|
Also not the case;
|
|
so there are 3 path entries for Lua, 1 doesn't exist in the system, the other 2 do not have the Got any follow ideas to look into @hishamhm ?? |
|
so here's more:
This is weird. It says it is installed, but doesn't list a single installed file. And hence also not the executable. Here's what I get locally: |
hishamhm
force-pushed
the
ci-win
branch
3 times, most recently
from
96ae9ae to
b6f97b1
Compare
|
managed to add Dependencies utility to check the exports, but they seem fine: I thought maybe the weird error was because it couldn't find the function to open the lib. |
Yeah, that's the mystery... Thanks for making the local build for comparing with the logs... the cl.exe and link.exe command line look effectively identical, as far as I can tell. |
|
Oh, looks like this may be the cause: https://github.com/leafo/gh-actions-lua/pull/38/files |
including various pending PRs from leafo/gh-actions-lua
|
@Tieske ...and we got a green build at last!! I pulled these four PRs into my fork of gh-actions-lua:
(I didn't pull leafo/gh-actions-lua#44 because that seems like a deeper change, which may also need fixing conflicts with the other ones, and I'm not sure if that would work out of the box anyway — I didn't test it, just wanted the issue we have at hand here to get over with.) I had to delete the cache for the Lua 5.4 windows build manually. And it looks like the cache key in gh-actions-lua doesn't check for gcc vs msvc. It probably needs a super-specific cache key with all of the msvc variants stuff... LuaRocks is also bad at this regard, we just call everything "win32" and only differ on CPU arch... so heads up on that. Another thing to note is that I think the combo luajit+msvc isn't supported by these actions yet, because we're using the Makefile to build luajit. (And so does leafo/gh-actions-lua#44, so that PR wouldn't help on this front either). But I think we're in a pretty good shape to get this PR cleaned up and your luasystem CI set up. And I think I'm looking good for releasing LuaRocks 3.11 as well! |
|
I'd say just clean up and merge using the forks as-is for the time being. If anything else happens with the actions, you can always get back here and have them point elsewhere. |
|
@Tieske do you have anything else you need to change here? I think even keeping the Dependencies Analyzer stuff around should be fine (I've gated it for the msvc run only). So I'd say how about we take this PR, squash it into a single commit and merge? You can always add more CI jobs later if you feel like it. |
|
over the weekend I tried adding also a MinGW job, but that failed. Ideally we'd have jobs for msvc and MinGW. Haven't looked into the cache key issue you mentioned before yet, maybe that's the culprit. Also; wondering if we should keep this as 1 workflow, or have separate ones for posix-like and Windows. |
|
The existing one for "windows-latest, luajit" is mingw already, isn't it? it's not MSVC, and uses gcc on Windows. |
|
using different Lua versions for the mingw and msvc builds avoids the cache issue for now. I can see the appeal of testing every NxM combination but the current list already provides quite decent coverage. |
to keep the caches not mixed (cache-key doesn't include toolchain)
|
ok, tried that, but now it keeps looking for the msvc toolchain; https://github.com/lunarmodules/luasystem/actions/runs/8297960620/job/22710219967?pr=17 Added a mingw build for a different Lua version. In the logs you can see that the 2 msvc specific steps are skipped (as they should). But it still looks for the |
|
ok, so turns out msvc is required to build Puc Rio Lua versions by the action, So no MinGW for those versions. So the LuaJIT builds will test the MinGW toolchain, since they are build using that one. And the PuC Rio ones will use MSVC. I think that should suffice. |
Co-authored-by: Hisham Muhammad <[email protected]>
testing ci additions