more on maildir modification times!

Nicolas Sebrecht nicolas.s-dev at laposte.net
Thu Apr 2 08:38:08 BST 2015


On Wed, Apr 01, 2015 at 10:29:15PM -0400, Janna Martl wrote:

> I think I know what's going on here, and if we don't do something about
> it, this problem will come back when our timezone thing gets merged. The
> commit that got reverted did the following (in folder/Maildir.py):
> 
> def savemessage(self, uid, content, flags, rtime):
>     """Writes a new message, with the specified uid.
>     <...>
> 
> +    if rtime is None:
> +        rtime = emailutil.get_message_date(content)
> +    messagename = self.new_message_filename(uid, flags, rtime=rtime)
> -    messagename = self.new_message_filename(uid, flags)
>     tmpname = self.save_to_tmp_file(messagename, content)
>     if rtime != None:
>         os.utime(os.path.join(self.getfullname(), tmpname), (rtime, rtime))
> 
> In other words, that last line messing with the mtime was always there,
> but the offending patch caused rtime to not be None. Why was it None?
> Look at folder/IMAP.py:
> 
> def cachemessagelist(self):
>     <...>
> 
>         res_type, response = imapobj.fetch("'%s'"%
>             msgsToFetch, '(FLAGS UID)')
>         <...>
> 
>     for messagestr in response:
>         <...>
>             rtime = imaplibutil.Internaldate2epoch(messagestr)
>             self.messagelist[uid] = {'uid': uid, 'flags': flags, 'time': rtime}
> 
> It was None because we weren't even asking the IMAP server for the
> INTERNALDATE, and then (penultimate line) tried to extract rtime from
> the response. Obviously, that failed.
> 
> That was a bug, which I fixed in the current timezone WIP by adding
> INTERNALDATE to the list of message properties the IMAP server should
> return. But a consequence of this is now rtime won't be None anymore in
> savemessage() above, and that os.utime call will actually happen.
> 
> Why don't we just get rid of the os.utime call? Why is it there in the
> first place? It seems to be trying (and, up until now, failing) to do
> exactly what people don't want.

I think rtime can be set by utime_from_message from 3bc66c0858ecc2e6
that I recently changed to utime_from_header in 3bc66c0858ecc2e6.

So, there is at least one use case where rtime was set and os.utime()
does the job we expect.

I don't care much about breaking an undocumented feature BUT I believe
this feature will come back again if we remove it. So, I'd rather keep
this line and not break it.

Requesting for INTERNALDATE is the good fix.

-- 
Nicolas Sebrecht




More information about the OfflineIMAP-project mailing list