[PATCH 2/3] Maildir relative paths change was not complete

Sebastian Spaeth Sebastian at SSpaeth.de
Wed Aug 17 13:22:47 BST 2011


On Wed, 17 Aug 2011 10:01:38 +0200, Vladimir.Marek at oracle.com wrote:
> From: Vladimir Marek <vlmarek at volny.cz>
> 
> In commit e023f190b0eed84fefc10e28bfe5e4e0467ff257 we missed one spot
> 
> Signed-off-by: Vladimir Marek <vlmarek at volny.cz>
> ---
>  offlineimap/folder/Maildir.py |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/offlineimap/folder/Maildir.py b/offlineimap/folder/Maildir.py
> index 3b53156..2646205 100644
> --- a/offlineimap/folder/Maildir.py
> +++ b/offlineimap/folder/Maildir.py
> @@ -128,7 +128,7 @@ class MaildirFolder(BaseFolder):
>          folderstr = ',FMD5=' + foldermd5
>          for dirannex in ['new', 'cur']:
>              fulldirname = os.path.join(self.getfullname(), dirannex)
> -            files.extend(os.path.join(fulldirname, filename) for
> +            files.extend(os.path.join(dirannex, filename) for
>                           filename in os.listdir(fulldirname))
>          for file in files:
>              messagename = os.path.basename(file)

Stop! You are right that something is not correct here, but we need to
be careful. Part of the confusion is because it is never documented what
the messagelist[UI]={filename: XXX} is supposed to contain. It used to
be absolute file paths, and I made it to be only the shorter version, to
save memory.

1) I think we should explicitly document what format the filename field
is supposed to take (I know you are not guilty of that omission, but we
should fix it while we are at it).

2) Now:

fulldirname = <absolute_folderroot>/<dirannex>
and "files" is now a list of absolute file names such as
/home/user/mail/INBOX/cur/1231231_FMD5=1_UID=123:2,S

You are making it a list of dirannex+filename:
cur/1231231_FMD5=1_UID=123:2,S

which is file for the value to stuff in the messagelist construct, but
the "file" variable is also used in this function:
  os.path.getsize(file)

and that would fail if it were in your new form, rather than the old
one. So we need to to use:
 os.path.getsize(os.path.join(self.getfullname(), file))

Does this make sense?
I'll send an updated patch based on your patch. Please let me know if
you are fine with that change.

P.S. Cool that you are actually using a test suite. Can you tell how to
use it and where to get it? Do you have a separate repo for that?

Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/offlineimap-project/attachments/20110817/97d1f370/attachment-0001.sig>


More information about the OfflineIMAP-project mailing list