gnus-cloud.el

gnus-cloud.el

[David Edmondson]

After some time away, I'm playing with Gnus again. In this instance,
gnus-cloud isn't working for me.

Specifically, I can add a gnus-cloud storage server (on nnimap) and
register another server (nntp) with it. ~u causes messages to be added
to Emacs-Cloud and they appear to have the right kind of content.

~d on another machine never does anything useful (e.g. mark some
articles as read) because this test:

              (if (and newer (not force-older))
                (gnus-message 3 "Skipping outdated cloud info for group %s, the info is from %s (now is %s)" group date now)

always returns true.

Looking at how `newer' is calculated, it compares the group timestamp
from the gnus-cloud message with the current time. It's not clear to me
how that test is ever going to return `nil', unless I have a version of
emacs running somewhere in the future that updates gnus-cloud.

Forcing `newer' to `nil' for debugging does indeed cause the local gnus
to pay attention to changes from the cloud.

Is this all expected to be working at the moment? The logic has not
changed for quite some time (I have the patch that Paul Eggert made a
couple of weeks ago - it doesn't seem directly relevant).

dme.
-- 
I was better off when I was on your side, and I was holding on.

Re: gnus-cloud.el

[Eric Abrahamsen]

David Edmondson <dme@dme.org> writes:

> After some time away, I'm playing with Gnus again. In this instance,
> gnus-cloud isn't working for me.
>
> Specifically, I can add a gnus-cloud storage server (on nnimap) and
> register another server (nntp) with it. ~u causes messages to be added
> to Emacs-Cloud and they appear to have the right kind of content.
>
> ~d on another machine never does anything useful (e.g. mark some
> articles as read) because this test:
>
>               (if (and newer (not force-older))
>                 (gnus-message 3 "Skipping outdated cloud info for group %s, the info is from %s (now is %s)" group date now)
>
> always returns true.
>
> Looking at how `newer' is calculated, it compares the group timestamp
> from the gnus-cloud message with the current time. It's not clear to me
> how that test is ever going to return `nil', unless I have a version of
> emacs running somewhere in the future that updates gnus-cloud.

It's true that doesn't make a lot of sense. Presumably we should be
checking against some value from `gnus-cloud-file-timestamps' instead.

And now that there's a iso8601 library, it would also be nice to use
that along with `time-less-p', instead of these string comparisons.

> Forcing `newer' to `nil' for debugging does indeed cause the local gnus
> to pay attention to changes from the cloud.
>
> Is this all expected to be working at the moment? The logic has not
> changed for quite some time (I have the patch that Paul Eggert made a
> couple of weeks ago - it doesn't seem directly relevant).

I once installed gnus-cloud, but I don't think I ever tried to use it.
I've never really looked into how it's supposed to work.

Eric

Re: gnus-cloud.el

[David Edmondson]

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

On Saturday, 2020-03-28 at 10:19:11 -07, Eric Abrahamsen wrote:

> David Edmondson <dme@dme.org> writes:
>
>> After some time away, I'm playing with Gnus again. In this instance,
>> gnus-cloud isn't working for me.
>>
>> Specifically, I can add a gnus-cloud storage server (on nnimap) and
>> register another server (nntp) with it. ~u causes messages to be added
>> to Emacs-Cloud and they appear to have the right kind of content.
>>
>> ~d on another machine never does anything useful (e.g. mark some
>> articles as read) because this test:
>>
>>               (if (and newer (not force-older))
>>                 (gnus-message 3 "Skipping outdated cloud info for group %s, the info is from %s (now is %s)" group date now)
>>
>> always returns true.
>>
>> Looking at how `newer' is calculated, it compares the group timestamp
>> from the gnus-cloud message with the current time. It's not clear to me
>> how that test is ever going to return `nil', unless I have a version of
>> emacs running somewhere in the future that updates gnus-cloud.
>
> It's true that doesn't make a lot of sense. Presumably we should be
> checking against some value from `gnus-cloud-file-timestamps' instead.

I'm not syncing any files with gnus-cloud, so I'm not sure how that
might help.

In general I would have expected to replay any changes with a higher
sequence number, irrespective of the date of the changes. That they
happened in the past is somewhat the point, isn't it? :-)

Another problem is that after a set of Emacs-Cloud messages have been
used to replay, the emacs instance doing that replay doesn't persist the
last seen sequence value as `gnus-cloud-sequence', so the next time it
attempts to replay it will unnecessarily replay the same sequence
numbered messages again.

Anyway, attached is a suggested patch. This makes things work for me in
some limited testing. Comments welcome!

> And now that there's a iso8601 library, it would also be nice to use
> that along with `time-less-p', instead of these string comparisons.
>
>> Forcing `newer' to `nil' for debugging does indeed cause the local gnus
>> to pay attention to changes from the cloud.
>>
>> Is this all expected to be working at the moment? The logic has not
>> changed for quite some time (I have the patch that Paul Eggert made a
>> couple of weeks ago - it doesn't seem directly relevant).
>
> I once installed gnus-cloud, but I don't think I ever tried to use it.
> I've never really looked into how it's supposed to work.
>
> Eric

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnus-cloud-Improve-cloud-sync.patch --]
[-- Type: text/x-patch, Size: 6196 bytes --]

From a3bdd2219855a72e2ee90de1b6f823c9678a470c Mon Sep 17 00:00:00 2001
From: David Edmondson <dme@dme.org>
Date: Sat, 28 Mar 2020 19:03:58 +0000
Subject: [PATCH] gnus-cloud: Improve cloud sync

After replaying a set of actions downloaded by gnus-cloud, persist the
highest sequence number seen as the local `gnus-cloud-sequence'
number, in order that a future download will not unnecessarily replay
previously seen actions and any future uploads from this emacs
instance use a higher sequence number than that downloaded.

Remove the test on whether individual newsrc entries are older than
the current time, as that is always going to be the case.
---
 lisp/gnus/gnus-cloud.el | 54 ++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/lisp/gnus/gnus-cloud.el b/lisp/gnus/gnus-cloud.el
index da6231d73300..7ea691e7220c 100644
--- a/lisp/gnus/gnus-cloud.el
+++ b/lisp/gnus/gnus-cloud.el
@@ -223,13 +223,10 @@ easy interactive way to set this from the Server buffer."
        (t
         (gnus-message 1 "Unknown type %s; ignoring" type))))))
 
-(defun gnus-cloud-update-newsrc-data (group elem &optional force-older)
-  "Update the newsrc data for GROUP from ELEM.
-Use old data if FORCE-OLDER is not nil."
+(defun gnus-cloud-update-newsrc-data (group elem)
+  "Update the newsrc data for GROUP from ELEM."
   (let* ((contents (plist-get elem :contents))
          (date (or (plist-get elem :timestamp) "0"))
-         (now (gnus-cloud-timestamp nil))
-         (newer (string-lessp date now))
          (group-info (gnus-get-info group)))
     (if (and contents
              (stringp (nth 0 contents))
@@ -238,15 +235,13 @@ Use old data if FORCE-OLDER is not nil."
             (if (equal (format "%S" group-info)
                        (format "%S" contents))
                 (gnus-message 3 "Skipping cloud update of group %s, the info is the same" group)
-              (if (and newer (not force-older))
-                (gnus-message 3 "Skipping outdated cloud info for group %s, the info is from %s (now is %s)" group date now)
-                (when (or (not gnus-cloud-interactive)
-                          (gnus-y-or-n-p
-                           (format "%s has older different info in the cloud as of %s, update it here? "
-				   group date)))
-		  (gnus-message 2 "Installing cloud update of group %s" group)
-		  (gnus-set-info group contents)
-		  (gnus-group-update-group group))))
+              (when (or (not gnus-cloud-interactive)
+			(gnus-y-or-n-p
+			 (format "%s has different info in the cloud from %s, update it here? "
+				 group date)))
+		(gnus-message 2 "Installing cloud update of group %s" group)
+		(gnus-set-info group contents)
+		(gnus-group-update-group group)))
           (gnus-error 1 "Sorry, group %s is not subscribed" group))
       (gnus-error 1 "Sorry, could not update newsrc for group %s (invalid data %S)"
                   group elem))))
@@ -380,8 +375,9 @@ When FULL is t, upload everything, not just a difference from the last full."
                   (gnus-cloud-files-to-upload full)
                   (gnus-cloud-collect-full-newsrc)))
           (group (gnus-group-full-name gnus-cloud-group-name gnus-cloud-method)))
+      (setq gnus-cloud-sequence (1+ (or gnus-cloud-sequence 0)))
       (insert (format "Subject: (sequence: %s type: %s storage-method: %s)\n"
-                      (or gnus-cloud-sequence "UNKNOWN")
+                      gnus-cloud-sequence
                       (if full :full :partial)
                       gnus-cloud-storage-method))
       (insert "From: nobody@gnus.cloud.invalid\n")
@@ -390,7 +386,6 @@ When FULL is t, upload everything, not just a difference from the last full."
       (if (gnus-request-accept-article gnus-cloud-group-name gnus-cloud-method
                                        t t)
           (progn
-            (setq gnus-cloud-sequence (1+ (or gnus-cloud-sequence 0)))
             (gnus-cloud-add-timestamps elems)
             (gnus-message 3 "Uploaded Gnus Cloud data successfully to %s" group)
             (gnus-group-refresh-group group))
@@ -459,18 +454,21 @@ instead of `gnus-cloud-sequence'.
 When UPDATE is t, returns the result of calling `gnus-cloud-update-all'.
 Otherwise, returns the Gnus Cloud data chunks."
   (let ((articles nil)
+	(highest-sequence-seen gnus-cloud-sequence)
         chunks)
     (dolist (header (gnus-cloud-available-chunks))
-      (when (> (gnus-cloud-chunk-sequence (mail-header-subject header))
-               (or sequence-override gnus-cloud-sequence -1))
-
-        (if (string-match (format "storage-method: %s" gnus-cloud-storage-method)
-                          (mail-header-subject header))
-            (push (mail-header-number header) articles)
-          (gnus-message 1 "Skipping article %s because it didn't match the Gnus Cloud method %s: %s"
-                        (mail-header-number header)
-                        gnus-cloud-storage-method
-                        (mail-header-subject header)))))
+      (let ((this-sequence (gnus-cloud-chunk-sequence (mail-header-subject header))))
+	(when (> this-sequence (or sequence-override gnus-cloud-sequence -1))
+
+	  (if (string-match (format "storage-method: %s" gnus-cloud-storage-method)
+			    (mail-header-subject header))
+	      (progn
+		(push (mail-header-number header) articles)
+		(setq highest-sequence-seen (max highest-sequence-seen this-sequence)))
+	    (gnus-message 1 "Skipping article %s because it didn't match the Gnus Cloud method %s: %s"
+			  (mail-header-number header)
+			  gnus-cloud-storage-method
+			  (mail-header-subject header))))))
     (when articles
       (nnimap-request-articles (nreverse articles) gnus-cloud-group-name)
       (with-current-buffer nntp-server-buffer
@@ -480,7 +478,9 @@ Otherwise, returns the Gnus Cloud data chunks."
           (push (gnus-cloud-parse-chunk) chunks)
           (forward-line 1))))
     (if update
-        (mapcar #'gnus-cloud-update-all chunks)
+        (progn
+	  (mapcar #'gnus-cloud-update-all chunks)
+	  (setq gnus-cloud-sequence highest-sequence-seen))
       chunks)))
 
 (defun gnus-cloud-server-p (server)
-- 
2.25.1


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


dme.
-- 
So clap your hands together, make beats with cutlery.

Re: gnus-cloud.el

[황병희]

Hellow David^^^

David Edmondson <dme@dme.org> writes:

> [......]
> From a3bdd2219855a72e2ee90de1b6f823c9678a470c Mon Sep 17 00:00:00 2001
> From: David Edmondson <dme@dme.org>
> Date: Sat, 28 Mar 2020 19:03:58 +0000
> Subject: [PATCH] gnus-cloud: Improve cloud sync
>
> After replaying a set of actions downloaded by gnus-cloud, persist the
> highest sequence number seen as the local `gnus-cloud-sequence'
> number, in order that a future download will not unnecessarily replay
> previously seen actions and any future uploads from this emacs
> instance use a higher sequence number than that downloaded.
>
> Remove the test on whether individual newsrc entries are older than
> the current time, as that is always going to be the case.
> ---
>  lisp/gnus/gnus-cloud.el | 54 ++++++++++++++++++++---------------------
>  1 file changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/lisp/gnus/gnus-cloud.el b/lisp/gnus/gnus-cloud.el
> index da6231d73300..7ea691e7220c 100644
> --- a/lisp/gnus/gnus-cloud.el
> +++ b/lisp/gnus/gnus-cloud.el
> @@ -223,13 +223,10 @@ easy interactive way to set this from the Server buffer."
>         (t
>          (gnus-message 1 "Unknown type %s; ignoring" type))))))
>  
> -(defun gnus-cloud-update-newsrc-data (group elem &optional force-older)
> -  "Update the newsrc data for GROUP from ELEM.
> -Use old data if FORCE-OLDER is not nil."
> +(defun gnus-cloud-update-newsrc-data (group elem)
> +  "Update the newsrc data for GROUP from ELEM."
>    (let* ((contents (plist-get elem :contents))
>           (date (or (plist-get elem :timestamp) "0"))
> -         (now (gnus-cloud-timestamp nil))
> -         (newer (string-lessp date now))
>           (group-info (gnus-get-info group)))
>      (if (and contents
>               (stringp (nth 0 contents))
> @@ -238,15 +235,13 @@ Use old data if FORCE-OLDER is not nil."
>              (if (equal (format "%S" group-info)
>                         (format "%S" contents))
>                  (gnus-message 3 "Skipping cloud update of group %s, the info is the same" group)
> -              (if (and newer (not force-older))
> -                (gnus-message 3 "Skipping outdated cloud info for group %s, the info is from %s (now is %s)" group date now)
> -                (when (or (not gnus-cloud-interactive)
> -                          (gnus-y-or-n-p
> -                           (format "%s has older different info in the cloud as of %s, update it here? "
> -				   group date)))
> -		  (gnus-message 2 "Installing cloud update of group %s" group)
> -		  (gnus-set-info group contents)
> -		  (gnus-group-update-group group))))
> +              (when (or (not gnus-cloud-interactive)
> +			(gnus-y-or-n-p
> +			 (format "%s has different info in the cloud from %s, update it here? "
> +				 group date)))
> +		(gnus-message 2 "Installing cloud update of group %s" group)
> +		(gnus-set-info group contents)
> +		(gnus-group-update-group group)))
>            (gnus-error 1 "Sorry, group %s is not subscribed" group))
>        (gnus-error 1 "Sorry, could not update newsrc for group %s (invalid data %S)"
>                    group elem))))
> @@ -380,8 +375,9 @@ When FULL is t, upload everything, not just a difference from the last full."
>                    (gnus-cloud-files-to-upload full)
>                    (gnus-cloud-collect-full-newsrc)))
>            (group (gnus-group-full-name gnus-cloud-group-name gnus-cloud-method)))
> +      (setq gnus-cloud-sequence (1+ (or gnus-cloud-sequence 0)))
>        (insert (format "Subject: (sequence: %s type: %s storage-method: %s)\n"
> -                      (or gnus-cloud-sequence "UNKNOWN")
> +                      gnus-cloud-sequence
>                        (if full :full :partial)
>                        gnus-cloud-storage-method))
>        (insert "From: nobody@gnus.cloud.invalid\n")
> @@ -390,7 +386,6 @@ When FULL is t, upload everything, not just a difference from the last full."
>        (if (gnus-request-accept-article gnus-cloud-group-name gnus-cloud-method
>                                         t t)
>            (progn
> -            (setq gnus-cloud-sequence (1+ (or gnus-cloud-sequence 0)))
>              (gnus-cloud-add-timestamps elems)
>              (gnus-message 3 "Uploaded Gnus Cloud data successfully to %s" group)
>              (gnus-group-refresh-group group))
> @@ -459,18 +454,21 @@ instead of `gnus-cloud-sequence'.
>  When UPDATE is t, returns the result of calling `gnus-cloud-update-all'.
>  Otherwise, returns the Gnus Cloud data chunks."
>    (let ((articles nil)
> +	(highest-sequence-seen gnus-cloud-sequence)
>          chunks)
>      (dolist (header (gnus-cloud-available-chunks))
> -      (when (> (gnus-cloud-chunk-sequence (mail-header-subject header))
> -               (or sequence-override gnus-cloud-sequence -1))
> -
> -        (if (string-match (format "storage-method: %s" gnus-cloud-storage-method)
> -                          (mail-header-subject header))
> -            (push (mail-header-number header) articles)
> -          (gnus-message 1 "Skipping article %s because it didn't match the Gnus Cloud method %s: %s"
> -                        (mail-header-number header)
> -                        gnus-cloud-storage-method
> -                        (mail-header-subject header)))))
> +      (let ((this-sequence (gnus-cloud-chunk-sequence (mail-header-subject header))))
> +	(when (> this-sequence (or sequence-override gnus-cloud-sequence -1))
> +
> +	  (if (string-match (format "storage-method: %s" gnus-cloud-storage-method)
> +			    (mail-header-subject header))
> +	      (progn
> +		(push (mail-header-number header) articles)
> +		(setq highest-sequence-seen (max highest-sequence-seen this-sequence)))
> +	    (gnus-message 1 "Skipping article %s because it didn't match the Gnus Cloud method %s: %s"
> +			  (mail-header-number header)
> +			  gnus-cloud-storage-method
> +			  (mail-header-subject header))))))
>      (when articles
>        (nnimap-request-articles (nreverse articles) gnus-cloud-group-name)
>        (with-current-buffer nntp-server-buffer
> @@ -480,7 +478,9 @@ Otherwise, returns the Gnus Cloud data chunks."
>            (push (gnus-cloud-parse-chunk) chunks)
>            (forward-line 1))))
>      (if update
> -        (mapcar #'gnus-cloud-update-all chunks)
> +        (progn
> +	  (mapcar #'gnus-cloud-update-all chunks)
> +	  (setq gnus-cloud-sequence highest-sequence-seen))
>        chunks)))
>  
>  (defun gnus-cloud-server-p (server)

Sorry man i don't know emacs lisp at all.
By the way the your codes are looks beautiful^^^

Sincerely, Gnus fan Byung-Hee

-- 
^고맙습니다 _白衣從軍_ 감사합니다_^))//

Re: gnus-cloud.el

[Eric Abrahamsen]

David Edmondson <dme@dme.org> writes:

> On Saturday, 2020-03-28 at 10:19:11 -07, Eric Abrahamsen wrote:
>
>> David Edmondson <dme@dme.org> writes:
>>
>>> After some time away, I'm playing with Gnus again. In this instance,
>>> gnus-cloud isn't working for me.
>>>
>>> Specifically, I can add a gnus-cloud storage server (on nnimap) and
>>> register another server (nntp) with it. ~u causes messages to be added
>>> to Emacs-Cloud and they appear to have the right kind of content.
>>>
>>> ~d on another machine never does anything useful (e.g. mark some
>>> articles as read) because this test:
>>>
>>>               (if (and newer (not force-older))
>>>                 (gnus-message 3 "Skipping outdated cloud info for group %s, the info is from %s (now is %s)" group date now)
>>>
>>> always returns true.
>>>
>>> Looking at how `newer' is calculated, it compares the group timestamp
>>> from the gnus-cloud message with the current time. It's not clear to me
>>> how that test is ever going to return `nil', unless I have a version of
>>> emacs running somewhere in the future that updates gnus-cloud.
>>
>> It's true that doesn't make a lot of sense. Presumably we should be
>> checking against some value from `gnus-cloud-file-timestamps' instead.
>
> I'm not syncing any files with gnus-cloud, so I'm not sure how that
> might help.

Oh, I had assumed that one of the files in that variable would be
.newsrc.eld, and that timestamp would be the relevant one. But you're
right -- this ought to be what `gnus-cloud-sequence' is for.

> In general I would have expected to replay any changes with a higher
> sequence number, irrespective of the date of the changes. That they
> happened in the past is somewhat the point, isn't it? :-)
>
> Another problem is that after a set of Emacs-Cloud messages have been
> used to replay, the emacs instance doing that replay doesn't persist the
> last seen sequence value as `gnus-cloud-sequence', so the next time it
> attempts to replay it will unnecessarily replay the same sequence
> numbered messages again.
>
> Anyway, attached is a suggested patch. This makes things work for me in
> some limited testing. Comments welcome!

I encourage you to open a bug report and put this patch on it. As
demonstrated, I don't know this code at all, and probably won't have
time to go through and learn it in the next week or so -- a bug report
is more likely to be seen by Lars or Ted (who I think wrote this
originally, I may be wrong).

Yours,
Eric