bug#10347: 24.0.50; archive-mode includes warning messages from unzip in the content of extracted files

bug#10347: 24.0.50; archive-mode includes warning messages from unzip in the content of extracted files

[Daniel Brooks]

[-- Attachment #1: Type: text/plain, Size: 226 bytes --]

archive-extract-by-stdout intermingles stderr and stdout unless the
caller passes in a buffer to send the stderr too. As none of the callers
do, I think it would be best to disable stderr unless the caller
supplies a buffer.


[-- Attachment #2: fix --]
[-- Type: application/octet-stream, Size: 1024 bytes --]

[-- Attachment #3: Type: text/plain, Size: 4926 bytes --]




In GNU Emacs 24.0.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.20.1)
 of 2010-07-23 on celebdil
configured using `configure  '--prefix=/home/db48x''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Group

Minor modes in effect:
  gnus-topic-mode: t
  gnus-undo-mode: t
  erc-track-mode: t
  erc-services-mode: t
  erc-ring-mode: t
  erc-page-mode: t
  erc-networks-mode: t
  erc-netsplit-mode: t
  erc-menu-mode: t
  erc-match-mode: t
  erc-log-mode: t
  erc-list-mode: t
  erc-dcc-mode: t
  erc-pcomplete-mode: t
  erc-capab-identify-mode: t
  erc-button-mode: t
  erc-stamp-mode: t
  erc-autojoin-mode: t
  iswitchb-mode: t
  erc-irccontrols-mode: t
  erc-noncommands-mode: t
  erc-move-to-prompt-mode: t
  erc-readonly-mode: t
  savehist-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
DEL DEL DEL DEL DEL r e p o r TAB RET w a r n i n g 
SPC m e s s a g e s SPC f r o m SPC z i p u DEL / u 
n z i p SPC g e t SPC i n c l u d e d ESC O D ESC O 
D ESC O D ESC O D ESC O D ESC O D ESC O D ESC O D ESC 
O D ESC O D ESC O D ESC O D ESC O D ESC O D ESC O D 
ESC O D ESC O D ESC O D ESC O D ESC O C DEL DEL DEL 
DEL DEL ESC [ 3 ~ C-a C-k a r c h i v e - m o d e SPC 
i n d l u c e DEL DEL d e s SPC w a r n i n g SPC ESC 
O D ESC O D ESC O D ESC O D ESC O D ESC O D ESC O D 
ESC O D ESC O D ESC O D ESC O D ESC O D ESC O D ESC 
O D DEL c C-e m e s s a g e s SPC f r o m SPC u n z 
i p SPC i n SPC t h e SPC c o n t e n t SPC o f SPC 
e x t r a c t e d SPC f i l e s RET ESC O A ESC O B 
C-x C-c q n o RET C-x k RET y e s RET C-x 1 ESC x g 
n u s RET y ESC c ESC x ESC p ESC p RET

Recent messages:
Pinging home mail+nnimap+home mail:INBOX.junk...done
INBOX.junk
Pinging home mail...done
INBOX.junk
Pinging home mail+nnimap+home mail:INBOX.junk...done
INBOX.junk
Pinging home mail...done
INBOX.junk
nnimap: Setting marks in INBOX...done
INBOX

Load-path shadows:
~/elisp/remember-latest/remember hides /home/db48x/share/emacs/24.0.50/lisp/textmodes/remember
~/elisp/xscheme hides /home/db48x/share/emacs/24.0.50/lisp/progmodes/xscheme
/home/db48x/share/emacs/24.0.50/lisp/erc/erc-list hides ~/elisp/erc-extras/erc-list

Features:
(gnus-topic gnus-mlspl utf-7 utf7 nnimap imap parse-time nnfolder
nndraft nnmh bbdb-gnus bbdb-snarf bbdb-com cl cl-19 auth-source netrc
gnus-agent gnus-srvr gnus-score score-mode nnvirtual gnus-msg gnus-cache
gnus-art mm-uu mml2015 epg-config mm-view smime password-cache dig
mailcap gnus-sum spam-stat gnus-demon nntp gnus-group gnus-undo nnmail
mail-source nnoo gnus-start gnus-spec gnus-int gnus-range gnus-win gnus
gnus-ems nnheader shadow gnus-util mail-extr message rfc822 mml mml-sec
mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mailabbrev mail-utils gmm-utils mailheader
emacsbug server add-log jka-compr find-func multi-isearch vc-git package
erc-track erc-services erc-ring erc-page erc-networks erc-netsplit
erc-menu erc-match erc-log erc-dcc erc-pcomplete time-date erc-capab
erc-button erc-fill erc-stamp erc-join iswitchb erc-goodies erc
erc-backend erc-compat format-spec planner-id compile comint ring
planner cal-menu calendar cal-loaddefs sort muse-html muse-xml-common
muse-colors pcomplete muse-publish muse-project muse-protocols mb-depth
two-column bookmark pp info muse-regexps derived muse muse-nested-tags
muse-mode bbdb-autoloads bbdb timezone xscheme scheme icicles
icicles-mode advice help-fns advice-preload dired regexp-opt
icicles-cmd2 easy-mmode icicles-cmd1 cus-edit cus-start cus-load
icicles-mcmd help-mode easymenu view icicles-mac icicles-fn icicles-var
icicles-opt edmacro kmacro ffap wid-edit icicles-face thingatpt hexrgb
savehist tooltip ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd
tool-bar dnd fontset image fringe lisp-mode register page menu-bar
rfn-eshadow timer select scroll-bar mldrag mouse jit-lock font-lock
syntax facemenu font-core frame cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
case-table epa-hook jka-cmpr-hook help simple abbrev loaddefs button
minibuffer faces cus-face files text-properties overlay md5 base64
format env code-pages mule custom widget hashtable-print-readable
backquote make-network-process dbusbind dynamic-setting
system-font-setting font-render-setting gtk x-toolkit x multi-tty emacs)

bug#10347: 24.0.50; archive-mode includes warning messages from unzip in the content of extracted files

[Juri Linkov]

> archive-extract-by-stdout intermingles stderr and stdout unless the
> caller passes in a buffer to send the stderr too. As none of the callers
> do, I think it would be best to disable stderr unless the caller
> supplies a buffer.

> [2. fix --- application/octet-stream; archive-mode-stdout.bundle]...

Unfortunately, I can't read what you've encrypted into this git bundle.

But I think we should use the same solution as we developed for
`archive-7z-extract' - to redirect stdout into a temporary file
and display its contents in the echo area.

This patch fixes `archive-zip-extract' by duplicating the logic of
`archive-7z-extract':

=== modified file 'lisp/arc-mode.el'
--- lisp/arc-mode.el	2011-12-15 07:24:10 +0000
+++ lisp/arc-mode.el	2011-12-22 20:05:51 +0000
@@ -1826,7 +1827,8 @@ (defun archive-zip-extract (archive name
     (let ((archive-7z-extract archive-zip-extract))
       (archive-7z-extract archive name)))
    (t
-    (archive-extract-by-stdout
+    (let ((tmpfile (make-temp-file "zip-stderr")))
+      (prog1 (archive-extract-by-stdout
      archive
      ;; unzip expands wildcards in NAME, so we need to quote it.  But
      ;; not on DOS/Windows, since that fails extraction on those
@@ -1839,7 +1841,13 @@ (defun archive-zip-extract (archive name
 	      (equal (car archive-zip-extract) "unzip"))
 	 (shell-quote-argument name)
        name)
-     archive-zip-extract))))
+	      archive-zip-extract
+	      tmpfile)
+	(with-temp-buffer
+	  (insert-file-contents tmpfile)
+	  (unless (search-forward "Everything is Ok" nil t)
+	    (message "%s" (buffer-string)))
+	  (delete-file tmpfile)))))))
 
 (defun archive-zip-write-file-member (archive descr)
   (archive-*-write-file-member

bug#10347: 24.0.50; archive-mode includes warning messages from unzip in the content of extracted files

[Daniel Brooks]

Juri Linkov <juri@jurta.org> writes:

>> archive-extract-by-stdout intermingles stderr and stdout unless the
>> caller passes in a buffer to send the stderr too. As none of the callers
>> do, I think it would be best to disable stderr unless the caller
>> supplies a buffer.
>
>> [2. fix --- application/octet-stream; archive-mode-stdout.bundle]...
>
> Unfortunately, I can't read what you've encrypted into this git bundle.
>
> But I think we should use the same solution as we developed for
> `archive-7z-extract' - to redirect stdout into a temporary file
> and display its contents in the echo area.
>

It is a binary file, which is unfortunate. Mercurial has a way to export
a commit (or a set of commits) with all of the necessary metadata as a
text file that I really like, but I digress.

> This patch fixes `archive-zip-extract' by duplicating the logic of
> `archive-7z-extract':

Yes, this is a good idea. However, let's avoid duplicating the code by
defining a function. Here's what I've got so far, although it doesn't
actually display the messages, for whatever reason. I'll debug it when I
get a spare minute.

diff --git a/lisp/arc-mode.el b/lisp/arc-mode.el
index 00023a6..442a258 100644
--- a/lisp/arc-mode.el
+++ b/lisp/arc-mode.el
@@ -1113,7 +1113,7 @@ using `make-temp-file', and the generated name is returned."
     (archive-delete-local tmpfile)
     success))
 
-(defun archive-extract-by-stdout (archive name command &optional stderr-file)
+(defun archive-extract-by-stdout-with-stderr (archive name command &optional stderr-file)
   (apply 'call-process
 	 (car command)
 	 nil
@@ -1121,6 +1121,16 @@ using `make-temp-file', and the generated name is returned."
 	 nil
 	 (append (cdr command) (list archive name))))
 
+(defun archive-extract-by-stdout (archive name command)
+  (let ((tmpfile (make-temp-file "zip-stderr")))
+      (prog1 (archive-extract-by-stdout archive name command tmpfile)
+        (with-temp-buffer
+          (insert-file-contents tmpfile)
+          (if (buffer-size)
+              (message "%s" (buffer-string)))
+          (delete-file tmpfile)))))
+
 (defun archive-extract-other-window ()
   "In archive mode, find this member in another window."
   (interactive)
@@ -1826,9 +1836,7 @@ This doesn't recover lost files, it just undoes changes in the buffer itself."
     (let ((archive-7z-extract archive-zip-extract))
       (archive-7z-extract archive name)))
    (t
-    (let ((tmpfile (make-temp-file "zip-stderr")))
-      (prog1 (archive-extract-by-stdout
-     archive
+    (archive-extract-by-stdout archive
      ;; unzip expands wildcards in NAME, so we need to quote it.  But
      ;; not on DOS/Windows, since that fails extraction on those
      ;; systems (unless w32-quote-process-args is nil), and file names
@@ -1840,13 +1848,7 @@ This doesn't recover lost files, it just undoes changes in the buffer itself."
 	      (equal (car archive-zip-extract) "unzip"))
 	 (shell-quote-argument name)
        name)
-              archive-zip-extract
-              tmpfile)
-        (with-temp-buffer
-          (insert-file-contents tmpfile)
-          (unless (search-forward "Everything is Ok" nil t)
-            (message "%s" (buffer-string)))
-          (delete-file tmpfile)))))))
+     archive-zip-extract))))
 
 (defun archive-zip-write-file-member (archive descr)
   (archive-*-write-file-member

bug#10347: 24.0.50; archive-mode includes warning messages from unzip in the content of extracted files

[Juri Linkov]

> Yes, this is a good idea. However, let's avoid duplicating the code by
> defining a function. Here's what I've got so far, although it doesn't
> actually display the messages, for whatever reason.

We can't remove special treatment of the 7z archiver because it produces
large output even on successful runs, so displaying it would be annoying.

I'd rather move the logic of displaying errors to `archive-extract-by-stdout'
where `stderr-test' defines the condition when not to display the output
redirected to stderr (in case of 7z it searches for the known regexp,
otherwise checks the buffer's size).

1. Tested it with "unzip": (setq archive-zip-extract '("unzip" "-qq" "-c"))

1.1. Visiting `omni.jar' displays in the echo area:

  warning [omni.jar]:  6552040 extra bytes at beginning or within zipfile
    (attempting to process anyway)
  error [omni.jar]:  reported length of central directory is
    -6552040 bytes too long (Atari STZip zipfile?  J.H.Holm ZIPSPLIT 1.1
    zipfile?).  Compensating...

1.2. Visiting the hand-edited `browser.jar' displays in the echo area:

  content/browser/aboutDialog.xul  bad CRC 1ded366d  (should be b80ec225)

2. Tested with using "7z": (setq archive-zip-extract '("7z" "x" "-so"))

2.1. Visiting `omni.jar' displays in the echo area:

  Processing archive: omni.jar
  Error: Can not open file as archive

2.2. Visiting the hand-edited `browser.jar' displays in the echo area:

  Processing archive: browser.jar
  Extracting  content/browser/aboutDialog.xul     CRC Failed
  Sub items Errors: 1

This all errors are reported to the user and easyly noticeable.

=== modified file 'lisp/arc-mode.el'
--- lisp/arc-mode.el	2011-12-15 07:24:10 +0000
+++ lisp/arc-mode.el	2011-12-25 21:27:01 +0000
@@ -1113,13 +1114,24 @@ (defun archive-*-extract (archive name c
     (archive-delete-local tmpfile)
     success))
 
-(defun archive-extract-by-stdout (archive name command &optional stderr-file)
-  (apply 'call-process
-	 (car command)
-	 nil
-	 (if stderr-file (list t stderr-file) t)
-	 nil
-	 (append (cdr command) (list archive name))))
+(defun archive-extract-by-stdout (archive name command &optional stderr-test)
+  (let ((stderr-file (make-temp-file "arc-stderr")))
+    (unwind-protect
+	(prog1
+	    (apply 'call-process
+		   (car command)
+		   nil
+		   (if stderr-file (list t stderr-file) t)
+		   nil
+		   (append (cdr command) (list archive name)))
+	  (with-temp-buffer
+	    (insert-file-contents stderr-file)
+	    (when (if (stringp stderr-test)
+		      (not (search-forward stderr-test nil t))
+		    (> (buffer-size) 0))
+	      (message "%s" (buffer-string)))))
+      (if (file-exists-p stderr-file)
+	  (delete-file stderr-file)))))
 
 (defun archive-extract-other-window ()
   "In archive mode, find this member in another window."
@@ -2095,17 +2125,11 @@ (defun archive-7z-summarize ()
       (apply 'vector files))))
 
 (defun archive-7z-extract (archive name)
-  (let ((tmpfile (make-temp-file "7z-stderr")))
-    ;; 7z doesn't provide a `quiet' option to suppress non-essential
-    ;; stderr messages.  So redirect stderr to a temp file and display it
-    ;; in the echo area when it contains error messages.
-    (prog1 (archive-extract-by-stdout
-	    archive name archive-7z-extract tmpfile)
-      (with-temp-buffer
-	(insert-file-contents tmpfile)
-	(unless (search-forward "Everything is Ok" nil t)
-	  (message "%s" (buffer-string)))
-	(delete-file tmpfile)))))
+  ;; 7z doesn't provide a `quiet' option to suppress non-essential
+  ;; stderr messages.  So redirect stderr to a temp file and display it
+  ;; in the echo area when it contains no message indicating success.
+  (archive-extract-by-stdout
+   archive name archive-7z-extract "Everything is Ok"))
 
 (defun archive-7z-write-file-member (archive descr)
   (archive-*-write-file-member

bug#10347: 24.0.50; archive-mode includes warning messages from unzip in the content of extracted files

[Juri Linkov]

Actually, `archive-rar-extract' has exactly the same problem.
When `unrar-free' can't extract some files from RAR archives,
it displays a confusing message:

  Removing old name: no such file or directory, /tmp/arc-rar1971TAO/file.txt

It needs the same treatment as 7z and zip.  But the difference is that
`unrar-free' outputs only to stdout, not to stderr.  And when the operation
is successful, it outputs long text with the known fixed message
"All OK" indicating success.  So we should check for this message,
and not display a message on success.

Here is a patch that adds `archive-extract-by-file' for RAR archives:

=== modified file 'lisp/arc-mode.el'
--- lisp/arc-mode.el	2011-12-15 07:24:10 +0000
+++ lisp/arc-mode.el	2011-12-25 21:27:01 +0000
@@ -1996,23 +2008,41 @@ (defun archive-rar-summarize (&optional
       (insert sep (make-string maxname ?-) "\n")
       (apply 'vector files))))
 
+(defun archive-extract-by-file (archive name command &optional stdout-test)
+  (let ((dest (make-temp-file "arc-dir" 'dir))
+	(stdout-file (make-temp-file "arc-stdout")))
+    (unwind-protect
+	(prog1
+	    (apply 'call-process
+		   (car command)
+		   nil
+		   `(:file ,stdout-file)
+		   nil
+		   (append (cdr command) (list archive name dest)))
+	  (with-temp-buffer
+	    (insert-file-contents stdout-file)
+	    (when (if (stringp stdout-test)
+		      (not (search-forward stdout-test nil t))
+		    (> (buffer-size) 0))
+	      (message "%s" (buffer-string))))
+	  (if (file-exists-p (expand-file-name name dest))
+	      (insert-file-contents-literally (expand-file-name name dest))))
+      (if (file-exists-p stdout-file)
+	  (delete-file stdout-file))
+      (if (file-exists-p (expand-file-name name dest))
+	  (delete-file (expand-file-name name dest)))
+      (while (file-name-directory name)
+	(setq name (directory-file-name (file-name-directory name)))
+	(delete-directory (expand-file-name name dest)))
+      (delete-directory dest))))
+
 (defun archive-rar-extract (archive name)
   ;; unrar-free seems to have no way to extract to stdout or even to a file.
   (if (file-name-absolute-p name)
       ;; The code below assumes the name is relative and may do undesirable
       ;; things otherwise.
       (error "Can't extract files with non-relative names")
-    (let ((dest (make-temp-file "arc-rar" 'dir)))
-      (unwind-protect
-          (progn
-            (call-process "unrar-free" nil nil nil
-                          "--extract" archive name dest)
-            (insert-file-contents-literally (expand-file-name name dest)))
-        (delete-file (expand-file-name name dest))
-        (while (file-name-directory name)
-          (setq name (directory-file-name (file-name-directory name)))
-          (delete-directory (expand-file-name name dest)))
-        (delete-directory dest)))))
+    (archive-extract-by-file archive name '("unrar-free" "--extract") "All OK")))
 
 ;;; Section: Rar self-extracting .exe archives.
 

bug#10347: 24.0.50; archive-mode includes warning messages from unzip in the content of extracted files

[Juri Linkov]

Version: 24.3

Thanks, this is fixed now and closed.