Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
Mixed whitespace is to be expected. The rpm coding style is a mixture of soft tabs of width four and hard tabs of width eight like you get with ```indent -kr``` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-466920864___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
Thanks for merging @pmatilai. Just had a look through now and code itself is fine. However compared to my branch I noticed the code on master has some mixed whitespace. Have a look through e.g. `rpmlua.c` for leading tabs. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-466593910___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
Okay, as of commit 6d4c68ba10b9713f3e599cf20c2455eb80e9bfe1 we now work on Lua 5.3 without needing any compatibility stuff enabled on Lua-side. Based on this PR but needed some preliminary work to avoid breaking the os.exit() override and three commits needed merging to avoid breaking things. I haven't actually tested on Lua 5.2 but based on function availability this should actually work there as well, so that's what I documented. Thanks @daurnimator for the patches and everybody for the patience :wink: -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-466015259___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
Closed #169. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#event-2155087026___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
Reopened #169. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#event-1911366930___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
Should I reopen then? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-430892851___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
Note that I do plan utilizing the rest of your cleanup + modernization work too, as time permits. Looking at my comment from yesterday, it probably wasn't at all clear from that, sorry. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-430525106___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
Closed #169. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#event-1908560112___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
Lest it all go to waste, I manually merged the bits that do not sacrifice 5.1 compatibility, without dragging in the actual compat layer. Which is non-trivial amount of cleanup already - thanks for all the work so far! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-430187755___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
And I've been telling you what I want: pull the bare minimum required compat magic into rpm. Remember the compatibility requirement for Lua is not a permanent thing but a relatively short-term thing (1-2 years at most) and there's so little change in our Lua department, new additions are hardly a concern. I'd like to see what the "~20 lines of copy-paste" would look like in reality before considering external dependencies. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-381921157___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
> Thanks for the update, but I've been asking NOT to drag in the entire > lua-compat thing Right. Which is why I've asked *many* times (including via email and IRC) how to introduce a new dependency for rpm. > If you need a place to stick the actually required "~20 lines" of > compat-macros/inline functions, I suppose rpmio/rpmlua.h could be used for > that, it's an internal header anyway. I don't think this is a good path: it would make it hard to merge in upstream changes of lua-compat-5.3 as well as require that people who change code to use new lua functions add in new definitions. > the integration should be along the lines of "download latest version from , > unpack and (re)run configure, enjoy". Okay. so that's all that is required for adding a dependency? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-381785133___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
Oh, and in case it turns out it's not "~20 lines" but "~2000 lines" (as in: the full compat-lua thing *is* required in practise): in no case do we want the compat-lua code imported into rpm git. If the entire compat-lua is required, the integration should be along the lines of "download latest version from , unpack and (re)run configure, enjoy". -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-381564514___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
Thanks for the update, but I've been asking NOT to drag in the entire lua-compat thing. If you need a place to stick the actually required "~20 lines" of compat-macros/inline functions, I suppose rpmio/rpmlua.h could be used for that, it's an internal header anyway. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-381560993___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
@daurnimator pushed 4 commits. 69dc569 rpmio/rpmlua.c: Use lua_rawlen instead of deprecated luaL_getn 2fb6dc4 rpmio/rpmlua.c: lua_pushglobaltable provided by lua-compat-5.3 ebb545a luaext/userconfig.c: Remove useless file 22d5899 rpmio/rpmlua.c: LUA_PATH global hasn't been used since lua 5.0 -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/169/files/2f0130e3f127ff569aa0e55eda709b3dbfaaf675..22d58996ca9cd3a3c653eb2f205553a9ecab95e2 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
PR updated with full solution (using lua-compat-5.3) that works with 5.1 and 5.3 on my computer. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-380004383___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
@daurnimator pushed 3 commits. 9c590a2 Squashed 'luaext/compat-5.3/' content from commit bc91f4091 8b7f1dc Use lua-compat-5.3 for backwards compatibility with lua 5.1 2f0130e lib/rpmliblua.c: lua 5.1 compat without compat-5.3 -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/169/files/c2169e607d8f0ff621838ea7fbb4b9a1c74752d2..2f0130e3f127ff569aa0e55eda709b3dbfaaf675 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
Btw, there were a few open questions around the further PRs. e.g. what is the split between rpm and rpmio: the lua library is *mostly* in rpmio, however the `rpmvercmp` (exposed as `rpm.vercmp` in lua) is not in rpmio -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-379987989___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
> This doesn't even compile against a build of Lua where the compat-stuff is > actually disabled, due to missing port of luaL_openlib() in rpmio/rpmlua.c This was brought up above with the open question of how best to solve the compatibilty issues. > Folks, if you're submitting untested code then at least have the courtesy of > stating so. Sorry, this was meant to be part 1 of a 3 part pull request (I was going to submit the rest after). Looks like I messed up the interim commit. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-379982324___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
This doesn't even compile against a build of Lua where the compat-stuff is
actually disabled, due to missing port of luaL_openlib() in rpmio/rpmlua.c.
What's worse, when using a version of lua that it does compile against, it
causes any attempt to use rpm macros crash:
```
$ ./rpm --eval "%{_lib}"
PANIC: unprotected error in call to Lua API (attempt to index a nil value)
Aborted (core dumped)
```
which in terms of the test-suite means:
```
## - ##
## Test results. ##
## - ##
ERROR: All 415 tests were run,
406 failed (4 expected failures).
```
Folks, if you're submitting untested code then at least have the courtesy of
stating so.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/169#issuecomment-379735523___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
> And I meant adding the couple of required #ifdef's into the code directly, > without pulling a whole external entity into our codebase for our rather > modest needs. Well I guess it's easy enough to pull ~20 lines out of compat-5.3 and paste them into the start of the file -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-363507584___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
>> What I'd prefer is a patch that adds support for Lua 5.3 without requiring >> LUA_COMPAT_MODULE for it, but allowing 5.1-5.2 to keep building without any >> extra hassle. >That's what compat-5.3 is for. The question in here was how to bring in a new dependency? Is vendoring the best way? And I meant adding the couple of required #ifdef's into the code directly, *without* pulling a whole external entity into our codebase for our rather modest needs. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-363343286___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
I think vendoring the code is probably what should be done. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-363155032___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
> What I'd prefer is a patch that adds support for Lua 5.3 without requiring > LUA_COMPAT_MODULE for it, but allowing 5.1-5.2 to keep building without any > extra hassle. That's what compat-5.3 is for. The question in here was how to bring in a new dependency? Is vendoring the best way? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-363153092___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
@pmatilai what's with the :green_apple: CI if this breaks something? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-363069682___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
pmatilai requested changes on this pull request. NAK in this form, see above. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#pullrequestreview-93967537___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
So, breaking build on RHEL/Centos-7 is a non-option for the time being, The only thing that qualifies as a precedent we have for this sort of thing is for bundled BDB support and don't really like that either. What I'd prefer is a patch that adds support for Lua 5.3 without requiring LUA_COMPAT_MODULE for it, but allowing 5.1-5.2 to keep building without any extra hassle. As in, keep my cake and eat it too ;) If it requires copying a trick or two from the lua-compat thing into rpm then so be it. Looking at the amount of changed code in is PR, it's not a whole lot really. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-363065548___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
What's the status here? I just ran into this because I'm trying to upgrade lua to 5.3 in Homebrew. It would be good not to have to force 5.3 to build with LUA_COMPAT_MODULE enabled, since that somewhat defeats the upstream purpose of deprecation. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-348862805___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
@daurnimator I don't know. I don't know enough about lua to make an educated decision. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-341964581___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
@Conan-Kudo How should we depend on it without vendoring? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-341949996___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
@daurnimator Generally speaking, subtrees/submodules aren't really acceptable. And we don't usually vendor in libraries. That said, maybe @pmatilai might make an exception for the Lua stuff, I don't know... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-341432247___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
@pmatilai ping? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-329948924___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
> In general we want to keep rpm buildable on recent RHEL which in this case > means RHEL-7, and that in turn means Lua 5.1. But compat stuff for older than > that can go, as far as I'm concerned. In that case I recommend you depend on https://github.com/keplerproject/lua-compat-5.3/ there. I'm not sure how you want to introduce that dependency? I often do it with a git subtree. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-284964810___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] work with lua 5.3 without compat mode (#169)
@Conan-Kudo in that case there is code that can be stripped out. e.g. the stuff inside ``` #if (LUA_VERSION_NUM < 502) || defined(LUA_COMPAT_MODULE) ``` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/169#issuecomment-284941483___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
