[PATCHv2 1/3] Re: Simplify the syncing strategy a bit

Nicolas Sebrecht nicolas.s-dev at laposte.net
Sun Mar 6 16:46:26 GMT 2011


On Sun, Mar 06, 2011 at 09:52:01AM +0100, Sebastian Spaeth wrote:
> 
> The previous syncing strategy was doing more than we needed to and was a
> bit underdocumented. This is an attempt to clean it up. Changes 1-3 are
> concerned with changes in accounts.py. Changes 4-5 are implemented in
> folder/Base.py.

Relying on Changeset position is not a good idea, FMPOV. Nothing assure
that git diff are displayed the same for everybody and/or will remain
the same accross git releases.

> 1) Do away with the previous different code paths depending on
> whether there is a LocalStatus file or not (the isnewfolder() test). We
> always use the same strategy now, which makes the strategy easier to underst
> 
> This strategy is simply:
> a) Sync remote to local folder first
> b) Sync local to remote
> 
> Where each sync implies a 4 pass strategy which does basically the same
> as before (explained below).
> 
> 2) Don't delete messages on LOCAL which don't exist on REMOTE right at
> the beginning anymore. This prevented us e.g. from keeping local
> messages rather than redownloading everything once LocalStatus got
> corrupted or deleted. This surprised many who put in an existing local
> maildir and expected it to be synced to the remote place. Instead, the
> local maildir was deleted. This is a data loss that actually occured to
> people!
> 
> 3) No need to separately sync the statusfolder, we update that one
> simultanously with the destfolders...
> 
> 3) Simplified the sync function API by only taking one destdir rather
> than a list of destdirs, we never used more anyway. This makes the code
> easier to read.
> 
> 4) Added plenty of code comments while I was going through to make sure
> the strategy is easy to understand.
> -----------------------------------------
> 
>  Pass1: Transfer new local messages
>         Upload msg with negative/no UIDs to dstfolder. dstfolder should
>         assign that message a new UID. Update statusfolder.
> 
>  Pass2: Copy existing messages
>         Copy messages in self, but not statusfolder to dstfolder if not
>         already in dstfolder. Update statusfolder.
> 
>  Pass3: Remove deleted messages
>         Get all UIDS in statusfolder but not self. These are messages
>         that we have locally deleted. Delete those from dstfolder and
>         statusfolder.
> 
>  Pass4: Synchronize flag changes
>         Compare flags in self with those in statusfolder. If msg has a
>         valid UID and exists on dstfolder (has not e.g.  been deleted
>         there), sync the flag change to dstfolder and statusfolder.
> 
> The user visible implications of this change should be unnoticable
> except in one situation:
>  Blowing away LocalStatus will not require you to redownload ALL of
>  your mails if you still have the local Maildir. It will simply recreate
>  LocalStatus.

Looks like it should be told in the Changelog.

> This commit is less scary than this commit message might suggest.

We don't need this kind of statement in the commit message. We all read
code. It could stand between the three dashes and the diffstat, though.

Code enhancements looks good.

-- 
Nicolas Sebrecht




More information about the OfflineIMAP-project mailing list