So, what you have at the end there (the .if ${OSVERSION} < 15 block) feels like an antipattern, but I can't think of any better way to do it. I've tried out a couple alternatives and they just made it more complicated. At the end of the day, it's a valid approach and it works, and also it only needs to be in there until 14 goes EOL (now that 15 is out, 14 has a defined death day).

I say, fix those small things, do a final sweep to make sure that the port is where you want it, and then let's get it committed!

Not that I doubted you, but I did rebuild after removing GH_PROJECT and GH_TAGNAME.

Oh relying upon the header file would also help in case inotify gets backported to stable/14 in the future.

In D54055#1235414, @dave_freedave.net wrote:

Oh relying upon the header file would also help in case inotify gets backported to stable/14 in the future.

Yeah, I really dig the approach they took. It's much better to do a capability check than a version check. I'd vote for that approach, though to be clear, either approach works and is acceptable. But the header test has a much lower chance of false negatives.

Why not use https://github.com/LuaLS/lua-language-server/releases/download/3.16.0/lua-language-server-3.16.0-submodules.zip ?

In D54055#1236171, @diizzy wrote:

Why not use https://github.com/LuaLS/lua-language-server/releases/download/3.16.0/lua-language-server-3.16.0-submodules.zip ?

I actually started with the submodules asset. I moved back to github tuples as I feel it provides more knobs to adjust moving forward.

In D54055#1236171, @diizzy wrote:

Why not use https://github.com/LuaLS/lua-language-server/releases/download/3.16.0/lua-language-server-3.16.0-submodules.zip ?

Good catch, diizzy. I'm glad to see Dave's response that he actively engaged that approach and made an informed decision about it.

Based on my own experience with the port, I think Dave is making a sound decision. I had to adjust each component separately on multiple occasions.

The consequence of upstream using both luamake and raw ninja is that the build is incredibly brittle. FreeBSD support of each component is the most after of afterthoughts, and one component or another has a habit of breaking the FreeBSD build (much less the runtime) and, in my own experience, needed to be adjusted to different versions.

It's definitely more work up-front, but my guess is that Dave is going to be very glad he took the time to deal with tuples.

Dave: I did the following:

/usr/ports$ curl <diff URL> | patch -p1
/usr/ports$ cd devel/lua-language-server
/usr/ports/devel/lua-language-server$ portlint -A

And I am getting a lot of stuff:

WARN: Makefile: [4]: whitespace before end of line.
WARN: Makefile: [4]: use tab (not space) to make indentation
WARN: Makefile: [13]: use a tab (not space) after a variable name
FATAL: Makefile: [19]: use a tab (not space) after a variable name
FATAL: Makefile: extra item "                                                       " placed in the PORTNAME section.
FATAL: Makefile: extra item "MAINTAINER" placed in the PORTNAME section.
FATAL: Makefile: extra item "COMMENT" placed in the PORTNAME section.
FATAL: Makefile: extra item "WWW" placed in the PORTNAME section.
FATAL: Makefile: extra item "LICENSE" placed in the MAINTAINER section.
FATAL: Makefile: extra item "LICENSE_FILE" placed in the MAINTAINER section.
WARN: Makefile: COMMENT is set externally to this port's Makefile, but this port is not configured as a slave port.
FATAL: Makefile: extra item "LIB_DEPENDS" placed in the LICENSE section.
FATAL: Makefile: extra item "_INOTIFY_" placed in the LICENSE section.
FATAL: Makefile: extra item "_LINK_INOTIFY_" placed in the LICENSE section.
FATAL: distinfo not under git.
FATAL: Makefile not under git.
FATAL: pkg-descr not under git.
FATAL: files/patch-3rd_bee.lua_compile_common.lua not under git.
FATAL: files/patch-3rd_luamake_compile_ninja_freebsd.ninja not under git.
FATAL: files/patch-3rd_bee.lua_test_test.lua not under git.
FATAL: files/lua-language-server.in not under git.
FATAL: files/patch-3rd_luamake_bee.lua_test_test.lua not under git.
18 fatal errors and 4 warnings found.

Clearly, not being under git is expected, and I have no clue what its issue is with your perfectly correct COMMENT. And, because this is a relatively complicated build, you get a lot of latitude for organizing things in a sensible way. But, the ordering of things is a big part of idiomaticity (my spellchecker says it's not a word, which baffles me). You should resolve the majority of those. Tabs vs. spaces and trailing whitespace is absolutely a problem and we need to be really diligent about it.

In my nvim config, I have:

vim.api.nvim_create_autocmd("Filetype", {
    pattern = "make",
    callback = function()
        vim.opt_local.tabstop    = 8
        vim.opt_local.shiftwidth = 8
        vim.opt_local.expandtab  = true
    end,
})

Additionally, I use mini.trailspace from the incredible mini.nvim plugin (https://github.com/nvim-mini/mini.nvim/blob/main/readmes/mini-trailspace.md) to automatically highlight trailing whitespace. I highly recommend it (or anything like it; search for "whitespace" in https://github.com/rockerBOO/awesome-neovim) for FreeBSD development; with it, I could see the issue as soon as I opened the file:

edit: And don't forget about set list to see spaces vs. tabs. If you haven't used it before, you'll want to set up listchars with something like set listchars=eol:↲,extends:…,nbsp:␣,precedes:…,space:◦,tab:» ,trail:·.

Totally forgot, I also have this guy:

vim.api.nvim_create_autocmd({"BufRead", "BufNewFile"}, {
    pattern = {"*/ports/*", "/etc/*", "/usr/local/etc/*",
    callback = function()
        vim.opt_local.tabstop    = 8
        vim.opt_local.shiftwidth = 8
        vim.opt_local.expandtab  = true
    end,
})

So, everything in /usr/ports (or any other ports tree), /etc, and /usr/local/etc will automatically get 8-width non-expanded tabs unless it matches some other known filetype. That has saved my bacon more times than I can count.

I only ran portclippy before. I'll go find portlint and track down the other errors too.

In D54055#1236298, @dave_freedave.net wrote:

I only ran portclippy before. I'll go find portlint and track down the other errors too.

I ran it too. I am actually pretty shocked that portclippy had nothing to say! Either it doesn't care about whitespace or it's buggy.

portclippy is newer, but both are actively maintained. They are both opinionated, and they disagree about as much as they agree. Neither of them are static analyzers, so use them as like, inspiration.

If only I'd been slower. I prefer the file check but can add comment. Before I do though, is the check in 2 places acceptable?

In D54055#1236307, @dave_freedave.net wrote:

If only I'd been slower. I prefer the file check but can add comment. Before I do though, is the check in 2 places acceptable?

OK, I'm behind that approach. Keep in mind that your goal there is to guess what future us will grep for. Good luck!

It is, but not ideal. If it were an OSVERSION check, it'd be trivial, but each filesystem check contributes to I/O latency. Maybe in the upper block, set BASE_LIBINOTIFY := true (make sure to use := there) and change the check to .if ! ${BASE_LIBINOTIFY}?

I'm considering asking portmgr about Mk/Uses/libinotify.mk. Dave, I'll Cc: you if I do that so you can see the process.

one sec I'm changing it to the way requested because both portlint and portclippy like it. so it has that plus grep going for it.

Dave, we're getting there, I promise!

I'm not sure how you're testing, but your build testing should be centered around poudriere testport:
poudriere testport -j <your_jail_name> devel/lua-language-server

Running it under there, I get the following:

===========================================================================
====> Running Q/A tests (stage-qa)
====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
Error: Orphaned: @dir %%DATADIR%%/meta/3rd/Cocos4.0
Error: Orphaned: @dir %%DATADIR%%/meta/3rd/Defold
Error: Orphaned: @dir %%DATADIR%%/meta/3rd/Jass
Error: Orphaned: @dir %%DATADIR%%/meta/3rd/OpenResty
Error: Orphaned: @dir %%DATADIR%%/meta/3rd/bee
Error: Orphaned: @dir %%DATADIR%%/meta/3rd/busted
Error: Orphaned: @dir %%DATADIR%%/meta/3rd/ffi-reflect
Error: Orphaned: @dir %%DATADIR%%/meta/3rd/lfs
Error: Orphaned: @dir %%DATADIR%%/meta/3rd/love2d
Error: Orphaned: @dir %%DATADIR%%/meta/3rd/lovr
Error: Orphaned: @dir %%DATADIR%%/meta/3rd/luaecs
Error: Orphaned: @dir %%DATADIR%%/meta/3rd/luassert
Error: Orphaned: @dir %%DATADIR%%/meta/3rd/luv
Error: Orphaned: @dir %%DATADIR%%/meta/3rd/skynet
===> Checking for items in pkg-plist which are not in STAGEDIR
===> Error: Plist issues found.
*** Error code 1

Stop.
make: stopped making "check-plist" in /usr/ports/devel/lua-language-server
=>> Error: check-plist failures detected

Each of those dirs is most likely empty (otherwise it would have registered those dirs automatically). For each of them, you'll need to decide whether they need to exist (in which case they must be added to pkg-plist):

@dir foo
@dir(<owner>,<group>,<mode>) bar
@dir(,,750) some/dir
@dir(adamw,wheel) another/dir

Or, if any shouldn't exist, then you'll need to explicitly ${RMDIR} them in post-install:.

Looking into it. I just got poudriere running in a bhyve VM. I can see the same errors so next update should be clean.

There it is! It's looking great, Dave. I'm just doing some final tests and--barring anything wacky--I'll get it committed shortly.

In D54055#1236567, @adamw wrote:

There it is! It's looking great, Dave. I'm just doing some final tests and--barring anything wacky--I'll get it committed shortly.

I feel like I just got the "Sensei Ishikawa" treatment. Where you look up at the hill with footholds while I start climbing... only to find you on the top of the hill that you walked around to the gentle path up the hill. But you know, it worked: I'm sold. I will be using poudriere going forward.

In D54055#1236570, @dave_freedave.net wrote:

In D54055#1236567, @adamw wrote:

There it is! It's looking great, Dave. I'm just doing some final tests and--barring anything wacky--I'll get it committed shortly.

I feel like I just got the "Sensei Ishikawa" treatment. Where you look up at the hill with footholds while I start climbing... only to find you on the top of the hill that you walked around to the gentle path up the hill. But you know, it worked: I'm sold. I will be using poudriere going forward.

Wow that's quite the vivid simile!

Honestly, there's a lot to be said for learning the system closer to the metal before stepping into an automated system. It'll pay dividends in the long run.

Seriously, man, great work on this. You should be proud of yourself.

Some ideas for next steps:

1. Make the luamake build support ccache
2. Make the luaLS build support ccache
3. See whether you can coax a verbose build (if we can see the actual commands rather than just "compiling foo/bar/baz.c" it makes debugging much easier)
4. Ensure the build is reproducible (IIRC, it used to include some opinionated timestamp that busted caches, dunno if that's still the case)
5. Be sure to dogfood updates before submitting update PRs (LuaLS updates used to have a 50/50 chance of building fine and crashing at runtime. I had to skip at least half of the updates)