lua5.1 compatibility

[smarek]

No description provided.

[generalmimon]

I don't think that different Lua versions should have separate directories. The current approach is that one language goes only into one directory, the language version doesn't matter. Using a special branch for every target in the ci_artifacts repo ensures that there are no conflicts.

See branches python/2.7 and python/3.4 for instance, all test artifacts go to directory test_out/python in both cases, not test_out/python2 or something like that.

[smarek]

Well, maybe I'm confused by ci-python, ci-python3, ci-python3 scripts, that use different directories. However it seems those are not run in Travis CI, only the ci-python one in differently configured environment.
I've reverted, and hopefuly the kaitai-io/kaitai_ci_ui#8 will suffice to differentiate test targets

[generalmimon]

I'd rather use only lua command in this script (not lua${CURRENT_LUA_VERSION}). The ci-* scripts are not exactly designed for selecting the language compiler/interpreter like you did here. This should be done in .travis.yml ideally by installing the right apt packages or running some commands, eventually in prepare-lua script.

If running this (prepare-lua:13) will make available only the lua5.3 command and not lua (I don't know, that's a question):

hererocks lua_install -r^ -l${VARIETY}

Can you rather add an alias or symlink the lua5.3 executable (in .travis.yml build matrix or prepare-lua) to be available also as lua?

[smarek]

Well, the Lua binary selection was already in place, I've only modified it to reflect on Travis env variable VARIETY
see https://github.com/kaitai-io/kaitai_struct_tests/blob/master/ci-lua#L13

If you want to change the way it was, i've just tested and hererocks always installs the lua into lua_install/bin/lua regardless of version in my environment. If the corect lua bin detection procedure was always pointless, we can safely get rid of that.

However, it might be useful for development if multiple lua binaries are installed, not through hererocks, just sayin

[generalmimon]

If you want to change the way it was, i've just tested and hererocks always installs the lua into lua_install/bin/lua regardless of version in my environment. If the corect lua bin detection procedure was always pointless, we can safely get rid of that.

Yeah, if that's the case, I'd definitely prefer it.

However, it might be useful for development if multiple lua binaries are installed, not through hererocks, just sayin

Well, I think it's easier to just temporarily symlink the lua command than rely on some magic variable, if one needs to run multiple Lua versions locally. Are the Lua versions 5.1, 5.2, and 5.3 really so different that it makes sense to test them all?

[smarek]

Ok, i'll change the script to only use lua binary, no detection whatsoever

They are different, especially 5.1 to 5.3 is big step, and 5.2 is the target we want because of Wireshark, so I'd really like to maintain high compatibility by testing with each version separately

[smarek]

Also, comparison

5.3

./run-lua
Ran 210 tests in 0.044 seconds, 195 successes, 9 failures, 6 errors

5.2

./run-lua
Ran 210 tests in 0.038 seconds, 191 successes, 13 failures, 6 errors

5.1

./run-lua
Ran 210 tests in 0.038 seconds, 190 successes, 12 failures, 8 errors

[generalmimon]

I think this check should be kept. If there is no lua executable, there's no point in doing anything else.

[smarek]

Almost none of the others ci-* scripts has this, so I don't see the point?

[generalmimon]

@smarek For your own convenience, you should never commit directly on master branch of your fork. Create a topic branch for every pull request you create. See kaitai-io/kaitai_struct_doc#38 (comment).

[generalmimon]

Looks good to me, thanks for the contribution!

[smarek]

Thanks, yes i'll do feature branches, once the development here takes off a bit :) I know the drill
Cheers!