no-delete-local patch for offlineimap

Nicolas Sebrecht nicolas.s-dev at laposte.net
Wed May 27 10:38:47 BST 2015


On Mon, May 25, 2015 at 09:45:54PM +0200, Petr Fischer wrote:

> OK. I exported the patch from github commit for your review.

Thank you.

> I'm not sure if it's a patch against the latest version of your source, but it's a few lines of code only.

You're right, this patch is a bit outdated. There are too many code
changes so I can fully review the patch.

> From 266c88f7a1b041ba169e0c0cab767bf412c23672 Mon Sep 17 00:00:00 2001
> From: "Edward Z. Yang" <ezyang at mit.edu>
> Date: Fri, 31 Aug 2012 16:51:33 -0400
> Subject: [PATCH] Add no-delete-local to avoid remote->local delete
>  synchronization.
> 
> Signed-off-by: Edward Z. Yang <ezyang at mit.edu>
> ---
>  offlineimap.conf           | 12 ++++++++++++
>  offlineimap/accounts.py    |  4 ++--
>  offlineimap/folder/Base.py | 29 ++++++++++++++++++++---------
>  3 files changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/offlineimap.conf b/offlineimap.conf
> index fccceab..51a196a 100644
> --- a/offlineimap.conf
> +++ b/offlineimap.conf
> @@ -252,6 +252,18 @@ remoterepository = RemoteExample
>  #maildir-windows-compatible = no
>  
>  
> +# In many cases, your remote repository is intended to contain a subset
> +# of the messages your local repository, because the remote repository
> +# has less disk space, etc.  However, if you just remove messages from
> +# the remote repository, OfflineIMAP will synchronize the changes back
> +# and delete your local copies.  If you set this to 'yes', OfflineIMAP
> +# will never delete local files even if they disappear on the remote
> +# side.  Local to remote deletes are still synchronized.  See also
> +# maxage.
> +#
> +# no-delete-local = no

We avoid dashes in configuration options.

> +
> +

Should have the "header": # This option stands in the [Account Test] section.

>  [Repository LocalExample]
>  
>  # Each repository requires a "type" declaration. The types supported for
> diff --git a/offlineimap/accounts.py b/offlineimap/accounts.py
> index 8b39c7e..4b4bd44 100644
> --- a/offlineimap/accounts.py
> +++ b/offlineimap/accounts.py
> @@ -435,7 +435,7 @@ def syncfolder(account, remotefolder, quick):
>          # Synchronize remote changes.
>          if not localrepos.getconfboolean('readonly', False):
>              ui.syncingmessages(remoterepos, remotefolder, localrepos, localfolder)
> -            remotefolder.syncmessagesto(localfolder, statusfolder)
> +            remotefolder.syncmessagesto(localfolder, statusfolder, False)
>          else:
>              ui.debug('imap', "Not syncing to read-only repository '%s'" \
>                           % localrepos.getname())
> @@ -443,7 +443,7 @@ def syncfolder(account, remotefolder, quick):
>          # Synchronize local changes
>          if not remoterepos.getconfboolean('readonly', False):
>              ui.syncingmessages(localrepos, localfolder, remoterepos, remotefolder)
> -            localfolder.syncmessagesto(remotefolder, statusfolder)
> +            localfolder.syncmessagesto(remotefolder, statusfolder, True)
>          else:
>              ui.debug('', "Not syncing to read-only repository '%s'" \
>                           % remoterepos.getname())

Looks OK.

> diff --git a/offlineimap/folder/Base.py b/offlineimap/folder/Base.py
> index 6f6f364..d5c5425 100644
> --- a/offlineimap/folder/Base.py
> +++ b/offlineimap/folder/Base.py
> @@ -294,7 +294,7 @@ def deletemessages(self, uidlist):
>          for uid in uidlist:
>              self.deletemessage(uid)
>  
> -    def copymessageto(self, uid, dstfolder, statusfolder, register = 1):
> +    def copymessageto(self, uid, dstfolder, statusfolder, always_sync_deletes, register = 1):

always_sync_deletes sounds a poor name but I'm more worried about why
this feature goes so deeply to low-level code. I don't get why we should
change the signature of such a central method.

>          """Copies a message from self to dst if needed, updating the status
>  
>          Note that this function does not check against dryrun settings,
> @@ -366,7 +366,7 @@ def copymessageto(self, uid, dstfolder, statusfolder, register = 1):
>                                 exc_info()[2]))
>              raise    #raise on unknown errors, so we can fix those
>  
> -    def syncmessagesto_copy(self, dstfolder, statusfolder):
> +    def syncmessagesto_copy(self, dstfolder, statusfolder, always_sync_deletes):
>          """Pass1: Copy locally existing messages not on the other side
>  
>          This will copy messages to dstfolder that exist locally but are
> @@ -401,16 +401,16 @@ def syncmessagesto_copy(self, dstfolder, statusfolder):
>                      self.getcopyinstancelimit(),
>                      target = self.copymessageto,
>                      name = "Copy message from %s:%s" % (self.repository, self),
> -                    args = (uid, dstfolder, statusfolder))
> +                    args = (uid, dstfolder, statusfolder, always_sync_deletes))
>                  thread.start()
>                  threads.append(thread)
>              else:
> -                self.copymessageto(uid, dstfolder, statusfolder,
> +                self.copymessageto(uid, dstfolder, statusfolder, always_sync_deletes,
>                                     register = 0)
>          for thread in threads:
>              thread.join()
>  
> -    def syncmessagesto_delete(self, dstfolder, statusfolder):
> +    def syncmessagesto_delete(self, dstfolder, statusfolder, always_sync_deletes):
>          """Pass 2: Remove locally deleted messages on dst
>  
>          Get all UIDS in statusfolder but not self. These are messages
> @@ -419,8 +419,19 @@ def syncmessagesto_delete(self, dstfolder, statusfolder):
>  
>          This function checks and protects us from action in ryrun mode.
>          """
> +        # This is functionally equivalent to having an empty deletelist
> +        # in the case of not always_sync_deletes and no-delete-local turned on; the
> +        # only difference is that in this regime we eagerly clear out
> +        # "stale" entries from statusfolder, i.e. ones that are not
> +        # present in the local or destination folder, whereas if we were
> +        # to skip this the entries hang around until a not always_sync_deletes
> +        # run.
> +        sync_deletes = always_sync_deletes or not self.config.getdefaultboolean("Account " + self.accountname, "no-delete-local", False)

So, this is where we read this configuration option. Why here?

> +        print sync_deletes

Forgot to remove debug statement?

> +        import sys

Wrong place for this import.

> +        sys.exit()

!?

>          deletelist = filter(lambda uid: uid>=0 \
> -                                and not self.uidexists(uid),
> +                                and not self.uidexists(uid) and (sync_deletes or not dstfolder.uidexists(uid)),
>                              statusfolder.getmessageuidlist())
>          if len(deletelist):
>              self.ui.deletingmessages(deletelist, [dstfolder])
> @@ -431,7 +442,7 @@ def syncmessagesto_delete(self, dstfolder, statusfolder):
>              for folder in [statusfolder, dstfolder]:
>                  folder.deletemessages(deletelist)
>  
> -    def syncmessagesto_flags(self, dstfolder, statusfolder):
> +    def syncmessagesto_flags(self, dstfolder, statusfolder, always_sync_deletes):
>          """Pass 3: Flag synchronization
>  
>          Compare flag mismatches in self with those in statusfolder. If
> @@ -485,7 +496,7 @@ def syncmessagesto_flags(self, dstfolder, statusfolder):
>              dstfolder.deletemessagesflags(uids, set(flag))
>              statusfolder.deletemessagesflags(uids, set(flag))
>                  
> -    def syncmessagesto(self, dstfolder, statusfolder):
> +    def syncmessagesto(self, dstfolder, statusfolder, always_sync_deletes):
>          """Syncs messages in this folder to the destination dstfolder.
>  
>          This is the high level entry for syncing messages in one direction.
> @@ -523,7 +534,7 @@ def syncmessagesto(self, dstfolder, statusfolder):
>              if offlineimap.accounts.Account.abort_NOW_signal.is_set():
>                  break
>              try:
> -                action(dstfolder, statusfolder)
> +                action(dstfolder, statusfolder, always_sync_deletes)
>              except (KeyboardInterrupt):
>                  raise
>              except OfflineImapError as e:


-- 
Nicolas Sebrecht




More information about the OfflineIMAP-project mailing list