lua5.1 compatibility

[smarek]

[smarek]

i intended to use BitOp, as introduced in CI tests setup

https://github.com/smarek/ci_targets/blob/master/prepare-lua#L22 https://bitop.luajit.org/

Is that snippet still correct?

[ildar]

Looks like this whole PR can be simplified with using https://github.com/keplerproject/lua-compat-5.3/

[Qix-]

A number of points:

• Not everyone has luarocks available, nor is luarocks compatible with all cases of Lua. Please don't make this assumption. If it must be used in CI, this is code smell for something not being available in the repository that users would otherwise need, and thus creates the dependency on Luarocks by end users. Testing (test libs, runtimes, etc.) is the only exception to this rule.
• Lua 5.1 and above all have bitwise operations and/or the bit[32] library. This should be sufficient for the use-cases here. Please don't introduce a dependency for it.
• Please don't use compat libs. They usually overwrite globals (though I know Keplar does not) and are NOT compatible with all environments where Lua is used. Further, I don't think 5.1 is missing anything that the Kaitai runtime needs aside from string.unpack, which is already ponyfilled (rightfully so).

As someone who is currently blocked (using kaitai) by lack of 5.1 support in this particular case (and have to resort to using hand-rolled parsers still), any of the three above things would still prevent me from using Kaitai even after this PR lands.

[smarek]

@Qix- points taken

• i do not expect luarocks to be available, only use them in testing (ci_targets) but since the luautf8 and luabitop rocks dependencies, i wonder if we can include them in runtime directly
• bitwise operations
  • from docs http://lua-users.org/wiki/BitwiseOperators
    • 5.0 => bitlib
    • 5.1 => BitOp (library)
    • 5.2 => bit32 (native)
    • 5.3 => bitwise operators (& | >> << etc.)
  • i suppose supporting Lua 5.0 does not make sense anymore, but 5.1+ does, so either it's bit (library) or bit32 (native) interface we want to use
  • for 5.3 we still have to use bit (library) because we cannot support the native bitwise operators in older versions

Is the bitwise support matrix right, or did i miss anything? Because you say Lua 5.1 has built-in bitwise operations, but I don't think it's true. Also I'm not sure about support in LuaJIT and whether we wanna support that too

[ildar]

Let me add my 10 cents

1. I referenced LuaRocks only as a big catalogue of Lua modules. A developer doesn't have to install or use it. The proposed code is starting to use external libraries. So a developer has to install them manually or with any tool he likes.
2. On bitwise operations: compat-5.3 library works on Lua 5.3 (haha!) so it may allow one code for all versions.
3. @Qix : general assumptions of "badness" of compat libraries should better be confirmed by issue ## or discussion references. Ideally --- by practice.

[smarek]

In my opinion, if anything, I'd like to replace all compiled dependencies (such as compat or lua-utf8) with pure-lua dependencies. Those can be easily included in runtime directly, and will allow for better cross-compatibility without need to install external libraries in platform-specific way (be it luarocks or apt)

I initially made a mistake presuming luarocks is standard way of dependencies management, included all on all kinds of platforms with lua support, but now I think we should use luarocks to install only test dependencies (luaunit and luafilesystem) and all the others have included in runtime directly.

So, given this, I'm actually against using compat53 library, as it needs to get compiled for target platform, and the same can be achieved by pure-lua probably.

[ildar]

I like your point of view. But I doubt about implementing (missing) bitwise operations in pure Lua. Do you have an example or idea?

[smarek]

@ildar well from wiki http://lua-users.org/wiki/BitwiseOperators it seems there are several pure-lua implementations, and since we will always use the bit or bit32 interface, it should be doable Only thing I've not yet done, is testing whether the interface methods are same or have differences (number and order of arguments, function names) Most promising is probably this one? https://github.com/davidm/lua-bit-numberlua/

[ildar]

Indeed.

https://github.com/davidm/lua-bit-numberlua/ Looks good.

[Qix-]

i suppose supporting Lua 5.0 does not make sense anymore, but 5.1+ does

Correct. 5.1+ is pretty typical due to LuaJIT's freeze at 5.1.

o either it's bit (library) or bit32 (native) interface we want to use

Please, no libraries.

local function try_require(name)
    local success, mod = pcall(require, name)
    if success then return mod else return nil end
end

local bit = rawget(_G, 'bit32') or rawget(_G, 'bit') or try_require('bit') or try_require('bit32') or error[[no bitwise library found]]

print[[bit library found]]

$ lua5.1 bit-test.lua
bit library found

$ lua5.2 bit-test.lua
bit library found

$ lua5.3 bit-test.lua
bit library found

for 5.3 we still have to use bit (library) because we cannot support the native bitwise operators in older versions

Correct.

Is the bitwise support matrix right

No. As I mentioned in my previous comments, Lua 5.1, 5.2, and 5.3 all have everything Kaitai would need aside from maybe utf-8 support and string.unpack, depending on what it needs there.

[smarek]

@Qix- i'm still not sure, whether clean Lua 5.1 has bit/bit32 interface? Why else would BitOp exist, if said versions contained built-in bitwise operations?

[Qix-]

Strange. I downloaded the source from lua/lua and compiled myself. It appears the Ubuntu distribution comes with bit installed but the PUC Rio (mainline) sources do not. Sounds like some Debian package manager needs to be yelled at, but that's a different issue.

I think most people working with 5.1 are using LuaJIT or are in an environment where bit exists.

I think the best solution here, personally, is to indicate that for 5.1 support, you need to provide bit or bit32 by way of require() yourself, whether that be provided by the implementation of Lua you're using or by importing some library (e.g. BitOp). We shouldn't provide it here as I agree that setting up compilation is going to be a worse idea.

Just a line in the documentation, probably bolded. Let's assume that bit or bit32 are available either as globals or as require()-able modules.

---

If we want to be really respectful of platforms/environments/etc, don't even check global - go straight to the require. That creates a very solid contract.

local function try_require(name)
    local success, mod = pcall(require, name)
    if success then return mod else return nil end
end

local bit = try_require('bit') or try_require('bit32') or error[[no bitwise library found]]

print[[bit library found]]

[smarek]

@Qix- yes, I've already found mentions of Debian lua maintainers adding the bit extension to versions where it isn't native, so I'm less surprised than you probably. Even tho, it might not be Debian maintainer fault, as lua-users BitwiseOperators mention backporting as the reason.

As of version 5.2, Lua ships with the library [bit32] that adds support for bitwise operations. Previous versions of Lua did not include bitwise operators, but bit32 has been backported to version 5.1. Alternatively, there are Lua libraries for this as well as some patched versions of Lua.*

Anyway, there is one more issue with those, and that is potential interface inconsistency. Is bit interface consistent with bit32? Is bit32 interface consistent with BitOp? And are those two consistent with https://github.com/davidm/lua-bit-numberlua/ ?

I'm thinking about writing some kind of sanity tests that would run before any of these methods are called, and would fail out hard, if the bit/bit32 interface found locally is not producing expected results, or the calls to the interface methods fail, because number/order of arguments differ from our expectations.

And that whole possible mess, i've just described, motivates me to just use some library, such as mentioned lua-bit-numberlua, pack that along in the runtime, and not worry about any platform-specific modifications/issues.

@Qix- do you think i worry too much, or how would you implement the precautions if you kinda agree with me?

[smarek]

Elaborating a bit more, in language where require('bit') might require 30 different file locations, be it user test script with the same name, to part of library, that is not at all providing bitwise operations method, i'm quite worried

Maybe, this kind of explicit require/mention of individual interface methods would guarantee that at least the correct methods exist in the imported interface/module?

local bnot = bit.bnot
local band, bor, bxor = bit.band, bit.bor, bit.bxor
local lshift, rshift, rol = bit.lshift, bit.rshift, bit.rol
-- etc...

source: https://bitop.luajit.org/api.html#shortcuts

[smarek]

I've updated PR with code for requiring utf8 and bit/bit32 modules, please let me know what you think

[smarek]

@Qix- I've tried to fix the whole UNICODE/UTF/CP437/SJIS/... mess, would you be please so kind, and looked at the changes in be0ed12ec90f81e15433ff9f336dae5551d1e10e ? The Unicode module is a bit modified version from NMAP And I think it should be relatively simple to extend it for SJIS (CP932) code-table, ie. by using this https://uic.win/en/charset/show/cp932/

[smarek]

Just a few suggestions.

Along with the above comment, I think you'll get a little bit of a performance increase (in non-LuaJIT environments) by caching bor, rshift, etc. since Lua does not optimize ASTs.

I'll probably do that as well, maybe not in current release, i'm actually thinking of creating separate todolist, that we can both work on independently, once this large changes are merged

---

One issue, that might be interesting to you @Qix- is the issue of de-coding 8-byte signed numbers so it works on all of the platforms we want the runtime to work on https://github.com/iryont/lua-struct/issues/3 struct.lua is taken directly from this repository, and even tho I tried to solve the issue, it's obviously far beyond my knowledge

It currently fails some of the tests as well, but it's easier to write some short test, than to run the whole kaitai test suite, but I'll leave that up to you, if you'll want to participate on this

[Qix-]

Losslessly storing 64-bit numbers in pure Lua is impossible without a bignum type, and even then I'd wager it's more work than it's worth (see my comment on your linked issue).

[smarek]

Ok, so we're left with two options, fail hard for reading 64bit numbers (s8be, s8le, u8be, u8le) or fail if the read value is bigger than what platform can handle (eg. reading 53/54 bits will yield correct number, reading more will fail hard)? Or is there better solution?

[generalmimon]

Losslessly storing 64-bit numbers in pure Lua is impossible without a bignum type

Ok, so we're left with two options, fail hard for reading 64bit numbers (s8be, s8le, u8be, u8le) or fail if the read value is bigger than what platform can handle (eg. reading 53/54 bits will yield correct number, reading more will fail hard)?

Note that this is not a new topic, the same problem has been also encountered by JavaScript, see https://github.com/kaitai-io/kaitai_struct/issues/183.

[smarek]

@generalmimon so it's about decision affecting whole kaitai environment? I still think, it should be declared, whether we

a. expect the target runtime to fail hard if the requested data type cannot be represented/worked with correctly (s8le/be, u8le/be methods will fail, regardless of the read data size) b. expect the target runtime to fail hard, only if the read value cannot be represented correctly (ie. returning uint32/int32 if the read value fits, and failing otherwise) c. expect the runtime to provide, if necessary, additional data type to handle such data (java BigInteger, lua BigNum, etc.), even if that means introducing new runtime dependency or target OS/platform constraints

[ildar]

A minor note: The top-level ‘unpack‘ function in 5.1 standard is usually associated with _G.unpack ( which is now table.unpack). So i suggest using ‘local sunpack‘ instead to avoid confusion.

[smarek]

Gentlemen, seems i've done everything i could, so it's now ready to merge

This PR is tightly related to https://github.com/kaitai-io/kaitai_struct_compiler/pull/206/files which is required for the 5.1/5.2 targets to work (bit manipulation, encoding unicode literals)

I'd like to solve all the mentioned issues in separate tickets/developments (bignumber, wider unicode/utf8 support, performance optimizations, lua-bit-numberlua instead of native/BitOp ), if we may

Also current statistics about tests, ran by different Lua versions with runtime in this PR

> lua -v
Lua 5.1.5  Copyright (C) 1994-2012 Lua.org, PUC-Rio
# Ran 210 tests in 0.031 seconds, 194 successes, 10 failures, 6 errors

> lua -v
Lua 5.2.4  Copyright (C) 1994-2015 Lua.org, PUC-Rio
# Ran 210 tests in 0.039 seconds, 194 successes, 10 failures, 6 errors

> lua -v
Lua 5.3.5  Copyright (C) 1994-2018 Lua.org, PUC-Rio
# Ran 210 tests in 0.049 seconds, 195 successes, 9 failures, 6 errors

Both Lua and modules (luaunit, lfs and bitop) installed through hererocks and subsequently luarocks

[ildar]

I see. The dot isn't for relative something, it's for overriding the built-in module.

[smarek]

@ildar I thought that was for import path/precendence, we certainly do not want to import different unicode module, than our own. Still does not do any harm, so I'll probably keep it now.