add rockspec

[teto]

• add rockspec: sqlite is a dependency but I chose not to expose it
• hack to fix tests: wouldn't it be simpler to use the envvar in all cases ?

[kkharji]

hey @teto, do you mean to say that this env var is auto set? or users need to set it manually in there zsh/bash config?

[kkharji]

Currently the ci tests failing for two reason:

• upper/lower type names: Had this issue for awhile, I guess we need to make sure that is "always" lower case.
• test/auto/stmt_spec.lua:96: types no longer return in ci, maybe this should just be ignored/commented out

[teto]

do you mean to say that this env var is auto set? or users need to set it manually in there zsh/bash config?

I tried to make luarocks test succeeds and it was failing because of that code: sqlite_clib_path is not set in the minimal_test.vim. It was not clear how to pass the value to vim while exporting the environment variable could fix it and removed one code path.
But I would understand if you prefer to keep it the current way, maybe the vim code could check the environment variable too ?

unrelated but would also be nice to have a vim.health script to check sqlite is correctly loaded.

[kkharji]

I tried to make luarocks test succeeds and it was failing because of that code: sqlite_clib_path is not set in the minimal_test.vim.

In that case, how about if not vim, try to read path from env var? or regardless if not set then read from env var. That way users may have the freedom to pick either one.

unrelated but would also be nice to have a vim.health script to check sqlite is correctly loaded.

Definitely it would, can you open an issue so we can later implement

[teto]

this should be more acceptable.

upper/lower type names: Had this issue for awhile, I guess we need to make sure that is "always" lower case.
test/auto/stmt_spec.lua:96: types no longer return in ci, maybe this should just be ignored/commented out

sry I didn't check those, could you rerun the workflow so I have a look please ?

[kkharji]

LGTM, I will fix test later, in another pr along with ci issues

[teto]

I've added a makefile target to run stylua locally so that I could fix the issues. The problem is that the stylua version I use locally may be different than the one on the CI. Since you are a nix user, would you mind if I added a flake just for development ? to get lualint/stylint and correctly export LIBSQLITE ?

[kkharji]

I've added a makefile target to run stylua locally so that I could fix the issues. The problem is that the stylua version I use locally may be different than the one on the CI. Since you are a nix user, would you mind if I added a flake just for development ? to get lualint/stylint and correctly export LIBSQLITE ?

Not at all. I like where this is going.

[teto]

I've added the flake, running nix develop followed by luarocks test seems to pass. I wouldn't mind this being merged if only so that the github workflows get run automatically in followup PRs ^^'

[kkharji]

I'm seeing luacheck and style lua still failing but for absolute nonsense it should have been warning instead of errors

[kkharji]

Thanks @teto

[kkharji]

Oh wait, sqlite.lua doesn't depend on plenary only on luv, have you seen script/gen_rockspec.lua

[teto]

I think I had seen it then forgot about it when I resumed the PR xD I've submitted an update that adds plenary as a test dependency though. It won't work because plenary is not in the root manifest of luarocks.org but I think this can be done with the help of @tjdevries

[kkharji]

😆 no worries, @teto what do u mean by it won't work .., isn't plenary up in luarocks and https://github.com/nvim-lua/plenary.nvim/blob/master/plenary.nvim-scm-1.rockspec? or is it having as test dependency breaking t?

[teto]

luarocks test ./sqlite-master-0.rockspec 
Missing dependencies for sqlite master-0:
   plenary.nvim (not installed)

sqlite master-0 depends on plenary.nvim (not installed)
Installing https://luarocks.org/plenary.nvim-scm-1.rockspec

fails .
adding my manifest fixes it (https://luarocks.org/modules/teto/plenary.nvim) but it is not scalable so I wonder if I should manage plenary myself or if @tjdevries has enough time/interest to do it (or another maintainer) .

I think it's fine to merge as is as luarocks install ./sqlite-master-0.rockspec --tree /tmp keeps working

[teto]

I've just pushed a linting fix. I think it would be best to have the CI use the flake to run the code rather than CI actions. We would have the same results locally as on github + it would work on other CI/CD pipelines without limiting oneself to github. but that's a suggestion.

[kkharji]

Yes, the CI setup was pre getting into nix. this would make things a lot simpler and won't have this out sync issue.