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.

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

In D14709#349765, @andrew_tao173.riddles.org.uk wrote:

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?)

In D14709#349874, @russ.haley_gmail.com wrote:

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?

In D14709#350017, @andrew_tao173.riddles.org.uk wrote:

In D14709#349874, @russ.haley_gmail.com wrote:

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!

Flavors stuff is up at D16494

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.

In D14709#478684, @andrew_tao173.riddles.org.uk wrote:

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.

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=199480

Is this patch for arc4random something that we should look at applying?

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.

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.

In D14709#483029, @andrew_tao173.riddles.org.uk wrote:

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

In D14709#483062, @russ.haley_gmail.com wrote:

In D14709#483029, @andrew_tao173.riddles.org.uk wrote:

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?

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.

In D14709#537546, @andrew_tao173.riddles.org.uk wrote:

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

Should work with patch -p1

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

In D14709#538585, @antoine wrote:

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 D14709#538595, @andrew_tao173.riddles.org.uk wrote:

In D14709#538585, @antoine wrote:

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 D14709#538596, @antoine wrote:

In D14709#538595, @andrew_tao173.riddles.org.uk wrote:

In D14709#538585, @antoine wrote:

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.

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

In D14709#571428, @russ.haley_gmail.com wrote:

• 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.

In D14709#571443, @andrew_tao173.riddles.org.uk wrote:

In D14709#571428, @russ.haley_gmail.com wrote:

• 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?

In D14709#571480, @russ.haley_gmail.com wrote:

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

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