Conversation

This PR updates the code in src/modules/wreck/luastack.c for compatibility with Lua 5.2.

Additionally, a minor testing fix is included (discovered on c9.io), along with the removal of accidentally committed debug code that used inspect.lua, which we do not install with the flux-core package.

Finally, Travis testing is updated to run at least one builder using lua5.2, to ensure continued support of that version. (have yet to see if that works)

This comment has been minimized.

Hm, that seems to be the last test run before the sharness tests start.
I'm not sure how this PR could have affected those tests as a whole, but perhaps I should try removing the last commit and retesting to see if including 2 lua packages is to blame.

This comment has been minimized.

Ok, I think I found the issue. The version of luarocks in Ubuntu 14.04 only works with Lua 5.1, so the built luaposix module is also for 5.1, which the lua5.2 interpreter will happily load, but then hangs -- which happens in the sharness.d/flux-sharness.sh code here:

Problem: luastack.c is not compatible with Lua 5.2, which removed
the LUA_GLOBALSINDEX special index. Also, per-coroutine globals
no longer seem to be supported, instead coroutines (as created by
luaL_newthread()) share global environment of the main Lua state,
so the hack to use these threads for each Lua script in order to get
a different per-script globals table no longer works.
Solution: Instead of using the hacky coroutine and shadow global table,
create a function environment (setfenv in 5.1, and _ENV/1st upvalue
in 5.2) for each compiled chunk (script), and save *that* table in
the registry, to later use to look up callbacks registered by the Lua
plugins. Dump the use of luaL_newthread(), it is no longer needed --
we weren't really using coroutines anyway.
Fixes#62

Problem: t2000-wreck-env.t 'setenv all' test fails if there are environment
variables that contain HOSTNAME, FLUX_URI, or ENVIRONMENT.
Restrict the regex used with `grep -ve` further to avoid false failures of
this test in some environments.

Lua test environment in src/bindings/lua/Makefile.am was overwriting
existing LUA_PATH and LUA_CPATH, which thwarted efforts to use a lua
installation from non-standard path. Instead of overwriting these env
vars, prepend to them to allow the side installed Lua installation or
modules to be found.

Problem: if './?.lua' is not in LUA_PATH, then fluxometer.lua will not
be found during 'make check'.
Ensure the in-tree fluxometer.lua is found by adding its path explicitly
to LUA_PATH in AM_TESTS_ENVIRONMENT.
Fixes#1585

This comment has been minimized.

Ok, after many, many, many attempts, and a few fixes along the way, I think I have this PR testing against Lua 5.1 and 5.2. The changes are:

Don't install the lua apt packages, instead use hererocks, which is a helper script that installs Lua to separate roots, including a copy of luarocks. hererocks is installed by pip and used in the travis-dep-builder.sh

This comment has been minimized.

Updated the travis-dep-builder.sh script so that it only uses hererocks when LUA_VERSION is set or the -L, --lua-version option is used. This should make it so no changes are required in flux-sched once this PR is merged.

I also figured out why I thought environment variables from the .travis.yml weren't being set in the before_install script (my fault), so I removed the changes to move everything into script:, which reduced the amount of churn in the PR.

This comment has been minimized.

Ok, finally got the error out of Travis for the python build and I'm still mystified. This only happens with LUA_VERSION=5.2, I guess some kind of environment issue? Anyone got any hints on what might be wrong here?

Allow testing with different Lua versions by optionally using the hererocks
package in travis-dep-builder.sh. The hererocks script builds and installs
Lua versions to different roots (including luarocks), which will allow
testing with multiple versions much more tractable.
The use of hererocks is only activated if LUA_VERSION is set, or a new
-L, --lua option is used.

Update .travis.yml to set LUA_VERSION for every build in order to trigger the
use of hererocks support in `travis-dep-builder.sh`.
Remove the lua* APT packages so they don't accidentally creep back into the
build, which can cause difficult to track down problems, especially when
Lua 5.1 and 5.2 packages are mixed together.

This comment has been minimized.

I was able to get some more information by setting SIDEFLUX_DEBUG in the travis environment and the failure above is what you get when flux start fails. In this case (not surprisingly) this was a Lua issue. I believe the problem is some Lua modules are being linked statically with liblua.a, and Lua 5.2 especially doesn't like this when liblua.so is also dynamically linked.

I'm not sure why this was a problem in the sideflux environment only (all other tests pass), but the problem was due to some lua APT packages still being installed in Travis. Once I removed those, the issue went away.

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.