[João Valverde <joao.valverde@xxxxxxxxxxxxxxxxxx>]

On 08/20/2016 02:03 PM, Jo�o Valverde wrote:
> On 08/20/2016 01:18 PM, Peter Wu wrote:
> > On Fri, Aug 19, 2016 at 05:38:57PM +0100, Jo�o Valverde wrote:
> > > On 08/19/2016 04:05 PM, Jo�o Valverde wrote:
> > > > On 08/19/2016 03:56 PM, Jo�o Valverde wrote:
> > > > > On 08/19/2016 02:54 PM, Peter Wu wrote:
> > > > > > On Mon, Aug 08, 2016 at 09:17:35PM +0100, Jo�o Valverde wrote:
> > > > > > > On 08/08/2016 05:58 PM, Pascal Quantin wrote:
> > > > > > > > Hi Jo�o,
> > > > > > > >
> > > > > > > > 2016-08-08 18:52 GMT+02:00 Jo�o Valverde
> > > > > > > > <joao.valverde@xxxxxxxxxxxxxxxxxx
> > > > > > > > <mailto:joao.valverde@xxxxxxxxxxxxxxxxxx>>:
> > > > > > > >
> > > > > > > >     Is moving to Lua 5.3 something we should look into?
> > > > > > > >
> > > > > > > >     The 64 bit integer support seems really promising.
> > > > > > > >
> > > > > > > >
> > > > > > > > Hariel explained us that Lua 5.3 was a completely different
> > > > > > > > language (a
> > > > > > > > bit like what you have between Python 2.X and 3.X). You can find
> > > > > > > > much
> > > > > > > > more info (from people knowing what they are taling about - so
> > > > > > > > not me
> > > > > > > > ;)) in bug 10881.
> > > > > > > >
> > > > > > > > Pascal.
> > > > > > >
> > > > > > >
> > > > > > > Thanks for that Pascal. The only sane way to approach the issue IMHO
> > > > > > > is to
> > > > > > > accept that this may and probably will break backward compatibility
> > > > > > > (not
> > > > > > > even think about supporting 5.1 or 5.2) and just consider whether
> > > > > > > that break
> > > > > > > is justified (hint: it is).
> > > > > >
> > > > > > Why is it justified to break backwards compatibility and move from
> > > > > > 5.2
> > > > > > to 5.3 without the ability to chose for 5.2? What is the killer
> > > > > > feature
> > > > > > of 5.3 that makes it totally worth to possibly break older
> > > > > > dissectors?
> > > > > >
> > > > > > The disadvantage of C plugins is that it had to be recompiled for
> > > > > > newer
> > > > > > versions. With a move from 5.2 to 5.3 and also removing GRegex and
> > > > > > bitop
> > > > > > you make it quite likely to break Lua dissectors in some way.
> > > > > >
> > > > > > I have once written a Lua library in C, interfacing with Libgcrypt
> > > > > > for
> > > > > > which I studied the Lua manual. The API changes with 5.3 were not
> > > > > > that
> > > > > > significant if I remember correctly (though you have to be careful
> > > > > > with
> > > > > > providing a compatibility layer), but the ABI is certainly not
> > > > > > compatible.
> > > > > >
> > > > > > In the recent proposed patches, you seem to have no issues with
> > > > > > breaking
> > > > > > backwards compatibility. Have you developed Lua dissectors before?
> > > > > > Breaking things for the sake of "shiny, new, future" is not an
> > > > > > acceptable motivation, there must be something more appealing to
> > > > > > justify
> > > > > > such breakage. Having 64-bit integer support, but taking away the
> > > > > > bitop
> > > > > > library is a net loss without even considering the other factors.
> > > > >
> > > > > Doesn't Lua 5.3 provide native bit operators? If so there is not net
> > > > > loss of functionality. That was my reasoning at least.
> > > > >
> > > > > The language incompatibilities between 5.2 and 5.3 are minor. The
> > > > > wireshark API is exactly the same.
> > > > >
> > > > > LPeg is more powerful and Lua-thonic than lrexlib, but there is a
> > > > > learning curve for that, no doubt. For anyone relying on lrexlib,
> > > > > it's a
> > > > > significant break. We can keep lrexlib, that's not a problem and it is
> > > > > orthogonal to the other changes.
> > > > >
> > > > > As far as killer features go, besides the obvious, how about better
> > > > > UTF-8 support?
> > > > >
> > > > > I don't have time for a more detailed answer right now but I'd like to
> > > > > say I think this change is entirely justified but I also completely
> > > > > understand disagreeing with that opinion.
> > > >
> > > > I'm referring to the upgrade to Lua 5.3 here, i.e, breaking backward
> > > > compatibility, same as any other Lua script moving from 5.1/5.2 to 5.3.
> > >
> > > Let me also ask, Peter, you're pushing back against building Lua from
> > > source
> > > and also pushing back against upgrading to 5.3.
> >
> > I am against pulling the Lua source tree in the tree, reasons were
> > already given in https://code.wireshark.org/review/#/c/17172
> >
> > I am not against adding Lua 5.3 compatibility, but would like to see
> > compatibility for existing dissectors. Keeping support for 5.2 (or even,
> > argh, 5.1) would allow distros/users to build with older versions if
> > they desire.
> >
> > > I'm not seeing how that is a viable long-term option on Linux.
> >
> > If 5.3 compatibility is added it should be no problem. After looking at
> > the actual changes (https://www.lua.org/manual/5.3/readme.html#changes
> > and also the linked incompatibilities), I see the following changes to
> > the Lua code:
> >
> >  - 64-bit integer support. Nice!
> >  - Bitwise operators. Cool, but we would have to add a compatibility
> >    library to keep old dissectors working.
> >  - UTF-8 "support". This just adds some helper functions that can
> >    calculate lengths, converts between codepoints and chars and adds a
> >    %U format specifier. It adds no new data type so won't really affect
> >    the WSLUA API.
> >
> > So you are right that there are not many changes in that area. On the
> > C library side we have also have no really window shattering changes
> > other than the integer stuff. Of course you would have to recompile any
> > C libraries, but I realize that it is probably a special case.
>
> I think there is a disconnect here because you are seeing Lua as a
> system dependency. I see it as Wireshark's own embedded language
> interpreter (although developed by the Lua team under a suitable license).
>
> (Sorry for my brevity, I really appreciate your input).

I also meant to add that it would be better to rip-off the compatibility 
band-aid, IMO, if the 5.3 features justify the upgrade.