The Wayback Machine - https://web.archive.org/web/20211028055401/https://github.com/Homebrew/homebrew-core/issues/68013
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

luajit probably needs to be deprecated #68013

Open
fxcoudert opened this issue Dec 30, 2020 · 18 comments
Open

luajit probably needs to be deprecated #68013

fxcoudert opened this issue Dec 30, 2020 · 18 comments

Comments

@fxcoudert
Copy link
Member

@fxcoudert fxcoudert commented Dec 30, 2020

  • The latest release is from 2015, and the latest beta is from 2017
  • It's heavily patched
  • Every new macOS version requires an additional patch
  • Upstream's recommendation is to “build from git HEAD”, and they won't apparently ship new releases: LuaJIT/LuaJIT#648 (comment)

The reason I'm not doing a pull request directly is that a lot of things depend on luajit, so I want to open a discussion and figure out the best way to handle this. Can some of these be migrated to one of the lua formulas?

@carlocab
Copy link
Member

@carlocab carlocab commented Dec 30, 2020

Can some of these be migrated to one of the lua formulas?

In theory, it should be possible to migrate them to [email protected]. That would, however, almost certainly cause performance regressions.

For reference, a number of other package managers actually ship 2.1.0-beta3, including Debian Stable and Ubuntu. https://repology.org/project/luajit/versions

@SMillerDev
Copy link
Member

@SMillerDev SMillerDev commented Dec 30, 2020

https://repology.org/project/luajit/versions indicates that there is apparently a 2.1.0-beta3? If there is a problem migrating to [email protected] we could choose to allow this beta.

@fxcoudert
Copy link
Member Author

@fxcoudert fxcoudert commented Dec 30, 2020

2.1.0-beta3 is from 2017, it would still need to be patched to build

@carlocab
Copy link
Member

@carlocab carlocab commented Dec 30, 2020

If we are deprecating luajit, we should also pick a date to disable it (absent a change of circumstances). I don't want to give users the impression that a deprecation warning is something that can be ignored indefinitely.

@thesamesam
Copy link
Contributor

@thesamesam thesamesam commented Jan 1, 2021

There are (and have been) a lot of LuaJIT forks. I've been looking at the OpenResty fork which seems to be consistently active. A fair amount of upstreams support just LuaJIT so replacing with a fork if possible will be best.

I'm interested if anyone has any views on other forks which look sustainable. The original LuaJIT isn't actually dead either, it's just.. not making releases for quite some time.

(We are having similar discussions in Gentoo).

@chrisfinazzo
Copy link
Contributor

@chrisfinazzo chrisfinazzo commented Jan 5, 2021

@carlocab I can confirm this is deprecated as well. Running brew uses [email protected] on my machine returns the following list:

gnuplot@4 libswiften mpv wireshark libquvi luabind stone-soup

YMMV, but this is just what I found when the warning appeared after invoking brew doctor.

@carlocab
Copy link
Member

@carlocab carlocab commented Jan 11, 2021

@chrisfinazzo I'm aware. I merged the PR deprecating [email protected] about a week ago.

Opened #68787. Let's see if this works. Thanks for the suggestion, @thesamesam.

@clason
Copy link
Contributor

@clason clason commented Jan 12, 2021

Speaking for neovim: this project heavily leverages LuaJIT, so replacing that with PUC Lua would be a significant performance regression. The last beta release (2.1.0-beta3) does not support Apple ARM, so it was necessary to switch to a post-release git commit (and adapt some tests and plugins to the breaking changes therein) for master (that is to become 0.5 soon(tm)). There have been requests to backport these changes to the 0.4 release branch. I don't think any of the forks (OpenResty, MoonJIT) are drop-in replacements, either.

It is very unfortunate that LuaJIT refuses to just release a beta4. If that means LuaJIT is removed from homebrew, it would probably be necessary to adapt the build scripts to let neovim pull in and build its own version of LuaJIT (like it does for the manual build).

@SMillerDev
Copy link
Member

@SMillerDev SMillerDev commented Jan 12, 2021

That would violate https://docs.brew.sh/Acceptable-Formulae#we-dont-like-install-scripts-that-download-unversioned-things so it would be more likely we would have to live with the performance regression.

@bfredl
Copy link

@bfredl bfredl commented Jan 12, 2021

@SMillerDev are you sure? to me it sounds like only unspecficied versions are banned, not if the scripts points to choosen known good commit with a checksum.

@carlocab
Copy link
Member

@carlocab carlocab commented Jan 12, 2021

I don’t think it violates that rule, actually. I think @clason just means that the neovim formula would add its own luajit resource, which would be a versioned dependency.

That said, it looks to me like the OpenResty fork could be a drop-in replacement... It also supports Apple ARM. See the PR I opened. I’ll try using it to build Neovim later today.

@SMillerDev
Copy link
Member

@SMillerDev SMillerDev commented Jan 12, 2021

I don’t think it violates that rule, actually. I think @clason just means that the neovim formula would add its own luajit resource, which would be a versioned dependency.

Might be okay then, even though I'd prefer to depend on a known good, working formula (lua) then installing a resource because upstream doesn't want to make their work packagable

@carlocab
Copy link
Member

@carlocab carlocab commented Jan 12, 2021

I dunno; trying to switch out a dependency that upstream have specified as what works for them sounds to me as very much like heavy patching of a formula. We don’t like patches; we probably shouldn’t like this either.

@bfredl
Copy link

@bfredl bfredl commented Jan 12, 2021

Even if neovim itself only uses PuC lua5.1 api, many plugins depend on luajit either explicitly or implicitly (optimizing for an acceptable user experience only with jit enabled). PuC lua fallback is meant more for niche platforms ( a slow neovim is better than no neovim on your MIPS router), not for major platforms and especially not a flagship product that is marketed primarily for speed and battery life (M1 mac).

@carlocab
Copy link
Member

@carlocab carlocab commented Jan 12, 2021

Tried building neovim, both release and HEAD against the latest tagged release of OpenResty LuaJIT. In both cases, it starts up perfectly fine and :checkhealth reveals no problems. (As mentioned, the latest LuaJIT release doesn't actually work for a HEAD neovim build. Cf. neovim/neovim#13529)

I'm going to put it on my tap for anyone who wants to test it (carlocab/homebrew-personal#27), but it's still building on GitHub runners at the moment. If you don't want to wait you can always check out the appropriate branch and build from there.

Update: Finally got off the GitHub runner queue, so I've merged the PR above. You can try Neovim built against OpenResty LuaJIT by

brew tap carlocab/personal
brew install neovim-openresty

neovim-openresty is keg-only, so you'll need to either add it to your PATH or call it using something like $(brew --prefix neovim-openresty)/bin/nvim.

@clason
Copy link
Contributor

@clason clason commented Jan 12, 2021

I'll give it a whirl later (on Intel) with a non-trivial actual use of Lua plugins.

@clason
Copy link
Contributor

@clason clason commented Jan 12, 2021

Seems to work well in preliminary tests (HEAD, playing with packer, treesitter, LSP, telescope)!

carlocab added a commit to carlocab/homebrew-core that referenced this issue Jan 14, 2021
BrewTestBot pushed a commit that referenced this issue Jan 14, 2021
See discussion at #68013 for context.

Closes #68787.

Signed-off-by: FX Coudert <[email protected]>
Signed-off-by: Carlo Cabrera <[email protected]>
@bernhardkaindl
Copy link
Contributor

@bernhardkaindl bernhardkaindl commented Aug 31, 2021

@carlocab : It looks like this has been discussed and is fixed. I guess you could close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants