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:
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.
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
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
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
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
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
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
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
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
> 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
>
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
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
@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
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:
@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
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.
> 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
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
> 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
>> 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?
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:
> 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?
--
@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:
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:
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
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
@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:
@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:
@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
@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
> 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.
@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:
33 matches
Mail list logo
