refactor : --rm is ignored with stdout

`zstd` CLI has progressively moved to the policy of
ignoring `--rm` command when the output is `stdout`.
The primary drive is to feature a behavior more consistent with `gzip`,
when `--rm` is the default, but is also ignored when output is `stdout`.
Other policies are certainly possible, but would break from this `gzip` convention.

The new policy was inconsistenly enforced, depending on the exact list of commands.
For example, it was possible to circumvent it by using `-c --rm` in this order,
which would re-establish source removal.

- Update the CLI so that it necessarily catch these situations and ensure that `--rm` is always disabled when output is `stdout`.
- Added a warning message in this case (for verbosity 3 `-v`).
- Added an `assert()`, which controls that `--rm` is no longer active with `stdout`
- Added tests, which control the behavior, even when `--rm` is added after `-c`
- Removed some legacy code which where trying to apply a specific policy for the `stdout` + `--rm` case, which is no longer possible

diff --git a/programs/fileio.c b/programs/fileio.c
index 367ade415ca2582d21b57266011d237059e770c4..2d79203de6d1f30372b087ad8c95b0a2b932f454 100644 (file)
--- a/programs/fileio.c
+++ b/programs/fileio.c
@@ -362,7 +362,7 @@ void FIO_setDictIDFlag(FIO_prefs_t* const prefs, int dictIDFlag) { prefs->dictID
 
 void FIO_setChecksumFlag(FIO_prefs_t* const prefs, int checksumFlag) { prefs->checksumFlag = checksumFlag; }
 
-void FIO_setRemoveSrcFile(FIO_prefs_t* const prefs, unsigned flag) { prefs->removeSrcFile = (flag>0); }
+void FIO_setRemoveSrcFile(FIO_prefs_t* const prefs, int flag) { prefs->removeSrcFile = (flag!=0); }
 
 void FIO_setMemLimit(FIO_prefs_t* const prefs, unsigned memLimit) { prefs->memLimit = memLimit; }
 
@@ -833,7 +833,7 @@ static void FIO_adjustMemLimitForPatchFromMode(FIO_prefs_t* const prefs,
  *         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).
- * However, if the output is stdout, we will always abort rather than displaying the warning prompt.
+ *         Note : --rm in combination with stdout is not allowed.
  */
 static int FIO_removeMultiFilesWarning(FIO_ctx_t* const fCtx, const FIO_prefs_t* const prefs, const char* outFileName, int displayLevelCutoff)
 {
@@ -850,14 +850,10 @@ static int FIO_removeMultiFilesWarning(FIO_ctx_t* const fCtx, const FIO_prefs_t*
             } 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 the original directory tree. \n")
+            DISPLAYLEVEL(2, "The concatenated output CANNOT regenerate original file names nor directory structure. \n")
             if (prefs->removeSrcFile) {
-                if (fCtx->hasStdoutOutput) {
-                    DISPLAYLEVEL(1, "Aborting. Use -f if you really want to delete the files and output to stdout\n");
-                    error = 1;
-                } else {
-                    error = g_display_prefs.displayLevel > displayLevelCutoff && UTIL_requireUserConfirmation("This is a destructive operation. Proceed? (y/n): ", "Aborting...", "yY", fCtx->hasStdinInput);
-                }
+                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);
             }
         }
     }

diff --git a/programs/fileio.h b/programs/fileio.h
index e37398ded254bcc0a3aa30049527156d1deaa6be..291d4d414585a5ab9405c45f2283ace44fe291d0 100644 (file)
--- a/programs/fileio.h
+++ b/programs/fileio.h
@@ -86,7 +86,7 @@ void FIO_setLdmMinMatch(FIO_prefs_t* const prefs, int ldmMinMatch);
 void FIO_setMemLimit(FIO_prefs_t* const prefs, unsigned memLimit);
 void FIO_setNbWorkers(FIO_prefs_t* const prefs, int nbWorkers);
 void FIO_setOverlapLog(FIO_prefs_t* const prefs, int overlapLog);
-void FIO_setRemoveSrcFile(FIO_prefs_t* const prefs, unsigned flag);
+void FIO_setRemoveSrcFile(FIO_prefs_t* const prefs, int flag);
 void FIO_setSparseWrite(FIO_prefs_t* const prefs, int sparse);  /**< 0: no sparse; 1: disable on stdout; 2: always enabled */
 void FIO_setRsyncable(FIO_prefs_t* const prefs, int rsyncable);
 void FIO_setStreamSrcSize(FIO_prefs_t* const prefs, size_t streamSrcSize);

diff --git a/programs/zstdcli.c b/programs/zstdcli.c
index 9937a9c9725e759ed0693044bc28aa736680a4cd..b3386988d04e57fb41dfe410974cf600eee96344 100644 (file)
--- a/programs/zstdcli.c
+++ b/programs/zstdcli.c
@@ -850,7 +850,8 @@ int main(int argCount, const char* argv[])
         defaultLogicalCores = 0,
         showDefaultCParams = 0,
         ultra=0,
-        contentSize=1;
+        contentSize=1,
+        removeSrcFile=0;
     unsigned nbWorkers = 0;
     double compressibility = 0.5;
     unsigned bench_nbSeconds = 3;   /* would be better if this value was synchronized from bench */
@@ -909,17 +910,17 @@ int main(int argCount, const char* argv[])
     if (exeNameMatch(programName, ZSTD_CAT)) { operation=zom_decompress; FIO_overwriteMode(prefs); forceStdout=1; followLinks=1; FIO_setPassThroughFlag(prefs, 1); outFileName=stdoutmark; g_displayLevel=1; }     /* supports multiple formats */
     if (exeNameMatch(programName, ZSTD_ZCAT)) { operation=zom_decompress; FIO_overwriteMode(prefs); forceStdout=1; followLinks=1; FIO_setPassThroughFlag(prefs, 1); outFileName=stdoutmark; g_displayLevel=1; }    /* behave like zcat, also supports multiple formats */
     if (exeNameMatch(programName, ZSTD_GZ)) {   /* behave like gzip */
-        suffix = GZ_EXTENSION; FIO_setCompressionType(prefs, FIO_gzipCompression); FIO_setRemoveSrcFile(prefs, 1);
+        suffix = GZ_EXTENSION; FIO_setCompressionType(prefs, FIO_gzipCompression); removeSrcFile=1;
         dictCLevel = cLevel = 6;  /* gzip default is -6 */
     }
-    if (exeNameMatch(programName, ZSTD_GUNZIP)) { operation=zom_decompress; FIO_setRemoveSrcFile(prefs, 1); }                                                     /* behave like gunzip, also supports multiple formats */
+    if (exeNameMatch(programName, ZSTD_GUNZIP)) { operation=zom_decompress; removeSrcFile=1; }                                                     /* behave like gunzip, also supports multiple formats */
     if (exeNameMatch(programName, ZSTD_GZCAT)) { operation=zom_decompress; FIO_overwriteMode(prefs); forceStdout=1; followLinks=1; FIO_setPassThroughFlag(prefs, 1); outFileName=stdoutmark; g_displayLevel=1; }   /* behave like gzcat, also supports multiple formats */
-    if (exeNameMatch(programName, ZSTD_LZMA)) { suffix = LZMA_EXTENSION; FIO_setCompressionType(prefs, FIO_lzmaCompression); FIO_setRemoveSrcFile(prefs, 1); }    /* behave like lzma */
-    if (exeNameMatch(programName, ZSTD_UNLZMA)) { operation=zom_decompress; FIO_setCompressionType(prefs, FIO_lzmaCompression); FIO_setRemoveSrcFile(prefs, 1); } /* behave like unlzma, also supports multiple formats */
-    if (exeNameMatch(programName, ZSTD_XZ)) { suffix = XZ_EXTENSION; FIO_setCompressionType(prefs, FIO_xzCompression); FIO_setRemoveSrcFile(prefs, 1); }          /* behave like xz */
-    if (exeNameMatch(programName, ZSTD_UNXZ)) { operation=zom_decompress; FIO_setCompressionType(prefs, FIO_xzCompression); FIO_setRemoveSrcFile(prefs, 1); }     /* behave like unxz, also supports multiple formats */
-    if (exeNameMatch(programName, ZSTD_LZ4)) { suffix = LZ4_EXTENSION; FIO_setCompressionType(prefs, FIO_lz4Compression); }                                       /* behave like lz4 */
-    if (exeNameMatch(programName, ZSTD_UNLZ4)) { operation=zom_decompress; FIO_setCompressionType(prefs, FIO_lz4Compression); }                                   /* behave like unlz4, also supports multiple formats */
+    if (exeNameMatch(programName, ZSTD_LZMA)) { suffix = LZMA_EXTENSION; FIO_setCompressionType(prefs, FIO_lzmaCompression); removeSrcFile=1; }    /* behave like lzma */
+    if (exeNameMatch(programName, ZSTD_UNLZMA)) { operation=zom_decompress; FIO_setCompressionType(prefs, FIO_lzmaCompression); removeSrcFile=1; } /* behave like unlzma, also supports multiple formats */
+    if (exeNameMatch(programName, ZSTD_XZ)) { suffix = XZ_EXTENSION; FIO_setCompressionType(prefs, FIO_xzCompression); removeSrcFile=1; }          /* behave like xz */
+    if (exeNameMatch(programName, ZSTD_UNXZ)) { operation=zom_decompress; FIO_setCompressionType(prefs, FIO_xzCompression); removeSrcFile=1; }     /* behave like unxz, also supports multiple formats */
+    if (exeNameMatch(programName, ZSTD_LZ4)) { suffix = LZ4_EXTENSION; FIO_setCompressionType(prefs, FIO_lz4Compression); }                        /* behave like lz4 */
+    if (exeNameMatch(programName, ZSTD_UNLZ4)) { operation=zom_decompress; FIO_setCompressionType(prefs, FIO_lz4Compression); }                    /* behave like unlz4, also supports multiple formats */
     memset(&compressionParams, 0, sizeof(compressionParams));
 
     /* init crash handler */
@@ -956,7 +957,7 @@ int main(int argCount, const char* argv[])
                 if (!strcmp(argument, "--help")) { usage_advanced(programName); CLEAN_RETURN(0); }
                 if (!strcmp(argument, "--verbose")) { g_displayLevel++; continue; }
                 if (!strcmp(argument, "--quiet")) { g_displayLevel--; continue; }
-                if (!strcmp(argument, "--stdout")) { forceStdout=1; outFileName=stdoutmark; FIO_setRemoveSrcFile(prefs, 0); g_displayLevel-=(g_displayLevel==2); continue; }
+                if (!strcmp(argument, "--stdout")) { forceStdout=1; outFileName=stdoutmark; removeSrcFile=0; continue; }
                 if (!strcmp(argument, "--ultra")) { ultra=1; continue; }
                 if (!strcmp(argument, "--check")) { FIO_setChecksumFlag(prefs, 2); continue; }
                 if (!strcmp(argument, "--no-check")) { FIO_setChecksumFlag(prefs, 0); continue; }
@@ -969,8 +970,8 @@ int main(int argCount, const char* argv[])
                 if (!strcmp(argument, "--no-asyncio")) { FIO_setAsyncIOFlag(prefs, 0); continue;}
                 if (!strcmp(argument, "--train")) { operation=zom_train; if (outFileName==NULL) outFileName=g_defaultDictName; continue; }
                 if (!strcmp(argument, "--no-dictID")) { FIO_setDictIDFlag(prefs, 0); continue; }
-                if (!strcmp(argument, "--keep")) { FIO_setRemoveSrcFile(prefs, 0); continue; }
-                if (!strcmp(argument, "--rm")) { FIO_setRemoveSrcFile(prefs, 1); continue; }
+                if (!strcmp(argument, "--keep")) { removeSrcFile=0; continue; }
+                if (!strcmp(argument, "--rm")) { removeSrcFile=1; continue; }
                 if (!strcmp(argument, "--priority=rt")) { setRealTimePrio = 1; continue; }
                 if (!strcmp(argument, "--show-default-cparams")) { showDefaultCParams = 1; continue; }
                 if (!strcmp(argument, "--content-size")) { contentSize = 1; continue; }
@@ -1167,7 +1168,7 @@ int main(int argCount, const char* argv[])
                         operation=zom_decompress; argument++; break;
 
                     /* Force stdout, even if stdout==console */
-                case 'c': forceStdout=1; outFileName=stdoutmark; FIO_setRemoveSrcFile(prefs, 0); argument++; break;
+                case 'c': forceStdout=1; outFileName=stdoutmark; removeSrcFile=0; argument++; break;
 
                     /* do not store filename - gzip compatibility - nothing to do */
                 case 'n': argument++; break;
@@ -1185,7 +1186,7 @@ int main(int argCount, const char* argv[])
                 case 'q': g_displayLevel--; argument++; break;
 
                     /* keep source file (default) */
-                case 'k': FIO_setRemoveSrcFile(prefs, 0); argument++; break;
+                case 'k': removeSrcFile=0; argument++; break;
 
                     /* Checksum */
                 case 'C': FIO_setChecksumFlag(prefs, 2); argument++; break;
@@ -1434,7 +1435,7 @@ int main(int argCount, const char* argv[])
     }
 
 #ifndef ZSTD_NODECOMPRESS
-    if (operation==zom_test) { FIO_setTestMode(prefs, 1); outFileName=nulmark; FIO_setRemoveSrcFile(prefs, 0); }  /* test mode */
+    if (operation==zom_test) { FIO_setTestMode(prefs, 1); outFileName=nulmark; removeSrcFile=0; }  /* test mode */
 #endif
 
     /* No input filename ==> use stdin and stdout */
@@ -1496,9 +1497,15 @@ int main(int argCount, const char* argv[])
 
     /* No status message in pipe mode (stdin - stdout) */
     hasStdout = outFileName && !strcmp(outFileName,stdoutmark);
-
     if ((hasStdout || !UTIL_isConsole(stderr)) && (g_displayLevel==2)) g_displayLevel=1;
 
+    /* don't remove source files when output is stdout */;
+    if (hasStdout && removeSrcFile) {
+        DISPLAYLEVEL(3, "Note: src files are not removed when output is stdout \n");
+        removeSrcFile = 0;
+    }
+    FIO_setRemoveSrcFile(prefs, removeSrcFile);
+
     /* IO Stream/File */
     FIO_setHasStdoutOutput(fCtx, hasStdout);
     FIO_setNbFilesTotal(fCtx, (int)filenames->tableSize);

diff --git a/tests/playTests.sh b/tests/playTests.sh
index 06e6ba95138a79f061a43ee621222178a2c9da9e..d0764d4d3fda90c7f7858ee9aeef016223c5cebb 100755 (executable)
--- a/tests/playTests.sh
+++ b/tests/playTests.sh
@@ -388,6 +388,14 @@ 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
+println "test: --rm is disabled when output is stdout"
+test -f tmp
+zstd --rm tmp -c > $INTOVOID
+test -f tmp # tmp shall still be there
+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 : should quietly not remove non-regular file"
 println hello > tmp
 zstd tmp -f -o "$DEVDEVICE" 2>tmplog > "$INTOVOID"
@@ -450,8 +458,6 @@ 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 'yes' | zstd tmp_rm_out tmp_rm3 -c --rm && die "compressing multiple files to stdout with --rm should fail unless -f is specified"
-echo 'yes' | zstd tmp_rm_out tmp_rm3 -c --rm -v && die "compressing multiple files to stdout with --rm should fail unless -f is specified"
 println gooder > tmpexists1
 zstd tmpexists1 tmpexists -c --rm -f > $INTOVOID