fdopen doesn't work properly on Lua 5.1

[rrthomas]

See https://github.com/luaposix/luaposix/commit/a83c89b8aaf530d43169bf5927db5b51a4dfd7e4#commitcomment-9978862

My inclination is to back out the compatibility code for now and simply not have this API on 5.1, until lua-compat-5.2/issues#22 is fixed (documentation for how to do this) and we can easily add it back properly.

If someone else would like to provide a patch according to the instructions in the comment above, that would also be most welcome.

[siffiejoe]

I implemented my suggestions from the above mentioned comment, and it works fine for PUC-Rio Lua 5.1. But when I tested on LuaJIT, I realized that file objects are a separate userdata type, and all methods explicitly check for that userdata type. It seems that Mike really doesn't want us to create our own file objects ... Sorry.

Btw.: The documentation for fdopen is wrong. The second argument (mode string) is missing.

[rrthomas]

Thanks for trying, @sifflejoe. Do you mean that you implemented your suggestions for fdopen? If so, can we please have a patch, we can then at least use it for Lua 5.1, and have a "not implemented" stub on LuaJIT?

Thanks for the documentation hint, which I've fixed in a62ff5c.

[siffiejoe]

I'm not sure how the "not implemented" stub would work. LuaJIT can load extensions compiled for Lua 5.1, so you would need a way to identify LuaJIT at runtime. Anyway, the code for PUC-Rio Lua 5.1 is in my fork: siffiejoe/luaposix@529a498. I can create a pull request if you want.

[rrthomas]

Sorry, I was thinking of detecting the difference at compile time, but you're right, that's not enough. That's annoying, because the code appears to work on LuaJIT (the Travis tests pass), but will silently leak resources (or other bad things). There are some ideas on telling the difference at runtime here:

http://stackoverflow.com/questions/20335340/how-can-i-detect-at-runtime-if-i-am-running-luajit-or-puc-lua-5-1

Is it worth the trouble, or should we just go back to having this only work on systems that properly support luaL_Stream, currently, Lua >= 5.2?

[siffiejoe]

The Travis tests shouldn't pass. LuaJIT throws an error as soon as you call a file method on a luaL_Stream userdata: luajit: bad argument #1 to '?' (FILE* expected, got userdata).

I usually use something like local is_luajit = (string.dump(function() end) or ""):sub(1, 3) == "\027LJ" to figure out if I'm running under LuaJIT. This seems a bit safer than relying on a simple global variable, but I would probably disable fdopen on Lua 5.1 altogether (LuaJIT is becoming the dominant reason to use Lua 5.1 anyway) ...

[rrthomas]

Indeed, it does fail when invoked manually, not sure why the spec doesn't have a problem. I'll file an issue about that. I agree that it's best to have fdopen only on Lua ≥ 5.2; as I say, wouldn't it therefore make sense to remove luaL_Stream from lua-compat-5.2?

[siffiejoe]

as I say, wouldn't it therefore make sense to remove luaL_Stream from lua-compat-5.2?

Probably. I thought about that before when I learnt that LuaJIT uses an extra type field in file objects. Now that it seems pretty much impossible to support luaL_Stream on LuaJIT it may cause more harm than good. And if you are fine with PUC-Rio Lua 5.1 only support, the extra four lines of

typedef luaL_Stream {
  FILE *f;
  lua_CFunction closef;
} luaL_Stream;

won't matter (you need extra code for the userdata environments anyway).

[gvvaughan]

I corrected the specs to run fdopen checks in a subprocess, and not conflate the example output and expectation output, and it now detects the incompatibility with Luajit:

https://travis-ci.org/luaposix/luaposix/builds/111900445

Is there any more progress on the state of the art here, than having a not implemented stub for luajit?

[siffiejoe]

No, nothing new. The only way I see to support fdopen() in LuaJIT is to io.open() a dummy file and replace the file pointer in the resulting userdata.

[gvvaughan]

Thanks for the confirmation.

Something else I am considering is to wrap the file descriptor in a userdata that luaposix APIs will understand, and forgo compatibility with Lua file objects entirely - I don't think it's too onerous to use luaposix APIs for luaposix handles and whatever version of Lua you are using handle its file handles separately. in exchange for that, code written with luaposix will work the same regardless of what (supported) Lua implementation your users run it on.

Unfortunately, this opens a can of worms for wrapping other POSIX APIs in userdata, where we currently use a LuaInteger, but the native types are 64bit, and don't fit... but despite the amount of work required across the board, I think that is the correct way to proceed.