From 8c85b29e3236d0f0be00e59de004d7297c4fddf0 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 23 Jan 2023 18:55:51 -0800 Subject: [PATCH] disable --rm on -o command make it more similar to -c (aka `stdout`) convention. --- programs/fileio.c | 97 +++++++++++-------- programs/util.c | 2 +- programs/zstdcli.c | 38 +++++--- .../compress-file-to-stdout.sh.stderr.exact | 2 + .../compress-stdin-to-stdout.sh.stderr.exact | 2 + .../decompress-file-to-stdout.sh.stderr.exact | 2 + ...decompress-stdin-to-stdout.sh.stderr.exact | 2 + tests/playTests.sh | 39 ++++++-- 8 files changed, 118 insertions(+), 66 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 2d79203de..abc90b659 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -612,7 +612,7 @@ FIO_openDstFile(FIO_ctx_t* fCtx, FIO_prefs_t* const prefs, if (!prefs->overwrite) { if (g_display_prefs.displayLevel <= 1) { /* No interaction possible */ - DISPLAY("zstd: %s already exists; not overwritten \n", + DISPLAYLEVEL(1, "zstd: %s already exists; not overwritten \n", dstFileName); return NULL; } @@ -723,7 +723,7 @@ int FIO_checkFilenameCollisions(const char** filenameTable, unsigned nbFiles) { filenameTableSorted = (const char**) malloc(sizeof(char*) * nbFiles); if (!filenameTableSorted) { - DISPLAY("Unable to malloc new str array, not checking for name collisions\n"); + DISPLAYLEVEL(1, "Allocation error during filename collision checking \n"); return 1; } @@ -740,7 +740,7 @@ int FIO_checkFilenameCollisions(const char** filenameTable, unsigned nbFiles) { prevElem = filenameTableSorted[0]; for (u = 1; u < nbFiles; ++u) { if (strcmp(prevElem, filenameTableSorted[u]) == 0) { - DISPLAY("WARNING: Two files have same filename: %s\n", prevElem); + DISPLAYLEVEL(2, "WARNING: Two files have same filename: %s\n", prevElem); } prevElem = filenameTableSorted[u]; } @@ -823,41 +823,59 @@ static void FIO_adjustMemLimitForPatchFromMode(FIO_prefs_t* const prefs, FIO_setMemLimit(prefs, (unsigned)maxSize); } -/* FIO_removeMultiFilesWarning() : +/* FIO_multiFilesConcatWarning() : + * This function handles logic when processing multiple files with -o or -c, displaying the appropriate warnings/prompts. * Returns 1 if the console should abort, 0 if console should proceed. - * This function handles logic when processing multiple files with -o, displaying the appropriate warnings/prompts. * - * If -f is specified, or there is just 1 file, zstd will always proceed as usual. - * If --rm is specified, there will be a prompt asking for user confirmation. - * If -f is specified with --rm, zstd will proceed as usual - * If -q is specified with --rm, zstd will abort pre-emptively - * If neither flag is specified, zstd will prompt the user for confirmation to proceed. - * If --rm is not specified, then zstd will print a warning to the user (which can be silenced with -q). - * Note : --rm in combination with stdout is not allowed. + * If output is stdout or test mode is active, check that `--rm` disabled. + * + * If there is just 1 file to process, zstd will proceed as usual. + * If each file get processed into its own separate destination file, proceed as usual. + * + * When multiple files are processed into a single output, + * display a warning message, then disable --rm if it's set. + * + * If -f is specified or if output is stdout, just proceed. + * If output is set with -o, prompt for confirmation. */ -static int FIO_removeMultiFilesWarning(FIO_ctx_t* const fCtx, const FIO_prefs_t* const prefs, const char* outFileName, int displayLevelCutoff) +static int FIO_multiFilesConcatWarning(const FIO_ctx_t* fCtx, FIO_prefs_t* prefs, const char* outFileName, int displayLevelCutoff) { - int error = 0; - if (fCtx->nbFilesTotal > 1 && !prefs->overwrite) { - if (g_display_prefs.displayLevel <= displayLevelCutoff) { - if (prefs->removeSrcFile) { - DISPLAYLEVEL(1, "zstd: Aborting... not deleting files and processing into dst: %s\n", outFileName); - error = 1; - } - } else { - if (!strcmp(outFileName, stdoutmark)) { - DISPLAYLEVEL(2, "zstd: WARNING: all input files will be processed and concatenated into stdout. \n"); - } else { - DISPLAYLEVEL(2, "zstd: WARNING: all input files will be processed and concatenated into a single output file: %s \n", outFileName); - } - DISPLAYLEVEL(2, "The concatenated output CANNOT regenerate original file names nor directory structure. \n") - if (prefs->removeSrcFile) { - assert(fCtx->hasStdoutOutput == 0); /* not possible : never erase source files when output == stdout */ - error = (g_display_prefs.displayLevel > displayLevelCutoff) && UTIL_requireUserConfirmation("This is a destructive operation. Proceed? (y/n): ", "Aborting...", "yY", fCtx->hasStdinInput); - } - } + if (fCtx->hasStdoutOutput) assert(prefs->removeSrcFile == 0); + if (prefs->testMode) { + assert(prefs->removeSrcFile == 0); + return 0; } - return error; + + if (fCtx->nbFilesTotal == 1) return 0; + assert(fCtx->nbFilesTotal > 1); + + if (!outFileName) return 0; + + if (fCtx->hasStdoutOutput) { + DISPLAYLEVEL(2, "zstd: WARNING: all input files will be processed and concatenated into stdout. \n"); + } else { + DISPLAYLEVEL(2, "zstd: WARNING: all input files will be processed and concatenated into a single output file: %s \n", outFileName); + } + DISPLAYLEVEL(2, "The concatenated output CANNOT regenerate original file names nor directory structure. \n") + + /* multi-input into single output : --rm is not allowed */ + if (prefs->removeSrcFile) { + DISPLAYLEVEL(2, "Since it's a destructive operation, input files will not be removed. \n"); + prefs->removeSrcFile = 0; + } + + if (fCtx->hasStdoutOutput) return 0; + if (prefs->overwrite) return 0; + + /* multiple files concatenated into single destination file using -o without -f */ + if (g_display_prefs.displayLevel <= displayLevelCutoff) { + /* quiet mode => no prompt => fail automatically */ + DISPLAYLEVEL(1, "Concatenating multiple processed inputs into a single output loses file metadata. \n"); + DISPLAYLEVEL(1, "Aborting. \n"); + return 1; + } + /* normal mode => prompt */ + return UTIL_requireUserConfirmation("Proceed? (y/n): ", "Aborting...", "yY", fCtx->hasStdinInput); } static ZSTD_inBuffer setInBuffer(const void* buf, size_t s, size_t pos) @@ -1767,9 +1785,9 @@ FIO_compressFilename_srcFile(FIO_ctx_t* const fCtx, &srcFileStat, compressionLevel); AIO_ReadPool_closeFile(ress.readCtx); - if ( prefs->removeSrcFile /* --rm */ - && result == 0 /* success */ - && strcmp(srcFileName, stdinmark) /* exception : don't erase stdin */ + if ( prefs->removeSrcFile /* --rm */ + && result == 0 /* success */ + && strcmp(srcFileName, stdinmark) /* exception : don't erase stdin */ ) { /* We must clear the handler, since after this point calling it would * delete both the source and destination files. @@ -1791,7 +1809,8 @@ checked_index(const char* options[], size_t length, size_t index) { #define INDEX(options, index) checked_index((options), sizeof(options) / sizeof(char*), (size_t)(index)) -void FIO_displayCompressionParameters(const FIO_prefs_t* prefs) { +void FIO_displayCompressionParameters(const FIO_prefs_t* prefs) +{ static const char* formatOptions[5] = {ZSTD_EXTENSION, GZ_EXTENSION, XZ_EXTENSION, LZMA_EXTENSION, LZ4_EXTENSION}; static const char* sparseOptions[3] = {" --no-sparse", "", " --sparse"}; @@ -1920,7 +1939,7 @@ int FIO_compressMultipleFilenames(FIO_ctx_t* const fCtx, assert(outFileName != NULL || suffix != NULL); if (outFileName != NULL) { /* output into a single destination (stdout typically) */ FILE *dstFile; - if (FIO_removeMultiFilesWarning(fCtx, prefs, outFileName, 1 /* displayLevelCutoff */)) { + if (FIO_multiFilesConcatWarning(fCtx, prefs, outFileName, 1 /* displayLevelCutoff */)) { FIO_freeCResources(&ress); return 1; } @@ -2742,7 +2761,7 @@ FIO_decompressMultipleFilenames(FIO_ctx_t* const fCtx, dRess_t ress = FIO_createDResources(prefs, dictFileName); if (outFileName) { - if (FIO_removeMultiFilesWarning(fCtx, prefs, outFileName, 1 /* displayLevelCutoff */)) { + if (FIO_multiFilesConcatWarning(fCtx, prefs, outFileName, 1 /* displayLevelCutoff */)) { FIO_freeDResources(ress); return 1; } diff --git a/programs/util.c b/programs/util.c index bfa2abed0..e017772ef 100644 --- a/programs/util.c +++ b/programs/util.c @@ -121,7 +121,7 @@ int UTIL_requireUserConfirmation(const char* prompt, const char* abortMsg, ch = getchar(); result = 0; if (strchr(acceptableLetters, ch) == NULL) { - UTIL_DISPLAY("%s", abortMsg); + UTIL_DISPLAY("%s \n", abortMsg); result = 1; } /* flush the rest */ diff --git a/programs/zstdcli.c b/programs/zstdcli.c index b3386988d..3adbae739 100644 --- a/programs/zstdcli.c +++ b/programs/zstdcli.c @@ -339,7 +339,7 @@ static const char* lastNameFromPath(const char* path) static void errorOut(const char* msg) { - DISPLAY("%s \n", msg); exit(1); + DISPLAYLEVEL(1, "%s \n", msg); exit(1); } /*! readU32FromCharChecked() : @@ -786,13 +786,13 @@ static unsigned init_nbThreads(void) { } else { \ argNb++; \ if (argNb >= argCount) { \ - DISPLAY("error: missing command argument \n"); \ + DISPLAYLEVEL(1, "error: missing command argument \n"); \ CLEAN_RETURN(1); \ } \ ptr = argv[argNb]; \ assert(ptr != NULL); \ if (ptr[0]=='-') { \ - DISPLAY("error: command cannot be separated from its argument by another command \n"); \ + DISPLAYLEVEL(1, "error: command cannot be separated from its argument by another command \n"); \ CLEAN_RETURN(1); \ } } } @@ -859,6 +859,7 @@ int main(int argCount, const char* argv[]) FIO_prefs_t* const prefs = FIO_createPreferences(); FIO_ctx_t* const fCtx = FIO_createContext(); + FIO_progressSetting_e progress = FIO_ps_auto; zstd_operation_mode operation = zom_compress; ZSTD_compressionParameters compressionParams; int cLevel = init_cLevel(); @@ -898,7 +899,7 @@ int main(int argCount, const char* argv[]) (void)recursive; (void)cLevelLast; /* not used when ZSTD_NOBENCH set */ (void)memLimit; assert(argCount >= 1); - if ((filenames==NULL) || (file_of_names==NULL)) { DISPLAY("zstd: allocation error \n"); exit(1); } + if ((filenames==NULL) || (file_of_names==NULL)) { DISPLAYLEVEL(1, "zstd: allocation error \n"); exit(1); } programName = lastNameFromPath(programName); #ifdef ZSTD_MULTITHREAD nbWorkers = init_nbThreads(); @@ -999,8 +1000,8 @@ int main(int argCount, const char* argv[]) if (!strcmp(argument, "--rsyncable")) { rsyncable = 1; continue; } if (!strcmp(argument, "--compress-literals")) { literalCompressionMode = ZSTD_ps_enable; continue; } if (!strcmp(argument, "--no-compress-literals")) { literalCompressionMode = ZSTD_ps_disable; continue; } - if (!strcmp(argument, "--no-progress")) { FIO_setProgressSetting(FIO_ps_never); continue; } - if (!strcmp(argument, "--progress")) { FIO_setProgressSetting(FIO_ps_always); continue; } + if (!strcmp(argument, "--no-progress")) { progress = FIO_ps_never; continue; } + if (!strcmp(argument, "--progress")) { progress = FIO_ps_always; continue; } if (!strcmp(argument, "--exclude-compressed")) { FIO_setExcludeCompressedFile(prefs, 1); continue; } if (!strcmp(argument, "--fake-stdin-is-console")) { UTIL_fakeStdinIsConsole(); continue; } if (!strcmp(argument, "--fake-stdout-is-console")) { UTIL_fakeStdoutIsConsole(); continue; } @@ -1057,7 +1058,7 @@ int main(int argCount, const char* argv[]) if (longCommandWArg(&argument, "--output-dir-flat")) { NEXT_FIELD(outDirName); if (strlen(outDirName) == 0) { - DISPLAY("error: output dir cannot be empty string (did you mean to pass '.' instead?)\n"); + DISPLAYLEVEL(1, "error: output dir cannot be empty string (did you mean to pass '.' instead?)\n"); CLEAN_RETURN(1); } continue; @@ -1073,7 +1074,7 @@ int main(int argCount, const char* argv[]) if (longCommandWArg(&argument, "--output-dir-mirror")) { NEXT_FIELD(outMirroredDirName); if (strlen(outMirroredDirName) == 0) { - DISPLAY("error: output dir cannot be empty string (did you mean to pass '.' instead?)\n"); + DISPLAYLEVEL(1, "error: output dir cannot be empty string (did you mean to pass '.' instead?)\n"); CLEAN_RETURN(1); } continue; @@ -1349,7 +1350,7 @@ int main(int argCount, const char* argv[]) int const ret = FIO_listMultipleFiles((unsigned)filenames->tableSize, filenames->fileNames, g_displayLevel); CLEAN_RETURN(ret); #else - DISPLAY("file information is not supported \n"); + DISPLAYLEVEL(1, "file information is not supported \n"); CLEAN_RETURN(1); #endif } @@ -1480,24 +1481,29 @@ int main(int argCount, const char* argv[]) if (showDefaultCParams) { if (operation == zom_decompress) { - DISPLAY("error : can't use --show-default-cparams in decompression mode \n"); + DISPLAYLEVEL(1, "error : can't use --show-default-cparams in decompression mode \n"); CLEAN_RETURN(1); } } if (dictFileName != NULL && patchFromDictFileName != NULL) { - DISPLAY("error : can't use -D and --patch-from=# at the same time \n"); + DISPLAYLEVEL(1, "error : can't use -D and --patch-from=# at the same time \n"); CLEAN_RETURN(1); } if (patchFromDictFileName != NULL && filenames->tableSize > 1) { - DISPLAY("error : can't use --patch-from=# on multiple files \n"); + DISPLAYLEVEL(1, "error : can't use --patch-from=# on multiple files \n"); CLEAN_RETURN(1); } - /* No status message in pipe mode (stdin - stdout) */ + /* No status message by default when output is stdout */ hasStdout = outFileName && !strcmp(outFileName,stdoutmark); - if ((hasStdout || !UTIL_isConsole(stderr)) && (g_displayLevel==2)) g_displayLevel=1; + if (hasStdout && (g_displayLevel==2)) g_displayLevel=1; + + /* when stderr is not the console, do not pollute it with status updates + * Note : the below code actually also silence more stuff, including completion report. */ + if (!UTIL_isConsole(stderr) && (g_displayLevel==2)) g_displayLevel=1; + FIO_setProgressSetting(progress); /* don't remove source files when output is stdout */; if (hasStdout && removeSrcFile) { @@ -1569,7 +1575,7 @@ int main(int argCount, const char* argv[]) operationResult = FIO_compressMultipleFilenames(fCtx, prefs, filenames->fileNames, outMirroredDirName, outDirName, outFileName, suffix, dictFileName, cLevel, compressionParams); #else (void)contentSize; (void)suffix; (void)adapt; (void)rsyncable; (void)ultra; (void)cLevel; (void)ldmFlag; (void)literalCompressionMode; (void)targetCBlockSize; (void)streamSrcSize; (void)srcSizeHint; (void)ZSTD_strategyMap; (void)useRowMatchFinder; /* not used when ZSTD_NOCOMPRESS set */ - DISPLAY("Compression not supported \n"); + DISPLAYLEVEL(1, "Compression not supported \n"); #endif } else { /* decompression or test */ #ifndef ZSTD_NODECOMPRESS @@ -1579,7 +1585,7 @@ int main(int argCount, const char* argv[]) operationResult = FIO_decompressMultipleFilenames(fCtx, prefs, filenames->fileNames, outMirroredDirName, outDirName, outFileName, dictFileName); } #else - DISPLAY("Decompression not supported \n"); + DISPLAYLEVEL(1, "Decompression not supported \n"); #endif } diff --git a/tests/cli-tests/file-stat/compress-file-to-stdout.sh.stderr.exact b/tests/cli-tests/file-stat/compress-file-to-stdout.sh.stderr.exact index e86c4eae3..7c690d20b 100644 --- a/tests/cli-tests/file-stat/compress-file-to-stdout.sh.stderr.exact +++ b/tests/cli-tests/file-stat/compress-file-to-stdout.sh.stderr.exact @@ -2,6 +2,8 @@ Trace:FileStat: > UTIL_isLink(file) Trace:FileStat: < 0 Trace:FileStat: > UTIL_isConsole(1) Trace:FileStat: < 0 +Trace:FileStat: > UTIL_isConsole(2) +Trace:FileStat: < 0 Trace:FileStat: > UTIL_getFileSize(file) Trace:FileStat: > UTIL_stat(file) Trace:FileStat: < 1 diff --git a/tests/cli-tests/file-stat/compress-stdin-to-stdout.sh.stderr.exact b/tests/cli-tests/file-stat/compress-stdin-to-stdout.sh.stderr.exact index 2e44511d8..8bf05e641 100644 --- a/tests/cli-tests/file-stat/compress-stdin-to-stdout.sh.stderr.exact +++ b/tests/cli-tests/file-stat/compress-stdin-to-stdout.sh.stderr.exact @@ -2,6 +2,8 @@ Trace:FileStat: > UTIL_isConsole(0) Trace:FileStat: < 0 Trace:FileStat: > UTIL_isConsole(1) Trace:FileStat: < 0 +Trace:FileStat: > UTIL_isConsole(2) +Trace:FileStat: < 0 Trace:FileStat: > UTIL_getFileSize(/*stdin*\) Trace:FileStat: > UTIL_stat(/*stdin*\) Trace:FileStat: < 0 diff --git a/tests/cli-tests/file-stat/decompress-file-to-stdout.sh.stderr.exact b/tests/cli-tests/file-stat/decompress-file-to-stdout.sh.stderr.exact index bbf66506b..7fe6dda15 100644 --- a/tests/cli-tests/file-stat/decompress-file-to-stdout.sh.stderr.exact +++ b/tests/cli-tests/file-stat/decompress-file-to-stdout.sh.stderr.exact @@ -2,6 +2,8 @@ Trace:FileStat: > UTIL_isLink(file.zst) Trace:FileStat: < 0 Trace:FileStat: > UTIL_isConsole(1) Trace:FileStat: < 0 +Trace:FileStat: > UTIL_isConsole(2) +Trace:FileStat: < 0 Trace:FileStat: > UTIL_isDirectory(file.zst) Trace:FileStat: > UTIL_stat(file.zst) Trace:FileStat: < 1 diff --git a/tests/cli-tests/file-stat/decompress-stdin-to-stdout.sh.stderr.exact b/tests/cli-tests/file-stat/decompress-stdin-to-stdout.sh.stderr.exact index 61487f61e..e36cb9d05 100644 --- a/tests/cli-tests/file-stat/decompress-stdin-to-stdout.sh.stderr.exact +++ b/tests/cli-tests/file-stat/decompress-stdin-to-stdout.sh.stderr.exact @@ -2,6 +2,8 @@ Trace:FileStat: > UTIL_isConsole(0) Trace:FileStat: < 0 Trace:FileStat: > UTIL_isConsole(1) Trace:FileStat: < 0 +Trace:FileStat: > UTIL_isConsole(2) +Trace:FileStat: < 0 Trace:FileStat: > UTIL_isDirectory(/*stdin*\) Trace:FileStat: > UTIL_stat(/*stdin*\) Trace:FileStat: < 0 diff --git a/tests/playTests.sh b/tests/playTests.sh index d0764d4d3..e064c86df 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -387,7 +387,7 @@ println "\n===> file removal" zstd -f --rm tmp test ! -f tmp # tmp should no longer be present zstd -f -d --rm tmp.zst -test ! -f tmp.zst # tmp.zst should no longer be present +test ! -f tmp.zst # tmp.zst should no longer be present println "test: --rm is disabled when output is stdout" test -f tmp zstd --rm tmp -c > $INTOVOID @@ -396,6 +396,20 @@ zstd -f --rm tmp -c > $INTOVOID test -f tmp # tmp shall still be there zstd -f tmp -c > $INTOVOID --rm test -f tmp # tmp shall still be there +println "test: --rm is disabled when multiple inputs are concatenated into a single output" +cp tmp tmp2 +zstd --rm tmp tmp2 -c > $INTOVOID +test -f tmp +test -f tmp2 +rm -f tmp3.zst +echo 'y' | zstd -v tmp tmp2 -o tmp3.zst --rm # prompt for confirmation +test -f tmp +test -f tmp2 +zstd -f tmp tmp2 -o tmp3.zst --rm # just warns, no prompt +test -f tmp +test -f tmp2 +zstd -q tmp tmp2 -o tmp3.zst --rm && die "should refuse to concatenate" + println "test : should quietly not remove non-regular file" println hello > tmp zstd tmp -f -o "$DEVDEVICE" 2>tmplog > "$INTOVOID" @@ -437,34 +451,35 @@ println hello > tmp1 println world > tmp2 zstd tmp1 tmp2 -o "$INTOVOID" -f zstd tmp1 tmp2 -c | zstd -t -zstd tmp1 tmp2 -o tmp.zst +echo 'y' | zstd -v tmp1 tmp2 -o tmp.zst test ! -f tmp1.zst test ! -f tmp2.zst zstd tmp1 tmp2 zstd -t tmp1.zst tmp2.zst zstd -dc tmp1.zst tmp2.zst zstd tmp1.zst tmp2.zst -o "$INTOVOID" -f -zstd -d tmp1.zst tmp2.zst -o tmp +echo 'y' | zstd -v -d tmp1.zst tmp2.zst -o tmp touch tmpexists zstd tmp1 tmp2 -f -o tmpexists zstd tmp1 tmp2 -q -o tmpexists && die "should have refused to overwrite" println gooder > tmp_rm1 println boi > tmp_rm2 println worldly > tmp_rm3 -echo 'y' | zstd tmp_rm1 tmp_rm2 -v -o tmp_rm3.zst --rm # tests the warning prompt for --rm with multiple inputs into once source -test ! -f tmp_rm1 -test ! -f tmp_rm2 +echo 'y' | zstd -v tmp_rm1 tmp_rm2 -v -o tmp_rm3.zst +test -f tmp_rm1 +test -f tmp_rm2 cp tmp_rm3.zst tmp_rm4.zst -echo 'Y' | zstd -d tmp_rm3.zst tmp_rm4.zst -v -o tmp_rm_out --rm -test ! -f tmp_rm3.zst -test ! -f tmp_rm4.zst +echo 'Y' | zstd -v -d tmp_rm3.zst tmp_rm4.zst -v -o tmp_rm_out --rm +test -f tmp_rm3.zst +test -f tmp_rm4.zst println gooder > tmpexists1 zstd tmpexists1 tmpexists -c --rm -f > $INTOVOID - # Bug: PR #972 if [ "$?" -eq 139 ]; then die "should not have segfaulted" fi +test -f tmpexists1 +test -f tmpexists println "\n===> multiple files and shell completion " datagen -s1 > tmp1 2> $INTOVOID datagen -s2 -g100K > tmp2 2> $INTOVOID @@ -1172,6 +1187,10 @@ zstd -t tmp3 && die "bad file not detected !" # detects 0-sized files as bad println "test --rm and --test combined " zstd -t --rm tmp1.zst test -f tmp1.zst # check file is still present +cp tmp1.zst tmp2.zst +zstd -t tmp1.zst tmp2.zst --rm +test -f tmp1.zst # check file is still present +test -f tmp2.zst # check file is still present split -b16384 tmp1.zst tmpSplit. zstd -t tmpSplit.* && die "bad file not detected !" datagen | zstd -c | zstd -t -- 2.47.2