unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
@ 2021-03-10 20:26 Jean Louis
  2021-08-31 21:33 ` Marco Centurion
  0 siblings, 1 reply; 28+ messages in thread
From: Jean Louis @ 2021-03-10 20:26 UTC (permalink / raw)
  To: 47058


This is the error:

Debugger entered--Lisp error: (file-missing "Reading directory" "No such file or directory" "CrossLine_linux_x86")
  access-file("CrossLine_linux_x86" "Reading directory")
  insert-directory("CrossLine_linux_x86" "--dired -al -d" nil nil)
  dired-insert-directory("/home/data1/protected/Downloads/" "-al -d" ("CrossLine_linux_x86"))
  dired-add-entry("/home/data1/protected/Downloads/CrossLine_linux_x8..." nil t)
  dired-update-file-line("/home/data1/protected/Downloads/CrossLine_linux_x8...")
  dired-compress()
  dired-map-over-marks-check(dired-compress nil compress t)
  dired-do-compress(nil)
  funcall-interactively(dired-do-compress nil)
  call-interactively(dired-do-compress nil nil)
  command-execute(dired-do-compress)

The error takes place when this file:
http://software.rochus-keller.info/CrossLine_linux_x86.tar.gz is
downloaded and in dired pressed Z:

insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86

In my opinion, this file contains just one file, not directory, and
maybe uncompressing feature is looking for directory, but it should
not. Uncompress should work without error even with single files in the
compressed package.



In GNU Emacs 28.0.50 (build 8, x86_64-pc-linux-gnu, X toolkit, cairo version 1.14.8, Xaw3d scroll bars)
 of 2021-03-07 built on protected.rcdrun.com
Repository revision: 468bb5ab7f949441f68c4133fcd5292dfbbfd83d
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.11907000
System Description: Hyperbola GNU/Linux-libre

Configured using:
 'configure --with-x-toolkit=lucid
 PKG_CONFIG_PATH=/home/data1/protected/GNUstep/Library/Libraries/pkgconfig:/usr/lib/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY PDUMPER PNG
RSVG SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XAW3D XDBE XIM XPM LUCID
ZLIB

Important settings:
  value of $LC_ALL: en_US.UTF-8
  value of $LANG: de_DE.UTF-8
  value of $XMODIFIERS: @im=exwm-xim
  locale-coding-system: utf-8-unix

Major mode: Dired by name

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort hashcash mail-extr emacsbug message rmc puny rfc822 mml
mml-sec epa derived epg epg-config gnus-util rmail rmail-loaddefs
auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json map text-property-search time-date subr-x seq
byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils help-fns radix-tree
cl-print debug backtrace help-mode find-func cus-start cus-load misearch
multi-isearch dired-aux cl-loaddefs cl-lib dired dired-loaddefs
iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core term/tty-colors frame minibuffer
cl-generic cham georgian utf-8-lang misc-lang vietnamese tibetan thai
tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian
slovak czech european ethiopic indian cyrillic chinese composite
charscript charprop case-table epa-hook jka-cmpr-hook help simple abbrev
obarray cl-preloaded nadvice button loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget hashtable-print-readable backquote threads
dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting cairo x-toolkit x multi-tty make-network-process
emacs)

Memory information:
((conses 16 74489 7983)
 (symbols 48 8670 1)
 (strings 32 24240 1273)
 (string-bytes 1 737012)
 (vectors 16 13446)
 (vector-slots 8 181429 12518)
 (floats 8 30 42)
 (intervals 56 1527 0)
 (buffers 992 14))

-- 
Thanks,
Jean Louis
⎔ λ 🄯 𝍄 𝌡 𝌚





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-03-10 20:26 bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86 Jean Louis
@ 2021-08-31 21:33 ` Marco Centurion
  2021-08-31 22:12   ` Arthur Miller
  0 siblings, 1 reply; 28+ messages in thread
From: Marco Centurion @ 2021-08-31 21:33 UTC (permalink / raw)
  To: 47058

I can confirm that this bug is present and pretty easy to reproduce.
Steps to reproduce:

----
$ touch test1
$ tar czf test.tar.gz test1
$ rm test1

In dired, press Z (`dired-do-compress`) when the point is on test.tar.gz
----

The error doesn't seem to be only because there's only a file inside the
compressed one, but also that the name of the decompressed file doesn't
coincide with the compressed one.  If one compresses and decompresses a
single file, dired works exactly as expected.

Also, when decompressing a `tar.gz` file, the dired buffer doesn't show
the tar.gz after pressing Z, which seems to imply that the original file
was deleted, but that's not the case.

Another case that I found is that if there's more than a single file
inside the compressed one, dired doesn't show the extracted files until
`revert-buffer` is invoked:

----
$ touch test test2
$ tar czf test.tar.gz test test2
$ rm test test2

In dired, press Z (`dired-do-compress`) when the point is on test.tar.gz

The expected result is a dired buffer that shows all extracted files,
but it only shows test
----

I'm not really sure how these problems could be fixed without some
pretty significant changes in how files are decompressed.

-- 
Marco Centurion
Unidad de Recursos Informáticos
Facultad de Ingeniería - UdelaR





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-08-31 21:33 ` Marco Centurion
@ 2021-08-31 22:12   ` Arthur Miller
  2021-09-01 13:59     ` Marco Centurion - URI
  0 siblings, 1 reply; 28+ messages in thread
From: Arthur Miller @ 2021-08-31 22:12 UTC (permalink / raw)
  To: Marco Centurion; +Cc: 47058

Marco Centurion <mcenturion@fing.edu.uy> writes:

> I can confirm that this bug is present and pretty easy to reproduce.
> Steps to reproduce:
>
> ----
> $ touch test1
> $ tar czf test.tar.gz test1
> $ rm test1
>
> In dired, press Z (`dired-do-compress`) when the point is on test.tar.gz
> ----
I just did all those steps in emacs -Q, and I can not confirm any errors. I
named files exactly as you show there, and decompressed file is correctly named
'test1'.

I tested in two different directories, you three shell commands from terminal
(st in my case), and Z from dired created correctly test1.

I also test M-! dance from Emacs, and even in that case everythign worked
correctly.

> The error doesn't seem to be only because there's only a file inside the
> compressed one, but also that the name of the decompressed file doesn't
> coincide with the compressed one.  If one compresses and decompresses a
> single file, dired works exactly as expected.
>
> Also, when decompressing a `tar.gz` file, the dired buffer doesn't show
> the tar.gz after pressing Z, which seems to imply that the original file
> was deleted, but that's not the case.

Press 'g'.

Observe that, if you do it while dired buffer is open, you have to manually
revert buffer after some operations ('g' key). Not all operations trigger
revert. I don't know exactly which ones do and which ones do not. This especialy
case if you update directory outside of Emacs.






^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-08-31 22:12   ` Arthur Miller
@ 2021-09-01 13:59     ` Marco Centurion - URI
  2021-09-01 16:06       ` Arthur Miller
  2021-09-20 22:12       ` Michalis V.
  0 siblings, 2 replies; 28+ messages in thread
From: Marco Centurion - URI @ 2021-09-01 13:59 UTC (permalink / raw)
  To: Arthur Miller; +Cc: 47058

Arthur Miller <arthur.miller@live.com> writes:

> Marco Centurion <mcenturion@fing.edu.uy> writes:
>
>> I can confirm that this bug is present and pretty easy to reproduce.
>> Steps to reproduce:
>>
>> ----
>> $ touch test1
>> $ tar czf test.tar.gz test1
>> $ rm test1
>>
>> In dired, press Z (`dired-do-compress`) when the point is on test.tar.gz
>> ----
> I just did all those steps in emacs -Q, and I can not confirm any errors. I
> named files exactly as you show there, and decompressed file is correctly named
> 'test1'.
>
> I tested in two different directories, you three shell commands from terminal
> (st in my case), and Z from dired created correctly test1.
>

Yes, the file is correctly decompressed.  The original report is about
an error message that shows up in the minibuffer "Reading directory: No
such file or directory, test".  And that's what I was able to reproduce
without having to download the file linked in the report, something that
I guess a lot of people wouldn't want to do.

> Press 'g'.

I do that, I just thought that it's a bit of a weird behaviour that
sometimes after pressing Z the dired buffer is consistent with what's
actually in the directory and sometimes it's not.  That is all.
Personally I wouldn't be opposed to reverting the buffer after every
operation that modifies files, but I'm sure most would and with good
reason.

-- 
Marco Centurion
Unidad de Recursos Informáticos
Facultad de Ingeniería - UdelaR





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-01 13:59     ` Marco Centurion - URI
@ 2021-09-01 16:06       ` Arthur Miller
  2021-09-20 22:12       ` Michalis V.
  1 sibling, 0 replies; 28+ messages in thread
From: Arthur Miller @ 2021-09-01 16:06 UTC (permalink / raw)
  To: Marco Centurion - URI; +Cc: 47058

Marco Centurion - URI <mcenturion@fing.edu.uy> writes:

> Arthur Miller <arthur.miller@live.com> writes:
>
>> Marco Centurion <mcenturion@fing.edu.uy> writes:
>>
>>> I can confirm that this bug is present and pretty easy to reproduce.
>>> Steps to reproduce:
>>>
>>> ----
>>> $ touch test1
>>> $ tar czf test.tar.gz test1
>>> $ rm test1
>>>
>>> In dired, press Z (`dired-do-compress`) when the point is on test.tar.gz
>>> ----
>> I just did all those steps in emacs -Q, and I can not confirm any errors. I
>> named files exactly as you show there, and decompressed file is correctly named
>> 'test1'.
>>
>> I tested in two different directories, you three shell commands from terminal
>> (st in my case), and Z from dired created correctly test1.
>>
Ahh, sorry, I saw just the mail I answered and thought the bug was about that.

> Yes, the file is correctly decompressed.  The original report is about
> an error message that shows up in the minibuffer "Reading directory: No
> such file or directory, test".  And that's what I was able to reproduce
> without having to download the file linked in the report, something that
> I guess a lot of people wouldn't want to do.
>
>> Press 'g'.
>
> I do that, I just thought that it's a bit of a weird behaviour that
> sometimes after pressing Z the dired buffer is consistent with what's
> actually in the directory and sometimes it's not.  That is all.
> Personally I wouldn't be opposed to reverting the buffer after every
> operation that modifies files, but I'm sure most would and with good
> reason.
Yes, it would be more consistent if all changes were reflected immidiately. I
have global-auto-revert-mode on, so it helps a bit, but I still have to
sometimes press 'g' to refresh the dired view.





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-01 13:59     ` Marco Centurion - URI
  2021-09-01 16:06       ` Arthur Miller
@ 2021-09-20 22:12       ` Michalis V.
  2021-09-21  4:32         ` Lars Ingebrigtsen
  2021-09-21  8:25         ` Michael Albinus
  1 sibling, 2 replies; 28+ messages in thread
From: Michalis V. @ 2021-09-20 22:12 UTC (permalink / raw)
  To: Marco Centurion - URI; +Cc: 47058, Arthur Miller

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

Marco Centurion - URI <mcenturion@fing.edu.uy> writes:

> Arthur Miller <arthur.miller@live.com> writes:
>
>> Marco Centurion <mcenturion@fing.edu.uy> writes:
>>
>>> I can confirm that this bug is present and pretty easy to reproduce.
>>> Steps to reproduce:
>>>
>>> ----
>>> $ touch test1
>>> $ tar czf test.tar.gz test1
>>> $ rm test1
>>>
>>> In dired, press Z (`dired-do-compress`) when the point is on test.tar.gz
>>> ----
>> I just did all those steps in emacs -Q, and I can not confirm any errors. I
>> named files exactly as you show there, and decompressed file is correctly named
>> 'test1'.
>>
>> I tested in two different directories, you three shell commands from terminal
>> (st in my case), and Z from dired created correctly test1.
>>
>
> Yes, the file is correctly decompressed.  The original report is about
> an error message that shows up in the minibuffer "Reading directory: No
> such file or directory, test".  And that's what I was able to reproduce
> without having to download the file linked in the report, something that
> I guess a lot of people wouldn't want to do.
>
>> Press 'g'.
>
> I do that, I just thought that it's a bit of a weird behaviour that
> sometimes after pressing Z the dired buffer is consistent with what's
> actually in the directory and sometimes it's not.  That is all.
> Personally I wouldn't be opposed to reverting the buffer after every
> operation that modifies files, but I'm sure most would and with good
> reason.


hi,

i've had a look into this some weeks ago and only just now managed to
assemble something presentable. As Marco correctly noted, the
problem is not with just a single file - any .tar.[gz|xz|zst|bz2] that
contains just files (no directories) will give the reported error.
The culprit is what 'dired-compress' expects: it assumes that the
uncompression result will produce just a single file or directory.

For example a test-file.gz will produce test-file, emacs-27.2.tar.gz
will give emacs-27.2/ etc. A special case is with formats that support
output directory like zip: boing.zip will be extracted to boing/
no-questions-asked. But for tars like this:

for i in a b c;do touch $i;done
tar cvzf abc.tar.gz a b c

doing Z on abc.tar.gz causes 'dired-compress' to expect abc but the
result is 3 new files a b c (and thus the error is shown). I tried to
think of some way to fix this and i've ended up with the attached
patch. What it does is to basically ask the user where to extract the
contents of the archive (even for zip files, so that the Z behavior is
somehow similar). To make this work for tar files i've added the -C
parameter in 'dired-compress-file-suffixes/. I've also added a missing
.tar.bz2 suffix (until now .tar.bz2 would just be decompressed & not
extracted).

The main work is done on a new function 'dired-uncompress-file' which
contains part of the code of 'dired-compress-file' (which handles both
compress/uncompress actions).
I thought it would be cleaner to have separate functions for these two
(and perhaps the latter function should be renamed to something better)
i've also added some new tests for .tar.gz and .zip formats.

the big downside of this patch is that it adds another prompt when
pressing Z: User must now enter the extraction directory (for file
/tmp/test.tar.gz the suggested default will be /tmp/test/). And that
behavior change might step on some people's toes so i'm a bit reserved
if this is the correct approach to solving the particular problem.

finally there's a corner case that is not solved: in the above scenario
with abc.tar.gz when you uncompress you still need to hit 'g' to refresh
the dired buffer (but there's no error anymore so at least that's
something). A fix for this would require some refactoring on what
'dired-compress' expects, perhaps make it expect a list of
files/directories and not just a single one, plus some more thinking into
the 'dired-compress-file' compression part.

thanks,
Michalis


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: dired-aux.patch --]
[-- Type: text/x-diff, Size: 7420 bytes --]

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index c728642917..2d4269daed 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -1134,9 +1134,10 @@ dired-compress-file-suffixes
     ;; "tar -zxf" isn't used because it's not available on the
     ;; Solaris 10 version of tar (obsolete in 2024?).
     ;; Same thing on AIX 7.1 (obsolete 2023?) and 7.2 (obsolete 2022?).
-    ("\\.tar\\.gz\\'" "" "gzip -dc %i | tar -xf -")
-    ("\\.tar\\.xz\\'" "" "xz -dc %i | tar -xf -")
-    ("\\.tgz\\'" "" "gzip -dc %i | tar -xf -")
+    ("\\.tar\\.gz\\'" "" "gzip -dc %i | tar -xf - -C %c")
+    ("\\.tar\\.xz\\'" "" "xz -dc %i | tar -xf - -C %c")
+    ("\\.tgz\\'" "" "gzip -dc %i | tar -xf - -C %c")
+    ("\\.tar\\.bz2\\'" "" "bunzip2 -c %i | tar -xf - -C %c")
     ("\\.gz\\'" "" "gzip -d")
     ("\\.lz\\'" "" "lzip -d")
     ("\\.Z\\'" "" "uncompress")
@@ -1148,8 +1149,8 @@ dired-compress-file-suffixes
     ("\\.bz2\\'" "" "bunzip2")
     ("\\.xz\\'" "" "unxz")
     ("\\.zip\\'" "" "unzip -o -d %o %i")
-    ("\\.tar\\.zst\\'" "" "unzstd -c %i | tar -xf -")
-    ("\\.tzst\\'" "" "unzstd -c %i | tar -xf -")
+    ("\\.tar\\.zst\\'" "" "unzstd -c %i | tar -xf - -C %c")
+    ("\\.tzst\\'" "" "unzstd -c %i | tar -xf - -C %c")
     ("\\.zst\\'" "" "unzstd --rm")
     ("\\.7z\\'" "" "7z x -aoa -o%o %i")
     ;; This item controls naming for compression.
@@ -1253,6 +1254,42 @@ dired-do-compress-to
                       (length in-files)
                       (file-name-nondirectory out-file)))))))
 
+;;;###autoload
+(defun dired-uncompress-file (file dirname command)
+  "Uncompress FILE using COMMAND. If file is a tar archive or some other
+format that supports output directory in its parameters, ask user the
+target directory to extract it (defaults to DIRNAME). Returns the
+directory or filename produced after the uncompress operation."
+  (if (string-match "%[ioc]" command)
+      (let ((extractdir (expand-file-name
+                         (read-file-name
+                          (format "Extract file to (default %s): " dirname)
+                          dirname))))
+        (prog1 (file-name-as-directory extractdir)
+          (when (not (file-directory-p extractdir))
+            (dired-create-directory extractdir))
+          (dired-shell-command
+           (replace-regexp-in-string
+            "%[oc]" (shell-quote-argument extractdir)
+            (replace-regexp-in-string
+             "%i" (shell-quote-argument file)
+             command
+             nil t)
+            nil t))))
+    ;; We found an uncompression rule without output dir argument
+    (let ((match (string-search " " command))
+          (msg (concat "Uncompressing " file)))
+      (unless (if match
+                  (dired-check-process
+                   msg
+                   (substring command 0 match)
+                   (substring command (1+ match))
+                   file)
+                (dired-check-process msg
+                                     command
+                                     file))
+        dirname))))
+
 ;;;###autoload
 (defun dired-compress-file (file)
   "Compress or uncompress FILE.
@@ -1277,28 +1314,7 @@ dired-compress-file
           ((file-symlink-p file)
            nil)
           ((and suffix (setq command (nth 2 suffix)))
-           (if (string-match "%[io]" command)
-               (prog1 (setq newname (file-name-as-directory newname))
-                 (dired-shell-command
-                  (replace-regexp-in-string
-                   "%o" (shell-quote-argument newname)
-                   (replace-regexp-in-string
-                    "%i" (shell-quote-argument file)
-                    command
-                    nil t)
-                   nil t)))
-             ;; We found an uncompression rule.
-             (let ((match (string-search " " command))
-                   (msg (concat "Uncompressing " file)))
-               (unless (if match
-                           (dired-check-process msg
-                                                (substring command 0 match)
-                                                (substring command (1+ match))
-                                                file)
-                         (dired-check-process msg
-                                              command
-                                              file))
-                 newname))))
+           (dired-uncompress-file file newname command))
           (t
            ;; We don't recognize the file as compressed, so compress it.
            ;; Try gzip; if we don't have that, use compress.
diff --git a/test/lisp/dired-aux-tests.el b/test/lisp/dired-aux-tests.el
index 7f1743f88d..5888f4cd99 100644
--- a/test/lisp/dired-aux-tests.el
+++ b/test/lisp/dired-aux-tests.el
@@ -158,5 +158,59 @@ dired-test-highlight-metachar
     (should (string-match (regexp-quote command) (nth 0 lines)))
     (dired-test--check-highlighting (nth 0 lines) '(8))))
 
+(ert-deftest dired-test-bug47058-tar ()
+  "test for https://debbugs.gnu.org/47058 ."
+  (dired-test-bug47058-fn "tar -cf - %i | gzip -c9 > %o"
+                           "gzip -dc %i | tar -xf - -C %c"
+                           ".tar.gz"))
+
+(ert-deftest dired-test-bug47058-zip ()
+  "test for https://debbugs.gnu.org/47058 ."
+  (dired-test-bug47058-fn "zip %o -r --filesync %i"
+                           "unzip -o -d %o %i"
+                           ".zip"))
+
+(defun dired-test-bug47058-fn (compress-cmd uncompress-cmd extension)
+  "helper fn for testing https://debbugs.gnu.org/47058 ."
+  (let* ((base-file (make-temp-file "dired-test-47058-"))
+         (archive-file (concat base-file extension))
+         (file1 (make-temp-file "a"))
+         (file2 (make-temp-file "b"))
+         (file3 (make-temp-file "c"))
+         (filelist (list file1 file2 file3))
+         (comprcmd (replace-regexp-in-string
+                  "%c" (shell-quote-argument temporary-file-directory)
+                  (replace-regexp-in-string
+                   "%i" (mapconcat 'identity filelist " ")
+                   (replace-regexp-in-string
+                    "%o" (shell-quote-argument archive-file)
+                    compress-cmd)))))
+    (cl-letf (((symbol-function 'read-file-name)
+               (lambda (&rest _) base-file)))
+      (dired-delete-file base-file)
+      (should-not (file-exists-p base-file))
+      (should-not (file-exists-p archive-file))
+      (dired-shell-command comprcmd)
+      (should (file-exists-p archive-file))
+      (mapcar (lambda (f) (should (file-exists-p f)))
+              filelist)
+      (mapcar (lambda (f) (delete-file f))
+              filelist)
+      (mapcar (lambda (f) (should-not (file-exists-p f)))
+              filelist)
+      (should (string-equal
+               (dired-uncompress-file archive-file
+                                      base-file
+                                      uncompress-cmd)
+               (file-name-as-directory base-file)))
+      (mapcar (lambda (f)
+                (should (file-exists-p
+                         (concat (file-name-as-directory base-file) f))))
+              filelist)
+      (dired-delete-file base-file 'always' nil)
+      (dired-delete-file archive-file 'always' nil)
+      (should-not (file-exists-p base-file))
+      (should-not (file-exists-p archive-file)))))
+
 (provide 'dired-aux-tests)
 ;; dired-aux-tests.el ends here

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-20 22:12       ` Michalis V.
@ 2021-09-21  4:32         ` Lars Ingebrigtsen
  2021-09-21  8:24           ` Eli Zaretskii
  2021-09-21  8:25         ` Michael Albinus
  1 sibling, 1 reply; 28+ messages in thread
From: Lars Ingebrigtsen @ 2021-09-21  4:32 UTC (permalink / raw)
  To: Michalis V.; +Cc: Marco Centurion - URI, 47058, Arthur Miller

"Michalis V." <mvar.40k@gmail.com> writes:

> the big downside of this patch is that it adds another prompt when
> pressing Z: User must now enter the extraction directory (for file
> /tmp/test.tar.gz the suggested default will be /tmp/test/). And that
> behavior change might step on some people's toes so i'm a bit reserved
> if this is the correct approach to solving the particular problem.

I think the new behaviour makes sense -- uncompressing the way we've
been doing (to the current dir) is pretty dangerous (because the
archives can overwrite files).  So I've installed your patch in Emacs 28
(with some trivial whitespace changes).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-21  4:32         ` Lars Ingebrigtsen
@ 2021-09-21  8:24           ` Eli Zaretskii
  2021-09-21  9:18             ` Michalis V.
  2021-09-21 17:07             ` Lars Ingebrigtsen
  0 siblings, 2 replies; 28+ messages in thread
From: Eli Zaretskii @ 2021-09-21  8:24 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: mvar.40k, mcenturion, 47058, arthur.miller

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Tue, 21 Sep 2021 06:32:42 +0200
> Cc: Marco Centurion - URI <mcenturion@fing.edu.uy>, 47058@debbugs.gnu.org,
>  Arthur Miller <arthur.miller@live.com>
> 
> "Michalis V." <mvar.40k@gmail.com> writes:
> 
> > the big downside of this patch is that it adds another prompt when
> > pressing Z: User must now enter the extraction directory (for file
> > /tmp/test.tar.gz the suggested default will be /tmp/test/). And that
> > behavior change might step on some people's toes so i'm a bit reserved
> > if this is the correct approach to solving the particular problem.
> 
> I think the new behaviour makes sense -- uncompressing the way we've
> been doing (to the current dir) is pretty dangerous (because the
> archives can overwrite files).  So I've installed your patch in Emacs 28
> (with some trivial whitespace changes).

I disagree that this is the right solution.  It solves the scenario of
the original report, but at a price of introducing an annoying
regression and backward-incompatible behavior in other, more important
use cases.

Let me take an important example: the Emacs release tarballs.  The
tarball's file name is emacs-X.Y.tar.gz, and the file names inside
have a leading directory of "emacs-X.Y/".  The name of the archive is
not important -- you can rename it at will, and the files are still
supposed to unpack into a sub-directory called "emacs-X.Y" under the
directory where you invoke the unpacking command.

This change will now cause the files by default to be unpacked into
emacs-X.Y/emacs-X.Y/, which is not our intent when we produce the
tarball.  (Yes, the user can override the default, but since the
default is identical to the correct directory name, many users will
not understand that they will get the files inside a subdirectory of
the directory they are prompted for, and will accept the default, to
their cost.)

This default is what the MS-Windows Explorer does when extracting
files from an archive.  It makes people inadvertently extract files
into a directory different from what the archive producer intended.
It is in particular nasty when unpacking binary distributions, which
are supposed to put files into the standard tree starting at "/usr" or
"/usr/local".  It is sad to see this silly, if not dangerous, default
seep into Emacs.

The original report is a rare and obscure use case: a tarball without
a leading directory.  Such tarballs should be avoided; they are not
well-formed tarballs.  But the use cases into which the "fix"
introduces annoying and incompatible behavior are much more important,
as this affects uncompressing every tarball out there.  So on balance,
I'd say it is a regression.  We should definitely not make this an
unconditional behavior change.

The root cause of this mess is that 'Z' was designed as a toggle
command, to support unpacking archives that were compressed by 'Z'
itself.  So it assumes that the files in the directory have the same
leading directory as the name of the archive, but that does not have
to be the case for archives created outside Dired.

So I think the correct solution for this bug is to catch the error of
the missing directory, and instead present the user with an
informative message saying that the files were extracted into a
directory such-and-such.  Because otherwise 'Z' in the OP's use case
did TRT: it unpacks the file into the current directory, as instructed
by the archive, the only problem is the error it signals.  That
archives should not generally have file names without leading
directories is not our problem; using 'tar' or some other unpacking
command would produce the same result, and there's no reason Emacs
should work differently.  In fact, one could argue that Emacs should
work the same, to be consistent with those other methods of unpacking.

I don't think we should have this "solution" in Emacs 28.





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-20 22:12       ` Michalis V.
  2021-09-21  4:32         ` Lars Ingebrigtsen
@ 2021-09-21  8:25         ` Michael Albinus
  2021-09-21  9:24           ` Michalis V.
  1 sibling, 1 reply; 28+ messages in thread
From: Michael Albinus @ 2021-09-21  8:25 UTC (permalink / raw)
  To: Michalis V.; +Cc: Marco Centurion - URI, 47058, Arthur Miller, tino.calancha

"Michalis V." <mvar.40k@gmail.com> writes:

> hi,

Hi,

> The main work is done on a new function 'dired-uncompress-file' which
> contains part of the code of 'dired-compress-file' (which handles both
> compress/uncompress actions).
> I thought it would be cleaner to have separate functions for these two
> (and perhaps the latter function should be renamed to something better)
> i've also added some new tests for .tar.gz and .zip formats.

dired-compress-file is also handled by remote file name handlers like
Tramp. How does this fit?

And note, that these days Tino Calancha is working in this area, see bug#50581.

> thanks,
> Michalis

Best regards, Michael.





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-21  8:24           ` Eli Zaretskii
@ 2021-09-21  9:18             ` Michalis V.
  2021-09-21  9:32               ` Eli Zaretskii
  2021-09-21 17:07             ` Lars Ingebrigtsen
  1 sibling, 1 reply; 28+ messages in thread
From: Michalis V. @ 2021-09-21  9:18 UTC (permalink / raw)
  To: Eli Zaretskii
  Cc: mvar.40k, Lars Ingebrigtsen, mcenturion, arthur.miller, 47058

Eli Zaretskii <eliz@gnu.org> writes:

> So I think the correct solution for this bug is to catch the error of
> the missing directory, and instead present the user with an
> informative message saying that the files were extracted into a
> directory such-and-such.  Because otherwise 'Z' in the OP's use case
> did TRT: it unpacks the file into the current directory, as instructed
> by the archive, the only problem is the error it signals.  That
> archives should not generally have file names without leading
> directories is not our problem; using 'tar' or some other unpacking
> command would produce the same result, and there's no reason Emacs
> should work differently.  In fact, one could argue that Emacs should
> work the same, to be consistent with those other methods of unpacking.
>
> I don't think we should have this "solution" in Emacs 28.


hi Eli,

thanks for the feedback. I agree that changing the behavior of the
command just to solve this corner case is not ideal. Actually i'm
neither for nor against merging this patch. It was more of an example
solution to trigger some feedback (and i got a first glimpse of ert :)

Btw one of the reasons i went with this approach and included -C
parameter for tars were some security concerns expressed in #25611. 

There's also a suggestion in the discussion there that Z should just
decompress and not untar the archive and the un-tarring should be a
separate action/procedure. That would be a drastic solution to this
problem but on the other hand it would make sense semantically
(extract != decompress). What is your opinion on this?

in any case i'll try to assemble another patch based on your suggestion.

thank you,
Michalis





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-21  8:25         ` Michael Albinus
@ 2021-09-21  9:24           ` Michalis V.
  2021-09-21 12:06             ` Michael Albinus
  0 siblings, 1 reply; 28+ messages in thread
From: Michalis V. @ 2021-09-21  9:24 UTC (permalink / raw)
  To: Michael Albinus
  Cc: Michalis V., Marco Centurion - URI, 47058, Arthur Miller,
	tino.calancha

Michael Albinus <michael.albinus@gmx.de> writes:

> Hi,

hi Michael,

> dired-compress-file is also handled by remote file name handlers like
> Tramp. How does this fit?

i stayed away from the handler part, basically i was scratching my head
trying to figure out when this gets called in dired-compress-file

    (cond (handler
           (funcall handler 'dired-compress-file file))

since i've never used tramp for compressing files etc. i'll have to dig
into this area a bit.

> And note, that these days Tino Calancha is working in this area, see bug#50581.

i'll have a look there too, thanks for the heads up!

> Best regards, Michael.


cheers,
Michalis





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-21  9:18             ` Michalis V.
@ 2021-09-21  9:32               ` Eli Zaretskii
  2021-09-21 17:10                 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2021-09-21  9:32 UTC (permalink / raw)
  To: Michalis V.; +Cc: mvar.40k, larsi, mcenturion, arthur.miller, 47058

> From: "Michalis V." <mvar.40k@gmail.com>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>,  mvar.40k@gmail.com,
>   mcenturion@fing.edu.uy,  47058@debbugs.gnu.org,  arthur.miller@live.com
> Date: Tue, 21 Sep 2021 12:18:50 +0300
> 
> Btw one of the reasons i went with this approach and included -C
> parameter for tars were some security concerns expressed in #25611. 

That's a separate issue.  And I don't see how is it a security issue
for Emacs, when unpacking an archive manually with 'tar' etc. would
produce the same results.  If the user wants to overwrite his/her
sensitive files, we should let them do it, in the same way as other
utilities do.  But that's MO, and it is a separate concern anyway.

> There's also a suggestion in the discussion there that Z should just
> decompress and not untar the archive and the un-tarring should be a
> separate action/procedure. That would be a drastic solution to this
> problem but on the other hand it would make sense semantically
> (extract != decompress). What is your opinion on this?

I'm okay with having a separate command for unpacking, yes.  We'd need
to provide a backward-compatibility option if we do that, since 'Z'
unpacks for some time now.

> in any case i'll try to assemble another patch based on your suggestion.

Thanks.





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-21  9:24           ` Michalis V.
@ 2021-09-21 12:06             ` Michael Albinus
  2022-03-11  8:29               ` Michael Albinus
  0 siblings, 1 reply; 28+ messages in thread
From: Michael Albinus @ 2021-09-21 12:06 UTC (permalink / raw)
  To: Michalis V.; +Cc: Marco Centurion - URI, 47058, Arthur Miller, tino.calancha

"Michalis V." <mvar.40k@gmail.com> writes:

> hi Michael,

Hi Michalis,

>> dired-compress-file is also handled by remote file name handlers like
>> Tramp. How does this fit?
>
> i stayed away from the handler part, basically i was scratching my head
> trying to figure out when this gets called in dired-compress-file
>
>     (cond (handler
>            (funcall handler 'dired-compress-file file))
>
> since i've never used tramp for compressing files etc. i'll have to dig
> into this area a bit.

This is a call of Tramp's own implementation of dired-compress-file when
default-directory is remote (ie, it has a syntax like
"/ssh:host:/path/to/file"). The reason is, that commands like "gzip"
must be executed on the remote "host" instead of the local one.

>> And note, that these days Tino Calancha is working in this area, see bug#50581.
>
> i'll have a look there too, thanks for the heads up!

Since Tramp's implementation tramp-sh-handle-dired-compress-file is
following the implementation in dired-compress-file, and the only other
handler is in ange-ftp, I'm curious whether we shall enable support for
remote systems directly in dired-compress-file. It uses already
process-file, so it shouldn't be too hard.

Opinions?

> cheers,
> Michalis

Best regards, Michael.





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-21  8:24           ` Eli Zaretskii
  2021-09-21  9:18             ` Michalis V.
@ 2021-09-21 17:07             ` Lars Ingebrigtsen
  2021-09-21 20:43               ` Michalis V.
  1 sibling, 1 reply; 28+ messages in thread
From: Lars Ingebrigtsen @ 2021-09-21 17:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: mvar.40k, mcenturion, 47058, arthur.miller

Eli Zaretskii <eliz@gnu.org> writes:

> This change will now cause the files by default to be unpacked into
> emacs-X.Y/emacs-X.Y/, which is not our intent when we produce the
> tarball.

I misread the patch somehow -- I thought in cases like that we'd end up
with just emacs-X.Y/ (when the queried-for directory was the same as the
one in the tar ball).

So I've now reverted the patch on the trunk (and I'm reopening this bug
report).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-21  9:32               ` Eli Zaretskii
@ 2021-09-21 17:10                 ` Lars Ingebrigtsen
       [not found]                   ` <83a6k5yfrb.fsf@gnu.org>
  2021-09-21 18:38                   ` Gregory Heytings
  0 siblings, 2 replies; 28+ messages in thread
From: Lars Ingebrigtsen @ 2021-09-21 17:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Michalis V., mcenturion, 47058, arthur.miller

Eli Zaretskii <eliz@gnu.org> writes:

> That's a separate issue.  And I don't see how is it a security issue
> for Emacs, when unpacking an archive manually with 'tar' etc. would
> produce the same results.  If the user wants to overwrite his/her
> sensitive files, we should let them do it, in the same way as other
> utilities do.  But that's MO, and it is a separate concern anyway.

It's an Emacs security issue because we make it so easy to unpack these
tar files.  We should ideally inspect the file first and see whether
it's an adversarial tar file first, and then prompt the user for what to
do.

> I'm okay with having a separate command for unpacking, yes.  We'd need
> to provide a backward-compatibility option if we do that, since 'Z'
> unpacks for some time now.

Separate commands here would be good; yes.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
       [not found]                   ` <83a6k5yfrb.fsf@gnu.org>
@ 2021-09-21 17:58                     ` Lars Ingebrigtsen
  0 siblings, 0 replies; 28+ messages in thread
From: Lars Ingebrigtsen @ 2021-09-21 17:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 47058

Eli Zaretskii <eliz@gnu.org> writes:

> By "easy" you mean the single-key command?  But then I could have the
> same via a shell alias.

You could, but that's not be an issue for Emacs.  :-)

> We can still ask the user for confirmation if we think the danger is
> real.

Yes, that's what I think we should do.






^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-21 17:10                 ` Lars Ingebrigtsen
       [not found]                   ` <83a6k5yfrb.fsf@gnu.org>
@ 2021-09-21 18:38                   ` Gregory Heytings
  2021-09-21 18:43                     ` Eli Zaretskii
  1 sibling, 1 reply; 28+ messages in thread
From: Gregory Heytings @ 2021-09-21 18:38 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Michalis V., mcenturion, arthur.miller, 47058


>> That's a separate issue.  And I don't see how is it a security issue 
>> for Emacs, when unpacking an archive manually with 'tar' etc. would 
>> produce the same results.  If the user wants to overwrite his/her 
>> sensitive files, we should let them do it, in the same way as other 
>> utilities do.  But that's MO, and it is a separate concern anyway.
>
> It's an Emacs security issue because we make it so easy to unpack these 
> tar files.  We should ideally inspect the file first and see whether 
> it's an adversarial tar file first, and then prompt the user for what to 
> do.
>

Would it not be easier to unconditionally untar the contents in a 
temporary directory, and to either move its contents to the current 
directory if it contains only one entry, or to rename it to a directory 
based on the tar file name when it contains more than one entry? 
Something like:

TMP=$(mktemp -d ./XXXXXXXX)
tar -C $TMP -x -z -f $FILE
if (($(ls $TMP | wc -l) == 1))
then
   mv $TMP/* .
   rmdir $TMP
else
   mv $TMP $(basename $FILE .tar.gz)
fi





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-21 18:38                   ` Gregory Heytings
@ 2021-09-21 18:43                     ` Eli Zaretskii
  2021-09-21 19:19                       ` Gregory Heytings
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2021-09-21 18:43 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: mvar.40k, larsi, mcenturion, arthur.miller, 47058

> Date: Tue, 21 Sep 2021 18:38:52 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Eli Zaretskii <eliz@gnu.org>, "Michalis V." <mvar.40k@gmail.com>, 
>     mcenturion@fing.edu.uy, 47058@debbugs.gnu.org, arthur.miller@live.com
> 
> Would it not be easier to unconditionally untar the contents in a 
> temporary directory, and to either move its contents to the current 
> directory if it contains only one entry, or to rename it to a directory 
> based on the tar file name when it contains more than one entry? 

Easier in what sense?

> Something like:
> 
> TMP=$(mktemp -d ./XXXXXXXX)
> tar -C $TMP -x -z -f $FILE
> if (($(ls $TMP | wc -l) == 1))
> then
>    mv $TMP/* .
>    rmdir $TMP
> else
>    mv $TMP $(basename $FILE .tar.gz)
> fi

Wouldn't that remove the files that are in the directory but not in
the archive?





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-21 18:43                     ` Eli Zaretskii
@ 2021-09-21 19:19                       ` Gregory Heytings
  2021-09-21 20:11                         ` Michalis V.
                                           ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Gregory Heytings @ 2021-09-21 19:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: mvar.40k, larsi, mcenturion, arthur.miller, 47058


>> Would it not be easier to unconditionally untar the contents in a 
>> temporary directory, and to either move its contents to the current 
>> directory if it contains only one entry, or to rename it to a directory 
>> based on the tar file name when it contains more than one entry?
>
> Easier in what sense?
>

In the sense of "DWIM".

>> Something like:
>>
>> TMP=$(mktemp -d ./XXXXXXXX)
>> tar -C $TMP -x -z -f $FILE
>> if (($(ls $TMP | wc -l) == 1))
>> then
>>    mv $TMP/* .
>>    rmdir $TMP
>> else
>>    mv $TMP $(basename $FILE .tar.gz)
>> fi
>
> Wouldn't that remove the files that are in the directory but not in the 
> archive?
>

No, it does what I explained above:

If all files in the tar file are under one directory (e.g. 
emacs-27.2.tar.gz whose files are all in a emacs-27.2 directory), the 
files will be in that directory.

If on the contrary the tar file is "broken" and its files are under 
multiple directories or not in a directory (say foobar.tar.gz with three 
files "/foo", "/bar" and "/baz"), the files will be put in a directory 
"foobar".





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-21 19:19                       ` Gregory Heytings
@ 2021-09-21 20:11                         ` Michalis V.
  2021-09-21 20:48                         ` Gregory Heytings
  2021-09-22  5:40                         ` Eli Zaretskii
  2 siblings, 0 replies; 28+ messages in thread
From: Michalis V. @ 2021-09-21 20:11 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: mvar.40k, 47058, mcenturion, arthur.miller, larsi

Gregory Heytings <gregory@heytings.org> writes:

>>> Something like:
>>>
>>> TMP=$(mktemp -d ./XXXXXXXX)
>>> tar -C $TMP -x -z -f $FILE
>>> if (($(ls $TMP | wc -l) == 1))
>>> then
>>>    mv $TMP/* .
>>>    rmdir $TMP
>>> else
>>>    mv $TMP $(basename $FILE .tar.gz)
>>> fi
>>
>> Wouldn't that remove the files that are in the directory but not in the
>> archive?
>>
>
> No, it does what I explained above:
>
> If all files in the tar file are under one directory (e.g. emacs-27.2.tar.gz
> whose files are all in a emacs-27.2 directory), the files will be in that
> directory.
>
> If on the contrary the tar file is "broken" and its files are under multiple
> directories or not in a directory (say foobar.tar.gz with three files "/foo",
> "/bar" and "/baz"), the files will be put in a directory "foobar".

hi Gregory,

i've gone down that road with make-temp-file until i realized that i
cannot trust that the temporary-file-directory (usually /tmp/ in linux)
has enough space to allow such action (e.g. it might be a small
ramdisk).
But in your example i see you create the temporary dir in the current
one where the archive also resides, correct?

and i just realized that this is possible in elisp:

(let ((temporary-file-directory "/home/mvar/"))
  (make-temp-file "boing-" t))

without altering the global var. I think this is quite feasible and will
give it a try, thank you!


Michalis





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-21 17:07             ` Lars Ingebrigtsen
@ 2021-09-21 20:43               ` Michalis V.
  2021-09-22  6:00                 ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Michalis V. @ 2021-09-21 20:43 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: mvar.40k, 47058

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> This change will now cause the files by default to be unpacked into
>> emacs-X.Y/emacs-X.Y/, which is not our intent when we produce the
>> tarball.
>
> I misread the patch somehow -- I thought in cases like that we'd end up
> with just emacs-X.Y/ (when the queried-for directory was the same as the
> one in the tar ball).
>
> So I've now reverted the patch on the trunk (and I'm reopening this bug
> report).

hi Lars,

I apologize, i should have provided an example. But still, this is only
if the user hits enter on the default suggestion. And to be fair, it
isn't any different with zip files, for example:

mkdir boing
dired-do-compress-to RET boing.zip

then hit Z on boing.zip and you'll have boing/boing/ created.

i.e. existing behavior across different formats isn't very consistent
either.

thanks,
Michalis





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-21 19:19                       ` Gregory Heytings
  2021-09-21 20:11                         ` Michalis V.
@ 2021-09-21 20:48                         ` Gregory Heytings
  2021-09-22  5:40                         ` Eli Zaretskii
  2 siblings, 0 replies; 28+ messages in thread
From: Gregory Heytings @ 2021-09-21 20:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: mvar.40k, larsi, mcenturion, arthur.miller, 47058

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


>>> Would it not be easier to unconditionally untar the contents in a 
>>> temporary directory, and to either move its contents to the current 
>>> directory if it contains only one entry, or to rename it to a 
>>> directory based on the tar file name when it contains more than one 
>>> entry?
>> 
>> Easier in what sense?
>
> In the sense of "DWIM".
>

Patch attached.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=Uncompress-tar-files-in-a-DWIMish-way.patch, Size: 3287 bytes --]

From 7a25f95b2a554ca281bb0e6d5000ef9ece980694 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Tue, 21 Sep 2021 20:43:27 +0000
Subject: [PATCH] Uncompress tar files in a DWIMish way

* lisp/dired-aux.el (dired-compress-file-suffixes): Add a '-C %o'
argument to tar commands.
(dired-compress-file): Extract files in a temporary directory; if
it contains a single file, use it as the output file, otherwise
rename the temporary directory and use it as the output file.
---
 lisp/dired-aux.el | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 53fbcfb6d0..66422e7b16 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -1134,9 +1134,9 @@ dired-compress-file-suffixes
     ;; "tar -zxf" isn't used because it's not available on the
     ;; Solaris 10 version of tar (obsolete in 2024?).
     ;; Same thing on AIX 7.1 (obsolete 2023?) and 7.2 (obsolete 2022?).
-    ("\\.tar\\.gz\\'" "" "gzip -dc %i | tar -xf -")
-    ("\\.tar\\.xz\\'" "" "xz -dc %i | tar -xf -")
-    ("\\.tgz\\'" "" "gzip -dc %i | tar -xf -")
+    ("\\.tar\\.gz\\'" "" "gzip -dc %i | tar -C %o -xf -")
+    ("\\.tar\\.xz\\'" "" "xz -dc %i | tar -C %o -xf -")
+    ("\\.tgz\\'" "" "gzip -dc %i | tar -C %o -xf -")
     ("\\.gz\\'" "" "gzip -d")
     ("\\.lz\\'" "" "lzip -d")
     ("\\.Z\\'" "" "uncompress")
@@ -1148,8 +1148,8 @@ dired-compress-file-suffixes
     ("\\.bz2\\'" "" "bunzip2")
     ("\\.xz\\'" "" "unxz")
     ("\\.zip\\'" "" "unzip -o -d %o %i")
-    ("\\.tar\\.zst\\'" "" "unzstd -c %i | tar -xf -")
-    ("\\.tzst\\'" "" "unzstd -c %i | tar -xf -")
+    ("\\.tar\\.zst\\'" "" "unzstd -c %i | tar -C %o -xf -")
+    ("\\.tzst\\'" "" "unzstd -c %i | tar -C %o -xf -")
     ("\\.zst\\'" "" "unzstd --rm")
     ("\\.7z\\'" "" "7z x -aoa -o%o %i")
     ;; This item controls naming for compression.
@@ -1278,15 +1278,23 @@ dired-compress-file
            nil)
           ((and suffix (setq command (nth 2 suffix)))
            (if (string-match "%[io]" command)
-               (prog1 (setq newname (file-name-as-directory newname))
+               (let ((tmp (file-name-as-directory (make-temp-file newname t))))
                  (dired-shell-command
                   (replace-regexp-in-string
-                   "%o" (shell-quote-argument newname)
+                   "%o" (shell-quote-argument tmp)
                    (replace-regexp-in-string
                     "%i" (shell-quote-argument file)
                     command
                     nil t)
-                   nil t)))
+                   nil t))
+                 (let ((files (directory-files
+                               tmp nil
+                               directory-files-no-dot-files-regexp)))
+                   (if (> (length files) 1)
+                       (rename-file tmp newname)
+                     (rename-file (concat tmp (car files)) newname)
+                     (delete-directory tmp)))
+                 (file-name-as-directory newname))
              ;; We found an uncompression rule.
              (let ((match (string-search " " command))
                    (msg (concat "Uncompressing " file)))
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-21 19:19                       ` Gregory Heytings
  2021-09-21 20:11                         ` Michalis V.
  2021-09-21 20:48                         ` Gregory Heytings
@ 2021-09-22  5:40                         ` Eli Zaretskii
  2021-09-22  7:15                           ` Gregory Heytings
  2 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2021-09-22  5:40 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: mvar.40k, larsi, mcenturion, arthur.miller, 47058

> Date: Tue, 21 Sep 2021 19:19:52 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: mvar.40k@gmail.com, larsi@gnus.org, mcenturion@fing.edu.uy, 
>     arthur.miller@live.com, 47058@debbugs.gnu.org
> 
> >> TMP=$(mktemp -d ./XXXXXXXX)
> >> tar -C $TMP -x -z -f $FILE
> >> if (($(ls $TMP | wc -l) == 1))
> >> then
> >>    mv $TMP/* .
> >>    rmdir $TMP
> >> else
> >>    mv $TMP $(basename $FILE .tar.gz)
> >> fi
> >
> > Wouldn't that remove the files that are in the directory but not in the 
> > archive?
> 
> No, it does what I explained above:

If you move a directory over another by renaming, the previous
directory is gone, and replaced by the one you moved in its place,
right?  I'm not talking about what GNU mv does, I'm talking about what
Emacs will do when renaming a directory into another one.

> If all files in the tar file are under one directory (e.g. 
> emacs-27.2.tar.gz whose files are all in a emacs-27.2 directory), the 
> files will be in that directory.
> 
> If on the contrary the tar file is "broken" and its files are under 
> multiple directories or not in a directory (say foobar.tar.gz with three 
> files "/foo", "/bar" and "/baz"), the files will be put in a directory 
> "foobar".

That was not my question.





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-21 20:43               ` Michalis V.
@ 2021-09-22  6:00                 ` Eli Zaretskii
  0 siblings, 0 replies; 28+ messages in thread
From: Eli Zaretskii @ 2021-09-22  6:00 UTC (permalink / raw)
  To: Michalis V.; +Cc: mvar.40k, larsi, 47058

> From: "Michalis V." <mvar.40k@gmail.com>
> Date: Tue, 21 Sep 2021 23:43:53 +0300
> Cc: mvar.40k@gmail.com, 47058@debbugs.gnu.org
> 
> I apologize, i should have provided an example. But still, this is only
> if the user hits enter on the default suggestion. And to be fair, it
> isn't any different with zip files, for example:
> 
> mkdir boing
> dired-do-compress-to RET boing.zip
> 
> then hit Z on boing.zip and you'll have boing/boing/ created.
> 
> i.e. existing behavior across different formats isn't very consistent
> either.

It's okay to make the behavior more consistent in future patches, but
let's not introduce regressions into existing behaviors as a side
effect.





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-22  5:40                         ` Eli Zaretskii
@ 2021-09-22  7:15                           ` Gregory Heytings
  2021-09-22  7:54                             ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Gregory Heytings @ 2021-09-22  7:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: mvar.40k, larsi, mcenturion, arthur.miller, 47058

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


>
> If you move a directory over another by renaming, the previous directory 
> is gone, and replaced by the one you moved in its place, right?  I'm not 
> talking about what GNU mv does, I'm talking about what Emacs will do 
> when renaming a directory into another one.
>

Sorry, I don't understand your question.  With the patch, if the output 
directory already exists, an "File already exists" is displayed, as I 
think should be the case.

I attach a slightly improved version, in which a tar file containing a 
single regular file is uncompressed into a directory.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=Uncompress-tar-files-in-a-DWIMish-way.patch, Size: 3466 bytes --]

From 5bbfd5d72d8543bac4baa6b6526e86a3f72feff6 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Wed, 22 Sep 2021 07:10:46 +0000
Subject: [PATCH] Uncompress tar files in a DWIMish way

* lisp/dired-aux.el (dired-compress-file-suffixes): Add a '-C %o'
argument to tar commands.
(dired-compress-file): Extract files in a temporary directory; if
it contains a single file, use it as the output file, otherwise
rename the temporary directory and use it as the output file.
---
 lisp/dired-aux.el | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 53fbcfb6d0..85e450c3f4 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -1134,9 +1134,9 @@ dired-compress-file-suffixes
     ;; "tar -zxf" isn't used because it's not available on the
     ;; Solaris 10 version of tar (obsolete in 2024?).
     ;; Same thing on AIX 7.1 (obsolete 2023?) and 7.2 (obsolete 2022?).
-    ("\\.tar\\.gz\\'" "" "gzip -dc %i | tar -xf -")
-    ("\\.tar\\.xz\\'" "" "xz -dc %i | tar -xf -")
-    ("\\.tgz\\'" "" "gzip -dc %i | tar -xf -")
+    ("\\.tar\\.gz\\'" "" "gzip -dc %i | tar -C %o -xf -")
+    ("\\.tar\\.xz\\'" "" "xz -dc %i | tar -C %o -xf -")
+    ("\\.tgz\\'" "" "gzip -dc %i | tar -C %o -xf -")
     ("\\.gz\\'" "" "gzip -d")
     ("\\.lz\\'" "" "lzip -d")
     ("\\.Z\\'" "" "uncompress")
@@ -1148,8 +1148,8 @@ dired-compress-file-suffixes
     ("\\.bz2\\'" "" "bunzip2")
     ("\\.xz\\'" "" "unxz")
     ("\\.zip\\'" "" "unzip -o -d %o %i")
-    ("\\.tar\\.zst\\'" "" "unzstd -c %i | tar -xf -")
-    ("\\.tzst\\'" "" "unzstd -c %i | tar -xf -")
+    ("\\.tar\\.zst\\'" "" "unzstd -c %i | tar -C %o -xf -")
+    ("\\.tzst\\'" "" "unzstd -c %i | tar -C %o -xf -")
     ("\\.zst\\'" "" "unzstd --rm")
     ("\\.7z\\'" "" "7z x -aoa -o%o %i")
     ;; This item controls naming for compression.
@@ -1278,15 +1278,26 @@ dired-compress-file
            nil)
           ((and suffix (setq command (nth 2 suffix)))
            (if (string-match "%[io]" command)
-               (prog1 (setq newname (file-name-as-directory newname))
+               (let ((tmp (file-name-as-directory (make-temp-file newname t))))
                  (dired-shell-command
                   (replace-regexp-in-string
-                   "%o" (shell-quote-argument newname)
+                   "%o" (shell-quote-argument tmp)
                    (replace-regexp-in-string
                     "%i" (shell-quote-argument file)
                     command
                     nil t)
-                   nil t)))
+                   nil t))
+                 (let* ((files (directory-files
+                                tmp nil
+                                directory-files-no-dot-files-regexp))
+                        (first-file (concat tmp (car files))))
+                   (if (or (> (length files) 1)
+                           (and (= (length files) 1)
+                                (file-regular-p first-file)))
+                       (rename-file tmp newname)
+                     (rename-file first-file newname)
+                     (delete-directory tmp)))
+                 (file-name-as-directory newname))
              ;; We found an uncompression rule.
              (let ((match (string-search " " command))
                    (msg (concat "Uncompressing " file)))
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-22  7:15                           ` Gregory Heytings
@ 2021-09-22  7:54                             ` Eli Zaretskii
  2021-09-22  8:07                               ` Gregory Heytings
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2021-09-22  7:54 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: mvar.40k, larsi, mcenturion, arthur.miller, 47058

> Date: Wed, 22 Sep 2021 07:15:36 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: mvar.40k@gmail.com, larsi@gnus.org, mcenturion@fing.edu.uy, 
>     arthur.miller@live.com, 47058@debbugs.gnu.org
> 
> > If you move a directory over another by renaming, the previous directory 
> > is gone, and replaced by the one you moved in its place, right?  I'm not 
> > talking about what GNU mv does, I'm talking about what Emacs will do 
> > when renaming a directory into another one.
> 
> Sorry, I don't understand your question.  With the patch, if the output 
> directory already exists, an "File already exists" is displayed, as I 
> think should be the case.

Should it?

The use case that I have in mind is this:

  . directory DIR exists on disk, and has files in it
  . the archive includes directory DIR with files in it, not
    necessarily identical to those on disk: some files on disk don't
    exist in the archive, and vice versa

When I unpack such an archive outside Emacs, what happens (and what
I'd expect to happen) is the following:

  . files in DIR on disk that don't exist in the archive are unaltered
  . files in DIR in the archive that don't exist on disk are unpacked
    into DIR on disk
  . non-directory files in DIR that exist both on disk and in the
    archive are overwritten with the version in the archive
  . sub-directories in DIR that exist both on disk and in the archive
    are NOT being overwritten; instead, they are recursively handled
    as described above, i.e. missing files are added and common files
    are overwritten
  . as a corollary from the last item, an empty sub-directory of DIR
    in the archive that also exists on disk will NOT be emptied on
    disk, but will be left intact with the files it had before
    unpacking

Will the unpacking you propose behave in the same way?  If not, we
need to augment it so it does.

In particular, unpacking into an existing directory should "just
work", it makes little sense to me to fail in that case, as that isn't
what Tar and similar utilities do, at least IME.  We may decide asking
the user for confirmation in that case, but given the confirmation
should proceed as above.





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-22  7:54                             ` Eli Zaretskii
@ 2021-09-22  8:07                               ` Gregory Heytings
  0 siblings, 0 replies; 28+ messages in thread
From: Gregory Heytings @ 2021-09-22  8:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: mvar.40k, larsi, mcenturion, 47058, arthur.miller


>
> The use case that I have in mind is this:
>
>  . directory DIR exists on disk, and has files in it
>  . the archive includes directory DIR with files in it, not necessarily identical to those on disk: some files on disk don't exist in the archive, and vice versa
>

Okay, now I see what you mean.  That's not something I ever do, but I 
understand it can make sense to do that.

>
> Will the unpacking you propose behave in the same way?  If not, we need 
> to augment it so it does.
>

It doesn't, indeed.  I'll think about this.





^ permalink raw reply	[flat|nested] 28+ messages in thread

* bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86
  2021-09-21 12:06             ` Michael Albinus
@ 2022-03-11  8:29               ` Michael Albinus
  0 siblings, 0 replies; 28+ messages in thread
From: Michael Albinus @ 2022-03-11  8:29 UTC (permalink / raw)
  To: Michalis V.; +Cc: Marco Centurion - URI, 47058, Arthur Miller, tino.calancha

Michael Albinus <michael.albinus@gmx.de> writes:

Hi Michalis,

> Since Tramp's implementation tramp-sh-handle-dired-compress-file is
> following the implementation in dired-compress-file, and the only other
> handler is in ange-ftp, I'm curious whether we shall enable support for
> remote systems directly in dired-compress-file. It uses already
> process-file, so it shouldn't be too hard.
>
> Opinions?

I have no idea about the status of this bug, but just for the records:
In Emacs 29, Tramp doesn't use an own handler for dired-compress-file anymore.

>> cheers,
>> Michalis

Best regards, Michael.





^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2022-03-11  8:29 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-10 20:26 bug#47058: 28.0.50; Dired Z: insert-directory: Reading directory: No such file or directory, CrossLine_linux_x86 Jean Louis
2021-08-31 21:33 ` Marco Centurion
2021-08-31 22:12   ` Arthur Miller
2021-09-01 13:59     ` Marco Centurion - URI
2021-09-01 16:06       ` Arthur Miller
2021-09-20 22:12       ` Michalis V.
2021-09-21  4:32         ` Lars Ingebrigtsen
2021-09-21  8:24           ` Eli Zaretskii
2021-09-21  9:18             ` Michalis V.
2021-09-21  9:32               ` Eli Zaretskii
2021-09-21 17:10                 ` Lars Ingebrigtsen
     [not found]                   ` <83a6k5yfrb.fsf@gnu.org>
2021-09-21 17:58                     ` Lars Ingebrigtsen
2021-09-21 18:38                   ` Gregory Heytings
2021-09-21 18:43                     ` Eli Zaretskii
2021-09-21 19:19                       ` Gregory Heytings
2021-09-21 20:11                         ` Michalis V.
2021-09-21 20:48                         ` Gregory Heytings
2021-09-22  5:40                         ` Eli Zaretskii
2021-09-22  7:15                           ` Gregory Heytings
2021-09-22  7:54                             ` Eli Zaretskii
2021-09-22  8:07                               ` Gregory Heytings
2021-09-21 17:07             ` Lars Ingebrigtsen
2021-09-21 20:43               ` Michalis V.
2021-09-22  6:00                 ` Eli Zaretskii
2021-09-21  8:25         ` Michael Albinus
2021-09-21  9:24           ` Michalis V.
2021-09-21 12:06             ` Michael Albinus
2022-03-11  8:29               ` Michael Albinus

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).