Prevent inferior emacs from asking confirmation for large files

[aagon]

Hello,

As mentioned by #111, when large files copies are attempted to remotes with some methods like ssh, the inferior emacs process never actually starts the copying and waits for ever for the user to confirm the copy (in the function files--ask-user-about-large-file, more specifically), with words to the effect :

File is large, really copy ?

The solution offered, namely to merely use another method only works around the problem, does not solve it. In fact, since there is no obvious way to write to the standard input of the inferior emacs process, it should be prevented, as much as possible, to block waiting on some user interaction that will never occur (since the user cannot actually write anything to the inferior process). This is how I interpret the setting of the variable create-dir in the function dired-async-create-files.

So, this commit does not purport to eliminate all the possible ways the inferior emacs could block on user interaction, but eliminates at least the confirmation for large files, which is liable to happen.

There may be better ways to do this.

Best,

Aymeric

[thierryvolpiatto]

aagon @.***> writes:

[[PGP Encrypted Part:OK]]

1. ( ) text/plain (*) text/html

Hello,

As mentioned by #111, when large files copies are attempted to remotes with some methods like ssh, the inferior emacs process never actually starts the copying and waits for ever for the user to confirm the copy (in the function files--ask-user-about-large-file, more specifically), with words to the effect :

File is large, really copy ?

The solution offered, namely to merely use another method only works around the problem, does not solve it. In fact, since there is no obvious way to write to the standard input of the inferior emacs process, it should be prevented, as much as possible, to block waiting on some user interaction that will never occur (since the user cannot actually write anything to the inferior process). This is how I interpret the setting of the variable create-dir in the function dired-async-create-files.

So, this commit does not purport to eliminate all the possible ways the inferior emacs could block on user interaction, but eliminates at least the confirmation for large files, which is liable to happen.

There may be better ways to do this.

I would be better to be sure the process abort instead of continuing running an emacs in the back eating all memory and at the end crash the system, what about this?

diff --git a/dired-async.el b/dired-async.el index 8294271..fdaa848 100644 --- a/dired-async.el +++ b/dired-async.el @@ -361,6 +361,8 @@ ESC or q' to not overwrite any of the remaining files, (async-start(lambda () (require 'cl-lib) (require 'dired-aux) (require 'dired-x) ,(async-inject-variables dired-async-env-variables-regexp)

• (advice-add 'files--ask-user-about-large-file
• :override (lambda (&rest args) 'abort)) (let ((dired-recursive-copies (quote always)) (dired-copy-preserve-time ,dired-copy-preserve-time)

NOTE: There is better tools to copy large files (remote) from emacs, using tramp specially with SSH method is perhaps the worst. I often copy large files (x GBs) from one machine to the other with helm-find-files and the rsync action (M-V), tramp is used only to select the destination on remote (or vice versa).

Best,

Aymeric

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

You can view, comment on, or merge this pull request online at:

https://github.com/jwiegley/emacs-async/pull/180

Commit Summary

• 430678d Prevent inferior emacs from asking confirmation

File Changes

(1 file)

• M dired-async.el (3)

Patch Links:

• https://github.com/jwiegley/emacs-async/pull/180.patch • https://github.com/jwiegley/emacs-async/pull/180.diff

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.*Message ID: @.***> [[End of PGP Encrypted Part]]

-- Thierry

[thierryvolpiatto]

Even better is to ask before starting async process:

diff --git a/dired-async.el b/dired-async.el
index 8294271..eb196fc 100644
--- a/dired-async.el
+++ b/dired-async.el
@@ -242,6 +242,17 @@ cases if `dired-async-skip-fast' is non-nil."
       (funcall old-func file-creator operation
                (nreverse quick-list) name-constructor marker-char))))

+(defun dired-async--abort-if-file-too-large (size op-type filename)
+  "If file SIZE larger than `large-file-warning-threshold', allow user to abort.
+Same as `abort-if-file-too-large' but without error."
+  (let ((choice (and large-file-warning-threshold size
+                (> size large-file-warning-threshold)
+                     ;; No point in warning if we can't read it.
+                     (file-readable-p filename)
+                     (files--ask-user-about-large-file
+                      size op-type filename nil))))
+    choice))
+
 (defvar overwrite-query)
 (defun dired-async-create-files (file-creator operation fn-list name-constructor
                                               &optional _marker-char)
@@ -299,14 +310,22 @@ ESC or `q' to not overwrite any of the remaining files,
                    (file-in-directory-p destname from)
                    (error "Cannot copy `%s' into its subdirectory `%s'"
                           from to)))
-            (if overwrite
-                (or (and dired-overwrite-confirmed
-                         (push (cons from to) async-fn-list))
-                    (progn
-                      (push (dired-make-relative from) failures)
-                      (dired-log "%s `%s' to `%s' failed\n"
-                                 operation from to)))
-              (push (cons from to) async-fn-list)))))
+            ;; Skip file if it is too large.
+            (if (and (memq operation '(copy rename))
+                     (eq (dired-async--abort-if-file-too-large
+                          (file-attribute-size
+                           (file-attributes (file-truename from)))
+                          (symbol-name operation) from)
+                         'abort))
+                (push from skipped)
+              (if overwrite
+                  (or (and dired-overwrite-confirmed
+                           (push (cons from to) async-fn-list))
+                      (progn
+                        (push (dired-make-relative from) failures)
+                        (dired-log "%s `%s' to `%s' failed\n"
+                                   operation from to)))
+                (push (cons from to) async-fn-list))))))
       ;; Fix tramp issue #80 with emacs-26, use "-q" only when needed.
       (setq async-quiet-switch
             (if (and (boundp 'tramp-cache-read-persistent-data)

[aagon]

NOTE: There is better tools to copy large files (remote) from emacs, using tramp specially with SSH method is perhaps the worst.

I completely agree. The performance penalty of the ssh method against scp or rsync is catastrophic.

I also agree with your solution of asking before launching the inferior process. I had to correct it a bit (apparently, operation is a capitalised string), so you might want to review it.

I also added an advice in the inferior to prevent it from asking. But my advice always returns nil, and not 'abort, since we already asked before and the user has decided to proceed at this point. In the case the inferior eats too much memory, I assumed it can always be killed with dired-async-kill-process.

I just tested it, it works.

Thank you for the constructive discussion and the substantial improvement.

Best,

Aymeric

[aagon]

37ea3e2 still needs some corrections.

(memq operation '(copy rename)) will always return nil, because operation holds a capitalised string like "Copy" or "Rename", so the form :

(eq (dired-async--abort-if-file-too-large
     (file-attribute-size
      (file-attributes (file-truename from)))
     (symbol-name operation) from)
    'abort)

will never be evaluated. A good replacement would be (member operation '("Copy" "Rename")).

For the same reason, (symbol-name operation) would produce an error. (downcase operation) works.

And more importantly, we do not deal with the issue : if the user decides to proceed, we still need to prevent the inferior emacs from asking anything. I've added an advice to replace files--ask-user-about-large-file with a lambda that always returns nil, as if the user always accepted (it has accepted already, after all).

The diff held by the pull request corresponds to the corrections I've detailed here and should be correct.

[thierryvolpiatto]

aagon @.***> writes:

37ea3e2 still needs some corrections.

(memq operation '(copy rename)) will always return nil, because operation holds a capitalised string like "Copy" or "Rename", so the form :

(eq (dired-async--abort-if-file-too-large (file-attribute-size (file-attributes (file-truename from))) (symbol-name operation) from) 'abort)

will never be evaluated. A good replacement would be (member operation '("Copy" "Rename")).

For the same reason, (symbol-name operation) would produce an error. (downcase operation) works.

Thanks fixed.

And more importantly, we do not deal with the issue : if the user decides to proceed, we still need to prevent the inferior emacs from asking anything. I've added an advice to replace files--ask-user-about-large-file with a lambda that always returns nil, as if the user always accepted (it has accepted already, after all).

Ah, yes right, forget to put back the advice, can you do it in the PR and I will merge it?

Thanks.

The diff held by the pull request corresponds to the corrections I've detailed here and should be correct.

Yes, sorry didn't realize you had updated the PR, can you apply latest correction on top of my changes for the PR? Sorry for extra work and thanks.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.*Message ID: @.***>

-- Thierry

[aagon]

Ah, yes right, forget to put back the advice, can you do it in the PR and I will merge it?

Yes, sorry didn't realize you had updated the PR, can you apply latest correction on top of my changes for the PR?

Done ! The PR now only holds the diff that adds the advice, and applies cleanly to master.

Sorry for extra work and thanks.

No worries, my pleasure.

[thierryvolpiatto]

aagon @.***> writes:

Done ! The PR now only holds the diff that adds the advice, and applies cleanly to master.

Great! Thank you, merged.

-- Thierry