search

swarn / fzy-lua

A lua implementation of the fzy fuzzy matching algorithm
MIT License
69 stars 6 forks source link

ci: fix Windows tests + enable for PRs #9

Closed mrcjkb closed 9 months ago

mrcjkb commented 9 months ago

See https://github.com/swarn/fzy-lua/pull/8#issuecomment-1839161863.

It appears busted 2.2.0 doesn't work on Windows.

mrcjkb commented 9 months ago

Also, this PR enables running the tests on PR.

swarn commented 9 months ago

Also, this PR enables running the tests on PR.

Thanks, I was going to ask that you add this.

mrcjkb commented 9 months ago

Nope, doesn't fix it. I'll try installing luaFileSystem

mrcjkb commented 9 months ago

Hmm, busted test/fzy_spec.lua also fails on Windows. @swarn can you rerun one of your previous builds to see if it's a flake caused by a recent version of busted?

swarn commented 9 months ago

I'm not that familiar with github's CI. How do I manually rerun a previous workflow? I can see the results in the old actions, but with no failure, there's no "rerun tests" button.

mrcjkb commented 9 months ago

I'm not that familiar with github's CI. How do I manually rerun a previous workflow? I can see the results in the old actions, but with no failure, there's no "rerun tests" button.

I'll open another PR that reverts my changes and adds pull_request.

mrcjkb commented 9 months ago

I'm not that familiar with github's CI. How do I manually rerun a previous workflow? I can see the results in the old actions, but with no failure, there's no "rerun tests" button.

I'll open another PR that reverts my changes and adds pull_request.

Yup, it's a flake :sob:

mrcjkb commented 9 months ago

I'll try replacing hererocks with hishamhm/gh-actions-luarocks, which has support for Windows.

mrcjkb commented 9 months ago

I don't get it... Why on earth has it started compaining about the strings.h include?

mrcjkb commented 9 months ago

Looks like the strings.h include is redundant?

...but luarocks test is still failing on Windows with the same error. I will try pinning older luarocks versions...

mrcjkb commented 9 months ago

Well, luarocks 3.3.0 is the oldest version that works with the GitHub action :disappointed:

mrcjkb commented 9 months ago

@swarn at this point I'm all out of ideas. My suggestion would be to disable tests for Windows for now and file a bug report with luarocks or busted.

Also, I'm not 100 % sure if the strings.h include is redundant or not. It seems to compile just fine without it. Keeping it might cause failures on Windows.

I seem to be hitting rate limits again, so I'll take a bit of a break.

mrcjkb commented 9 months ago

Current status:

I have the gut feeling it'll be easier to get this working without hererocks.

swarn commented 9 months ago

Yeah, I'm noticing that hererocks is unmaintained for five years, probably time to switch.

I see you have comments on https://github.com/leafo/gh-actions-luarocks/pull/14 last week about build-time paths not found. This is a runtime path error; is it related?

This was all working a year ago, I suppose I should have pinned more versions...

mrcjkb commented 9 months ago

I see you have comments on leafo/gh-actions-luarocks#14 last week about build-time paths not found. This is a runtime path error; is it related?

It could be something similar. But I don't think it's those paths I commented on - it's working for other lua versions. And correcting those environment variables didn't help solve my problem.

swarn commented 9 months ago

Well, thanks for the effort. I'll merge this now. The code hasn't changed in years, so I'm not too worried about skipping Windows tests for the moment.

If you don't mind, can I ask: are you using this library for something? Neovim and Roblox seem to be the main use cases.

mrcjkb commented 9 months ago

Well, thanks for the effort. I'll merge this now. The code hasn't changed in years, so I'm not too worried about skipping Windows tests for the moment.

No problem 😄

If you don't mind, can I ask: are you using this library for something? Neovim and Roblox seem to be the main use cases.

We intend to use it for rocks.nvim, a neovim package manager that uses luarocks. Right now, it has auto-completion for packages to install/prune and it would be nice to make it fuzzy 😅

swarn commented 9 months ago

Cool!

swarn commented 9 months ago

@mrcjkb If you're curious, I got this working.

The problem was the compiler. The windows-latest image comes with mingw. LuaRocks finds and attempts to use mingw. Somehow, it's using the wrong flags — the lua runtime finds the DLL files, but doesn't see that they're valid modules.

Adding the MSVC install action fixes it. LuaRocks uses MSVC and the DLLs work fine. I know you added this action, so I'm not sure what else was going on.

I'm also not sure what changed between now and last January, when the workflow last ran. Either luarocks or the windows-latest changed in some way.

mrcjkb commented 9 months ago

Nice! Thanks for the update. That's definitely useful information. Looks like the hererocks installation picks up the toolchain correctly, but the GH action doesn't...

Btw, I've added fuzzy autocompletion to rocks.nvim and it works really nicely :smile: