Bring Lua to 5.4.6. #875
Bring Lua to 5.4.6. #875
Conversation
This prepares the ground for bringing Lua up to 5.4.6. It adds some missing copyrights, removes some overlooked copyrights, makes vertical whitespace consistent across files and adds `[jart]` comments in various places to make subsequent Lua comparisons easier. Well. Hopefully. In theory, nothing here is controversial. If we merge this, the 5.4.4 update will be “all signal, no noise.” But the noise level isn’t all that high, so it probably doesn’t matter all that much.
I discovered that five test files were encoded with ISO-8859-1 rather than UTF-8: ``` $ iconv -f UTF-8 third_party/lua/test/*.lua >/dev/null iconv: third_party/lua/test/db.lua:299:33: cannot convert iconv: third_party/lua/test/files.lua:95:18: cannot convert iconv: third_party/lua/test/pm.lua:76:19: cannot convert iconv: third_party/lua/test/sort.lua:292:6: cannot convert iconv: third_party/lua/test/strings.lua:94:39: cannot convert $ file third_party/lua/test/db.lua third_party/lua/test/db.lua: ISO-8859 text $ file third_party/lua/test/files.lua third_party/lua/test/files.lua: ISO-8859 text $ file third_party/lua/test/pm.lua third_party/lua/test/pm.lua: ISO-8859 text $ file third_party/lua/test/sort.lua third_party/lua/test/sort.lua: ISO-8859 text $ file third_party/lua/test/strings.lua third_party/lua/test/strings.lua: ISO-8859 text ```
|
In case it isn't obvious: the "Cosmo Lua 5.4.x to 5.4.y" links I've given show only the changes from .x to .y. The "Files changed" in the pull request, by contrast, show changes from master. |
Some tests — in `files.lua` and `pm.lua` — count bytes. Since the byte count of encoded characters changes from ISO-8859-1 to UTF-8, the tests fail.
|
@michaellenaghan, the patch for 5.4.6 looks good to me. Can you rebase against the current master (as there were several dlopen-related changes that created a conflict)? Thank you! |
|
@pkulchenko Sorry, Paul; I tried to get to this before we leave, but it looks like I've run out of time. (The conflicts aren't, for the most part, difficult, but some of them require some thought.) I can do this when I'm back in mid-Jan? And just to make sure we're on the same page: would you commit the individual releases (5.4.3, 5.4.4, etc.) or just the final one (5.4.6)? (Obviously, I don't want to waste time rebasing PRs that you don't plan to merge anyway!) In addition to "merge intermediates" and "merge final" there's a third option: you could "merge final" without squashing. Then there'd be just one merge, but the intermediates would still be part of the history. |
|
@michaellenaghan any updates on this? @pkulchenko do you have any preferences on doing it in one PR vs multiple? FWIW I do not have much of a preference myself; if I had to call it one way or the other I'd probably say "merge final" for expedience's sake. |
|
One PR should be fine and would be my preference. I'd just commit 5.4.6 changes and wouldn't worry about the intermediate steps (as long as the end result is the same). |
|
When I took another look at this in Jan it seemed like there were in fact so many changes that I'd effectively have to start from scratch. Given that the original PR was never accepted, I didn't know if that effort would pay off. Would it? (If @mrdomino is taking over the answer doesn't matter.) |
|
I can't speak for anyone else, but when I first showed up I just ignored any PRs that had already been open when I arrived. Sorry about that. Barring unforeseen circumstances (e.g. getting hit by a bus) I'll be around to make sure this gets reviewed and merged when the time comes. I would like to see this PR land. I was not planning on implementing it myself though. |
|
OK, I'll put this back on my list and take a look next week. If I don't think I'll be able to complete it I'll let you know. I have three other PRs (#868, #869, #870) that fixed timezone issues and were also overlooked. I rebased some of them at the end of last year, but... crickets. At this point I can't see trying again. Should I close them? |
|
All right, sounds good, keep me posted. I'll take a look at those PRs and see if there is anything to be done. I would request that you not close them yet. |
|
@michaellenaghan just want to reiterate that I am sorry we were not able to integrate your contributions last time and also I would really like to have your help with the project. One thing is that a lot of us usually hang out on redbean discord, and we are not averse to being poked / harangued about stuff like PRs there. If something is not getting attention, do feel free to shout a bit about it. |
The upstream Lua 5.4.5 to 5.4.6 diff is here.
The Cosmo Lua 5.4.5 to 5.4.6 diff is here.
===
You can either apply the pull requests step by step, or just this one. In my opinion, step by step is better; the work was already done, and the first two changes are complicated. I think the intermediate steps are like an insurance policy; most of the time you never need insurance, but when things go wrong…