Page MenuHomeFreeBSD

devel/lua-language-server: Update 3.16.1 => 3.17.1
Needs ReviewPublic

Authored by dave_freedave.net on Sun, Jan 11, 3:53 PM.
Tags
None
Referenced Files
Restricted File
Mon, Feb 9, 12:36 PM
Unknown Object (File)
Sat, Feb 7, 8:15 PM
Unknown Object (File)
Sat, Feb 7, 3:09 PM
Unknown Object (File)
Mon, Jan 26, 11:29 AM
Unknown Object (File)
Fri, Jan 23, 5:57 PM
Restricted File
Thu, Jan 22, 10:02 PM
Unknown Object (File)
Thu, Jan 22, 9:23 PM
Unknown Object (File)
Thu, Jan 22, 2:23 PM

Details

Reviewers
adamw
Summary

updating to latest version.

This uses a bee.lua based on lua 5.5 (not yet in ports). This means I've been wrong all along that this required lua:build in USES. I verified that in poudriere(8).

Test Plan

passes make test on stable/14, stable/15, main.
Also verified it from neovim against my config for all of the above versions.
That is where I discovered I now need a ~/.config/nvim/.luarc.json for latest version to get symbols.

Diff Detail

Repository
R11 FreeBSD ports repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Removed files from patch that didn't actually change (just date change from when I ran make makepatch).

Confirmed via poudriere testport that USES does not need lua:build as it only uses its own modified bee.lua which is now based on lua 5.5.

{F141925403} is output from poudriere testport ... devel/lua-language-server.

dave_freedave.net edited the test plan for this revision. (Show Details)

I decided to change a variable name to match the Uses/inotify.mk coming in D54116. That really just meant changing LINK_INOTIFY to INOTIFY_LIB. But while at it I wanted a better variable name for what I had set as INOTIFY, so it is now LM_INOTIFY_LINK.

If D54116 arrives first I have already tested the patch to switch to it (and will update this review):

--- Makefile    2026-01-12 06:21:43.155262000 -0300
+++ M   2026-01-12 06:37:51.898022000 -0300
@@ -10,7 +10,7 @@
 LICENSE=       MIT
 LICENSE_FILE=  ${WRKSRC}/LICENSE

-USES=          dos2unix ninja:make
+USES=          dos2unix inotify ninja:make
 DOS2UNIX_REGEX=        .*\.(cpp|h|lua|md|obj|json)
 USE_GITHUB=    yes
 GH_ACCOUNT=    LuaLS
@@ -82,12 +82,9 @@
 do-test:
        cd ${WRKSRC} && 3rd/luamake/luamake unit-test

-.include <bsd.port.options.mk>
+.include <bsd.port.mk>

-.if ${OPSYS} == FreeBSD && ${OSVERSION} < 1500050
-LIB_DEPENDS+=  libinotify.so:devel/libinotify
+# `luamake' has its own syntax for linking of inotify library
+.if !empty(INOTIFY_PORT)
 LM_INOTIFY_LINK=       links = "inotify",
-INOTIFY_LIB=   -linotify
 .endif
-
-.include <bsd.port.mk>
dave_freedave.net retitled this revision from devel/lua-language-server: Update 3.16.1 => 3.16.4 to devel/lua-language-server: Update 3.16.1 => 3.17.1.
dave_freedave.net edited the summary of this revision. (Show Details)
dave_freedave.net edited the test plan for this revision. (Show Details)

lua-language-server has updated before this could be committed. skipping ahead to latest release 3.17.1

NOTE: I can't tell if its nvim-lspconfig that changed or if its lua-language-server but there I now require a ..luarc.json in my ~/.config/nvim to correctly pick up symbols. It took me a while to track down which delayed me putting this up.

As LuaCATS did *not* change, I think it was an nvim-lspconfig update. Here is what you are going to need if you want lua-language-server to pick up symbols when editing your neovim config:

{
  "schema": "https://raw.githubusercontent.com/LuaLS/vscode-lua/master/setting/schema.json",
  "runtime": {
    "version": "LuaJIT",
     "path": [
       "?.lua",
       "?/init.lua"
     ],
    //"pathStrict": true
  },
  "workspace": {
    "library": [
      "$VIMRUNTIME",
      "${3rd}/luv/library",
    ],
    "checkThirdParty": false
  }
}

re-ran poudriere testport -j FreeBSD15 -p local devel/lua-language-server

{F142736553}

devel/lua-language-server/Makefile
14

Everything else in DOS2UNIX_REGEX is alphabetized. It's a nothing thing, but you sure you want to break that pattern?

88

I love what you did here! You clearly read the Uses/inotify.mk patch I'd submitted, and you're using the appropriate variable name that that patch will export, If that was unintentional then don't tell me—I want to continue believing that you simply nailed it.

So, here's the deal. This patch looks great, it tests perfectly for me, and I'm happy to commit it. Just give me a sign when you're ready and I'll make it happen.

This revision is now accepted and ready to land.Sat, Feb 7, 9:34 PM

Here is what you are going to need if you want lua-language-server to pick up symbols when editing your neovim config:
[...]

Yeah, LuaLS has gotten really picky about needing its environment set correctly. I'm really not sure why lspconfig doesn't have an option to set LuaLS up for neovim config editing.

I'd suggest putting the luarc into ${EXAMPLESDIR} (and adding an EXAMPLES option). You could even put a note about it in pkg-message. I do see your luarc has a comment in it though; does LuaLS handle JSONC?

Moved to using the new USES=inotify, verified it worked:

stable/14 » pkg info -d lua-language-server
lua-language-server-3.17.1,1:
        libinotify-20240724_3
        libinotify-20240724_3 (libinotify.so.0)

stable/15 » pkg info -d lua-language-server
lua-language-server-3.17.1,1:

current » pkg info -d lua-language-server
lua-language-server-3.17.1,1:

Added example config for neovim and EXAMPLES option. I built both with and without, and installed to verify the pkg-message is only provided when that option is selected.

re-tested on stable/14, stable/15, and current against my neovim config.

I'll attach latest log from poudriere testport after getting these changes up.

This revision now requires review to proceed.Mon, Feb 9, 12:35 PM

Latest output from poudriere testport -j FreeBSD15 -p local devel/lua-language-server
{F144553961}

devel/lua-language-server/Makefile
47

Consider whether it'd be beneficial to install the pkg-message regardless, so that users with EXAMPLES disabled have an opportunity to see the solution to a problem they might be having. What do you think?

104

Having anything after <bsd.port.mk> is an antipattern; that's what bsd.port.{pre,post}.mk are for. See if that works instead.

devel/lua-language-server/files/pkg-message.in
10

Is it worth mentioning that it also prevents LuaLS from complaining about undefined symbols?

All suggestions taken:

  • always install pkg-message
  • use bsd.port.{pre,post}.mk as suggested
  • update pkg-message to explain undefined symbols.
devel/lua-language-server/Makefile
102–103

Typically, all variable-handling logic lives above targets. Unless there's a reason for this block to live here, you want to move it up.

devel/lua-language-server/Makefile
102–103

I had run portlint -A and portclippy --strict Makefile and neither complained which is probably why I didn't move this. It just has to be after the .include <bsd.port.pre.mk>

It was part of the antipattern I had before. But I've just restarted rebuilding world and ports so I may or may not get to this before Carnival 😦

Actually I just hit the same test failure pkg-fallout had emailed me about. bee has a test here that I don't believe is correct.

sleep(3) does not promise to return within 1 seconds (or indeed any amount) of time after your requested sleep amount has elapsed. I'm sure it usually is going to be within 1 second, but I think this is a bad test.

I can fix this test, but lua-language-server has another known flaky test. Should I move the do-test target to some other name so that pkg-fallout doesn't run it? Is there a standard name like maintainer-test or something? I can then file a bug upstream.

Oh! I should patch this, its the build of luamake that runs as part of do-build. That runs a test on its older version of bee. I mean if I could get rid of those tests without running do-test I would. Anyway little more work here... may as well fix this now.

Actually I just hit the same test failure pkg-fallout had emailed me about. bee has a test here that I don't believe is correct.

sleep(3) does not promise to return within 1 seconds (or indeed any amount) of time after your requested sleep amount has elapsed. I'm sure it usually is going to be within 1 second, but I think this is a bad test.

Honestly, anything designed to guess how long an asynchronous process is going to take is just missing the point. What about patching out (or mark for skipping, or whatever) that one test and letting the others proceed? If there's benefit to running the test suite, it'd be a shame to neutralize the entire thing just for one unreliable test.

And yes, maintainer-* is probably what I'd go for. Once upon a time you'd see maintainer-whatever targets all the time, but with both the power and confines of our tooling, you don't see them as often. But they're great to have, because they implicitly document things you've learned about the port, you know?

Moved LM_INOTIFY_LINK= above all the rules.
No longer run luamake tests as part of build. Only run for explicitly requested make test as they have a flaky (actually broken) test too.

Once I realized the test failure isn't coming from do-test I realized I didn't need a maintainer-test target.

Sorry about all the churn on this one, but I think its in better shape now. Shouldn't get email from pkg-fallout now. Have a nice message and example luarc.json too.

My FreeBSD14 bulk is still running, but I verified with:

root@poudriere16:~ # pkg info -d --file /usr/local/poudriere/data/packages/FreeBSD14-local/.building/All/lua-language-server-3.17.1,1.pkg
lua-language-server-3.17.1,1:
        libinotify-20240724_3
root@poudriere16:~ # pkg info -d --file /usr/local/poudriere/data/packages/FreeBSD15-local/All/lua-language-server-3.17.1,1.pkg
lua-language-server-3.17.1,1:

I don't want to jinx myself, but I believe this is (finally) ready 😄