Packer.nvim: Math.max changes in LuaJIT

[clason]

So it appears that LuaJIT made some breaking changes to math.max and math.min (possibly in https://github.com/LuaJIT/LuaJIT/commit/03208c8162af9cc01ca76ee1676ca79e5abe9b60?) which lead to an

Error executing vim.schedule lua callback: .../packer.nvim/lua/packer/plugin_utils.lua:48: bad argument #1 to 'max' (number expected, got no value)

on :PackerUpdate.

Unfortunately, this change is pulled in when bumping LuaJIT to a commit required to get it building on ARM64 (specifically, Apple M1), see https://github.com/neovim/neovim/pull/12624. (You can see the problem in action with the latest snapshot of VimR, which is built with that PR (even on Intel).) Not sure what the best course of action is here; maybe do nothing until that PR (or a similar one) is merged? Or just implement your own max/min in this line for the sake of compatibility?

[wbthomason]

Yes, this was also encountered by #96 - my plan was to wait until a version of LuaJIT with that patch was formally released, but if people are already running into this issue by building from LuaJIT master, I may need to add a shim or change my use of max/min.

Do you know of a textual summary of the changes? I'm having a hard time telling what's changed from reading the LuaJIT commit you linked, as I'm not familiar with that codebase.

[clason]

Oops, sorry, didn't see that issue! Yes, that's the same. It's not on master yet, but I expect it will be by the time 0.5 is released to allow native builds on Apple silicon. (At least I hope so ;))

Unfortunately, I couldn't find any explanation of these changes, either (nor a closed issue that looks related) 😞 (And that specific commit seems more like switching arguments around, so not really sure how this leads to the observed error.)

[wbthomason]

Yup. I think I'll still hold off on taking action on this for the moment, but will try to find a chance to play with the new LuaJIT and see if I can figure out what the change introduces. I do wonder if this is a regression in LuaJIT, since using max/min on an unpack'd table _seems_ like something that should still be fine...

[clason]

@wbthomason I'm sorry to say, time is up :) Neovim just merged the Apple silicon PR, which bumped LuaJIT to the version with the changed max across the board. Expect to get a flood of issues on this in the coming days... (Might want to pin this issue?)

[wbthomason]

Most unfortunate. Time to see if I can figure out what that change introduced...

[clason]

It seems the change is specifically to how multiple values are handled; I get the error when unpack(util.map(vim.fn.getftime, txts)) returns multiple values.

[nanotee]

Can reproduce on latest Neovim master

I don't know if this is related, but there seems to be some kind of weird interaction between math.max and unpack on older versions of LuaJIT:

print(math.max(unpack({})))

Unpacking an empty table in a math.max call used to return a (seemingly random) number, whereas latest LuaJIT throws an error.

[wbthomason]

I think this is either the interaction @nanotee notes (which would imply that there's been a previously-hidden bug in packer's helptags updating) or actually a change to table.insert, since the results of the map call in plugin_utils.lua:48 should not be empty.

[wbthomason]

In contrast to @clason's comment, I'm getting this when the call returns nil or {}...

[clason]

If I vim.inspect the result of util.map instead of unpack, I also get {} when the error throws. (It might be that there's a race condition with printing, so that may not be 100% reliable.) So I'd go with @nanotee's comment.

There have been changes to string hashing that affect the ordering of tables; not sure that this is relevant here?

I have not been able to find out _anything_ related to the LuaJIT commit -- no issue, no PR, no post on the mailing list -- explaining the changes or what they are supposed to fix. But a change to handling of max for empty arguments sounds quite likely.

[babariviere]

@wbthomason I have found what might be the issue.

Some plugins don't have the doc folder. So the vim.fn.glob will always return an empty table and raise an exception.
Maybe ignoring doc update for these plugins should do it?

[wbthomason]

123 fixes this for me - could others here test that and confirm that it also fixes the issue for them?

@babariviere: The changes in #123 implicitly do that, so good thinking!
@clason: Sounds good. I suspect that @nanotee's thinking is right, and there was a bug in math.max previously that caused helptags to sometimes be rebuilt (or not) erroneously

[babariviere]

Fixes it for me too 👍

[nanotee]

Same here, no more errors after updating packer!