From 8f22def8f7b71fb9855d435e99d624bc89d1e8d9 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Mon, 17 Jul 2017 14:33:51 -0700 Subject: [PATCH] return to `psub --file` being the default The recent change to switch `psub` to use `argparse` caused it to use a fifo by default because it inadvertently fixed a long standing bug in the fish script. This changes the behavior back to `psub --file` being the default behavior and introduces a `--fifo` flag. It also updates the documentation to make it clearer when and why `--fifo` mode should not be used. Fixes #4222 --- doc_src/psub.txt | 13 +++++++---- share/functions/psub.fish | 15 ++++++++---- tests/psub.err | 0 tests/psub.in | 49 +++++++++++++++++++++++++++++++++++++++ tests/psub.out | 7 ++++++ tests/test9.in | 35 +--------------------------- tests/test9.out | 7 ------ 7 files changed, 75 insertions(+), 51 deletions(-) create mode 100644 tests/psub.err create mode 100644 tests/psub.in create mode 100644 tests/psub.out diff --git a/doc_src/psub.txt b/doc_src/psub.txt index 42fb7257e..d0c59e0ac 100644 --- a/doc_src/psub.txt +++ b/doc_src/psub.txt @@ -2,17 +2,20 @@ \subsection psub-synopsis Synopsis \fish{synopsis} -COMMAND1 ( COMMAND2 | psub [-f] [-s SUFFIX]) +COMMAND1 ( COMMAND2 | psub [-F | --fifo] [-f | --file] [-s SUFFIX]) \endfish \subsection psub-description Description -Posix shells feature a syntax that is a mix between command substitution and piping, called process substitution. It is used to send the output of a command into the calling command, much like command substitution, but with the difference that the output is not sent through commandline arguments but through a named pipe, with the filename of the named pipe sent as an argument to the calling program. `psub` combined with a regular command substitution provides the same functionality. +Some shells (e.g., ksh, bash) feature a syntax that is a mix between command substitution and piping, called process substitution. It is used to send the output of a command into the calling command, much like command substitution, but with the difference that the output is not sent through commandline arguments but through a named pipe, with the filename of the named pipe sent as an argument to the calling program. `psub` combined with a regular command substitution provides the same functionality. -If the `-f` or `--file` switch is given to `psub`, `psub` will use a regular file instead of a named pipe to communicate with the calling process. This will cause `psub` to be significantly slower when large amounts of data are involved, but has the advantage that the reading process can seek in the stream. +The following options are available: -If the `-s` or `---suffix` switch is given, `psub` will append SUFFIX to the filename. +- `-f` or `--file` will cause psub to use a regular file instead of a named pipe to communicate with the calling process. This will cause `psub` to be significantly slower when large amounts of data are involved, but has the advantage that the reading process can seek in the stream. This is the default. +- `-F` or `--fifo` will cause psub to use a named pipe rather than a file. You should only use this if the command produces no more than 8 KiB of output. The limit on the amount of data a FIFO can buffer varies with the OS but is typically 8 KiB, 16 KiB or 64 KiB. If you use this option and the command on the left of the psub pipeline produces more output a deadlock is likely to occur. + +- `-s` or `--suffix` will append SUFFIX to the filename. \subsection psub-example Example @@ -20,6 +23,6 @@ If the `-s` or `---suffix` switch is given, `psub` will append SUFFIX to the fil diff (sort a.txt | psub) (sort b.txt | psub) # shows the difference between the sorted versions of files `a.txt` and `b.txt`. -source-highlight -f esc (cpp main.c | psub -s .c) +source-highlight -f esc (cpp main.c | psub -f -s .c) # highlights `main.c` after preprocessing as a C source. \endfish diff --git a/share/functions/psub.fish b/share/functions/psub.fish index 5380030a3..61d8cf4cc 100644 --- a/share/functions/psub.fish +++ b/share/functions/psub.fish @@ -1,5 +1,5 @@ function psub --description "Read from stdin into a file and output the filename. Remove the file when the command that called psub exits." - set -l options 'h/help' 'f/file' 's/suffix=' + set -l options -x 'f,F' -x 'F,s' 'h/help' 'f/file' 'F/fifo' 's/suffix=' 'T-testing' argparse -n psub --max-args=0 $options -- $argv or return @@ -21,10 +21,10 @@ function psub --description "Read from stdin into a file and output the filename set -q TMPDIR and set tmpdir $TMPDIR - if not set -q _flag_file + if set -q _flag_fifo # Write output to pipe. This needs to be done in the background so # that the command substitution exits without needing to wait for - # all the commands to exit + # all the commands to exit. set dirname (mktemp -d $tmpdir/.psub.XXXXXXXXXX) or return set filename $dirname/psub.fifo"$_flag_suffix" @@ -35,13 +35,18 @@ function psub --description "Read from stdin into a file and output the filename cat >$filename else set dirname (mktemp -d $tmpdir/.psub.XXXXXXXXXX) - set filename $dirname/psub"$_flag_suffix" + set filename "$dirname/psub$_flag_suffix" cat >$filename end # Write filename to stdout echo $filename + # This flag isn't documented. It's strictly for our unit tests. + if set -q _flag_testing + return + end + # Find unique function name while true set funcname __fish_psub_(random) @@ -53,7 +58,7 @@ function psub --description "Read from stdin into a file and output the filename # Make sure we erase file when caller exits function $funcname --on-job-exit caller --inherit-variable filename --inherit-variable dirname --inherit-variable funcname command rm $filename - if count $dirname >/dev/null + if test -n "$dirname" command rmdir $dirname end functions -e $funcname diff --git a/tests/psub.err b/tests/psub.err new file mode 100644 index 000000000..e69de29bb diff --git a/tests/psub.in b/tests/psub.in new file mode 100644 index 000000000..5a4808477 --- /dev/null +++ b/tests/psub.in @@ -0,0 +1,49 @@ +# Test psub behavior +set -l filename (echo foo | psub --testing) +test -f $filename +or echo 'psub is not a regular file' >&2 +rm $filename + +set -l filename (echo foo | psub --testing --file) +test -f $filename +or echo 'psub is not a regular file' >&2 +rm $filename + +set -l filename (echo foo | psub --testing --fifo) +test -p $filename +or echo 'psub is not a fifo' >&2 +rm $filename + +cat (echo foo | psub) +cat (echo bar | psub --fifo) +cat (echo baz | psub) + +set -l filename (echo foo | psub) +if test -e $filename + echo 'psub file was not deleted' +else + echo 'psub file was deleted' +end + +# The --file flag is the default behavior. +if count (echo foo | psub -s .cc | grep -o '\.cc$') >/dev/null + echo 'psub filename ends with .cc' +else + echo 'psub filename does not end with .cc' +end + +# Make sure we get the same result if we explicitly ask for a temp file. +if count (echo foo | psub -f -s .cc | grep -o '\.cc$') >/dev/null + echo 'psub filename ends with .cc' +else + echo 'psub filename does not end with .cc' +end + +set -l filename (echo foo | psub -s .fish) +if test -e (dirname $filename) + echo 'psub directory was not deleted' +else + echo 'psub directory was deleted' +end + +diff -q (__fish_print_help psub | psub) (psub -hs banana | psub) diff --git a/tests/psub.out b/tests/psub.out new file mode 100644 index 000000000..6f9d6710c --- /dev/null +++ b/tests/psub.out @@ -0,0 +1,7 @@ +foo +bar +baz +psub file was deleted +psub filename ends with .cc +psub filename ends with .cc +psub directory was deleted diff --git a/tests/test9.in b/tests/test9.in index fcc45a816..c38f2e58f 100644 --- a/tests/test9.in +++ b/tests/test9.in @@ -7,7 +7,7 @@ cat ../test/temp/file_truncation_test.txt echo -n > ../test/temp/file_truncation_test.txt cat ../test/temp/file_truncation_test.txt -# Test events. +# Test events. # This pattern caused a crash; github issue #449 @@ -85,39 +85,6 @@ eval 'status -n status -n status -n' -# Test psub -cat (echo foo | psub) -cat (echo bar | psub) -cat (echo baz | psub) - -set -l filename (echo foo | psub) -if test -e $filename - echo 'psub file was not deleted' -else - echo 'psub file was deleted' -end - -if count (echo foo | psub -s .cc | grep -o '\.cc$') >/dev/null - echo 'psub filename ends with .cc' -else - echo 'psub filename does not end with .cc' -end - -if count (echo foo | psub -f -s .cc | grep -o '\.cc$') >/dev/null - echo 'psub filename ends with .cc' -else - echo 'psub filename does not end with .cc' -end - -set -l filename (echo foo | psub -s .fish) -if test -e (dirname $filename) - echo 'psub directory was not deleted' -else - echo 'psub directory was deleted' -end - -diff -q (__fish_print_help psub | psub) (psub -hs banana | psub) - # Test support for unbalanced blocks function try_unbalanced_block ../test/root/bin/fish -c "echo $argv | source " 2>&1 | grep "Missing end" 1>&2 diff --git a/tests/test9.out b/tests/test9.out index 8419e6975..0fd2acf88 100644 --- a/tests/test9.out +++ b/tests/test9.out @@ -13,13 +13,6 @@ Testing for loop 1 2 3 -foo -bar -baz -psub file was deleted -psub filename ends with .cc -psub filename ends with .cc -psub directory was deleted bom_test not#a#comment is