[PATCH] bug#18130: rmail.el: handle imaps, pops, and local source mailboxes (maildir, MH, etc.)

[PATCH] bug#18130: rmail.el: handle imaps, pops, and local source mailboxes (maildir, MH, etc.)

[Sergey Poznyakoff]

Hello,

When used with GNU Mailutils movemail, rmail handles only imap and pop
mailboxes[1].  However, the Mailutils movemail supports a much wider
set of mailbox formats (both remote and local).  This patch makes
rmail.el fully use the flexibility of GNU Mailutils.  In particular,
it makes it possible to use imaps and pops encrypted protocols.  The
patch is against the current Emacs trunk.  The bug has been reported by
Carlos Pita to both GNU Emacs[2] and Mailutuls[3].

[1] http://lists.gnu.org/archive/html/bug-mailutils/2014-07/msg00008.html.
[2] http://lists.gnu.org/archive/html/bug-gnu-emacs/2014-07/msg00867.html
    http://lists.gnu.org/archive/html/bug-gnu-emacs/2014-07/msg00869.html
[3] http://lists.gnu.org/archive/html/bug-mailutils/2014-08/msg00000.html.

* lisp/mail/rmail.el (rmail-remote-proto-p): New function.
(rmail-parse-url): Return actual proto in the 2nd list element
instead of the mere t. All callers updated. Do not require
password unless the argument refers to a remote mailbox.
(rmail-insert-inbox-text): Select a proper message depending
on whether source mailbox is a remote one or not.
Hanlde non-plainfile local source mailboxes (maildir, MH, etc.)
---
 lisp/mail/rmail.el | 58 +++++++++++++++++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 25 deletions(-)

diff --git a/lisp/mail/rmail.el b/lisp/mail/rmail.el
index f769204..a3d6343 100644
--- a/lisp/mail/rmail.el
+++ b/lisp/mail/rmail.el
@@ -1869,13 +1869,16 @@ not be a new one).  It returns non-nil if it got any new messages."
 	(setq result (> new-messages 0))
 	result))))
 
+(defun rmail-remote-proto-p (proto)
+  (string-match "^\\(\\(imap\\|pop\\)s?\\)$" proto))
+
 (defun rmail-parse-url (file)
-  "Parse the supplied URL. Return (list MAILBOX-NAME REMOTE PASSWORD GOT-PASSWORD)
+  "Parse the supplied URL. Return (list MAILBOX-NAME PROTO PASSWORD GOT-PASSWORD)
 WHERE MAILBOX-NAME is the name of the mailbox suitable as argument to the
-actual version of `movemail', REMOTE is non-nil if MAILBOX-NAME refers to
-a remote mailbox, PASSWORD is the password if it should be
-supplied as a separate argument to `movemail' or nil otherwise, GOT-PASSWORD
-is non-nil if the user has supplied the password interactively.
+actual version of `movemail', PROTO is the protocol (use rmail-remote-proto-p
+to see if it refers to a remote mailbox), PASSWORD is the password if it
+should be supplied as a separate argument to `movemail' or nil otherwise,
+GOT-PASSWORD is non-nil if the user has supplied the password interactively.
 "
   (cond
    ((string-match "^\\([^:]+\\)://\\(\\([^:@]+\\)\\(:\\([^@]+\\)\\)?@\\)?.*" file)
@@ -1886,24 +1889,26 @@ is non-nil if the user has supplied the password interactively.
 	    (host  (substring file (or (match-end 2)
 				       (+ 3 (match-end 1))))))
 
-	(if (not pass)
-	    (when rmail-remote-password-required
-	      (setq got-password (not (rmail-have-password)))
-	      (setq supplied-password (rmail-get-remote-password
-				       (string-equal proto "imap"))))
-	  ;; The password is embedded.  Strip it out since movemail
-	  ;; does not really like it, in spite of the movemail spec.
-	  (setq file (concat proto "://" user "@" host)))
+	(if (rmail-remote-proto-p proto)
+	    (if (not pass)	
+		(when rmail-remote-password-required
+		  (setq got-password (not (rmail-have-password)))
+		  (setq supplied-password (rmail-get-remote-password
+					   (string-match "imaps?" proto))))
+	      ;; FIXME
+	      ;; The password is embedded.  Strip it out since movemail
+	      ;; does not really like it, in spite of the movemail spec.
+	      (setq file (concat proto "://" user "@" host))))
 
 	(if (rmail-movemail-variant-p 'emacs)
 	    (if (string-equal proto "pop")
 		(list (concat "po:" user ":" host)
-		      t
+		      proto
 		      (or pass supplied-password)
 		      got-password)
 	      (error "Emacs movemail does not support %s protocol" proto))
 	  (list file
-		(or (string-equal proto "pop") (string-equal proto "imap"))
+		proto
 		(or supplied-password pass)
 		got-password))))
 
@@ -1971,18 +1976,18 @@ Value is the size of the newly read mail after conversion."
   (or (memq (file-locked-p buffer-file-name) '(nil t))
       (error "RMAIL file %s is locked"
 	     (file-name-nondirectory buffer-file-name)))
-  (let (file tofile delete-files movemail popmail got-password password)
+  (let (file tofile delete-files movemail proto got-password password)
     (while files
       ;; Handle remote mailbox names specially; don't expand as filenames
       ;; in case the userid contains a directory separator.
       (setq file (car files))
       (let ((url-data (rmail-parse-url file)))
 	(setq file (nth 0 url-data))
-	(setq popmail (nth 1 url-data))
+	(setq proto (nth 1 url-data))
 	(setq password (nth 2 url-data))
 	(setq got-password (nth 3 url-data)))
 
-      (if popmail
+      (if proto
 	  (setq renamep t)
 	(setq file (file-truename
 		    (substitute-in-file-name (expand-file-name file)))))
@@ -2003,14 +2008,16 @@ Value is the size of the newly read mail after conversion."
 		     (expand-file-name buffer-file-name))))
       ;; Always use movemail to rename the file,
       ;; since there can be mailboxes in various directories.
-      (when (not popmail)
+      (when (not proto)
 	;; On some systems, /usr/spool/mail/foo is a directory
 	;; and the actual inbox is /usr/spool/mail/foo/foo.
 	(if (file-directory-p file)
 	    (setq file (expand-file-name (user-login-name)
 					 file))))
-      (cond (popmail
-	     (message "Getting mail from the remote server ..."))
+      (cond (proto
+	     (message (if (rmail-remote-proto-p proto)
+			  "Getting mail from the remote server ..."
+			(concat "Getting mail from " proto))))
 	    ((and (file-exists-p tofile)
 		  (/= 0 (nth 7 (file-attributes tofile))))
 	     (message "Getting mail from %s..." tofile))
@@ -2021,7 +2028,7 @@ Value is the size of the newly read mail after conversion."
       ;; rename or copy the file FILE to TOFILE if and as appropriate.
       (cond ((not renamep)
 	     (setq tofile file))
-	    ((or (file-exists-p tofile) (and (not popmail)
+	    ((or (file-exists-p tofile) (and (not proto)
 					     (not (file-exists-p file))))
 	     nil)
 	    (t
@@ -2056,9 +2063,10 @@ Value is the size of the newly read mail after conversion."
 		   ;; If we just read the password, most likely it is
 		   ;; wrong.  Otherwise, see if there is a specific
 		   ;; reason to think that the problem is a wrong passwd.
-		   (if (or got-password
-			   (re-search-forward rmail-remote-password-error
-					      nil t))
+		   (if (and (rmail-remote-proto-p proto)
+			    (or got-password
+				(re-search-forward rmail-remote-password-error
+						   nil t)))
 		       (rmail-set-remote-password nil))
 
 		   ;; If using Mailutils, remove initial error code
-- 
1.7.12.1

bug#18130: Fwd: [PATCH] bug#18130: rmail.el: handle imaps, pops, and local source mailboxes (maildir, MH, etc.)

[Carlos Pita]

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

Sergey has kindly contributed this patch I'm adding to the report because
I'm afraid it could get lost otherwise.
---------- Forwarded message ----------
From: "Sergey Poznyakoff" <gray@gnu.org>
Date: Aug 16, 2014 10:02 AM
Subject: [PATCH] bug#18130: rmail.el: handle imaps, pops, and local source
mailboxes (maildir, MH, etc.)
To: <bug-gnu-emacs@gnu.org>
Cc: "Carlos Pita" <carlosjosepita@gmail.com>, <bug-mailutils@gnu.org>

Hello,

When used with GNU Mailutils movemail, rmail handles only imap and pop
mailboxes[1].  However, the Mailutils movemail supports a much wider
set of mailbox formats (both remote and local).  This patch makes
rmail.el fully use the flexibility of GNU Mailutils.  In particular,
it makes it possible to use imaps and pops encrypted protocols.  The
patch is against the current Emacs trunk.  The bug has been reported by
Carlos Pita to both GNU Emacs[2] and Mailutuls[3].

[1] http://lists.gnu.org/archive/html/bug-mailutils/2014-07/msg00008.html.
[2] http://lists.gnu.org/archive/html/bug-gnu-emacs/2014-07/msg00867.html
    http://lists.gnu.org/archive/html/bug-gnu-emacs/2014-07/msg00869.html
[3] http://lists.gnu.org/archive/html/bug-mailutils/2014-08/msg00000.html.

* lisp/mail/rmail.el (rmail-remote-proto-p): New function.
(rmail-parse-url): Return actual proto in the 2nd list element
instead of the mere t. All callers updated. Do not require
password unless the argument refers to a remote mailbox.
(rmail-insert-inbox-text): Select a proper message depending
on whether source mailbox is a remote one or not.
Hanlde non-plainfile local source mailboxes (maildir, MH, etc.)
---
 lisp/mail/rmail.el | 58
+++++++++++++++++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 25 deletions(-)

diff --git a/lisp/mail/rmail.el b/lisp/mail/rmail.el
index f769204..a3d6343 100644
--- a/lisp/mail/rmail.el
+++ b/lisp/mail/rmail.el
@@ -1869,13 +1869,16 @@ not be a new one).  It returns non-nil if it got
any new messages."
        (setq result (> new-messages 0))
        result))))

+(defun rmail-remote-proto-p (proto)
+  (string-match "^\\(\\(imap\\|pop\\)s?\\)$" proto))
+
 (defun rmail-parse-url (file)
-  "Parse the supplied URL. Return (list MAILBOX-NAME REMOTE PASSWORD
GOT-PASSWORD)
+  "Parse the supplied URL. Return (list MAILBOX-NAME PROTO PASSWORD
GOT-PASSWORD)
 WHERE MAILBOX-NAME is the name of the mailbox suitable as argument to the
-actual version of `movemail', REMOTE is non-nil if MAILBOX-NAME refers to
-a remote mailbox, PASSWORD is the password if it should be
-supplied as a separate argument to `movemail' or nil otherwise,
GOT-PASSWORD
-is non-nil if the user has supplied the password interactively.
+actual version of `movemail', PROTO is the protocol (use
rmail-remote-proto-p
+to see if it refers to a remote mailbox), PASSWORD is the password if it
+should be supplied as a separate argument to `movemail' or nil otherwise,
+GOT-PASSWORD is non-nil if the user has supplied the password
interactively.
 "
   (cond
    ((string-match
"^\\([^:]+\\)://\\(\\([^:@]+\\)\\(:\\([^@]+\\)\\)?@\\)?.*" file)
@@ -1886,24 +1889,26 @@ is non-nil if the user has supplied the password
interactively.
            (host  (substring file (or (match-end 2)
                                       (+ 3 (match-end 1))))))

-       (if (not pass)
-           (when rmail-remote-password-required
-             (setq got-password (not (rmail-have-password)))
-             (setq supplied-password (rmail-get-remote-password
-                                      (string-equal proto "imap"))))
-         ;; The password is embedded.  Strip it out since movemail
-         ;; does not really like it, in spite of the movemail spec.
-         (setq file (concat proto "://" user "@" host)))
+       (if (rmail-remote-proto-p proto)
+           (if (not pass)
+               (when rmail-remote-password-required
+                 (setq got-password (not (rmail-have-password)))
+                 (setq supplied-password (rmail-get-remote-password
+                                          (string-match "imaps?" proto))))
+             ;; FIXME
+             ;; The password is embedded.  Strip it out since movemail
+             ;; does not really like it, in spite of the movemail spec.
+             (setq file (concat proto "://" user "@" host))))

        (if (rmail-movemail-variant-p 'emacs)
            (if (string-equal proto "pop")
                (list (concat "po:" user ":" host)
-                     t
+                     proto
                      (or pass supplied-password)
                      got-password)
              (error "Emacs movemail does not support %s protocol" proto))
          (list file
-               (or (string-equal proto "pop") (string-equal proto "imap"))
+               proto
                (or supplied-password pass)
                got-password))))

@@ -1971,18 +1976,18 @@ Value is the size of the newly read mail after
conversion."
   (or (memq (file-locked-p buffer-file-name) '(nil t))
       (error "RMAIL file %s is locked"
             (file-name-nondirectory buffer-file-name)))
-  (let (file tofile delete-files movemail popmail got-password password)
+  (let (file tofile delete-files movemail proto got-password password)
     (while files
       ;; Handle remote mailbox names specially; don't expand as filenames
       ;; in case the userid contains a directory separator.
       (setq file (car files))
       (let ((url-data (rmail-parse-url file)))
        (setq file (nth 0 url-data))
-       (setq popmail (nth 1 url-data))
+       (setq proto (nth 1 url-data))
        (setq password (nth 2 url-data))
        (setq got-password (nth 3 url-data)))

-      (if popmail
+      (if proto
          (setq renamep t)
        (setq file (file-truename
                    (substitute-in-file-name (expand-file-name file)))))
@@ -2003,14 +2008,16 @@ Value is the size of the newly read mail after
conversion."
                     (expand-file-name buffer-file-name))))
       ;; Always use movemail to rename the file,
       ;; since there can be mailboxes in various directories.
-      (when (not popmail)
+      (when (not proto)
        ;; On some systems, /usr/spool/mail/foo is a directory
        ;; and the actual inbox is /usr/spool/mail/foo/foo.
        (if (file-directory-p file)
            (setq file (expand-file-name (user-login-name)
                                         file))))
-      (cond (popmail
-            (message "Getting mail from the remote server ..."))
+      (cond (proto
+            (message (if (rmail-remote-proto-p proto)
+                         "Getting mail from the remote server ..."
+                       (concat "Getting mail from " proto))))
            ((and (file-exists-p tofile)
                  (/= 0 (nth 7 (file-attributes tofile))))
             (message "Getting mail from %s..." tofile))
@@ -2021,7 +2028,7 @@ Value is the size of the newly read mail after
conversion."
       ;; rename or copy the file FILE to TOFILE if and as appropriate.
       (cond ((not renamep)
             (setq tofile file))
-           ((or (file-exists-p tofile) (and (not popmail)
+           ((or (file-exists-p tofile) (and (not proto)
                                             (not (file-exists-p file))))
             nil)
            (t
@@ -2056,9 +2063,10 @@ Value is the size of the newly read mail after
conversion."
                   ;; If we just read the password, most likely it is
                   ;; wrong.  Otherwise, see if there is a specific
                   ;; reason to think that the problem is a wrong passwd.
-                  (if (or got-password
-                          (re-search-forward rmail-remote-password-error
-                                             nil t))
+                  (if (and (rmail-remote-proto-p proto)
+                           (or got-password
+                               (re-search-forward
rmail-remote-password-error
+                                                  nil t)))
                       (rmail-set-remote-password nil))

                   ;; If using Mailutils, remove initial error code
--
1.7.12.1

[-- Attachment #2: Type: text/html, Size: 10649 bytes --]

bug#18278: [PATCH] bug#18130: rmail.el: handle imaps, pops, and local source mailboxes (maildir, MH, etc.)

[Stefan Monnier]

[ Thanks Carlos for pinging us about it.  ]

> set of mailbox formats (both remote and local).  This patch makes
> rmail.el fully use the flexibility of GNU Mailutils.  In particular,
> it makes it possible to use imaps and pops encrypted protocols.  The
> patch is against the current Emacs trunk.  The bug has been reported by
> Carlos Pita to both GNU Emacs[2] and Mailutuls[3].

Thank you Sergey,

The patch looks OK for me, but as I'm not familiar with Rmail, I'd like
to have the opinion of someone else.


        Stefan

bug#18278: [PATCH] bug#18130: rmail.el: handle imaps, pops, and local source mailboxes (maildir, MH, etc.)

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Date: Mon, 08 Sep 2014 09:00:22 -0400
> Cc: Carlos Pita <carlosjosepita@gmail.com>, bug-mailutils@gnu.org,
> 	18278@debbugs.gnu.org
> 
> [ Thanks Carlos for pinging us about it.  ]
> 
> > set of mailbox formats (both remote and local).  This patch makes
> > rmail.el fully use the flexibility of GNU Mailutils.  In particular,
> > it makes it possible to use imaps and pops encrypted protocols.  The
> > patch is against the current Emacs trunk.  The bug has been reported by
> > Carlos Pita to both GNU Emacs[2] and Mailutuls[3].
> 
> Thank you Sergey,
> 
> The patch looks OK for me, but as I'm not familiar with Rmail, I'd like
> to have the opinion of someone else.

I do use Rmail, but not movemail from Mailutils, and not for imap and
other modern protocols.

So the only issue that bothers me here is: would the new code still
work with the old movemail, and how does the code detect whether it
invokes old or new one?

If this change is 100% backwards compatible, I see no reasons not to
install it.

bug#18278: [PATCH] bug#18130: rmail.el: handle imaps, pops, and local source mailboxes (maildir, MH, etc.)

[Carlos Pita]

Hi Eli,

> So the only issue that bothers me here is: would the new code still
> work with the old movemail, and how does the code detect whether it
> invokes old or new one?

please take into account that this is mainly a bug fix (see bug report
#18130, which originated this new report and corresponding patch). It's
not an improvement on or an enhancement of the "old movemail". Currently
the emacs documentation on rmail states that it must work with the
external movemail and that it must support additional protocols (by
using this external tool). But both assertions are currently false. The
rmail code already supports both movemail backends, indeed, but the
support for the mailutils one is mostly broken.

Cheers
--
Carlos

bug#18278: [PATCH] bug#18130: rmail.el: handle imaps, pops, and local source mailboxes (maildir, MH, etc.)

[Eli Zaretskii]

> From: Carlos Pita <carlosjosepita@gmail.com>
> Cc: Stefan Monnier <monnier@IRO.UMontreal.CA>,  gray@gnu.org,  bug-mailutils@gnu.org,  18278@debbugs.gnu.org
> Date: Tue, 09 Sep 2014 12:46:08 -0300
> 
> > So the only issue that bothers me here is: would the new code still
> > work with the old movemail, and how does the code detect whether it
> > invokes old or new one?
> 
> please take into account that this is mainly a bug fix (see bug report
> #18130, which originated this new report and corresponding patch). It's
> not an improvement on or an enhancement of the "old movemail". Currently
> the emacs documentation on rmail states that it must work with the
> external movemail and that it must support additional protocols (by
> using this external tool). But both assertions are currently false. The
> rmail code already supports both movemail backends, indeed, but the
> support for the mailutils one is mostly broken.

Yes, I understand all that (I've read the patch).  But I don't think a
bugfix necessarily avoids the danger of breaking backwards
compatibility, which is why I asked.  I think my question still
stands.

bug#18278: [PATCH] bug#18130: rmail.el: handle imaps, pops, and local source mailboxes (maildir, MH, etc.)

[Glenn Morris]

Thank you for the patch. I am sorry it was forgotten about.
Finally applied as 48536f6.