gnus-sync.el patch and notes

gnus-sync.el patch and notes

[Matt Ford]

Hi,

I've been trying to use the gnus-sync.el to, well, keep things in sync.
And after much to and fro I've got it working and I've even made a
partial patch for the bit I'm interested in (the file based sync).

Firstly, there seems to be a bug in the both the couchdb and the file
base gnus-sync-read code.  Both methods of syncing assume that a group
node in the back-end will match the structure of a group node in the
active gnus-newsrc-alist.  Not true!  If you have no marks or ticks in
the active group node then you have a structure like this

("gwene.net.debian.planet" 3 ((1 . 5491)))

and you could easily have a group node looking like this in the sync
backend

("gwene.net.debian.planet" 3
 ((1 . 5491))
 ((seen
   (5287 . 5491))
  (tick 5460)))

The (setf (nth X ...) type merging won't work as the tail element
doesn't exist in the active groups gnus-newsrc-alist entry.  You get
sync failures along the lines of invalid type listp or consp.

In the end, I decided the best bet was to remove the entry if it exists
from the existing gnus-newsrc-alist and push the backend entry into
gnus-newsrc-alist.  i.e., just replace it.  Whilst I was at it I decided
that invalid groups were actually missing groups and I pop those into
the hash also.

> As an aside the merge code for the file based syncing looks wrong to
> me even if the structures are assumed to be the same.  The loop over
> the store needs an integer counter to use as the index for the
> `(nth...)'  function not `(car store)'.

Other things I've learned:

 (setq gnus-sync-backend "/ssh:home:lesync/gnus-sync" 
            gnus-sync-global-vars '(gnus-newsrc-last-checked-date
                              gnus-topic-alist
                              gnus-topic-topology
                              gnus-category-alist
                              )
      gnus-sync-newsrc-groups '("gwene" "gmane"))

- The global variables to sync as listed above are all pretty safe.  It is
  certainly not safe to use (gnus-newsrc-alist) as I think was previously
  suggested in the comment docs (especially if you are using offine imap
  and slave imap servers).  What's more it clashes with the
  gnus-sync-newsrc-groups functionality as both operate on the same data.

- The actual functionality is simple enough the backend file is read in
  as a lisp buffer and is essentially executed as is.  Global-var are
  over-written and the gnus-sync-newsrc-groups are merged and added.

- Let IMAP manage IMAP syncing don't use this stuff for that.  Obvious
  in hindsight.

Hope this helps some people.

Cheers!


--8<---------------cut here---------------start------------->8---
diff --git a/site-elisp/gnus/gnus-sync.el b/site-elisp/gnus/gnus-sync.el
index 7e13b88..eb11676 100644
--- a/site-elisp/gnus/gnus-sync.el
+++ b/site-elisp/gnus/gnus-sync.el
@@ -843,31 +843,33 @@ With a prefix, SUBSCRIBE-ALL is set and unknown groups will be subscribed."
             (load gnus-sync-backend nil t)
           (error
            (error "Error in %s: %s" gnus-sync-backend (cadr var)))))
-      (let ((valid-count 0)
-            invalid-groups)
+      (let ((count 0)
+            missing-groups)
         (dolist (node gnus-sync-newsrc-loader)
           (if (gnus-gethash (car node) gnus-newsrc-hashtb)
               (progn
-                (incf valid-count)
-                (loop for store in (cdr node)
-                      do (setf (nth (car store)
-                                    (assoc (car node) gnus-newsrc-alist))
-                               (cdr store))))
-            (push (car node) invalid-groups)))
+                (incf count)
+                (setq gnus-newsrc-alist
+                      (delq (assoc (car node) gnus-newsrc-alist)
+                            gnus-newsrc-alist))
+                (push node gnus-newsrc-alist))  
+            (progn
+              (push (car node) missing-groups)
+              (push node gnus-newsrc-alist))))
         (gnus-message
          7
-         "gnus-sync-read: loaded %d groups (out of %d) from %s"
-         valid-count (length gnus-sync-newsrc-loader)
+         "gnus-sync-read: loaded %d present groups (out of %d) from %s"
+         count (length gnus-sync-newsrc-loader)
          gnus-sync-backend)
-        (when invalid-groups
+        (when missing-groups
           (gnus-message
            7
-           "gnus-sync-read: skipped %d groups (out of %d) from %s"
-           (length invalid-groups)
+           "gnus-sync-read: added missing %d groups (out of %d) from %s"
+           (length missing-groups)
            (length gnus-sync-newsrc-loader)
            gnus-sync-backend)
-          (gnus-message 9 "gnus-sync-read: skipped groups: %s"
-                        (mapconcat 'identity invalid-groups ", ")))))
+          (gnus-message 9 "gnus-sync-read: missing groups: %s"
+                        (mapconcat 'identity missing-groups ", ")))))
      (nil))
 
     (gnus-message 9 "gnus-sync-read: remaking the newsrc hashtable") 
--8<---------------cut here---------------end--------------->8---

-- 
Matt

p.s., Gnus and it's developers rock - a big thank-you for all your
effort. :-)

Re: gnus-sync.el patch and notes

[Ted Zlatanov]

On Mon, 16 Jul 2012 23:18:51 +0100 Matt Ford <matt@dancingfrog.co.uk> wrote: 

MF> I've been trying to use the gnus-sync.el to, well, keep things in sync.
MF> And after much to and fro I've got it working and I've even made a
MF> partial patch for the bit I'm interested in (the file based sync).

MF> Firstly, there seems to be a bug in the both the couchdb and the file
MF> base gnus-sync-read code.  Both methods of syncing assume that a group
MF> node in the back-end will match the structure of a group node in the
MF> active gnus-newsrc-alist.  Not true!  If you have no marks or ticks in
MF> the active group node then you have a structure like this

Matt, thanks for catching this.  I appreciate the time you put into it.

...

>> As an aside the merge code for the file based syncing looks wrong to
>> me even if the structures are assumed to be the same.  The loop over
>> the store needs an integer counter to use as the index for the
>> `(nth...)'  function not `(car store)'.

I'm cool with any fixes you propose for this.  My time is extremely
limited due to external commitments, so I would appreciate it if you put
a patch together for these two issues, plus some documentation changes
to mention the following:

MF> Other things I've learned:

MF>  (setq gnus-sync-backend "/ssh:home:lesync/gnus-sync" 
MF>             gnus-sync-global-vars '(gnus-newsrc-last-checked-date
MF>                               gnus-topic-alist
MF>                               gnus-topic-topology
MF>                               gnus-category-alist
MF>                               )
MF>       gnus-sync-newsrc-groups '("gwene" "gmane"))

MF> - The global variables to sync as listed above are all pretty safe.  It is
MF>   certainly not safe to use (gnus-newsrc-alist) as I think was previously
MF>   suggested in the comment docs (especially if you are using offine imap
MF>   and slave imap servers).  What's more it clashes with the
MF>   gnus-sync-newsrc-groups functionality as both operate on the same data.

MF> - The actual functionality is simple enough the backend file is read in
MF>   as a lisp buffer and is essentially executed as is.  Global-var are
MF>   over-written and the gnus-sync-newsrc-groups are merged and added.

MF> - Let IMAP manage IMAP syncing don't use this stuff for that.  Obvious
MF>   in hindsight.

I'll commit your patch, with or without the suggested changes.  Let me
know.

Lars was thinking about using IMAP as the sync backend instead of
CouchDB and after many months of heavy usage I have to agree.  CouchDB
is very good to get started and has a good interface, but for pure Lisp
data and specifically for synchronization it's not as good as a native
Lisp interface.  The big reason is that Lisp data doesn't map cleanly
into JSON.  So gnus-sync.el may be changing again... when Lars or I get
around to it...  but it's definitely been useful to me and many others,
so it's not going away.

Ted

Re: gnus-sync.el patch and notes

[Steinar Bang]

>>>>> Ted Zlatanov <tzz@lifelogs.com>:

> On Mon, 16 Jul 2012 23:18:51 +0100 Matt Ford <matt@dancingfrog.co.uk> wrote: 

MF> Firstly, there seems to be a bug in the both the couchdb and the file
MF> base gnus-sync-read code.  Both methods of syncing assume that a group
MF> node in the back-end will match the structure of a group node in the
MF> active gnus-newsrc-alist.  Not true!  If you have no marks or ticks in
MF> the active group node then you have a structure like this

Hm... will this fix the quirkiness that sometimes happens when using
gnus-sync-read?  If so, I would definitely like to see this go in.

[snip!]
> I'll commit your patch, with or without the suggested changes.  Let me
> know.

Has it been committed?  Doesn't look like it from the top of the gnus
git history...?

Re: gnus-sync.el patch and notes

[Matt Ford]

Steinar Bang <sb@dod.no> writes:

> Hm... will this fix the quirkiness that sometimes happens when using
> gnus-sync-read?  If so, I would definitely like to see this go in.

It will fix it for syncing to a file - I have a much better time of it
now.  I might even put the read hooks back into place for my setup!

The patch doesn't address the issue if you use couchdb.  I can point you
to the bit that needs fixing.

In the following function:

> (defun gnus-sync-lesync-install-group-entry (name)
> [...]
>           ;; install the subscription level
>           (when (gnus-sync-lesync-get-prop 'level name)
>             (setf (nth 1 master) (gnus-sync-lesync-get-prop 'level name)))
>           ;; install the read and other marks
>           (setf (nth 2 master) (gnus-sync-lesync-get-prop 'read name))
>           (setf (nth 3 master) (gnus-sync-lesync-get-prop 'marks name))
>           (gnus-sync-lesync-set-prop 'checksum
>                                      name
>                                      (gnus-sync-deep-print master))
>           nil)
>       (gnus-error 1 =22%s: invalid LeSync group %s=22 loc name)
>       'invalid-name)))

the final `(setf (nth 3 master))' may fail as it might not exist.  A
similar fix to that of the file based solution would be to change this
code to remove the group entry from master and build up a new group
entry from gnus-sync-lesync-get-prop and push back onto master (maybe
you can get the full lesync entry to stop having to rebuild it).

Hope that helps.

-- 
Matt

Re: gnus-sync.el patch and notes

[Steinar Bang]

>>>>> Matt Ford <matt@dancingfrog.co.uk>:

> The patch doesn't address the issue if you use couchdb. 

Bummer!  That's what I use.

> I can point you to the bit that needs fixing.

Ok.  Is that the stuff in the rest of your email?  (ie. the part I
haven't quoted?)  If so, I will study it closer (but not right now).

Thanks!


- Steinar

Re: gnus-sync.el patch and notes

[Ted Zlatanov]

On Mon, 16 Jul 2012 23:18:51 +0100 Matt Ford <matt@dancingfrog.co.uk> wrote: 

MF> I've been trying to use the gnus-sync.el to, well, keep things in sync.
MF> And after much to and fro I've got it working and I've even made a
MF> partial patch for the bit I'm interested in (the file based sync).

Thanks, Matt, and sorry for the delay.  Do you have papers on file?

The patch looks good and I don't see any problems with it.  The delay in
responding was entirely due to external factors.

Ted

Re: gnus-sync.el patch and notes

[Ted Zlatanov]

On Fri, 21 Dec 2012 14:36:09 -0500 Ted Zlatanov <tzz@lifelogs.com> wrote: 

TZ> On Mon, 16 Jul 2012 23:18:51 +0100 Matt Ford <matt@dancingfrog.co.uk> wrote: 
MF> I've been trying to use the gnus-sync.el to, well, keep things in sync.
MF> And after much to and fro I've got it working and I've even made a
MF> partial patch for the bit I'm interested in (the file based sync).

TZ> Thanks, Matt, and sorry for the delay.  Do you have papers on file?

Ping again...

Ted