Redirecting input of value-consuming builtin from file hangs

[xiaq]

This hangs:

echo "line" > file
each $echo~ < file

OTOH cat file | each $echo~ is fine.

[krader1961]

The proximate cause is that the for v := range fm.ports[0].Chan { in function IterateInputs never terminates when the input is from a file redirection rather than a pipe. It seems likely that setting up a pipe correctly initializes fm.ports[0].Chan while redirection does not do so.

[krader1961]

The problem is that the input redirection from a file is using a BlackholeChan, meant to discard value output, when it should be using a ClosedChan.

[krader1961]

Note that the current behavior silently discards value output written to a file:

> put yes > x
> ls -l x
-rw-r----- 1 krader staff 0 Sep 10 22:18 x
> cat x

Note that there is no output from the cat x because the file is empty.

I think that behavior is a mistake. Writing value output to a file, via redirection, should be a fatal error. The only question is whether it should cause a panic or cause an exception that it can be trapped by a try...except block.

[krader1961]

The fix for this problem is literally a five line change (not counting comments). I'll work on adding a few regression tests tomorrow as well as updating the documentation. Then open a P.R.

I'd still like to create a consensus regarding writing (discarding) value output redirected to a file. The current behavior is to silently discard the data. I think it should be an error but, for the moment, will retain the current behavior (which is why it's a five line rather than one line change).

[hanche]

@krader1961

Writing value output to a file, via redirection, should be a fatal error. The only question is whether it should cause a panic or cause an exception that it can be trapped by a try...except block.

Surely you're joking when you suggest a panic here? IMO, panics are for internal elvish errors only, not for user errors. A user should not be able to crash or panic the shell except by sending it an actual SIGKILL og SIGSEGV or some such thing.

But I think it may be a good idea to make it an exception. After all, the only-bytes function is available for use in a pipeline that might produce values in addition to byte output.

[krader1961]

Surely you're joking when you suggest a panic here?

Not so much joking as pointing out that if the only options are to discard output the user probably expects to be written to the file or panic the panic is preferable. However, obviously there is a third option: Replace the use of BlackholeChan with a value sink that will result in an exception if it receives any values. In fact, it's not obvious that the current BlackholeChan is the correct behavior for any situation. I'll need to look at the couple of other places it is used. I've opened issue #1149 since I would prefer not to conflate that problem with this issue.

[krader1961]

This is a duplicate of #600. It's a shame that @scurest didn't follow-up with a P.R. two years ago to fix the incorrect use of a BlackholeChan for redirection. And @scurest also noted the same thing I did: writing values to a file silently discards those values.