Subject: Re: bug#19865: tar-untar-buffer: should honor default-directory

From: Ivan Shmakov <[email-masked]>
Date: Sat, 14 Feb 2015 16:27:29 +0000

>>>>> Ivan Shmakov <[email-masked]> writes:

[…]

 > Avoiding switching buffers until around the actual write-region call
 > may still be a better alternative (as that should make mistakes of
 > this kind hardier to introduce), but results in a lengthier patch.
 > Any opinion on which way I should go there?

	Per my reading of the code, with-current-buffer is generally
	used in tar-mode.el around the smallest fragments possible.

	Please thus consider the revised patch MIMEd, which I’ve tried
	to make consistent with such an approach.

	Also to note is that the awareness of the general “data buffer
	default-directory” issue dates back to 2001 at the least
	(considering the excerpt below, for instance), so I don’t seem
	to understand how making tar-untar-buffer consistent with the
	rest of the tar-mode.el code could ever be harmful?

	TIA.

commit e8421604cdd386af0c32fb7cf698882ec6b74015
Author: Gerd Moellmann <[email-masked]>
Date:   2001-08-07 13:36:14 +0000

    (tar-extract): Avoid generating a new buffer
    for each file visited.  From Markus Rost <[email-masked]>.

$ git archive --format=tar  e8421604cdd3 -- tar-mode.el | tar -xO | nl -ba 
…
   749			;; Set the default-directory to the dir of the
   750			;; superior buffer. 
   751			(setq default-directory
   752			      (save-excursion
   753				(set-buffer tar-buffer)
   754				default-directory))
…
$ 

-- 
FSF associate member #7257  np. Meditation — David Modica 3013 B6A0 230E 334A
diff --git a/lisp/tar-mode.el b/lisp/tar-mode.el
index 6c7f755..c6eef01 100644
--- a/lisp/tar-mode.el
+++ b/lisp/tar-mode.el
@@ -531,25 +531,28 @@ defun tar-untar-buffer ()
   "Extract all archive members in the tar-file into the current directory."
   (interactive)
   ;; FIXME: make it work even if we're not in tar-mode.
-  (let ((descriptors tar-parse-info))   ;Read the var in its buffer.
-    (with-current-buffer
-        (if (tar-data-swapped-p) tar-data-buffer (current-buffer))
-      (set-buffer-multibyte nil)          ;Hopefully, a no-op.
-      (dolist (descriptor descriptors)
-        (let* ((name (tar-header-name descriptor))
-               (dir (if (eq (tar-header-link-type descriptor) 5)
-                        name
-                      (file-name-directory name)))
-               (start (tar-header-data-start descriptor))
-               (end (+ start (tar-header-size descriptor))))
-          (unless (file-directory-p name)
-            (message "Extracting %s" name)
-            (if (and dir (not (file-exists-p dir)))
-                (make-directory dir t))
-            (unless (file-directory-p name)
-	      (let ((coding-system-for-write 'no-conversion))
-		(write-region start end name)))
-            (set-file-modes name (tar-header-mode descriptor))))))))
+  (let ((data-buf (if (tar-data-swapped-p) tar-data-buffer (current-buffer))))
+    (with-current-buffer data-buf
+      (set-buffer-multibyte nil))       ; Hopefully, a no-op.
+    (dolist (descriptor tar-parse-info)
+      (let* ((name (tar-header-name descriptor))
+	     (dir (if (eq (tar-header-link-type descriptor) 5)
+		      name
+		    (file-name-directory name)))
+	     (start (tar-header-data-start descriptor))
+	     (end (+ start (tar-header-size descriptor))))
+	(unless (file-directory-p name)
+	  (message "Extracting %s" name)
+	  (if (and dir (not (file-exists-p dir)))
+	      (make-directory dir t))
+	  (unless (file-directory-p name)
+	    (let ((coding-system-for-write 'no-conversion)
+		  ;; Note that default-directory may have a different
+		  ;; value in the data buffer.
+		  (name (expand-file-name name default-directory)))
+	      (with-current-buffer data-buf
+		(write-region start end name))))
+	  (set-file-modes name (tar-header-mode descriptor)))))))
 
 (defun tar-summarize-buffer ()
   "Parse the contents of the tar file in the current buffer."