Lua 5.4.0
Needs ReviewPublic

Authored by russ.haley_gmail.com on Mar 16 2018, 6:28 AM.

Details

Summary

Hi,

This is a port for Lua 5.4.0 work2.

The Lua Manual can be found here:
http://www.lua.org/work/doc/manual.html

Andrew Gierth has added editline and documentation options to the port.

Test Plan

The current expectation on the mailing list is 5.4 should be generally backwards compatible with 5.3. Incompatibilities with 5,3 are listed in the manual:
http://www.lua.org/work/doc/manual.html#8

Build testing:
This was created and tested on Ghost BSD (11.1-p4). I have a clean VM for 11.1 and a TrueOS VM (Some version of CURRENT) that I will test this on. I'll pull out an arm board if I can.

Lua Testing:

  • I ran some heavy garbage collector tests on work1 that chewed up 8gb of memory without issue
  • I would like to request @dbn for a expeditionary run to look at bumping all ports possible to 5.4.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

The so isn't working because there is nothing here to build it; the makefile distributed with Lua does not try to build one and never did, the ports for earlier Lua versions patch the makefile to add it.

Turning on LUA_NILINTABLE is simply not appropriate; at most you could make it a selectable option, off by default.

lua54/Makefile
38 ↗(On Diff #40339)

this should be MYLIBS, not MYLDFLAGS if you're going to do it this way

The so isn't working because there is nothing here to build it; the makefile distributed with Lua does not try to build one and never did, the ports for earlier Lua versions patch the makefile to add it.

Thank you, I didn't remember that I had stripped that out to revise it. I get a few hours here and there for these things.

Turning on LUA_NILINTABLE is simply not appropriate; at most you could make it a selectable option, off by default.

Quite right. I'll make sure it's off for the final port and perhaps figure out how to make it an option.

  • Updated to build shared object.
  • Removed LUA_NILINTABLE. Will turn it into an option. Any other useful options?
  • Fixed to use MYLIBS not MYLDFLAGS for -ledit
russ.haley_gmail.com marked an inline comment as done.Mar 17 2018, 4:10 AM
russ.haley_gmail.com edited the summary of this revision. (Show Details)Mar 17 2018, 4:23 AM
russ.haley_gmail.com edited the test plan for this revision. (Show Details)
russ.haley_gmail.com edited the summary of this revision. (Show Details)
lua54/files/patch-src_Makefile
33 ↗(On Diff #40372)

If you're going to remove -lreadline here, it seems more logical to also remove -DLUA_USE_READLINE and pass that in via MYCFLAGS. This has the advantage that you can make it an option for the port (on by default).

lua54/Makefile
37 ↗(On Diff #40372)

I've been testing this:

OPTIONS_SINGLE= EDIT
OPTIONS_SINGLE_EDIT=EDITNONE LIBEDIT READLINE

OPTIONS_DEFINE= NILINTABLE 
OPTIONS_DEFAULT+= LIBEDIT

EDITNONE_DESC=  No interactive command-line editing
LIBEDIT_DESC=   Use base system libedit
READLINE_DESC=  Use GNU Readline from ports

NILINTABLE_DESC=Experimental (incompatible) nil-in-table feature

LIBEDIT_CPPFLAGS=-DLUA_USE_READLINE -I/usr/include/edit
LIBEDIT_LIBS=-ledit

READLINE_USES=readline
READLINE_CPPFLAGS=-DLUA_USE_READLINE
READLINE_LIBS=-lreadline

NILINTABLE_CPPFLAGS=-DLUA_NILINTABLE

CFLAGS+= -fPIC

WITHOUT_NO_STRICT_ALIASING=yes

MAKE_ARGS+=     CC="${CC}" \
                MYCFLAGS="${CPPFLAGS} ${CFLAGS}" \
                MYLDFLAGS="${LDFLAGS}" \
                MYLIBS="${LIBS}" \

Specific things to note:

  1. Passing CPPFLAGS/CFLAGS/LDFLAGS down via the MY* variables preserves the system default compile options and also any user-supplied ones, whereas your version clobbers those. The following points depend on this, as does the use of OPT_CPPFLAGS and so on.
  1. WITHOUT_NO_STRICT_ALIASING removes the system default -fno-strict-aliasing which is applied at -O2 and up.
  1. USES=readline handles adding -L/usr/local/lib and -I/usr/local/include automatically.
lua54/files/patch-src_Makefile
23 ↗(On Diff #40372)

LDFLAGS already had MYLDFLAGS included in it; don't add it again.

russ.haley_gmail.com updated this revision to Diff 40410.EditedMar 18 2018, 4:31 PM

Updated with improvements from @andrew_tao173.riddles.org.uk :

  • Removed hard coded options
  • Added options for editline/readline library source
  • Added option for experimental NILINTABLE
  • Andrew commented that command names should come from lua.mk. I added a timestamp for tracking. (Updating lua.mk is on my todo list. See D13691)
lua54/files/patch-src_Makefile
33 ↗(On Diff #40410)

-DLUA_USE_READLINE definitely needs to be removed from here now, the EDITNONE option of the port breaks if it's left in. (Sorry, forgot to mention that when I sent the makefile changes.)

russ.haley_gmail.com marked an inline comment as done.Mar 18 2018, 7:13 PM

Corrected src/Makefile patch to remove -DLUA_USE_READLINE as per AG. Compiles with and without interactive mode.

New diff with my suggested version for the work2 release.

NILINTABLE is gone, and I've added a DOCS option and reworked a few minor things.

Makefile
108

oops, typo, should be soname=${PORTNAME}-${LUA_VER}, will fix

Fix silly mistake in previous diff

Makefile
54

+1

russ.haley_gmail.com edited the summary of this revision. (Show Details)Sat, Jul 14, 8:06 PM
russ.haley_gmail.com edited the test plan for this revision. (Show Details)Sat, Jul 14, 8:10 PM
russ.haley_gmail.com added reviewers: mat, dbn.
russ.haley_gmail.com added a project: Restricted Project.
russ.haley_gmail.com edited the test plan for this revision. (Show Details)
russ.haley_gmail.com added a subscriber: dbn.
Makefile
61

Would it be wrong of me to add a lua-docs shell script to /usr/local/bin that looks for firefox or chromium and opens readme.html?

Makefile
61

It ... doesn't seem to be common style as far as I can see. I'm not keen on the idea.

It ... doesn't seem to be common style as far as I can see. I'm not keen on the idea.

Well it was fun to write anyways. :)

https://pastebin.com/9fRuejBp

Note that work2 is not ready for prime time yet (garbage collector bugs, at least)

russ.haley_gmail.com added a comment.EditedSat, Jul 14, 9:38 PM

Note that work2 is not ready for prime time yet (garbage collector bugs, at least)

I didn't see anything on the mailing list that would preclude adding it to the ports collection, or at the very least performing an expeditionary run (if that's possible). @mat, @dbn, I'm keen to get 5.3.5 or preferably 5.4 "build tested" against other packages because I've got a week off work right now with no kids.

Note that work2 is not ready for prime time yet (garbage collector bugs, at least)

I didn't see anything on the mailing list that would preclude adding it to the ports collection, or at the very least performing an expeditionary run (if that's possible). @mat, @dbn, I'm keen to get 5.3.5 or preferably 5.4 "build tested" against other packages because I've got a week off work right now with no kids.

The "uservalue has an incorrect value" thread is the GC bug I know about. Won't affect pure-lua code, but anything that uses uservalues can explode (even just doing require "re" collectgarbage() triggers an invalid access, though it won't crash immediately unless you built with assertions on).

Note that work2 is not ready for prime time yet (garbage collector bugs, at least)

I didn't see anything on the mailing list that would preclude adding it to the ports collection, or at the very least performing an expeditionary run (if that's possible). @mat, @dbn, I'm keen to get 5.3.5 or preferably 5.4 "build tested" against other packages because I've got a week off work right now with no kids.

The "uservalue has an incorrect value" thread is the GC bug I know about. Won't affect pure-lua code, but anything that uses uservalues can explode (even just doing require "re" collectgarbage() triggers an invalid access, though it won't crash immediately unless you built with assertions on).

I may be mistaken, but the experimental (not expeditionary!) run only build packages so I'm hoping we can get the build output and see what needs upgrading. I see now that I can request an experimantal run on Bugzilla so I'll submit one today.

I may be mistaken, but the experimental (not expeditionary!) run only build packages so I'm hoping we can get the build output and see what needs upgrading. I see now that I can request an experimantal run on Bugzilla so I'll submit one today.

I know of some outstanding (possibly unreported) issues with ports: devel/lua-lpeg sets LUA_32BITS which makes it useless against the default lua build, and also fails to install re.lua

Is it worth adding options for enabling assertions and apicheck in the port? I use apicheck enough that it would be helpful, but my usage is possibly atypical.

Added ASSERT and APICHECK options.

Conform port Makefile to conventions specified in ports manual and run portlint (1 warning remains, for EDITNONE not needing any implementation)

Remove -O2 from src/Makefile in accordance with porting guidelines; -O should be provided system-wide.

Conform src/Makefile's use of $(AR) to respect $(ARFLAGS)

Ensure liblua.so is linked to libm without risking linking it to undesired additional libs.

mat added a comment.Mon, Jul 16, 7:37 AM

Could you start this by doing a svn cp lua53 lua54 and then only uploading the diff from the actual changes from 5.3 ?

Makefile
3–5

Please remove all this.

10

Remove.

This is now a diff against the result of 'svn cp lua53 lua54' as requested by mat.

Also includes mat's other requested changes.

mat added a comment.Mon, Jul 16, 2:07 PM

Could you use devel/arcanist, or at least generate a diff with full context like it does, with svn diff -x -U9999 or git diff -U9999.

Remove use of ${PORTNAME} in line with discussions over on the 5.3.5 patch.