teto/haskell tools nvim enable tests

[teto]

• luaPackages.nlua: disable patchShebangAuto
• luaPackages.haskell-tools-nvim: enable tests

Description of changes

cc @mrcjkb

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[teto]

I dont know if it's an ideal we should strive for as it's pretty hypthetical in the current state of the ecosystem but it should be nice if we could just run luarocks test for all lua packages.

Do you think it's something achievable with haskell-tools-nvim ? it might already be the case I haven't tested.
Adding a .busted file that sets lua=nlua might be enough ? https://lunarmodules.github.io/busted/#overview
luarocks test wil detect the file and run busted automatically ?

[mrcjkb]

but it should be nice if we could just run luarocks test for all lua packages.

I think there will still be quite a few neovim plugins that use plenary or something else, but it should be possible to run it automatically if there's a .busted file.

Adding a .busted file that sets lua=nlua might be enough?

That night work, if nlua is added to the rockspec's test_dependencies. I'll try it out in haskell-tools later.
On the Nix side, we would have to modify luarocks-nix to add test_dependencies to the nativeCheckInputs.

[teto]

great. luarocks-nix should already add test_dependencies to checkInputs though on the nix side it may need some adjustements. If you do the change in a haskell-tools-nvim, I can try it out and make the required changes in nixpkgs

Anyway waiting for #294057 (comment) to get merged before merging this

[teto]

so this worked in a devShell but because I had #300905 in my local fork so we need to wait for that to reach master.
Getting it to work in sandbox was tricker than I thought because disabling patchShebang (which removes the nvim call and breaks nlua) leaves /usr/bin/env untouched which is not available in sandbox.
So I decided to hardcode the nlua shebang instead, which should work once #300905 is available.

[teto]

Result of nixpkgs-review pr 302147 run on x86_64-linux 1

11 packages built:

• lua51Packages.haskell-tools-nvim
• lua51Packages.nlua
• lua52Packages.haskell-tools-nvim
• lua52Packages.nlua
• lua53Packages.haskell-tools-nvim
• lua53Packages.nlua
• lua54Packages.haskell-tools-nvim
• lua54Packages.nlua
• luajitPackages.haskell-tools-nvim
• luajitPackages.nlua
• vimPlugins.haskell-tools-nvim

[teto]

I had to enable the test only for lua5.1 else it would fail for missing dependencies. I haven't looked much into it, I suppose since lua is the interpreter, it might be looking in the wrong folders (eg with "5.1" in its paths) while the installed dependencies are named "5.3" and so on. Perfection being the enemy of good, let's improve this incrementally. Merging once darwin is confirmed to work

[mrcjkb]

I had to enable the test only for lua5.1 else it would fail for missing dependencies. I haven't looked much into it, I suppose since lua is the interpreter, it might be looking in the wrong folders (eg with "5.1" in its paths) while the installed dependencies are named "5.3" and so on. Perfection being the enemy of good, let's improve this incrementally. Merging once darwin is confirmed to work

🤔 Maybe neovim plugins shouldn't have lua > 5.1 listed as supported?

[teto]

Maybe neovim plugins shouldn't have lua > 5.1 listed as supported?

in the lua package set, we dont know it's a neovim plugin (we could look at the rockspec tags/labels "neovim" ?). We follow the plugin recommendation. We could have haskell-tools.nvim requires lua == 5.1 then ?

[mrcjkb]

Maybe neovim plugins shouldn't have lua > 5.1 listed as supported?

in the lua package set, we dont know it's a neovim plugin (we could look at the rockspec tags/labels "neovim" ?). We follow the plugin recommendation. We could have haskell-tools.nvim requires lua == 5.1 then ?

🤔 Perhaps it would make sense for luarocks-tag-release to specify lua == 5.1 if both the following conditions are true:

• No lua dependency has been explicitly specified
• One of the labels is neovim (indicating that it's a Neovim plugin).
  This is either set via an input or picked up from the GitHub repo metadata.

Is there any case in which we would want to be able to build a Neovim plugin for any other lua versions?

[teto]

Is there any case in which we would want to be able to build a Neovim plugin for any other lua versions?

None I can think of.

One of the labels is neovim (indicating that it's a Neovim plugin).
This is either set via an input or picked up from the GitHub repo metadata.

I think defining it by an input that then enables the label makes more sense. How do you set copy_directories (= plugin/after/doc...) today ? isn't there a neovim input already ?

[mrcjkb]

How do you set copy_directories (= plugin/after/doc...) today ?

It defaults to adding the rtp directories if they exist (if you leave the input empty).

isn't there a neovim input already ?

No. I decided to cater to neovim plugins by default, making it possible to disable neovim specific behaviour by setting the inputs explicitly.