The notmuch library includes a full blown message header parser. Yet
the same message headers are parsed by gmime during indexing. Switch
to gmime parsing completely.
These are the main changes:
* Gmime stops header parsing at the first invalid header, and presumes
the message body starts from there. The current parser is quite
liberal in accepting broken headers. The change means we will be
much pickier about accepting invalid messages.
* The current parser converts tabs used in header folding to
spaces. Gmime preserve the tabs. Due to a broken python library used
in mailman, there are plenty of mailing lists that produce headers
with tabs in header folding, and we'll see plenty of tabs. (This
change has been mitigated in preparatory patches.)
* For pure header parsing, the current parser is likely faster than
gmime, which parses the whole message rather than just the
headers. Since we parse the message and its headers using gmime for
indexing anyway, this avoids and extra header parsing round when
adding new messages. In case of duplicate messages, we'll end up
parsing the full message although just headers would be
sufficient. All in all this should still speed up 'notmuch new'.
* Calls to notmuch_message_get_header() may be slightly slower than
previously for headers that are not indexed in the database, due to
parsing of the whole message. Within the notmuch code base, notmuch
reply is the only such user.
In xapian terms, convert folder: prefix from probabilistic to boolean
prefix, matching the paths, relative from the maildir root, of the
message files, ignoring the maildir new and cur leaf directories.
folder:foo matches all message files in foo, foo/new, and foo/cur.
folder:foo/new does *not* match message files in foo/new.
folder:"" matches all message files in the top level maildir and its
new and cur subdirectories.
This change constitutes a database change: bump the database version
and add database upgrade support for folder: terms. The upgrade also
adds path: terms.
Finally, fix the folder search test for literal folder: search, as
some of the folder: matching capabilities are lost in the
probabilistic to boolean prefix change.
The path: prefix is a literal boolean prefix matching the paths,
relative from the maildir root, of the message files.
path:foo matches all message files in foo (but not in foo/new or
foo/cur).
path:foo/new matches all message files in foo/new.
path:"" matches all message files in the top level maildir.
path:foo/** matches all message files in foo and recursively in all
subdirectories of foo.
path:** matches all message files recursively, i.e. all messages.
In case previous notmuch compact has been interrupted there is old
work-in-progress database compact directory partially filled. Remove
it just before starting to fill the directory with new files.
It is less error prone and window of failure opportunity is smaller
if the old (backup) database is always renamed (instead of sometimes
rmtree'd) before new (compacted) database is put into its place.
Finally rmtree() old database in case old database backup is not kept.
catch Xapian::Error in compact code in lib/database.cc to be consistent
with other code in addition to not making software crash on uncaught
other Xapian error.
The extra path component added by the lib is a magic value that the
caller just has to know. This is demonstrated by the current code,
which indeed has "xapian.old" both sides of the interface. Use the
backup path provided by the lib caller verbatim, without adding
anything to it.
This function uses Xapian's Compactor machinery to compact the notmuch
database. The compacted database is built in a temporary directory and
later moved into place while the original uncompacted database is
preserved.
Signed-off-by: Ben Gamari <bgamari.foss@gmail.com>
As explained by Jeffrey Stedfast, the author of GMime, quoted in [1]:
> Passing the GMIME_ENABLE_RFC2047_WORKAROUNDS flag to g_mime_init()
> *should* solve the decoding problem mentioned in the thread. This
> flag should be safe to pass into g_mime_init() without any bad side
> effects and my unit tests do test that code-path.
The thread being referred to is [2].
[1] id:87bo56viyo.fsf@nikula.org
[2] id:08cb1dcd-c5db-4e33-8b09-7730cb3d59a2@gmail.com
Presently, the code which finds the parent of a message as it is being
added to the database assumes that the first Message-ID-like substring
of the In-Reply-To header is the parent Message ID. Some mail clients,
however, put stuff other than the Message-ID of the parent in the
In-Reply-To header, such as the email address of the sender of the
parent. This can fool notmuch.
The updated algorithm prefers the last Message ID in the References
header. The References header lists messages oldest-first, so the last
Message ID is the parent (RFC2822, p. 24). The References header is
also less likely to be in a non-standard
syntax (http://cr.yp.to/immhf/thread.html,
http://www.jwz.org/doc/threading.html). In case the References header
is not to be found, fall back to the old behavior.
V2 of this patch, incorporating feedback from Jani and (indirectly)
Austin.
Previously, we would treat multi-message mboxes as one giant email,
which, besides the obvious incorrect indexing, often led to
out-of-memory errors for archival mboxes. Now we explicitly reject
multi-message mboxes. For historical reasons, we retain support for
single-message mboxes, but official deprecate this behavior.
Add a custom value range processor to enable date and time searches of
the form date:since..until, where "since" and "until" are expressions
understood by the previously added date/time parser, to restrict the
results to messages within a particular time range (based on the Date:
header).
If "since" or "until" describes date/time at an accuracy of days or
less, the values are rounded according to the accuracy, towards past
for "since" and towards future for "until". For example,
date:november..yesterday would match from the beginning of November
until the end of yesterday. Expressions such as date:today..today
means since the beginning of today until the end of today.
Open-ended ranges are supported (since Xapian 1.2.1), i.e. you can
specify date:..until or date:since.. to not limit the start or end
date, respectively.
CAVEATS:
Xapian does not support spaces in range expressions. You can replace
the spaces with '_', or (in most cases) '-', or (in some cases) leave
the spaces out altogether.
Entering date:expr without ".." (for example date:yesterday) will not
work as you might expect. You can achieve the expected result by
duplicating the expr both sides of ".." (for example
date:yesterday..yesterday).
Open-ended ranges won't work with pre-1.2.1 Xapian, but they don't
produce an error either.
Signed-off-by: Jani Nikula <jani@nikula.org>
Previously, _notmuch_database_filename_to_direntry would abort with an
internal error when called on a read-only database. Now that creating
the directory document is optional,
notmuch_database_find_message_by_filename can disable directory
document creation (as it should) and, as a result, not abort on
read-only databases.
Using the new support from _notmuch_directory_create, this makes
notmuch_database_get_directory a read-only operation that simply
returns the directory object if it exists or NULL otherwise. This
also means that notmuch_database_get_directory can work on read-only
databases.
This change breaks the directory mtime workaround in notmuch-new.c by
fixing the exact issue it was working around. This permits mtime
update races to prevent scans of changed directories, which
non-deterministically breaks a few tests. The next patch fixes this.
Now _notmuch_database_filename_to_direntry takes a flags argument and
can indicate if the necessary directory documents do not exist.
Again, callers have been updated, but retain their original behavior.
Now _notmuch_database_find_directory_id takes a flags argument, which
it passes through to _notmuch_directory_create and can indicate if the
directory does not exist. Again, callers have been updated, but
retain their original behavior.
Previously this function would create directory documents if they
didn't exist. As a result, it could only be used on writable
databases. This adds an argument to make creation optional and to
make this function work on read-only databases. We use a flag
argument to avoid a bare boolean and to permit future expansion.
Both callers have been updated, but currently retain the old behavior.
We'll take advantage of the new argument in the following patches.
Previously, notmuch_database_get_directory had no way to indicate how
it had failed. This changes its prototype to return a status code and
set an out-argument to the retrieved directory, like similar functions
in the library API. This does *not* change its currently broken
behavior of creating directory objects when they don't exist, but it
does document it and paves the way for fixing this. Also, it can now
check for a read-only database and return
NOTMUCH_STATUS_READ_ONLY_DATABASE instead of crashing.
In the interest of atomicity, this also updates calls from the CLI so
that notmuch still compiles.
This is the notmuch_database_create equivalent of the previous change.
In this case, there were places where errors were not being propagated
correctly in notmuch_database_create or in calls to it. These have
been fixed, using the new status value.
It has been a long-standing issue that notmuch_database_open doesn't
return any indication of why it failed. This patch changes its
prototype to return a notmuch_status_t and set an out-argument to the
database itself, like other functions that return both a status and an
object.
In the interest of atomicity, this also updates every use in the CLI
so that notmuch still compiles. Since this patch does not update the
bindings, the Python bindings test fails.
Formerly notmuch_database_close closed the xapian database and
destroyed the talloc structure associated with the notmuch database
object. Split notmuch_database_close into notmuch_database_close and
notmuch_database_destroy.
This makes it possible for long running programs to close the xapian
database and thus release the lock associated with it without
destroying the data structures obtained from it.
This also makes the api more consistent since every other data
structure has a destructor function.
The comments in notmuch.h are a courtesy of Austin Clements.
Signed-off-by: Justus Winter <4winter@informatik.uni-hamburg.de>
Formerly it was possible for *message_ret to be left
uninitialized. The documentation however clearly states that "[o]n any
failure or when the message is not found, this function initializes
'*message' to NULL".
Signed-off-by: Justus Winter <4winter@informatik.uni-hamburg.de>
Formerly the xapian database object was deleted and closed in its
destructor once the object was garbage collected. Explicitly call
close() so that the database and the associated lock is released
immediately.
The comment is a courtesy of Austin Clements.
Signed-off-by: Justus Winter <4winter@informatik.uni-hamburg.de>
Previously opening a notmuch database in read write mode that has been
locked resulted in the notmuch_database_open function executing
notmuch_database_close as a cleanup function. notmuch_database_close
failed to check whether the xapian database has in fact been created.
Add a check whether the xapian database object has actually been
created before trying to call its flush method.
Signed-off-by: Justus Winter <4winter@informatik.uni-hamburg.de>
Previously, we manually "free"d various pointers in
notmuch_database_open. Use a local talloc context instead to simplify
cleanup and eliminate various NULL pointer initializations and
conditionals.
In the error-handling paths of notmuch_database_open, we call
notmuch_database_close, which "delete"s several objects referenced by
the notmuch_database_t object. However, some of these pointers may be
uninitialized, resulting in undefined behavior. Hence, allocate the
notmuch_database_t with talloc_zero to make sure these pointers are
NULL so that "delete"ing them is harmless.
As reported in
id:"CAEbOPGyuHnz4BPtDutnTPUHcP3eYcRCRkXhYoJR43RUMw671+g@mail.gmail.com"
sometimes gmime tries to access a NULL pointer, e.g. g_mime_iconv_open()
tries to access iconv_cache that is NULL if g_mime_init() is not called.
This causes notmuch to segfault when calling gmime functions.
Calling g_mime_init() initializes iconv_cache and others variables needed
by gmime, making sure they are initialized when notmuch calls gmime
functions.
Test marked fix by db.
Commit 567bcbc2 introduced two new values for each message (content of the
"From" and "Subject" headers), but the comments about the database schema had
not been updated accordingly.
This is a rebase and cleanup of Istvan Marko's patch from
id:m3pqnj2j7a.fsf@zsu.kismala.com
Search retrieves these headers for every message in the search
results. Previously, this required opening and parsing every message
file. Storing them directly in the database significantly reduces IO
and computation, speeding up search by between 50% and 10X.
Taking full advantage of this requires a database rebuild, but it will
fall back to the old behavior for messages that do not have headers
stored in the database.
Apparently the method was renamed in Xapian 1.1.0 but the old method
name will stay around for a while. It seems better to stick with the
old name to make notmuch compile with older versions of Xapian, at
least for now.
We keep the lib/xutil.c version. As a consequence, also factor out
_internal_error and associated macros. It might be overkill to make a
new file error_util.c for this, but _internal_error does not really
belong in database.cc.
Previously, the functions notmuch_database_find_message() and
notmuch_database_find_message_by_filename() functions did not properly
report error condition to the library user.
For more information, read the thread on the notmuch mailing list
starting with my mail "id:871uv2unfd.fsf@gmail.com"
Make these functions accept a pointer to 'notmuch_message_t' as argument
and return notmuch_status_t which may be used to check for any error
condition.
restore: Modify for the new notmuch_database_find_message()
new: Modify for the new notmuch_database_find_message_by_filename()
Adding a message may involve changes to multiple database documents,
and thus needs to be done in a transaction. This makes add_message
(and, I think, the whole library) atomicity-safe: library callers only
needs to use atomic sections if they needs atomicity across multiple
library calls.
notmuch_database_find_message_by_filename is mostly stolen from
notmuch_database_remove_message, so this patch also vastly simplfies
the latter using the former.
This API is also useful in its own right and will be used in a later
patch for eager maildir flag synchronization.
Previously, notmuch_database_remove_message would remove the message
file name, sync the change to the message document, re-find the
message document, and then delete it if there were no more file names.
An interruption after sync'ing would result in a file-name-less,
permanently un-removable zombie message that would produce errors and
odd results in searches. We could wrap this in an atomic section, but
it's much simpler to eliminate the round-about approach and just
delete the message document instead of sync'ing it if we removed the
last filename.
notmuch_database_t now keeps a nesting count and we only start a
transaction or commit for the outermost atomic section.
Introduces a new error, NOTMUCH_STATUS_UNBALANCED_ATOMIC.
Various typo fixes in comments within the source code.
Signed-off-by: Pieter Praet <pieter@praet.org>
Edited-by: Carl Worth <cworth@cworth.org> Restricted to just
source-code comments, (and fixed fix of "descriptios" to "descriptors"
rather than "descriptions").
