[PATCH v4] Re: make maxage use UIDs to avoid timezone issues

[Janna Martl]

On Wed, Mar 25, 2015 at 09:52:47PM +0100, Nicolas Sebrecht wrote:
>Could you please tell me if this last patch does it for you? I've put
>the topic here:
>
>  http://github.com/nicolas33/offlineimap.git ns/timezone-v4

More quibbling about the _msgs_to_fetch() part -- I fixed a couple bugs in
your latest version. (Also, you only need two queries if both maxage and
maxsize are enabled.)

 offlineimap/folder/IMAP.py | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/offlineimap/folder/IMAP.py b/offlineimap/folder/IMAP.py
index 25dd26d..dfe9595 100644
--- a/offlineimap/folder/IMAP.py
+++ b/offlineimap/folder/IMAP.py
@@ -204,12 +204,11 @@ class IMAPFolder(BaseFolder):
         if maxsize != None:
             conditions.append("SMALLER %d"% maxsize)
 
-        if len(conditions) > 1:
+        if len(conditions) >= 1:
             # Build SEARCH command.
-            if maxage == None:
-                search_cond = "(%s)"% ' '.join(conditions)
-                search_result = search(search_cond)
-            else:
+            search_cond = "(%s)"% ' '.join(conditions)
+            search_result = search(search_cond)
+            if maxage != None:
                 # Get the messages within maxage is not enough. We want all
                 # messages with UID > min_uid from these within-maxage messages.
                 # We can't rely on maxage only to get the proper min_uid because
@@ -225,17 +224,18 @@ class IMAPFolder(BaseFolder):
                     return None # Nothing to sync.
                 min_msn = min(msg_seq_numbers)
                 # If no maxsize, can just ask for all messages with MSN > min_msn.
-                search_cond = "%d:*"% min_msn
-                if maxsize != None:
+                if maxsize == None:
+                    return "%d:*"% min_msn
+                else:
                     # Restrict the range min_msn:* to those with acceptable size.
                     # Single-quotes prevent imaplib2 from quoting the sequence.
-                    search_cond = "'%s (SMALLER %d)'"% (min_msn, maxsize)
-                # Having to make a second query sucks but this is only for
-                # IMAP/IMAP configurations with maxage enabled. We assume this
-                # is not so common. This time overhead should be acceptable
-                # regarding the benefits introduced by all the avoided sync of
-                # maxage.
-                search_result = search(search_cond)
+                    search_cond = "'%s:* (SMALLER %d)'"% (min_msn, maxsize)
+                    # Having to make a second query sucks but this is only for
+                    # IMAP/IMAP configurations with maxage and maxsize enabled. We
+                    # assume this is not so common. This time overhead should be
+                    # acceptable regarding the benefits introduced by all the avoided
+                    # sync of maxage.
+                    search_result = search(search_cond)
             # Resulting MSN are separated by space, coalesce into ranges
             return imaputil.uid_sequence(search_result)
 
-- 
2.3.4

These were found when I tried to sync an empty Maildir with a
pre-existing IMAP account; this case now works for me with the patch
above. This code is also used in IMAP-to-IMAP sync, which I think is
still broken. I made a new (empty) gmail account and tried to sync it
against a pre-existing one; offlineimap doesn't want to use Gmail as a
local folder type (but does allow IMAP?), and if I force it to, things
start going really badly: IMAP_local is empty, so we fetch a list of
within-maxage messages from IMAP_remote, with UID's N+1 ,..., N+r on the
remote. These get copied to IMAP_local, where they get UID's 1,...,r. I
get a bunch of error messages like

ERROR: IMAP backend cannot change a messages UID from 18272 to 258
<... and then later ...>
ERROR: IMAP backend cannot change a messages UID from 255 to 18316

Traceback:
File "/usr/lib/python2.7/site-packages/offlineimap/folder/Base.py", line 719, in copymessageto
self.change_message_uid(uid, new_uid)
File "/usr/lib/python2.7/site-packages/offlineimap/folder/IMAP.py", line 869, in change_message_uid
'%d to %d'% (uid, new_uid), OfflineImapError.ERROR.MESSAGE)

which makes sense -- IMAP servers probably don't want to randomly
change UIDs of their messages. Now on the next sync, IMAP_local's
list of recent messages starts with UID 1 (the messages that were just
copied), so min_uid = 1, so IMAP_remote's messagelist also starts with
*it's* UID 1, which is not the same as IMAP_remote's UID 1. So you end
up syncing everything in IMAP_remote, even though a lot of it is old.

To be clear, the thing I'm blaming for this is the general strategy of:
(1) get local messages within maxage
(2) min_uid = min(uid's of the messages in (1))
(3) local messagelist = local messages with uid >= min_uid
(4) remote messagelist = remote messages with uid >= min_uid

Even if we take out (3) (which is just there for edge cases), this is
still a problem. General reason why: in the Maildir-IMAP case, both
sides share a consistent notion of UID's. This is not the case for
IMAP-IMAP.


-- J.M.