From 5f8ac02b7707c03f93ff10ac1362d2c618bd3c42 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 19 Apr 2019 17:08:40 -0700 Subject: cli: display a warning whenever default output is stdout while input != stdin This behavior has been preserved for compatibility with existing ecosystem. But it's problematic, as some environment start `lz4` without identifying stdout as console by default, leading to a change of behavior for a same line of script. A more sensible policy would be to default to stdout only when input is stdin. Soft change for the time being : keep the behavior, just print a warning message. User should prefer `-c` to explicitly select `stdout`. Also : updated tests in Makefile to explicitly select `stdout` with `-c`. --- programs/lz4cli.c | 36 +++++++++++++++++++++++++----------- tests/Makefile | 26 +++++++++++++------------- 2 files changed, 38 insertions(+), 24 deletions(-) diff --git a/programs/lz4cli.c b/programs/lz4cli.c index 8bd7042..279aaf1 100644 --- a/programs/lz4cli.c +++ b/programs/lz4cli.c @@ -652,7 +652,15 @@ int main(int argc, const char** argv) /* No output filename ==> try to select one automatically (when possible) */ while ((!output_filename) && (multiple_inputs==0)) { - if (!IS_CONSOLE(stdout)) { output_filename=stdoutmark; break; } /* Default to stdout whenever possible (i.e. not a console) */ + + if (!IS_CONSOLE(stdout)) { + /* Default to stdout whenever stdout is not the console. + * Note : this policy is likely to change in the future, don't rely on it ! + * prefer using an explicit `-c` flag */ + DISPLAYLEVEL(1, "Warning : using stdout as default output. Do not rely on this behavior: use explicit `-c` instead ! \n"); + output_filename=stdoutmark; + break; + } if (mode == om_auto) { /* auto-determine compression or decompression, based on file extension */ mode = determineOpMode(input_filename); } @@ -682,10 +690,14 @@ int main(int argc, const char** argv) break; } - /* Check if output is defined as console; trigger an error in this case */ + if (multiple_inputs==0) assert(output_filename); + /* when multiple_inputs==1, output_filename may simply be useless, + * however, output_filename must be !NULL for next strcmp() tests */ if (!output_filename) output_filename = "*\\dummy^!//"; + + /* Check if output is defined as console; trigger an error in this case */ if (!strcmp(output_filename,stdoutmark) && IS_CONSOLE(stdout) && !forceStdout) { - DISPLAYLEVEL(1, "refusing to write to console without -c\n"); + DISPLAYLEVEL(1, "refusing to write to console without -c \n"); exit(1); } /* Downgrade notification level in stdout and multiple file mode */ @@ -701,21 +713,23 @@ int main(int argc, const char** argv) LZ4IO_setNotificationLevel((int)displayLevel); if (ifnIdx == 0) multiple_inputs = 0; if (mode == om_decompress) { - if (multiple_inputs) - operationResult = LZ4IO_decompressMultipleFilenames(prefs, inFileNames, ifnIdx, !strcmp(output_filename,stdoutmark) ? stdoutmark : LZ4_EXTENSION); - else + if (multiple_inputs) { + assert(ifnIdx <= INT_MAX); + operationResult = LZ4IO_decompressMultipleFilenames(prefs, inFileNames, (int)ifnIdx, !strcmp(output_filename,stdoutmark) ? stdoutmark : LZ4_EXTENSION); + } else { operationResult = DEFAULT_DECOMPRESSOR(prefs, input_filename, output_filename); + } } else { /* compression is default action */ if (legacy_format) { DISPLAYLEVEL(3, "! Generating LZ4 Legacy format (deprecated) ! \n"); LZ4IO_compressFilename_Legacy(prefs, input_filename, output_filename, cLevel); } else { - if (multiple_inputs) - operationResult = LZ4IO_compressMultipleFilenames(prefs, inFileNames, ifnIdx, !strcmp(output_filename,stdoutmark) ? stdoutmark : LZ4_EXTENSION, cLevel); - else + if (multiple_inputs) { + assert(ifnIdx <= INT_MAX); + operationResult = LZ4IO_compressMultipleFilenames(prefs, inFileNames, (int)ifnIdx, !strcmp(output_filename,stdoutmark) ? stdoutmark : LZ4_EXTENSION, cLevel); + } else { operationResult = DEFAULT_COMPRESSOR(prefs, input_filename, output_filename, cLevel); - } - } + } } } _cleanup: if (main_pause) waitEnter(); diff --git a/tests/Makefile b/tests/Makefile index 70cae63..579999c 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -176,15 +176,15 @@ test-install: lz4 lib liblz4.pc test-lz4-sparse: lz4 datagen @echo "\n ---- test sparse file support ----" ./datagen -g5M -P100 > tmplsdg5M - $(LZ4) -B4D tmplsdg5M | $(LZ4) -dv --sparse > tmplscB4 + $(LZ4) -B4D tmplsdg5M -c | $(LZ4) -dv --sparse > tmplscB4 $(DIFF) -s tmplsdg5M tmplscB4 - $(LZ4) -B5D tmplsdg5M | $(LZ4) -dv --sparse > tmplscB5 + $(LZ4) -B5D tmplsdg5M -c | $(LZ4) -dv --sparse > tmplscB5 $(DIFF) -s tmplsdg5M tmplscB5 - $(LZ4) -B6D tmplsdg5M | $(LZ4) -dv --sparse > tmplscB6 + $(LZ4) -B6D tmplsdg5M -c | $(LZ4) -dv --sparse > tmplscB6 $(DIFF) -s tmplsdg5M tmplscB6 - $(LZ4) -B7D tmplsdg5M | $(LZ4) -dv --sparse > tmplscB7 + $(LZ4) -B7D tmplsdg5M -c | $(LZ4) -dv --sparse > tmplscB7 $(DIFF) -s tmplsdg5M tmplscB7 - $(LZ4) tmplsdg5M | $(LZ4) -dv --no-sparse > tmplsnosparse + $(LZ4) tmplsdg5M -c | $(LZ4) -dv --no-sparse > tmplsnosparse $(DIFF) -s tmplsdg5M tmplsnosparse ls -ls tmpls* ./datagen -s1 -g1200007 -P100 | $(LZ4) | $(LZ4) -dv --sparse > tmplsodd # Odd size file (to generate non-full last block) @@ -200,7 +200,7 @@ test-lz4-sparse: lz4 datagen cat tmplsdg1M tmplsdg1M > tmpls2M $(LZ4) -B5 -v tmplsdg1M tmplsc $(LZ4) -d -v tmplsc tmplsr - $(LZ4) -d -v tmplsc >> tmplsr + $(LZ4) -d -v tmplsc -c >> tmplsr ls -ls tmp* $(DIFF) tmpls2M tmplsr @$(RM) tmpls* @@ -208,8 +208,8 @@ test-lz4-sparse: lz4 datagen test-lz4-contentSize: lz4 datagen @echo "\n ---- test original size support ----" ./datagen -g15M > tmplc1 - $(LZ4) -v tmplc1 | $(LZ4) -t - $(LZ4) -v --content-size tmplc1 | $(LZ4) -d > tmplc2 + $(LZ4) -v tmplc1 -c | $(LZ4) -t + $(LZ4) -v --content-size tmplc1 -c | $(LZ4) -d > tmplc2 $(DIFF) -s tmplc1 tmplc2 @$(RM) tmplc* @@ -218,10 +218,10 @@ test-lz4-frame-concatenation: lz4 datagen @echo -n > tmp-lfc-empty @echo hi > tmp-lfc-nonempty cat tmp-lfc-nonempty tmp-lfc-empty tmp-lfc-nonempty > tmp-lfc-src - @$(LZ4) -zq tmp-lfc-empty > tmp-lfc-empty.lz4 - @$(LZ4) -zq tmp-lfc-nonempty > tmp-lfc-nonempty.lz4 + $(LZ4) -zq tmp-lfc-empty -c > tmp-lfc-empty.lz4 + $(LZ4) -zq tmp-lfc-nonempty -c > tmp-lfc-nonempty.lz4 cat tmp-lfc-nonempty.lz4 tmp-lfc-empty.lz4 tmp-lfc-nonempty.lz4 > tmp-lfc-concat.lz4 - $(LZ4) -d tmp-lfc-concat.lz4 > tmp-lfc-result + $(LZ4) -d tmp-lfc-concat.lz4 -c > tmp-lfc-result cmp tmp-lfc-src tmp-lfc-result @$(RM) tmp-lfc-* @echo frame concatenation test completed @@ -341,8 +341,8 @@ test-lz4-dict: lz4 datagen for l in 0 1 4 128 32767 32768 32769 65535 65536 65537 98303 98304 98305 131071 131072 131073; do \ ./datagen -g$$l > tmp-dict-$$l; \ $(DD) if=tmp-dict-$$l of=tmp-dict-$$l-tail bs=1 count=65536 skip=$$((l > 65536 ? l - 65536 : 0)); \ - < tmp-dict-$$l $(LZ4) -D stdin tmp-dict-data-128KB | $(LZ4) -dD tmp-dict-$$l-tail | $(DIFF) - tmp-dict-data-128KB; \ - < tmp-dict-$$l-tail $(LZ4) -D stdin tmp-dict-data-128KB | $(LZ4) -dD tmp-dict-$$l | $(DIFF) - tmp-dict-data-128KB; \ + < tmp-dict-$$l $(LZ4) -D stdin tmp-dict-data-128KB -c | $(LZ4) -dD tmp-dict-$$l-tail | $(DIFF) - tmp-dict-data-128KB; \ + < tmp-dict-$$l-tail $(LZ4) -D stdin tmp-dict-data-128KB -c | $(LZ4) -dD tmp-dict-$$l | $(DIFF) - tmp-dict-data-128KB; \ done @$(RM) tmp-dict* -- cgit v0.12 From 467e352de929fa664af03ece065c89e9997d1bf1 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Sun, 21 Apr 2019 12:42:13 -0700 Subject: tests/Makefile : created CMP variable for potential redirection, if need be. note : should probably converge all comparison operations onto CMP --- tests/Makefile | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/Makefile b/tests/Makefile index 579999c..209a530 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -144,6 +144,8 @@ ifneq (,$(filter $(shell uname), Darwin )) MD5:=md5 -r endif +# note : we should probably settle on a single compare utility +CMP:=cmp DIFF:=diff ifneq (,$(filter $(shell uname),SunOS)) DIFF:=gdiff @@ -222,7 +224,7 @@ test-lz4-frame-concatenation: lz4 datagen $(LZ4) -zq tmp-lfc-nonempty -c > tmp-lfc-nonempty.lz4 cat tmp-lfc-nonempty.lz4 tmp-lfc-empty.lz4 tmp-lfc-nonempty.lz4 > tmp-lfc-concat.lz4 $(LZ4) -d tmp-lfc-concat.lz4 -c > tmp-lfc-result - cmp tmp-lfc-src tmp-lfc-result + $(CMP) tmp-lfc-src tmp-lfc-result @$(RM) tmp-lfc-* @echo frame concatenation test completed @@ -241,15 +243,15 @@ test-lz4-multiple: lz4 datagen mv tmp-tlm2 tmp-tlm2-orig mv tmp-tlm3 tmp-tlm3-orig $(LZ4) -d -f -m tmp-tlm*.lz4 - cmp tmp-tlm1 tmp-tlm1-orig # must be identical - cmp tmp-tlm2 tmp-tlm2-orig - cmp tmp-tlm3 tmp-tlm3-orig + $(CMP) tmp-tlm1 tmp-tlm1-orig # must be identical + $(CMP) tmp-tlm2 tmp-tlm2-orig + $(CMP) tmp-tlm3 tmp-tlm3-orig # compress multiple files into stdout cat tmp-tlm1.lz4 tmp-tlm2.lz4 tmp-tlm3.lz4 > tmp-tlm-concat1 $(RM) *.lz4 $(LZ4) -m tmp-tlm1 tmp-tlm2 tmp-tlm3 -c > tmp-tlm-concat2 test ! -f tmp-tlm1.lz4 # must not create .lz4 artefact - cmp tmp-tlm-concat1 tmp-tlm-concat2 # must be equivalent + $(CMP) tmp-tlm-concat1 tmp-tlm-concat2 # must be equivalent # decompress multiple files into stdout $(RM) tmp-tlm-concat1 tmp-tlm-concat2 $(LZ4) -f -m tmp-tlm1 tmp-tlm2 tmp-tlm3 # generate .lz4 to decompress @@ -257,7 +259,7 @@ test-lz4-multiple: lz4 datagen $(RM) tmp-tlm1 tmp-tlm2 tmp-tlm3 $(LZ4) -d -m tmp-tlm1.lz4 tmp-tlm2.lz4 tmp-tlm3.lz4 -c > tmp-tlm-concat2 test ! -f tmp-tlm1 # must not create file artefact - cmp tmp-tlm-concat1 tmp-tlm-concat2 # must be equivalent + $(CMP) tmp-tlm-concat1 tmp-tlm-concat2 # must be equivalent # compress multiple files, one of which is absent (must fail) ! $(LZ4) -f -m tmp-tlm-concat1 notHere tmp-tlm-concat2 # must fail : notHere not present @$(RM) tmp-tlm* -- cgit v0.12 From 22fae16c6f4c4f118e93468ede884295d365fcef Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Sun, 21 Apr 2019 17:01:03 -0700 Subject: ensure tests work when `stdout` is not the console ensure this case is continuously tested on travis. Update documentation on implicit output, invite to not rely on implicit output in scripts. --- .travis.yml | 2 +- programs/lz4.1.md | 27 +++++++++++++-------------- programs/lz4cli.c | 5 +++-- tests/Makefile | 2 +- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/.travis.yml b/.travis.yml index 301d294..2065478 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,7 +8,7 @@ matrix: os: osx compiler: clang script: - - make -C tests test-lz4 MOREFLAGS='-Werror -Wconversion -Wno-sign-conversion' + - make -C tests test-lz4 MOREFLAGS='-Werror -Wconversion -Wno-sign-conversion' | tee # test scenario where `stdout` is not the console - CFLAGS=-m32 make -C tests clean test-lz4-contentSize # Container-based 12.04 LTS Server Edition 64 bit (doesn't support 32-bit includes) diff --git a/programs/lz4.1.md b/programs/lz4.1.md index 10449a0..ffda3ff 100644 --- a/programs/lz4.1.md +++ b/programs/lz4.1.md @@ -31,29 +31,29 @@ The native file format is the `.lz4` format. `lz4` supports a command line syntax similar _but not identical_ to `gzip(1)`. Differences are : - * `lz4` preserves original files * `lz4` compresses a single file by default (see `-m` for multiple files) * `lz4 file1 file2` means : compress file1 _into_ file2 * `lz4 file.lz4` will default to decompression (use `-z` to force compression) + * `lz4` preserves original files * `lz4` shows real-time notification statistics during compression or decompression of a single file (use `-q` to silence them) - * If no destination name is provided, result is sent to `stdout` - _except if stdout is the console_. - * If no destination name is provided, __and__ if `stdout` is the console, - `file` is compressed into `file.lz4`. - * As a consequence of previous rules, note the following example : - `lz4 file | consumer` sends compressed data to `consumer` through `stdout`, - hence it does _not_ create `file.lz4`. - * Another consequence of those rules is that to run `lz4` under `nohup`, - you should provide a destination file: `nohup lz4 file file.lz4`, - because `nohup` writes the specified command's output to a file. + * When no destination is specified, result is sent on implicit output, + which depends on `stdout` status. + When `stdout` _is Not the console_, it becomes the implicit output. + Otherwise, if `stdout` is the console, the implicit output is `filename.lz4`. + * It is considered bad practice to rely on implicit output in scripts. + because the script's environment may change. + Always use explicit output in scripts. + `-c` ensures that output will be `stdout`. + Conversely, providing a destination name, or using `-m` + ensures that the output will be either the specified name, or `filename.lz4` respectively. Default behaviors can be modified by opt-in commands, detailed below. * `lz4 -m` makes it possible to provide multiple input filenames, which will be compressed into files using suffix `.lz4`. - Progress notifications are also disabled by default (use `-v` to enable them). + Progress notifications become disabled by default (use `-v` to enable them). This mode has a behavior which more closely mimics `gzip` command line, with the main remaining difference being that source files are preserved by default. * Similarly, `lz4 -m -d` can decompress multiple `*.lz4` files. @@ -81,8 +81,7 @@ In some cases, some options can be expressed using short command `-x` or long command `--long-word`. Short commands can be concatenated together. For example, `-d -c` is equivalent to `-dc`. -Long commands cannot be concatenated. -They must be clearly separated by a space. +Long commands cannot be concatenated. They must be clearly separated by a space. ### Multiple commands diff --git a/programs/lz4cli.c b/programs/lz4cli.c index 279aaf1..82f2fac 100644 --- a/programs/lz4cli.c +++ b/programs/lz4cli.c @@ -655,8 +655,9 @@ int main(int argc, const char** argv) if (!IS_CONSOLE(stdout)) { /* Default to stdout whenever stdout is not the console. - * Note : this policy is likely to change in the future, don't rely on it ! - * prefer using an explicit `-c` flag */ + * Note : this policy may change in the future, therefore don't rely on it ! + * To ensure `stdout` is explicitly selected, use `-c` command flag. + * Conversely, to ensure output will not become `stdout`, use `-m` command flag */ DISPLAYLEVEL(1, "Warning : using stdout as default output. Do not rely on this behavior: use explicit `-c` instead ! \n"); output_filename=stdoutmark; break; diff --git a/tests/Makefile b/tests/Makefile index 209a530..ddc6d1e 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -317,7 +317,7 @@ test-lz4-basic: lz4 datagen unlz4 lz4cat ! $(LZ4) -c --fast=-1 tmp-tlb-dg20K # lz4 should fail when fast=-1 # Test for #596 @echo "TEST" > tmp-tlb-test - $(LZ4) tmp-tlb-test + $(LZ4) -m tmp-tlb-test $(LZ4) tmp-tlb-test.lz4 tmp-tlb-test2 $(DIFF) -q tmp-tlb-test tmp-tlb-test2 @$(RM) tmp-tlb* -- cgit v0.12