Page MenuHomeFreeBSD

[new port] lang/lua54 - new port for Lua 5.4.0
ClosedPublic

Authored by russ.haley_gmail.com on Mar 16 2018, 6:28 AM.
Tags
  • Restricted Project
Referenced Files
F80137741: D14709.diff
Thu, Mar 28, 10:50 AM
Unknown Object (File)
Thu, Mar 21, 4:01 AM
Unknown Object (File)
Mon, Mar 18, 12:53 PM
Unknown Object (File)
Mon, Mar 18, 10:42 AM
Unknown Object (File)
Sun, Mar 10, 3:48 PM
Unknown Object (File)
Sun, Mar 10, 3:48 PM
Unknown Object (File)
Sun, Mar 10, 3:48 PM
Unknown Object (File)
Sun, Mar 10, 3:48 PM

Details

Summary

Hi,

This is a port for Lua 5.4.0

The Lua Manual can be found here:
https://www.lua.org/manual/5.4/

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

Built and lightly tested on Ghost BSD 20 (FreeBSD 12.1 STABLE).

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Makefile
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.

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.

andrew_tao173.riddles.org.uk retitled this revision from Lua 5.4.0 to lang/lua54 - new port for Lua 5.4.0.Jul 18 2018, 2:21 PM
andrew_tao173.riddles.org.uk set the repository for this revision to rP FreeBSD ports repository.

Change OPTIONS_DEFAULT+= to just = in accordance with nitpicking on the 5.3.5 patch.

Makefile
4–5

This should probably be:

DISTVERSION= 5.4.0-work2

Use DISTVERSION. This corrects the package version too (now 5.4.0.w2)

Sync with 5.3.5 work.

This adds the dynamic-libedit patch (which is an extra patch here, rather than unconditionally patching lua.c as in 5.3.5, because unlike 5.3.5 we don't otherwise have any need to patch that file). Other changes are just cosmetic.

Changes in line with 5.3.5 patch:

  • move DOCSDIR to lua54/lua to free up lua54/${PORTNAME} for module ports
  • change doc install to avoid complaints from check-plist

So are we done here? I have nothing more to add.

So are we done here? I have nothing more to add.

I'm happy with it.

I cannot comment on the specifics of lua (i.e. do these changes work), I've never used lua and haven't tested it. However, I am happy with the above changes with the following comments:

  1. I assume the indentation is correct, despite it not rendering correctly in Chrome
  2. Please try to upstream all patches

@jbeich: your thoughts?

And, who will commit this change?

In D14709#349808, @dbn wrote:

I cannot comment on the specifics of lua (i.e. do these changes work), I've never used lua and haven't tested it. However, I am happy with the above changes with the following comments:

  1. I assume the indentation is correct, despite it not rendering correctly in Chrome
  2. Please try to upstream all patches

@jbeich: your thoughts?

And, who will commit this change?

Hi @dbn, Lua 5.4 is still in a beta phase so my preference is to hold this port until the Lua 5.4 official release, then update and push it to the repository. @andrew_tao173.riddles.org.uk has already indicated his preference to hold the port until the Lua release (unless something has changed?)

Hi @dbn, Lua 5.4 is still in a beta phase so my preference is to hold this port until the Lua 5.4 official release, then update and push it to the repository. @andrew_tao173.riddles.org.uk has already indicated his preference to hold the port until the Lua release (unless something has changed?)

I didn't say to hold it until release, I said -work2 was "not ready for prime time" or some such. But in the absence of any statement from the Lua devs about the likely timing of a -work3 or a final 5.4.0, I see these options:

  1. make the port available anyway as is (it doesn't seem unusual to have ports for non-final releases of languages, see e.g. tcl87)
  2. add a patch to stop it enabling the generational GC by default in lua.c (generational GC has known crash/corruption bugs in -work2) and make it available
  3. combine options 1 and 2 by making the patch an option
  4. sit on it until there's a (hopefully more stable) -work3 or final release, but we have no idea of the timeline for this

I'm not all that keen on option 1, but any of options 1-3 might be useful from a Lua development point of view.

Another factor to consider is updating Uses/lua.mk (which doesn't recognize 5.4 yet). I have been testing the idea of giving USES=lua a flavors option for lua modules to use and it seems to work well - I could put that work up as a separate revision for review?

Hi @dbn, Lua 5.4 is still in a beta phase so my preference is to hold this port until the Lua 5.4 official release, then update and push it to the repository. @andrew_tao173.riddles.org.uk has already indicated his preference to hold the port until the Lua release (unless something has changed?)

I didn't say to hold it until release, I said -work2 was "not ready for prime time" or some such. But in the absence of any statement from the Lua devs about the likely timing of a -work3 or a final 5.4.0, I see these options:

  1. make the port available anyway as is (it doesn't seem unusual to have ports for non-final releases of languages, see e.g. tcl87)
  2. add a patch to stop it enabling the generational GC by default in lua.c (generational GC has known crash/corruption bugs in -work2) and make it available
  3. combine options 1 and 2 by making the patch an option
  4. sit on it until there's a (hopefully more stable) -work3 or final release, but we have no idea of the timeline for this

I'm not all that keen on option 1, but any of options 1-3 might be useful from a Lua development point of view.

Another factor to consider is updating Uses/lua.mk (which doesn't recognize 5.4 yet). I have been testing the idea of giving USES=lua a flavors option for lua modules to use and it seems to work well - I could put that work up as a separate revision for review?

I had started a review for updating Uses/lua.mk last year (https://reviews.freebsd.org/D13691 - feel free to re-use this or start a new one). This review is when I was informed about the previous exp-run and why Lua was still at 5.2. My plan had been to follow the Lua releases and try to get everything updated to 5.3, but I have nothing against your suggestions. Option 2 would be my preference; it seems like the best "bang for the buck".

I'm keen to see your flavors work!

Track changes requested by mat for the 5.3 port.

Now that a 5.4.0-beta release has been made, and there are signs that features are actually stabilizing (there have been many changes since work2), perhaps we could pick this up again. The distinfo needs an update of course.

Now that a 5.4.0-beta release has been made, and there are signs that features are actually stabilizing (there have been many changes since work2), perhaps we could pick this up again. The distinfo needs an update of course.

Yes, I'll start on that now. Thanks for the ping.

russ.haley_gmail.com edited the summary of this revision. (Show Details)

Updated for the latest Lua 5.4 Beta release.

  • When the ASSERT option is selected, the extra-patch-assert file fails to apply cleanly. I tried to port it over but I there has been a change to lgc.c and I don't know enough about lgc to apply the patch (Help Andrew?).
  • I tested all the other options and they all compiled.
  • I installed Lua54 beta in the default configuration without issue.

hmmmm?

> Installing for lua54-5.4.0.b_1

> Checking if lua54 is already installed

> Registering installation for lua54-5.4.0.b_1

pkg-static: duplicate file listing: /usr/local/share/doc/lua54/lua/contents.html, ignoring
pkg-static: duplicate file listing: /usr/local/share/doc/lua54/lua/index.css, ignoring
pkg-static: duplicate file listing: /usr/local/share/doc/lua54/lua/logo.gif, ignoring
pkg-static: duplicate file listing: /usr/local/share/doc/lua54/lua/lua.css, ignoring
pkg-static: duplicate file listing: /usr/local/share/doc/lua54/lua/manual.css, ignoring
pkg-static: duplicate file listing: /usr/local/share/doc/lua54/lua/manual.html, ignoring
pkg-static: duplicate file listing: /usr/local/share/doc/lua54/lua/osi-certified-72x60.png, ignoring
pkg-static: duplicate file listing: /usr/local/share/doc/lua54/lua/readme.html, ignoring
Installing lua54-5.4.0.b_1...

  • When the ASSERT option is selected, the extra-patch-assert file fails to apply cleanly. I tried to port it over but I there has been a change to lgc.c and I don't know enough about lgc to apply the patch (Help Andrew?).

The assert patch no longer needs to touch lgc.c, since that was fixing a bug with a misplaced assert that is now fixed upstream. Only the change to lprefix.h is needed now.

You seem to have duplicated a patch? There should be no patch for lua.c other than the extra-patch-libedit-dl one.

lua54/pkg-plist
13 ↗(On Diff #63394)

These %%PORTDOCS%% lines should not be here, PORTDOCS in the makefile automatically handles adding them to the plist. I think this explains your duplication warnings.

New patch to address comments, thank you Andrew.

Update: retested the default LIBEDIT_DL configuration and I don't get command line history

russellh@gbsd2 ~/f/p/l/l/w/l/src> ./lua54
Lua 5.4.0  Copyright (C) 1994-2019 Lua.org, PUC-Rio
> print('test')
test
> ^[[A^C⏎

but it works fine with libreadline:

russellh@gbsd2 ~/f/p/l/l/w/l/src> ./lua54
Lua 5.4.0  Copyright (C) 1994-2019 Lua.org, PUC-Rio
> print('test')
test
> print('test')
test

Build output for LIBEDIT_DL is here: https://pastebin.com/UTVrdZVX

Addressed andrews comments. Fixed missing patch for LIBEDIT_DL and tested LIBEDIT_DL option. twice. :-/

Added option for arc4random random seed. I tested running math.random and it looked randomish to me? I don't know what I'm looking for but the flags were present in the build output:

...
--- liolib.o ---
cc  -O2 -Wall -Wextra -DLUA_COMPAT_5_3 -DLUA_USE_POSIX -DLUA_USE_DLOPEN -DLUA_USE_READLINE_DL -isystem /usr/local/include -O2 -pipe  -fPIC -D__BSD_VISIBLE -DLUA_USER_H='<stdlib.h>' -D'lua_makeseed()=cast(unsigned int, arc4random())' -fstack-protector-strong -isystem /usr/local/include  -c liolib.c -o liolib.o
--- lmathlib.o ---
cc  -O2 -Wall -Wextra -DLUA_COMPAT_5_3 -DLUA_USE_POSIX -DLUA_USE_DLOPEN -DLUA_USE_READLINE_DL -isystem /usr/local/include -O2 -pipe  -fPIC -D__BSD_VISIBLE -DLUA_USER_H='<stdlib.h>' -D'lua_makeseed()=cast(unsigned int, arc4random())' -fstack-protector-strong -isystem /usr/local/include  -c lmathlib.c -o lmathlib.o
...

Looks like Roberto snuck some hardcoded -Os options into the makefile, we need to either patch those back out or think of something else to do with them.

Looks like Roberto snuck some hardcoded -Os options into the makefile, we need to either patch those back out or think of something else to do with them.

You should ask about that on the mailing list. -O2 is specified twice, as well as -Os. Which option does the compiler take?

cc -O2 -Wall -Wextra -DLUA_COMPAT_5_3 -DLUA_USE_POSIX -DLUA_USE_DLOPEN -DLUA_USE_READLINE_DL -isystem /usr/local/include -O2 -pipe -fPIC -D__BSD_VISIBLE -DLUA_USER_H='<stdlib.h>' -D'lua_makeseed()=cast(unsigned int, arc4random())' -fstack-protector-strong -isystem /usr/local/include -Os -c lcode.c

Complete build output here: https://pastebin.com/pB2N7D3w

Patch out targets that added -Os to llex.o, lparser.o, and lcode.o.

Build output here https://pastebin.com/2tLGT9RA

Looks like Roberto snuck some hardcoded -Os options into the makefile, we need to either patch those back out or think of something else to do with them.

You should ask about that on the mailing list. -O2 is specified twice, as well as -Os. Which option does the compiler take?

Generally the last one.

cc -O2 -Wall -Wextra -DLUA_COMPAT_5_3 -DLUA_USE_POSIX -DLUA_USE_DLOPEN -DLUA_USE_READLINE_DL -isystem /usr/local/include -O2 -pipe -fPIC -D__BSD_VISIBLE -DLUA_USER_H='<stdlib.h>' -D'lua_makeseed()=cast(unsigned int, arc4random())' -fstack-protector-strong -isystem /usr/local/include -Os -c lcode.c

That first -O2 needs to be patched out as well - wasn't there something for that in files/patch-src_Makefile before?

../lua54/Makefile
116 ↗(On Diff #63951)

Did the base FreeBSD bug that caused us to require this ever get fixed? I tried following the bug report but lost track of it.

Update to lua 5.4.0-rc1

Update to match recent changes to Lua framework; connect to the build; add the new version to the list of supported versions in default-versions and Uses/lua.mk.

Allowing 5.4 in Uses/lua.mk does have the potential to break builds of ports that specify, e.g., USES=lua:53+ (so that the default version of 5.2 is not in range) and where they use interfaces that change from 5.3 to 5.4. I'm not sure how many cases of this there are (if any); I will do my own test run soon, but an exp-run will likely be needed at some point.

Now that there's an actual release candidate, we can think about pushing this forward.

Test results: adding support for 5.4 causes only two ports (archivers/urbackup-server and x11-toolkits/termit) to switch from lua 5.3 to 5.4 when LUA_DEFAULT is 5.2. (They would not switch if the default were 5.3; both are declared as lua:53+, and the default version is chosen if it is in range, otherwise the latest available version in the range.)

Both of them fail to build as a result.

For archivers/urbackup-server, it's because it has a configure script that looks only for lua53 and older versions.

For x11-toolkits/termit it's because it uses cmake to build, and cmake has no idea that lua54 exists so it also has a hardcoded check for lua53 and older.

In both cases, adding appropriate CONFIGURE_ENV or CMAKE_ARGS *might* fix the issue; I'll have a poke at them.

Update to lua 5.4.0-rc1

Update to match recent changes to Lua framework; connect to the build; add the new version to the list of supported versions in default-versions and Uses/lua.mk.

Allowing 5.4 in Uses/lua.mk does have the potential to break builds of ports that specify, e.g., USES=lua:53+ (so that the default version of 5.2 is not in range) and where they use interfaces that change from 5.3 to 5.4. I'm not sure how many cases of this there are (if any); I will do my own test run soon, but an exp-run will likely be needed at some point.

Now that there's an actual release candidate, we can think about pushing this forward.

I'm not sure what I'm doing wrong but I get the lua54 port files in my ports root directory if I apply this patch.

russellh@gbsd2 ~/f/ports> patch <  ~/freebsd/patches/D14709.diff

Change the two ports that declared themselves lua:53+ to just lua:53, since they don't seem to respond sanely to attempts to force configuration with 5.4 using environment vars.

(In my documentation patch for the porter's manual, I've added a note about why this style of version specification should be avoided. See https://reviews.freebsd.org/D24430 for details.)

In my opinion, it's a bug in Mk/Uses/lua.mk that USES=lua:53+ chooses lua 5.4 over 5.3

In my opinion, it's a bug in Mk/Uses/lua.mk that USES=lua:53+ chooses lua 5.4 over 5.3

What alternative logic do you suggest?

In my opinion, it's a bug in Mk/Uses/lua.mk that USES=lua:53+ chooses lua 5.4 over 5.3

What alternative logic do you suggest?

Maybe chose the closest from default version that fits the constraints

In my opinion, it's a bug in Mk/Uses/lua.mk that USES=lua:53+ chooses lua 5.4 over 5.3

What alternative logic do you suggest?

Maybe chose the closest from default version that fits the constraints

In other words, choose the lowest version in the range if the default version is below the start of the range, or the highest version if it is above the end?

Should the rule be the same for 53+ as for, say, 53-54 ?

(I should note, I did not change the version selection logic much in my patch for lua.mk; the old lua.mk would choose the highest version in these kinds of cases too.)

I'm honestly not sure that there's a rule that works well for all kinds of ports. The only (unflavored) ports for which lua:53+ really make sense would be ones that are actually written in Lua, rather than embedding Lua into C code (or whatever other language), because the Lua API has breaking changes between every version (while the 5.3 to 5.4 changes are relatively small, there's no guarantee at all about future versions).

The major benefit I can see from your suggested rule is that it would ensure that advancing LUA_DEFAULT would not cause any port to suddenly switch to an older version of Lua, whereas with the current logic, a 53+ port will build with 5.4 once that is added, but revert to 5.3 if LUA_DEFAULT was changed from the current value of 5.2 to 5.3.

The downside would seem to be that ports that really can benefit from a newer Lua version would be stuck on an older one.

In the specific case of the two ports referenced here, I think it's clearly a bug in the ports that they specify 53+ when they cannot actually build with anything other than exactly 5.3.

Others have supported the "closest to default" rule, so I'll put up a separate patch for that.

russ.haley_gmail.com edited the summary of this revision. (Show Details)
russ.haley_gmail.com edited the test plan for this revision. (Show Details)

Updated to use the official release Lua 5.4.0

Please make sure that when committed, this is done by first copying lang/lua53 to lang/lua54.

lang/lua54/Makefile
95–122 ↗(On Diff #74830)

All this needs to happen before the options block, see https://www.freebsd.org/doc/en/books/porters-handbook/porting-order.html.

  • Removed non-relevant ports packages included by mistake
  • Copied current Lua53 makefile and adjusted for Lua54.
  • Moved OPTIONS variable block below build variables (will submit a correction to lua53 as well)
russ.haley_gmail.com retitled this revision from lang/lua54 - new port for Lua 5.4.0 to [new port] lang/lua54 - new port for Lua 5.4.0.Jul 24 2020, 9:40 PM
  • Removed non-relevant ports packages included by mistake

They were not included by mistake; they were the ports which would otherwise have broken by adding lua54 as per previous discussion.

However, the change to version selection logic from D24492 does hopefully obviate that issue, since now adding lua54 won't change the lua version used by any existing port.

  • Removed non-relevant ports packages included by mistake

They were not included by mistake; they were the ports which would otherwise have broken by adding lua54 as per previous discussion.

However, the change to version selection logic from D24492 does hopefully obviate that issue, since now adding lua54 won't change the lua version used by any existing port.

Ah, I thought I had mucked something. Shall I restore them?

Ah, I thought I had mucked something. Shall I restore them?

Nope. Their maintainers can deal with them in due course.

Accepting as portmgr for the Mk bits.

Update to 5.4.1

Better handling of ASSERT option (no need for a patch file)

Reorder things to pacify portclippy

This revision was not accepted when it landed; it landed in state Needs Review.Oct 15 2020, 5:55 PM
This revision was automatically updated to reflect the committed changes.