posix.pipeline: please review

[rrthomas]

I've just added a function posix.pipeline to allow a pipeline of processes to be run simply from Lua, including both shell commands and Lua functions.

Please take a look and see what issues there are with the code. Obvious things to consider:

1. Should it handle some signals? Currently it doesn't, reasoning that Lua is not a shell, so if the signal ends up killing the calling process, that's fine; but that's not ideal for use at the REPL. (See e.g. http://www.cons.org/cracauer/sigint.html )
2. Should it die or return some error code on error? die-ing seems the logical thing to do to "return" from a process, but would it be nice to have some way to distinguish an error inside the pipeline function from errors in the called processes?
3. Is it really necessary to fork as much as it does? I think so, but I'm not entirely sure. In particular: the child shell processes can't use os.execute, as system(3) blocks SIGCHLD. It might seem unnecessary to fork again when the sub-process is actually a Lua function, but in fact it is, as otherwise it's not run in parallel, risking the pipeline blocking. But I'm not 100% convinced that it's necessary to fork AGAIN when there's another stage to the pipeline.
4. Any other hacks needed/not needed? For example, there are lots of calls to posix.close to make sure SIGPIPE will be delivered as per signal(7). The original stdout is saved and then restored at the end, but strictly that's only needed at the top level (I think?). Maybe the function could be restructured to avoid the nasty "if t2..." blocks.
5. What other features would be nice? The most obvious is a way to get the output of the last stage. Feeding input to the first stage is easily done by prepending an extra Lua function to the list, but owing to the one-way nature of pipes, and the fact that we're using fork here, it's harder to get data out: I propose to write a wrapper to set up a pipe and add an extra process to the end of the list to return the last stage's stdout like io.lines. Another idea would be to somehow wrap functions so they can take stdin as an argument and write to stdout with return values; this would require designing a calling convention that works with as little modification required to existing code as possible. For example, provide a pattern to split input into arguments, and specify a separator to string.join the results when writing them out.

I think this is a really interesting feature (I've been meaning to implement it for months) to make it easier to write shell scripts in Lua. I don't know of a similar feature in other scripting languages, either. In general I think it should work "just like" the shell, and extra functionality should be wrapped on top. But getting it right is tricky both in terms of robustness and exact API, so I'm very open to suggestions.

[rrthomas]

I've refined posix.pipeline, in particular adding an optional parameter that gives the function to use to make pipes between processes; this defaults to posix.pipe, but for example posix.openpty can also be used. See https://github.com/rrthomas/cw for an example use of this, where posix.pipeline is used to color the output of an arbitrary process in about 30 lines of code (for the process-related bit, including handling SIGINT). This also illustrates putting a shell command in parallel with a Lua function.

I've also beefed up posix.system to accept either a shell command, a series of arguments to execp, or a Lua function to run in a subprocess. posix.pipeline now uses posix.system.

Currently I believe that posix.pipeline_slurp (and hence posix.pipeline_iter?) has some problems, because it doesn't work when I try to use it to simplify cw further. (However, it's not really desirable for cw, which should process output as it is produced, not collect it all and then color it.) I welcome insights from others. However, I'd happily release the code as-is for now, because it works for basic examples.

If I don't get any comments soon, I shall make a new release so I can rely on it for cw; I suspect releasing these new functions more widely may also be the best way to get feedback and improvements.

[agladysh]

I'll try to give some feedback today. Busy week.

[agladysh]

NB: it is usually a good idea to organize review requests as pull requests (note that it is possible to do that from the same repo — as long as you have your code in a separate branch). This way you do not pollute master with experimental code. Pull requests do handle original branch updates nicely, so all your changes would be visible to reviewers.

P.S. Looking at the code now.

[agladysh]

Looked at the code. All this stuff with running Lua functions in forks looks a little cheesy to me. Is this really a luaposix business?

As for implementation correctness feedback, please post a request for review in Lua ML, I think that this needs more eyes.

That being said, please do release a new version — that Lua 5.1 breakage is not nice at all. (Speaking of pull requests and experimental code...)

My 2c,
Alexander.

P.S. I wish luaposix had a test suite...

[rrthomas]

Thanks for the suggestions. Obviously it's not a POSIX function, but where else would you put it? It seems rather bureaucratic to start a new library just for this, and it's basically expressing the POSIX shell "a | b | c" idiom conveniently in Lua.

Luaposix does have a test suite, of course. I'll add an issue about testing building on 5.1 and 5.2 a good idea.

[agladysh]

Obviously it's not a POSIX function, but where else would you put it? It seems rather bureaucratic to start a new library just for this, and it's basically expressing the POSIX shell "a | b | c" idiom conveniently in Lua.

It is not a bureaucracy issue, but a design one. You have to draw a line somewhere, what belongs to luaposix, and what is not.

[pygy]

It would be nice if sometihng like this were possible:

posix.pipeline({
  "cat foo.txt",
  { 1000, coroutine.create(function (s) -- Pass the input in 1000 bytes chunks.
    while s = coroutine.yield(s) do
      stuff(s)
    end
  end)},
  "other-cmd" -- It's early morning, I an't brain yet...
})

Can't. Can't brain yet.

It would be even better if it was possible to mimick the file:read() API(no idea it it would be efficient, though, I don;t have any idea of how Unix pipes are implemented):

posix.pipeline({
  ...
  { 1000, "*line", coroutine.create(function (s) -- Pass 1000 bytes plus the rest of the current line.
    while s = coroutine.yield(s) do
      stuff(s)
    end
  end)},
  ...
})

[rrthomas]

You have to draw a line somewhere, what belongs to luaposix, and what is not.

Sure: the line is quite clear with lposix.c: any, but only, POSIX functions and constants can be directly wrapped. The line is less clear with posix.lua. Suggestions welcome. Note that posix.lua currently contains a mixture of Lua implementations of POSIX APIs (creat), GNU system APIs (euidaccess), and novel extensions (system, pipeline). All are essentially POSIX system level features.

[rrthomas]

It would be nice if sometihng like this were possible:

Did you look at pipeline_iterator?

[agladysh]

You have to draw a line somewhere, what belongs to luaposix, and what is not.

Sure: the line is quite clear with lposix.c: any, but only, POSIX functions and constants can be directly wrapped. The line is less clear with posix.lua. Suggestions welcome. Note that posix.lua currently contains a mixture of Lua implementations of POSIX APIs (creat), GNU system APIs (euidaccess), and novel extensions (system, pipeline). All are essentially POSIX system level features.

Well, I have no useful design suggestions at this point, sorry. Maybe others?

[agladysh]

Luaposix does have a test suite, of course. I'll add an issue about testing building on 5.1 and 5.2 a good idea.

Aha, missed that somehow. (Too many files in the root directory, too little concentration. :-) )

But no tests for the new pipeline?

[rrthomas]

But no tests for the new pipeline?

I didn't write any while I was in doubt about the API, but now I'm writing one.

[rrthomas]

Closing now, as I'm releasing 5.1.28 with an announcement of pipeline_iterator and pipeline_slurp, and there is now a test. Thanks for all your input.