On Tue, Dec 13, 2011 at 12:00:57PM -0500, W. Trevor King wrote:
> I've attached a patch that does fix the problem…
Oops, *now* I've attached the patch and logs ;).
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
From 3067b1b4dfb00d165bd9480ea49f446adb12991d Mon Sep 17 00:00:00 2001
From: W. Trevor King <wking@drexel.edu>
Date: Tue, 13 Dec 2011 11:26:00 -0500
Subject: [PATCH] Only scan children in _getfolders_scandir if extension is set.
When sep is '/', MaildirRepository._getfolders_scandir recursively
checks sub-directories for additional maildirs. The old loop logic
always checked the top directory and its children. This lead to
children being found twice, once from their parent, with dirname
matching their directory name, and once from themselves, with a
dirname of ''.
This patch fixes the problem by only checking the top directory when
extension is not set (i.e. for the root directory).
Do not read in custom maildir flags, or we would try to sync them over
the wire. The next step will be to merge flag writes with existing
custom flags, so we don't lose information.
The long term goal will be to attempt to sync flags to the other side,
of course.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Blinkenlights UI 6.5.0 regression fixes only.
* Sleep led to crash ('abort_signal' not existing)
* Make exit via 'q' key work again cleanly
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
With the new abort signal handler, we can send a signal that lets us
exit cleanly. Make use of this, rather than crashing out in ugly ways.
This affects only the Blinkenlights UI when pressing 'q'.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
This is a regression introduced when renaming signals due to the
improved CTRL-C handling. Regression in 6.5.0
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
OfflineIMAP v6.5.1 (2012-01-07) - "Quest for stability"
=======================================================
* Fixed Maildir regression "flagmatchre" not found. (regressed in 6.5.0)
* Have console output go by default to STDOUT and not STDERR (regression
in 6.5.0)
* Fixed MachineUI to urlencode() output lines again, rather than
outputting multi-line items. It's ugly as hell, but it had been that
way for years.
* Remove the old global locking system. We lock only the accounts that
we currently sync, so you can invoke OfflineImap multiple times now as
long as you sync different accounts. This system is compatible with
all releases >= 6.4.0, so don't run older releases simultanous to this
one.
THe new logging framwork spit putput to STDERR by default (as that is
pythons default), but we used to have STDERR, so make it go there again.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
We were using the internal imaplib2 _get_untagged_response() functions a
few times. Replace 3 of these calls with 2 calls to the public function
response() rather than fudging with internals that could change anytime.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Remove the old global locking system. We lock only the accounts that we
currently sync, so you can invoke OfflineImap multiple times now as long
as you sync different accounts. This system is compatible with all
releases >= 6.4.0, so don't run older releases simultanous to this one.
This mostly reverts commit 0d95651417,
disabling the old global lock system that we had in parallel to the new one.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
The logging rework led to multipline output as we stopped urlencoding
the output lines. Urrg. Fixed this, so output is urlencoded again.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
These were needed for python <2.6 compatability, but since we depend on
python 2.6 now, these can go.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
This is a CRITICAL bug fix release for everyone who is on the 6.4.x
series. Please upgrade to avoid potential data loss! The version has
been bumped to 6.5.0, please let everyone know to stay away from 6.5.x!
I am sorry for this.
See details in the Changelog and even more gory details in commit
message for commit 8fc7227189.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
This change looks harmless, but it fixes a severe bugfix, potentially
leading to data loss! It fixes the "on n new uploads, it will redownload
n-1, n-2, n-3,... messages during the next syncs" condition, and this is
what happens:
If there are more than one Mails to upload to a server, we do that by
repeatedly invoking folder.IMAP.savemessage(). If the server supports
the UIDPLUS extension we query the resulting UID by doing a:
imapobj._get_untagged_response('APPENDUID', True)
and that is exactly the problem. The "True" part causes the reply to
remain in the "response stack" of the imaplib2 library. When we do
the same call on a subsequent message and the connection is still on the
same folder, we will get the same UID response back (imaplib2 only looks
for the first matching response and returns that). The only time we
clear the response stack, is when the IMAP connection SELECTS a
different folder.
This means that when we upload 10 messages, the IMAP server gives us
always the same UID (that of the first one) back. And trying to write
out 10 different messages with the same UID will confuse OfflineIMAP.
This is the reason why we saw the ongoing UPLOADING/DOWNLOADING behavior
that people reported. And this is the reason why we saw the
inconsistency in the UID mapping in the IMAP<->IMAP case.
I urge everyone to upgrade ASAP. Sorry for that, I don't know why the
problem only became prevalent in the recent few releases as this code
has been there for quite a while.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Rather than always parsing the filename, we only need to do so if the flags
have actually changed, otherwise we can keep the filename.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Previously, assigning a new UID to a mapped IMAP or Maildir repository
was done by loading the "local" item, saving it under a new UID and
deleting the old one. This involved lots of disk activity for nothing
more than an effective file rename in Maildirs, and lots of network
usage in the MappedUID cases.
We do this on every upload from a local to a remote item, so that can
potentially be quite expensive. This patch lets backends that support it
(Maildir, MappedUID) efficiently rename the file rather than having to
read the mail content, write it out as a new file and delete the old
file. This speeds up uploads from Maildir and the MappedUID server.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Various functions (such as change_message_uid) will want to construct
maildir filenames, so factor out the code into a helper.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Create a helper function that retrieves the UID, folder MD5, and Flags from
a message filename.
We need these items when we simply want to rename (=new UID) a Maildir
message file later. The new function can give us these components.
Rework, so we cache the calculation of the folder's md5 value once, it
never changes and we call it a lot.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
If someone had a custom :2,a flag, adding a new flag would lead to the
invalid maildir filename ...a:2,... due to regex deficiencies not coping
with this. Fix this so we alway produce valid maildir names.
Note that custom flags are still problematic: as the syncing to the
remote IMAP server will fail, the next sync will assume that they have
been removed from the remote IMAP side and they will be removed from the
local Maildir then. We will need to think about how to handle this. At
least, with this patch we won't lose standard flags and won't produce
invalid maildir names.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
The regex for catching Maildir message flags was
self.infosep + '.*2,([A-Z]+)' (infosep being ':').
The .* is bogus, as there is nothing between the : and the 2, per
maildir name specification, so remove that unneeded piece.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
When a new remote folder was detected, we tried to create the folder
locally on the Maildir and called repository.forgetfolders() to force a
new scanning of the Maildir. However, that implementation used the
inherited base function that did nothing. We simply needed to implement
forgetfolders() to set self.folder=None, so we would force a new read in
of the updated local folder structure.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
1) Rename the unintuitive repository.syncfoldersto() to
sync_folder_structure()
2) We were checking if the local repository is readonly and then turning
off any folder creation. But as we can create folders on a remote
repository too, we need to be more fine grained here. Just don't create
a folder on the repository that is marked readonly=True.
This still does not do away with the error message that one currently
gets on missing local folders.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
The quiet UI should only output errors, and the final "Finished account
X in 2 seconds" clearly is none, so the message debug level needed to be
reduced to INFO to suppress it in the quiet ui.
Fixes https://github.com/spaetz/offlineimap/issues/6
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Merge in the new logging mechanism, and provide an --info feature that
will help with debugging. There is still some unstableness, so there
will be another release soon, but this should be no worse than 6.4.2 at
least...
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Previously, we would simply bail out in an ugly way, potentially leaving
temporary files around etc, or while writing status files. Hand SIGINT
and SIGTERM as an event to the Account class, and make that bail out
cleanly at predefined points. Stopping on ctrl-c can take a few seconds
(it will e.g. finish to transfer the ongoing message), but it will shut
down cleanly.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
As reported in https://github.com/spaetz/offlineimap/pull/2, we would
fail when files are empty because file.read() would throw attribute
errors.
Fix this by removing the superfluous read() check and additionally log
some warning message.
Reported-by: Ralf Schmitt
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
When sep='/' in a Maildir, we were doing a os.path.join(dirname,'') on
the top level maildir, which results in a "dirname/", so all our maildir
folder names had slashes appended. Which is pretty much wrong, so this
fixes it by only using os.path.join when we actually have something to
append.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
We need the list of folders and the folder delimiter, but it was not
always retrieved early enough. E.g. when doing IMAP<->IMAP sync and the
local IMAP being readonly, we would bunk out with a mysterious error
message become repository.getsel() would still return None.
This commit fixes this error.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
imapserver.getdelim() was not used at all, so remove this function. The
folder delimiter is available via the repository.getsep() call.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
The folder delimiter is only initialized after a call to
acquireconnection(), so we must never call this function too
early. Include an assert() to make sure we get notified when we do.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Rather than keeping a separate queue of all logged lines in memory, we
rely on the curses window scrolling functionality to scroll lines. On
resizing the terminal this means, we'll clear the screen and start
filling it afresh, but that should be acceptable.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
1) Rework the sleep abort request to set the skipsleep configuration
setting that the sleep() code checks.
2) Only output 15 rather than 50 debug messages on abort...
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
The remote|local|statusrepo is an anttribute of each SyncableAccount()
anyway, so we don't need to pass it in, we can simply get it from the
Account().
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Rename some variables, simplify the hotkeys treatment. Refresh/exit
signals still don't work as of yet, but will come.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Resizing a Blinkenlights terminal doesn't crash anymore, and actually
seems to be changing the size, with this patch.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
We were still referring to s.gettf() in sleeping(self, ...) causing each
attempt to sleep to crash. Fix this, and the CursesAccountFrame.sleeping()
method. I am sure, there is still wrong and broken but we are getting there.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
This function can IMHO lead to possible deadlocks when waiting for the
connectionlock. Do add a comment to that regard, this will need to audit.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
The huge UI rework patch removed some obscure logic and special handling of
thread exit messages. It turns out that this was in fact still needed as a
specific exit message of the SyncRunner thread signified the threatmonitor
to quit.
We will want a nicer machinery for this in the future I guess, but fix the
eternal hang on exit by reintroducing a special exit message for the
SyncRunner thread, and return from the infinite monitor loop if SyncRunner
finishes.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
To make sure, the lock gets released even if we raise an exception between
acquire() and release()
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
All ExitNotifyThreads and InstanceLimitThreads are setDaemon(True) in their
constructor, so there is no need to do that again in the code.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
1) Differentiate error messages between imaplib.abort and imaplib.error
exceptions in the log.
2) Drop connections in the case of imapobj.error, it also might denote a
broken connection.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
During cleanup we often call releaseconnection in a finally: block. But
in cases of error, we might have dropped the connection earlier already
and set it to "None". In this case don't fail releaseconnection() but
make it a NOOP.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
The default parameter value was "None", and we were comparing that
directly to the imaplib2 value of is_readonly which is False or True, so
the comparison always returned "False".
Fix this by setting the default parameter to "False" and not
"None". Also convert all users of that function to use False/True.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
It is basically unused by now. Rework to be able to make use of it
later, no functional changes.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Logging was flawed as the output was e.g. heavily buffered and people
complained about missing log entries. Fix this by making use of the
standard logging facilities that offlineimap offers.
This is one big ugly patch that does many things. It fixes the
Blinkenlights backend to work again with the logging facilities.
Resize windows and hotkeys are still not handled absolut correctly, this
is left for future fixing. THe rest of the backends should be working fine.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
If folder creation failed, we would output the wrong repository and
folder name (copy'n paste error). Fix this so we actually output the
correct values.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Only import the lock, that we actually need. Also import the with statement
for use with python 2.5. We'll need it for sure in this file.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Drop a connection, if the NOOP to keep a connection open fails due to
broken connections.
Note that I believe this function is not working as intended. We grab
one random connection and send a NOOP. This is not enough to keep all
connections open, and if we invoke this function multiple times, we
might well always get the same connection to send a NOOP through.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
A repositories 'reference value is always prefixed to the full folder
path, so we should do so when creating a new one. The code had existed
but was commented out since 2003, I guess the "reference" option is not
too often used.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
If port is None, we would try to format an empty string with %d wich
fails. Fix it by using %s.
Reported-by: Iain Dalton <iain.dalton@gmail.com>
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
This outputs a handy summary of your server configuration and version
strings etc, which is useful for bug reporting.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Was getting too large, split into an parse_cmd_options and a sync()
function. Moving config and ui to self.config and self.ui to make them
available through the OfflineImap instance.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
It happens quick, and clutters the log. So we can usually skip this. We
will output a log entry when we actually create a new folder anyway.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
If we create a new folder we would previously not update our folder
list, which led to us skipping the synchronization of those new folders
during the initial run (subsequent runs would pick it up).
Invalidate the folder cache when we create a folder during folder
structure sync. Regetting the whole list from an IMAP server might be
slightly suboptimal from a performance point, but it is easy and will
lead to consistent results. Hopefully we will not have to create new
folders on each new run.
Reported-by: Daniel Shahaf <d.s@daniel.shahaf.name>
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Modify the UI:acct and acctdone functions to keep tab of the time
inbetween. Put self.ui.acct() and acctdone() at the right places in
accounts.py so that the timing happens at the right places.
While modifying that loop, flatten the nested try: try: except: finally:
constructs, we require python 2.5 now which copes with that.
At the end of each account sync you will now see something like:
*** Finished account 'test' in 0:05
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Thread names are used to determine the logging header in the TTY ui. A
recent change made them too terse (basically only changing the account
name and not the folder names). Unbreak.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Output (2 of 500) when logging message copying. This required moving of
self.ui.copyingmessage into a different function where we actually have
the information about the progress handy.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Rather than setting a global threadutil/profiledir variable, we make
set_profiledir a class function that sets the class variable profiledir.
While touching theprofiledir code, add warning to the user if the
profile directory existed before.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Registering a thread (associating it with a certain account name) would
fail if it was already registered. However, as we a) never unregister most
threads (bad) and b) single-threaded mode reuses threads, we failed when
syncing multiple accounts in single-threading mode.
This commit cleans up the functions to not make re-registering a thread
fatal (it could be legitimate, however it *should* not occur). Future
work needs to be done to unregister new threads at the appropriate places.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
When checking for the IMAP4.abort() exception, we need of course to
perform:
except imapobj.abort:
and not
except imapobj.abort():
Thanks to Johannes Stezenbach <js@sig21.net> for pointing to the glitch.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
apply() has been deprecated since Python 2.3, and won't be working in
python 3 anymore. Use the functional equivalent throughout.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Dave identified a case where our new dropped connection handling did
not work out correctly: we use the retry_left variable to signify
success (0=success if no exception occured).
However, we were decrementing the variable AFTER all the exception
checks, so if there was one due to a dropped connection, it
could well be that we 1) did not raise an exception (because we want to
retry), and 2) then DECREMENTED retry_left, which indicated "all is
well, no need to retry".
The code then continued to check() the append, which failed with the
above message (because we obtained a new connection which had not even
selected the current folder and we were still in mode AUTH). The fix is
of course, to fix our logic: Decrement retry_left first, THEN decide
whether to raise() (retry_left==0) or retry (retry_left>0) which would
then correctly attempt another loop. I am sorry for this newbie type of
logic error. The retry count loop was too hastily slipped in, it seems.
Reported-by: Dave Abrahams <dave@boostpro.com>
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
If APPEND raises abort(), the (typ, dat) variables will not be set, so
we should not be using it for the OfflineImapError Exception
string. Fixing and prettifying the string formatting a bit at the same
time.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
When syncfolder() fails, we output an error message containing the
foldername per the localfolder variable. However, the localfolder
variable is assigned inside our try: block and when the error occurs
there, we will have no localfolder variable to use for output. This
caused the errormsg to cause an Exception itself which unhelpfully
distracts from the root cause of the error.
Reconstruct the folder name in a bit more complex way, but in a way so
it is guaranteed to work (by relying on parameters passed in to the
function).
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
A report by Dave Abrahams showed that the dequote() function failed when
invoked with an empty string. This fixes the function to be more robust.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
nametrans rules can lead to different visiblename names for the
top-level directory, specifically both '.' and '' (the latter was
recently introduced). However, we need to be able to compare folder
names to see if we need to create a new directory or whether a directory
already exists, so we need to be able to compare a repositories
visiblename (=transposed via nametrans rule) with another folder.
To make the top-level directory comparison happen, we enforce a
top-level name of '', so that comparisons work.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Some Webservers (I am looking at you Gmail) send different capabilities
before and after login, so they can tailor their server capabilities to
the user. While legal, this is uncommon and we were not updating our
server capabilities. Doing so allows us to detect that Gmail actually
supports the UIDPLUS extension, and we will stop mangling headers when
uploading to Gmail. This could lead to some performance gains when we
upload many messages to Gmail.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Commit b0e88622c4 changed dst_hash[folder.visiblename] to
dst_hash[folder.name] but we did not adapt all places where it is needed
to use visiblename again. This led to attempting to create a name on
REMOTE ignoring the nametrans setting on the LOCAL repository.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Previously, we only checked if a LOCAL folder falls under the local
repositories folderfilter rule when deciding whether a folder should be
created on REMOTE.
However, we also do not want to create the folder on REMOTE if it would
fall under a folderfilter rule there. This patch prevents us from doing
so.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
It is not easy to think through when to use visiblenames() and whatnot. It seems I managed to not think it through properly. Which might be fixed now.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
The thread ID is not really useful and looks ugly. It also makes lines
longer than needed, there is more useful information we can put in the
log. So do away with it.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
This will ignore any nametrans rules, so we might want to limit this
only to cases where no nametrans has been specified, or we might want to
use the nametrans setting of the dest repo.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
getvisiblename() was only defined on IMAP(derived) foldertypes, but we
want it on eg. Maildirs too, so we define it centrally in Folder.Base.py
rather than only in folder.IMAP.py.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
