Re: [Bug-tar] [PATCH] Do not fail when 'compress' is unable to provide sufficient compress ratio
On Sunday, April 28, 2013 07:23:05 PM Antonio Diaz Diaz wrote: Paul Eggert wrote: Converting gzip to the general API seems the right thing to do in the long term. That might be wise, yes, but there might be other users who expect the current behavior, and we'd need to consult with them. To my surprise I have found that the current behavior is that gzip does not return '2' even if the compressed file is larger than the original: Thanks, there are probably other circumstances in 'gzip' invoking warning? And there are a lot of other compressor programs to update. In the meantime 'tar' should work with what's out there. If it is not broken, don't fix it. I think nobody has reported a problem caused by any compressor other than compress returning 2. I would not wait for a bugreport.. Tar should really accept API of compressors it uses imo. Most probably it is the documentation of those other compressors (including gzip) what is wrong, not the code. I guess it is not bug in gzip doc, but afaik Paul is also 'gzip' maintainer so he would be able to talk about gzip more. Pavel
Re: [Bug-tar] [PATCH] Do not fail when 'compress' is unable to provide sufficient compress ratio
If you look at the gzip source code you'll find a number of circumstances that will cause it to exit with status 2, as a warning. This API isn't likely to change any time soon, I'm afraid, as gzip is fairly frozen.
Re: [Bug-tar] [PATCH] Do not fail when 'compress' is unable to provide sufficient compress ratio
Paul Eggert wrote: Converting gzip to the general API seems the right thing to do in the long term. That might be wise, yes, but there might be other users who expect the current behavior, and we'd need to consult with them. To my surprise I have found that the current behavior is that gzip does not return '2' even if the compressed file is larger than the original: $ dd if=/dev/urandom of=testfile bs=1K count=1 $ ls -go testfile* -rw-r--r-- 1 1024 2013-04-28 19:04 testfile $ gzip testfile ; echo $? 0 $ ls -go testfile* -rw-r--r-- 1 1056 2013-04-28 19:04 testfile.gz And there are a lot of other compressor programs to update. In the meantime 'tar' should work with what's out there. If it is not broken, don't fix it. I think nobody has reported a problem caused by any compressor other than compress returning 2. Most probably it is the documentation of those other compressors (including gzip) what is wrong, not the code. Best regards, Antonio.
Re: [Bug-tar] [PATCH] Do not fail when 'compress' is unable to provide sufficient compress ratio
On 04/26/2013 12:56 AM, Pavel Raiskup wrote: So you think something like remove the leading path '/usr/bin/' before comparison with 'GZIP_PROGRAM'? I'm afraid that will lead to even more confusion; among other things, tar shouldn't care what the program names are. It may be better to have two different options. One, designed for programs that behave like 'compress' does, where exit status 2 is OK. The other, for normal programs, where only exit status 0 is OK. When tar infers a compressor it can also infer which of these two options to use.
Re: [Bug-tar] [PATCH] Do not fail when 'compress' is unable to provide sufficient compress ratio
When tar infers a compressor it can also infer which of these two options to use. That means, using the -Z, -z, .. options will setup some new flag, same as the new use-compress-program-like option will. Do you have some requirements for this option name? This is the hard one.. Would you be OK if I tried to prepare the patch?
Re: [Bug-tar] [PATCH] Do not fail when 'compress' is unable to provide sufficient compress ratio
On 04/26/2013 01:17 AM, Pavel Raiskup wrote: Do you have some requirements for this option name? We could have --use-compress-program for the compress API, and --use-filter-program for the more-vanilla API. Or something like that. I'm fine with your preparing a patch.
Re: [Bug-tar] [PATCH] Do not fail when 'compress' is unable to provide sufficient compress ratio
On Friday, April 26, 2013 01:36:02 AM Paul Eggert wrote: On 04/26/2013 01:17 AM, Pavel Raiskup wrote: Do you have some requirements for this option name? We could have --use-compress-program for the compress API, and --use-filter-program for the more-vanilla API. Or something like that. I'm fine with your preparing a patch. I agree that this would be the most logical. But there is a problem with backward compatibility. The behaviour of --use-compress-program will change.. what about this: -I, --use-filter-program (general 'vanilla' API) --use-compress-program (obsoleted alias to ^^^) // remove from --help man info to not confuse people --use-compress-like-program (with a *proper* definition what it does) // or choose a better name? Pavel
Re: [Bug-tar] [PATCH] Do not fail when 'compress' is unable to provide sufficient compress ratio
On 04/26/2013 03:07 AM, Pavel Raiskup wrote: -I, --use-filter-program (general 'vanilla' API) --use-compress-program (obsoleted alias to ^^^) // remove from --help man info to not confuse people --use-compress-like-program (with a *proper* definition what it does) // or choose a better name? Something like this sounds good. But how about dropping the use- for the new options? That will simplify things a bit. Like this: -I, --filter-program (general 'vanilla' API) --use-compress-program (obsoleted alias for --filter-program) // remove from --help man info to not confuse people --compress-program (with a *proper* definition what it does)
Re: [Bug-tar] [PATCH] Do not fail when 'compress' is unable to provide sufficient compress ratio
On 04/26/2013 10:07 AM, Antonio Diaz Diaz wrote: Converting gzip to the general API seems the right thing to do in the long term. That might be wise, yes, but there might be other users who expect the current behavior, and we'd need to consult with them. And there are a lot of other compressor programs to update. In the meantime 'tar' should work with what's out there.
Re: [Bug-tar] [PATCH] Do not fail when 'compress' is unable to provide sufficient compress ratio
On Wednesday, April 24, 2013 10:58:10 PM Paul Eggert wrote: I think I'd rather have tar assume the gzip exit-status convention, and depart from that only for bzip2 and lzip (which don't use the convention). That's a bit simpler and is more likely to match some random future compressor. Should I fix that patch then? This will probably bring incompatibilities into dealing with user-defined scripts passed by --use-compress-program (until that change, tar was accepted non-zero exit value as an error). Don't really know how much we should care about this.. Pavel
Re: [Bug-tar] [PATCH] Do not fail when 'compress' is unable to provide sufficient compress ratio
Paul Eggert wrote: I think I'd rather have tar assume the gzip exit-status convention, and depart from that only for bzip2 and lzip (which don't use the convention). That's a bit simpler and is more likely to match some random future compressor. Only if the random future compressor continues repeating the errors of the past. Bzip2 and lzip follow a deeper convention; that any non-zero exit status means failure[1]: [1]http://www.gnu.org/software/bash/manual/html_node/Exit-Status.html#Exit-Status For the shell's purposes, a command which exits with a zero exit status has succeeded. A non-zero exit status indicates failure. This seemingly counter-intuitive scheme is used so there is one well-defined way to indicate success and a variety of ways to indicate various failure modes. Routing around gzip's quirks may be necessary, but if we begin considering some non-zero exit statuses as success, chaos will develop. The --use-compress-program is a general mechanism, not only for compressors[2]: [2]http://www.gnu.org/software/tar/manual/html_node/gzip.html#SEC132 The '--use-compress-program' option, in particular, lets you implement your own filters, not necessarily dealing with compression/decompression. For example, suppose you wish to implement PGP encryption on top of compression, using gpg And it happens that '2' is in the range gpg reserves for fatal errors[3]: [3]http://www.gnupg.org/documentation/manuals/gnupg/GPG-Examples.html#GPG-Examples The program returns 0 if everything was fine, 1 if at least a signature was bad, and other error codes for fatal errors. So, please, don't break --use-compress-program. Best regards, Antonio.
Re: [Bug-tar] [PATCH] Do not fail when 'compress' is unable to provide sufficient compress ratio
I think I'd rather have tar assume the gzip exit-status convention, and depart from that only for bzip2 and lzip (which don't use the convention). That's a bit simpler and is more likely to match some random future compressor.
Re: [Bug-tar] [PATCH] Do not fail when 'compress' is unable to provide sufficient compress ratio
On Tuesday, February 26, 2013 08:32:25 AM Pavel Raiskup wrote: On Mon, 2013-02-25 at 11:39 -0800, Paul Eggert wrote: I suggest something simpler: namely, just declare that the -Z (compress) program must conform to the compress API, so that its exit status 2 really means OK. That way, we can change only src/system.c and NEWS. The compress API is obsolete and isn't likely to change so this sounds safe. Thanks for comments! Attaching approach with just src/system.c and NEWS edited. Hi, I have looked once again on usual compressors and I see now that not only 'compress' API exits with 2 exit status in case of warning .. 'gzip', 'xz', 'lzma' and 'lzop' behave similarly. Attaching probably better fix, any comment is welcome! Pavel From 5f316c5500c6c62bae9c5a83de711eacd2615611 Mon Sep 17 00:00:00 2001 From: Pavel Raiskup prais...@redhat.com Date: Mon, 25 Feb 2013 10:18:15 +0100 Subject: [PATCH] tar: do not fail hardly when compressor just warns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Usual compressor exit status API is that 0 means successful compression, 1 is fatal error and 2 is warning. The two exceptions used by tar by default are Bzip2 and lzip at the moment — any non-zero value is fatal error. * src/system.c (sys_wait_for_child): Warn only when child process exited with 2 and the compressing command wasn't bzip2 or lzip. * NEWS: Document. --- NEWS | 9 + src/system.c | 22 +++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index 3108798..aa8c82e 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,15 @@ version 1.26.90 (Git) * Bug fixes +** tar successes now when compressor warns only + +Tar does not fail with fatal exit status 2 if the tar's child +compressor exited with warning exit status 2. This is usual default +for 'xz', 'gzip', 'compress', 'lzop' and 'lzma' compressors. The +'bzip2' and 'lzip' compressors are throwing non-zero exit status in +case of fatal error. Tar still fails hardly if user specified script +passed by --use-compress-program returns non-zero status. + ** Sparse files with large data When creating a PAX-format archive, tar no longer arbitrarily restricts diff --git a/src/system.c b/src/system.c index e1fd263..4cd12e9 100644 --- a/src/system.c +++ b/src/system.c @@ -189,9 +189,25 @@ sys_wait_for_child (pid_t child_pid, bool eof) if (!(!eof sig == SIGPIPE)) FATAL_ERROR ((0, 0, _(Child died with signal %d), sig)); } - else if (WEXITSTATUS (wait_status) != 0) - FATAL_ERROR ((0, 0, _(Child returned status %d), - WEXITSTATUS (wait_status))); + /* Exit value of common compressors usually conforms to following + defaults: 0 is successful compression, 1 is fatal error and 2 is + warning. + Bzip2 and lzip return non-zero value in case of fatal error - use + this as default behavior also for user-defined scripts. + */ + else if (WEXITSTATUS (wait_status) == 0) +return; + else if (WEXITSTATUS (wait_status) == 2 +(!strcmp (use_compress_program_option, GZIP_PROGRAM) +|| !strcmp (use_compress_program_option, XZ_PROGRAM) +|| !strcmp (use_compress_program_option, LZMA_PROGRAM) +|| !strcmp (use_compress_program_option, LZOP_PROGRAM) +|| !strcmp (use_compress_program_option, COMPRESS_PROGRAM))) +WARN ((0, 0, _(%lu: Child (compressor '%s') exited with warning), + (unsigned long) child_pid, use_compress_program_option)); + else +FATAL_ERROR ((0, 0, _(Child '%s' returned exit status %d), + use_compress_program_option, WEXITSTATUS (wait_status))); } } -- 1.8.1.4
Re: [Bug-tar] [PATCH] Do not fail when 'compress' is unable to provide sufficient compress ratio
I'm attaching one possible solution, could you look at it? Sorry for the typo, re-attaching because of this: | diff --git a/src/system.c b/src/system.c | index ca0ad05..d722352 100644 | --- a/src/system.c | +++ b/src/system.c | @@ -192,7 +192,7 @@ sys_wait_for_child (pid_t child_pid, bool eof) | #ifdef HAVE_COMPRESS_EXIT_HACK |else if (!strcmp (use_compress_program_option, HAVE_COMPRESS_EXIT_HACK) | WEXITSTATUS (wait_status) == 2) | -WARN ((0, 0, _(%lu: Child says that compressed output is larger | +WARN ((0, 0, _(%lu: Child says that compressed output is larger |then original data.), |(unsigned long) child_pid)); | #endif From 3281269662a7d361ef8a360600352e93cc2ebdcf Mon Sep 17 00:00:00 2001 From: Pavel Raiskup prais...@redhat.com Date: Mon, 25 Feb 2013 10:18:15 +0100 Subject: [PATCH] tar: The Lempel-Ziv coding bugfix Do not exit with fatal error 2 after compressing tarball by the 'compress' utility (-Z option) when the child process running the compressor exited with just-a-warning exit value 2. There is still possible to restore the old tar behavior by running './configure --without-compress-exit-hack' (e.g. in situations when there resides different compress implementation on the target system). * acinclude.m4 (TAR_COMPRESS_EXIT_HACK): New function. * configure.ac: Use TAR_COMPRESS_EXIT_HACK. * src/system.c (sys_wait_for_child) [#ifdef HAVE_COMPRESS_EXIT_HACK]: Warn when the compressed data are larger then the original data if the child process command exited with 2 while the compressing command was called 'compress'. * NEWS: Document. --- NEWS | 7 +++ acinclude.m4 | 19 +++ configure.ac | 3 +++ src/system.c | 7 +++ 4 files changed, 36 insertions(+) diff --git a/NEWS b/NEWS index 3108798..863cf52 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,13 @@ version 1.26.90 (Git) * Bug fixes +** The Lempel-Ziv coding compression bugfix (compress) + +Do not exit with fatal error during compressing by the 'compress' +utility (-Z option) when the child process running the compressor exits +with warning exit value 2. It just means that the compressed output is +bigger then original data. Just warn the user now instead of hard fail. + ** Sparse files with large data When creating a PAX-format archive, tar no longer arbitrarily restricts diff --git a/acinclude.m4 b/acinclude.m4 index d48c881..1367eed 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -52,3 +52,22 @@ AC_DEFUN([TAR_HEADERS_ATTR_XATTR_H], ) fi ]) + +# Allow maintainer to disable compress's exit-2-value hack +AC_DEFUN([TAR_COMPRESS_EXIT_HACK],[ + AC_ARG_WITH( +[compress-exit-hack], +AS_HELP_STRING( + [--without-compress-exit-hack], + [fail when -Z (compress) compressor exists with non-error value 2]), +[with_compress_exit_hack=no] + ) + if test x$with_compress_exit_hack != xno; then +AC_DEFINE_UNQUOTED( + HAVE_COMPRESS_EXIT_HACK, + $tar_cv_compressor_compress, + Define to name of compress utility when we don't want to fail hardly + if this utility exits with exit code 2. It should just mean that the + compressed file is bigger than original file.) + fi +]) diff --git a/configure.ac b/configure.ac index 3303c53..b0a4f14 100644 --- a/configure.ac +++ b/configure.ac @@ -253,6 +253,9 @@ TAR_COMPR_PROGRAM(lzma) TAR_COMPR_PROGRAM(lzop) TAR_COMPR_PROGRAM(xz) +# must be called after TAR_COMPR_PROGRAM(compress) call +TAR_COMPRESS_EXIT_HACK + AC_MSG_CHECKING(for default archive format) AC_ARG_VAR([DEFAULT_ARCHIVE_FORMAT], diff --git a/src/system.c b/src/system.c index e1fd263..d722352 100644 --- a/src/system.c +++ b/src/system.c @@ -189,6 +189,13 @@ sys_wait_for_child (pid_t child_pid, bool eof) if (!(!eof sig == SIGPIPE)) FATAL_ERROR ((0, 0, _(Child died with signal %d), sig)); } +#ifdef HAVE_COMPRESS_EXIT_HACK + else if (!strcmp (use_compress_program_option, HAVE_COMPRESS_EXIT_HACK) + WEXITSTATUS (wait_status) == 2) +WARN ((0, 0, _(%lu: Child says that compressed output is larger + then original data.), + (unsigned long) child_pid)); +#endif else if (WEXITSTATUS (wait_status) != 0) FATAL_ERROR ((0, 0, _(Child returned status %d), WEXITSTATUS (wait_status))); -- 1.8.1.2
Re: [Bug-tar] [PATCH] Do not fail when 'compress' is unable to provide sufficient compress ratio
I suggest something simpler: namely, just declare that the -Z (compress) program must conform to the compress API, so that its exit status 2 really means OK. That way, we can change only src/system.c and NEWS. The compress API is obsolete and isn't likely to change so this sounds safe.
Re: [Bug-tar] [PATCH] Do not fail when 'compress' is unable to provide sufficient compress ratio
On Mon, 2013-02-25 at 11:39 -0800, Paul Eggert wrote: I suggest something simpler: namely, just declare that the -Z (compress) program must conform to the compress API, so that its exit status 2 really means OK. That way, we can change only src/system.c and NEWS. The compress API is obsolete and isn't likely to change so this sounds safe. Thanks for comments! Attaching approach with just src/system.c and NEWS edited. Pavel From eab80fb7c5a5500a9d4c20321b4f00e019cb3735 Mon Sep 17 00:00:00 2001 From: Pavel Raiskup prais...@redhat.com Date: Mon, 25 Feb 2013 10:18:15 +0100 Subject: [PATCH] tar: The Lempel-Ziv coding handler bugfix Do not exit with fatal error 2 after compressing tarball by the 'compress' utility (-Z option) when the child process running the compressor exited with just-a-warning exit value 2 (compressed output is larger than original data). * src/system.c (sys_wait_for_child): Warn only when child process exited with 2 and the compressing command was called 'compress'. * NEWS: Document. --- NEWS | 11 +++ src/system.c | 7 +++ 2 files changed, 18 insertions(+) diff --git a/NEWS b/NEWS index 3108798..17df594 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,17 @@ version 1.26.90 (Git) * Bug fixes +** The Lempel-Ziv coding compression bugfix (compress) + +Do not exit with fatal error 2 after the tarball compressing when +the tar's child 'compress' process (-Z option) exited with +just-a-warning exit value 2. It just means that output of the +'compress' utility became bigger than it's original data +(insufficient compress ratio). Tar just warns now under these +circumstances instead of failing hard. Any compressor used by the +'-Z' option must conform to compress API now - thus the exit value +'2' must signify successful exit status. + ** Sparse files with large data When creating a PAX-format archive, tar no longer arbitrarily restricts diff --git a/src/system.c b/src/system.c index e1fd263..10f76a3 100644 --- a/src/system.c +++ b/src/system.c @@ -189,6 +189,13 @@ sys_wait_for_child (pid_t child_pid, bool eof) if (!(!eof sig == SIGPIPE)) FATAL_ERROR ((0, 0, _(Child died with signal %d), sig)); } + /* Any compressor used by the '-Z' option must conform to the 'compress' + API - value 2 is OK exit status (not a fatal error) */ + else if (WEXITSTATUS (wait_status) == 2 + !strcmp (use_compress_program_option, COMPRESS_PROGRAM)) +WARN ((0, 0, _(%lu: Child says that compressed output is larger + than original data.), + (unsigned long) child_pid)); else if (WEXITSTATUS (wait_status) != 0) FATAL_ERROR ((0, 0, _(Child returned status %d), WEXITSTATUS (wait_status))); -- 1.8.1.2
