This updates the thread linking code to use ghost messages instead of
user metadata to link messages into threads.
In contrast with the old approach, this is actually correct.
Previously, thread merging updated only the thread IDs of message
documents, not thread IDs stored in user metadata. As originally
diagnosed by Mark Walters [1] and as demonstrated by the broken
T260-thread-order test, this can cause notmuch to fail to link
messages even though they're in the same thread. In principle the old
approach could have been fixed by updating the user metadata thread
IDs as well, but these are not indexed and hence this would have
required a full scan of all stored thread IDs. Ghost messages solve
this problem naturally by reusing the exact same thread ID and message
ID representation and indexing as regular messages.
Furthermore, thanks to this greater symmetry, ghost messages are also
algorithmically simpler. We continue to support the old user metadata
format, so this patch can't delete any code, but when we do remove
support for the old format, several functions can simply be deleted.
[1] id:8738h7kv2q.fsf@qmul.ac.uk
This updates the message abstraction to support ghost messages: it
adds a message flag that distinguishes regular messages from ghost
messages, and an internal function for initializing a newly created
(blank) message as a ghost message.
In the interest of robustness, avoid undefined behavior of
sortable_unserialise if the date value is missing. This shouldn't
happen now, but ghost messages will have blank date values.
This moves the code to retrieve and clear the metadata thread ID out
of _notmuch_database_link_message into its own function. This will
simplify future changes.
Previously, this was performed by notmuch_database_add_message. This
happens to be the only caller currently (which is why this was safe),
but we're about to introduce more callers, and it makes more sense to
put responsibility for ID compression in the lower-level function
rather than requiring each caller to handle it.
In Xapian, closing a database implicitly aborts any outstanding
transaction and commits changes. For historical reasons,
notmuch_database_close had grown to almost, but not quite duplicate
this behavior. Before closing the database, it would explicitly (and
unnecessarily) commit it. However, if there was an outstanding
transaction (ie atomic section), commit would throw a Xapian
exception, which notmuch_database_close would unnecessarily print to
stderr, even though notmuch_database_close would ultimately abort the
transaction anyway when it called close.
This patch simplifies notmuch_database_close to explicitly abort any
outstanding transaction and then just call Database::close. This
works for both read-only and read/write databases, takes care of
committing changes, unifies the exception handling path, and codifies
aborting outstanding transactions. This is currently the only way to
abort an atomic section (and may remain so, since it would be
difficult to roll back things we may have cached from rolled-back
modifications).
as stated in thread.cc:115
/* Construct an authors string from matched_authors_array and
* authors_array. The string contains matched authors first, then
* non-matched authors (with the two groups separated by '|'). Within
* each group, authors are listed in date order. */
this is, however, not reflected in the public API documentation in
notmuch.h:970. This patch a paragraph explaining how | separates the
group of authors of messages matching the query and those of messages
that do not, but are still contained in the thread.
48db8c8 introduced a disagreement between when
notmuch_database_needs_upgrade returned TRUE and when
notmuch_database_upgrade actually performed an upgrade. As a result,
if a database had a version less than 3, but no new features were
required, notmuch new would call notmuch_database_upgrade to perform
an upgrade, but notmuch_database_upgrade would return immediately
without updating the database version. Hence, the next notmuch new
would do the same, and so on.
Fix this by ensuring that the upgrade-required logic is identical
between the two.
Previously, there was no protection against a caller invoking an
operation on an old database version that would effectively corrupt
the database by treating it like a newer version.
According to notmuch.h, any caller that opens the database in
read/write mode is supposed to check if the database needs upgrading
and perform an upgrade if it does. This would protect against this,
but nobody (even the CLI) actually does this.
However, with features, it's easy to protect against incompatible
operations on a fine-grained basis. This lightweight change allows
callers to safely operate on old database versions, while preventing
specific operations that would corrupt the database with an
informative error message.
Commit 567bcbc2 introduced support for storing various headers in
document values. However, doing so in a backwards-compatible way
meant that genuinely empty header values could not be distinguished
from the old behavior of not storing the headers at all, so these
required parsing the original message.
Now that we have database features, new databases can declare that all
messages have header values, so if we have this feature flag, we can
use the stored header value even if it's the empty string.
This requires slight cleanup to notmuch_message_get_header, since the
code previously couldn't distinguish between empty headers and headers
that are never stored in the database (previously this distinction
didn't matter).
Previously, some parts of upgrade didn't report progress and for
others it was possible for the progress meter to restart at 0 part way
through the upgrade because each stage was reported separately.
Fix this by computing the total amount of work that needs to be done
up-front and updating completed work monotonically.
Rather than potentially making multiple passes over the same type of
data in the database, reorganize upgrade around each type of data that
may be upgraded. This eliminates code duplication, will make
multi-version upgrades faster, and will let us improve progress
reporting.
Previously, we had database version information hard-coded in the
upgrade code. Slightly re-organize the upgrade process around the set
of new database features to be enabled by the upgrade.
Previously, the upgrade was organized as two passes -- an upgrade
pass, and a separate cleanup pass -- so the database was always in a
valid state. This change substantially simplifies this code by
performing the upgrade in a transaction and combining both passes in
to one. This 1) eliminates a lot of duplicate code between the
passes, 2) speeds up the upgrade process, 3) makes progress reporting
more accurate, 4) eliminates the potential for stale data if the
upgrade is interrupted during the cleanup pass, and 5) makes it easier
to reason about the safety of the upgrade code.
Previously, our database schema was versioned by a single number.
Each database schema change had to occur "atomically" in Notmuch's
development history: before some commit, Notmuch used version N, after
that commit, it used version N+1. Hence, each new schema version
could introduce only one change, the task of developing a schema
change fell on a single person, and it all had to happen and be
perfect in a single commit series. This made introducing a new schema
version hard. We've seen only two schema changes in the history of
Notmuch.
This commit introduces database schema version 3; hopefully the last
schema version we'll need for a while. With this version, we switch
from a single version number to "features": a set of named,
independent aspects of the database schema.
Features should make backwards compatibility easier. For many things,
it should be easy to support databases both with and without a
feature, which will allow us to make upgrades optional and will enable
"unstable" features that can be developed and tested over time.
Features also make forwards compatibility easier. The features
recorded in a database include "compatibility flags," which can
indicate to an older version of Notmuch when it must support a given
feature to open the database for read or for write. This lets us
replace the old vague "I don't recognize this version, so something
might go wrong, but I promise to try my best" warnings upon opening a
database with an unknown version with precise errors. If a database
is safe to open for read/write despite unknown features, an older
version will know that and issue no message at all. If the database
is not safe to open for read/write because of unknown features, an
older version will know that, too, and can tell the user exactly which
required features it lacks support for.
According to RFC2822 References and In-Reply-To headers are supposed
to contain one or more Message-IDs, however older RFC822 allowed
almost any content. When both References and In-Reply-To headers ends
with something else that a Message-ID (see e.g. [1]), the thread
structure presented by notmuch is incorrect. The reason is that
notmuch treats this case as if the email contained no "replyto"
information (see _notmuch_database_link_message_to_parents).
This patch changes the parse_references() function to return the last
valid Message-ID encountered rather than NULL resulting from the last
hunk of text not being the Message-ID.
[1] https://lkml.org/lkml/headers/2014/5/19/864
Previously, we invalidated stored message metadata in
_notmuch_message_add_term and _notmuch_message_remove_term, but not in
_notmuch_message_gen_terms. This doesn't currently result in any bugs
because of our limited uses of _notmuch_message_gen_terms, but it may
could cause trouble in the future.
As noted in devel/STYLE, every private library function should start
with _notmuch. This patch corrects function naming that did not adhere
to this style in lib/notmuch-private.h. In particular, the old function
names that now begin with _notmuch are
notmuch_sha1_of_file
notmuch_sha1_of_string
notmuch_message_file_close
notmuch_message_file_get_header
notmuch_message_file_open
notmuch_message_get_author
notmuch_message_set_author
Signed-off-by: Charles Celerier <cceleri@cs.stanford.edu>
notmuch_database_close may fail in Xapian ->flush() or ->close(), so
report the status. Similarly for notmuch_database_destroy which calls
close.
This is required for notmuch insert to report error status if message
indexing failed.
This adds a 100 termpos gap between all phrases indexed by
_notmuch_message_gen_terms. This fixes a bug where terms from the end
of one header and the beginning of another header could match together
in a single phrase and a separate bug where term positions of
un-prefixed terms overlapped.
This fix only affects newly indexed messages. Messages that are
already indexed won't benefit from this fix without re-indexing, but
the fix won't make things any worse for existing messages.
Previously, we indexed the name and address parts of from/to headers
with two calls to _notmuch_message_gen_terms. In general, this
indicates that these parts are separate phrases. However, because of
an implementation quirk, the two calls to _notmuch_message_gen_terms
generated adjacent term positions for the prefixed terms, which
happens to be the right thing to do in this case, but the wrong thing
to do for all other calls. Furthermore, _notmuch_message_gen_terms
produced potentially overlapping term positions for the un-prefixed
copies of the terms, which is simply wrong.
This change indexes both the name and address in a single call to
_notmuch_message_gen_terms, indicating that they should be part of a
single phrase. This masks the problem with the un-prefixed terms
(fixing the two known-broken tests) and puts us in a position to fix
the unintentionally phrases generated by other calls to
_notmuch_message_gen_terms.
This is effectively a revert of
commit 6812136bf5
Author: Jani Nikula <jani@nikula.org>
Date: Mon Mar 31 00:21:48 2014 +0300
lib: drop support for single-message mbox files
The intention was to drop support for indexing new single-message mbox
files (and whether that was a good idea in the first place is
arguable). However this inadvertently broke support for reading
headers from previously indexed single-message mbox files, which is
far worse.
Distinguishing between the two cases would require more code than
simply bringing back support for single-message mbox files.
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.
We've supported mbox files containing a single message for historical
reasons, but the support has been deprecated, with a warning message
while indexing, since Notmuch 0.15. Finally drop the support, and
consider all mbox files non-email.
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.
Clarify that using the directory after destroying the corresponding
database is not permitted.
This is implicit in the description of notmuch_database_destroy, but
it doesn't hurt to be explicit, and we do express similar "ownership"
relationships at other places in the docs.
Currently if a Xapian exception happens in notmuch_message_get_header,
the exception is not caught leading to crash. In
notmuch_message_get_date the exception is caught, but an internal error
is raised, again leading to crash.
This patch fixes the error handling by making both functions catch the
Xapian exceptions, print an error and return NULL or 0.
The 'notmuch->exception_reported' is also set, as is done elsewhere,
even if I don't really get the idea of that field.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@iki.fi>
With some combination of clang and talloc, not using the return value
of talloc_steal() produces a warning. Ignore it, as talloc_steal() has
no failure modes per documentation.
This version of the library introduces LIBNOTMUCH_CHECK_VERSION and
the *_VERSION macros. Bumping the version number is also necessary to
make the comment on LIBNOTMUCH_CHECK_VERSION no longer a lie.
This makes it clear that these macros refer to the *library* version,
and not to the notmuch application-level release. Since there are no
consumers of these macros yet, this is now or never.
We have two distinct "library version" numbers: the soname version and
the version macros. We need both for different reasons: the version
macros enable easy compile-time version detection (and conditional
compilation), while the soname version enables runtime version
detection (which includes the version checking done by things like the
Python bindings).
However, currently, these two version numbers are different, which is
unnecessary and can lead to confusion (especially in things like
Debian, which include the soname version in the package name). This
patch makes them the same by bumping the version macros up to agree
with the soname version.
(We should probably keep the version number in just one place so they
can't get out of sync, but that can be done in another patch.)
Unfortunately old versions of GCC and clang do not provide byte order
macros, so we re-invent them.
If UTIL_BYTE_ORDER is not defined or defined to 0, we fall back to
macros supported by recent versions of GCC and clang
Previously PLATFORM_BYTE_ORDER and IS_LITTLE_ENDIAN were not defined,
so the little endian code was always compiled in.
This will have the effect that the "SHA1s" on big endian architectures
will change (i.e. become actual sha1s). So someone re-indexing their
database could conceivable lose tags on messages without a message-id
header.
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.
There have been some plans for making build incompatible changes to
the library API. This is inconvenient, but it is much more so without
a way to easily conditional build against multiple versions of
notmuch.
The macro has been lifted from glib.
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.
The queries don't really work after a database is closed, and we would
like them to be freed if the database is destroyed.
Acknowledged-by: David Bremner <david@tethera.net>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
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
notmuch_message_tags_to_maildir_flags() unconditionally moves messages from
maildir directory "new/" to maildir directory "cur/", which makes messages lose
their "new" status in the MUA. However some users want to keep this "new"
status after, for instance, an auto-tagging of new messages.
However, as Austin mentioned and according to the maildir specification,
messages living in "new/" are not allowed to have flags, even if mutt allows it
to happen. For this reason, this patch prevents moving messages from "new/" to
"cur/", only if no flags have to be changed. It's hopefully enough to satisfy
mutt (and maybe other MUAs showing the "new" status) users checking the "new"
status.
Changelog:
* v2: Fix bool type as well as NULL returned despite having no errors (Austin
Clements)
* v4: Tag the related test (contributed by Michal Sojka) as working
Signed-off-by: Louis Rilling <l.rilling@av7.net>
[Condition for keeping messages in new/ was extended to satisfy all
tests from the previous patch. -Michal Sojka]
[Added by David Bremner, to keep the tests passing at each commit]
update insert tests for new maildir synchronization rules
As of id:1355952747-27350-4-git-send-email-sojkam1@fel.cvut.cz
we are more conservative about moving messages from ./new to ./cur.
This updates the insert tests to match
Long story short, fix build on recent (3.2+) clang.
The long story for posterity follows.
gcc 4.6 added new warnings about structs with greater visibility than
their fields. The warnings were silenced by adjusting visibility in
commit d5523ead90
Author: Carl Worth <cworth@cworth.org>
Date: Wed May 11 13:23:13 2011 -0700
Mark some structures in the library interface with visibility=default attribute.
Later on,
commit 3b76adf9e2
Author: Austin Clements <amdragon@MIT.EDU>
Date: Sat Jan 14 19:17:33 2012 -0500
lib: Add support for automatically excluding tags from queries
changed visibility of struct _notmuch_string_list for the same reason, and
commit 1a53f9f116
Author: Mark Walters <markwalters1009@gmail.com>
Date: Thu Mar 1 22:30:38 2012 +0000
lib: Add the exclude flag to notmuch_query_search_threads
split the struct _notmuch_string_list and its typedef
notmuch_string_list_t as a way to make a forward declaration for
_notmuch_thread_create().
The subtle difference was that the struct definition now had 'visible'
in it, while the typedef didn't, and it was within the #pragma GCC
visibility push(hidden) block. This went unnoticed, as the then common
versions of clang didn't care about this.
A later change in clang (I did not dig into when this change was
introduced) caused the following error:
CXX -O2 lib/database.o
In file included from lib/database.cc:21:
In file included from ./lib/database-private.h:33:
./lib/notmuch-private.h:479:8: error: visibility does not match previous declaration
struct visible _notmuch_string_list {
^
./lib/notmuch-private.h:67:33: note: expanded from macro 'visible'
^
./lib/notmuch-private.h:52:13: note: previous attribute is here
^
1 error generated.
make: *** [lib/database.o] Error 1
This is slightly misleading due to the reference to the #pragma. The
real culprit is the typedef within the #pragma.
We could just add 'visible' to the typedef, or move the typedef
outside of the #pragma, and be done with it, but juggle the
declarations a bit to accommodate moving the typedef back with the
struct, and keep the visibility attribute in one place.
The problem was originally reported by Simonas Kazlauskas
<s@kazlauskas.me> in id:20130418102507.GA23688@godbox but I was only
able to reproduce and investigate now that I upgraded clang.
notmuch_message_get_header started returning some headers straight
from the database in 567bcbc, but this comment explicitly claimed all
headers were read from the message file.
Add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t so that it can
cover all four values of search --exclude in the cli.
Previously the way to avoid any message being marked excluded was to
pass in an empty list of excluded tags: since we now have an explicit
option we might as well honour it.
The enum is in a slightly strange order as the existing FALSE/TRUE
options correspond to the new
NOTMUCH_EXCLUDE_FLAG/NOTMUCH_EXCLUDE_TRUE options so this means we do
not need to bump the version number.
Indeed, an example of this is that the cli count and show still use
FALSE/TRUE and still work.
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.
Xapian::TermIterator::operator* returns std::string which is destroyed
as soon as (*i).c_str() finishes. The remembered pointer 'term' then
references invalid memory.
Signed-off-by: Vladimir Marek <vlmarek@volny.cz>
Notmuch automatically re-orders names of the format "Last, First" to
"First Last" when the associated email address is
First.Last@example.com. But, if a name is of the format "Last,First"
then notmuch will format the name as "irst Last". Handle any number of
spaces after the comma, including none.
Previously, getting the list of all messages in a thread required
recursively traversing the thread's message hierarchy, which was both
difficult and resulted in messages being out of order. This adds a
public function to retrieve an iterator over all of the messages in a
thread in oldest-first order.
Previously, thread.cc built up a list of all messages, then
proceeded to tear it apart to transform it into a list of
top-level messages. Now we simply build a new list of top-level
messages.
This simplifies the interface to _notmuch_message_add_reply,
eliminates the pointer acrobatics from
_resolve_thread_relationships, and will enable us to do things
with the list of all messages in the following patches.
Previously, there were various opportunities for memory leaks in the
error-handling paths of this function. Use a local talloc context and
some reparenting to make eliminate these leaks, while keeping the
control flow simple.
Using char instead of int allows for simpler definitions of the
DOCIDSET macros so the code is easier to understand and consistent with
respect to memory-usage. Estimated reduction of memory-usage for
bitmap about 8 times.
Apparently as of GMime 2.4, you don't need to call
internet_address_list_destroy anymore, but you still need to call
g_object_unref (from the GMime Changelog).
On the medium performance corpus, valgrind shows "possibly lost"
leakage in "notmuch new" dropping from 7M to 300k.
The message->headers hash table values get data returned by
g_mime_utils_header_decode_text ().
The pointer returned by g_mime_utils_header_decode_text is from the
following line in rfc2047_decode_tokens
return g_string_free (decoded, FALSE);
The docs for g_string_free say
Frees the memory allocated for the GString. If free_segment is TRUE
it also frees the character data. If it's FALSE, the caller gains
ownership of the buffer and must free it after use with g_free().
The remaining frees and allocations referencing to message->headers hash
values have been changed to use g_free and g_malloc functions.
This combines and completes the changes started by David Bremner.
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>
OpenBSD's build flags are identical to FreeBSD, except that libraries
need to be explicitly linked against libc. No code changes are
necessary.
From: Cody Cutler <ccutler@csail.mit.edu>
Fix the COERCE_STATUS macro to handle _internal_error being declared
as void function.
Note that the function _internal_error does not return. Evaluating to
NOTMUCH_STATUS_SUCCESS is done purely to appease the compiler.
Signed-off-by: Justus Winter <4winter@informatik.uni-hamburg.de>
The API documentation (notmuch.h) states that the parameter may be NULL,
but the implementation only checked the current element, potentially
dereferencing a NULL pointer in the process.
Signed-off-by: Sascha Silbe <sascha-pgp@silbe.org>
Previously, notmuch new only synchronized maildir flags to tags for
files with a maildir "info" part. Since messages in new/ don't have
an info part, notmuch would ignore them for flag-to-tag
synchronization.
This patch makes notmuch consider messages in new/ to be legitimate
maildir messages that simply have no maildir flags set. The most
visible effect of this is that such messages now automatically get the
unread tag.
Previously, we synchronized flags to tags for any message that looked
like it had maildir flags in its file name, regardless of whether it
was in a maildir-like directory structure. This was asymmetric with
tag-to-flag synchronization, which only applied to messages in
directories named new/ and cur/ (introduced by 95dd5fe5).
This change makes our interpretation stricter and addresses this
asymmetry by only synchronizing flags to tags for messages in
directories named new/ or cur/. It also prepares us to treat messages
in new/ as maildir messages, even though they lack maildir flags.
This way notmuch_message_maildir_flags_to_tags can call it. It makes
more sense for this to be just above all of the maildir
synchronization code rather than mixed in the middle.
Previously, if passed a filename with a directory that did not exist
in the database, _notmuch_message_remove_filename would needlessly
create that directory document. Fix it so that doesn't happen.
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.
We've changed the APIs of notmuch_database_open,
notmuch_database_create, and notmuch_database_close.
Amended by db: also bump string in bindings/python/notmuch/globals.py
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>
Implicit typecast from 'void *' to 'T *' is okay in C, but not in
C++. In talloc_steal, an explicit cast is provided for type safety in
some GCC versions. Otherwise, a cast is required. Provide a template
function for this to maintain type safety, and redefine talloc_steal
to use it.
The template must be outside the extern "C" block (NOTMUCH_BEGIN_DECLS
and NOTMUCH_END_DECLS), but keep it within the GCC visibility #pragma.
No functional changes, apart from making the library build with
compilers other than recent GCC.
Signed-off-by: Jani Nikula <jani@nikula.org>
When the exclude tags contain a tag that does not occur anywhere in
the Xapian database the exclusion fails. We modify the way the query
is constructed to `work around' this. (In fact the new code is cleaner
anyway.)
It also seems to fix another exclusion failure bug reported by
jrollins but we have not yet worked out why it helps in that case.
Allow query debugging to be enabled at run-time by setting the
NOTMUCH_DEBUG_QUERY environment variable to a non-empty string.
Previously, enabling query debugging required recompiling, but parsed
queries are often useful for tracking down bugs in situations where
recompiling is inconvenient.
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>
Add the NOTMUCH_MESSAGE_FLAG_EXCLUDED flag to
notmuch_query_search_threads. Implemented by inspecting the tags
directly in _notmuch_thread_create/_thread_add_message rather than as
a Xapian query for speed reasons.
Note notmuch_thread_get_matched_messages now returns the number of
non-excluded matching messages. This API is not totally desirable but
fixing it means breaking binary compatibility so we delay that.
Add a flag NOTMUCH_MESSAGE_FLAG_EXCLUDED which is set by
notmuch_query_search_messages for excluded messages. Also add an
option omit_excluded_messages to the search that we do not want the
excludes at all.
This exclude flag will be added to notmuch_query_search threads in the
next patch.
This fixes a bug that didn't allow to search for non-ASCII words such
parts. The code here was copied from show_text_part_content(), because
the show command already does the needed conversion when showing the
message.
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.
This is useful for tags like "deleted" and "spam" that people
generally want to exclude from query results. These exclusions will
be overridden if a tag is explicitly mentioned in a query.
lib/messages.c: In function ‘notmuch_messages_move_to_next’:
lib/messages.c:131:2: warning: ISO C forbids ‘return’ with expression, in function returning void [-pedantic]
Signed-off-by: Jani Nikula <jani@nikula.org>
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.
It appears to be an oversight that encrypted parts were indexed
previously. The terms generated from encrypted parts are meaningless
and do nothing but add bloat to the database. It is not worth
indexing the encrypted content, just as it's not worth indexing the
signatures in signed parts.
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.
For some reason, on my machine, the link is picking up
/usr/lib/libutil.so instead of util/libutil.a. This causes there to be
undefined symbols in libnotmuch, making it unuseable. This patch causes
the link to fail instead.
Add function notmuch_query_count_threads() to get the number of threads
matching a search. This is done by performing a search and figuring out the
number of unique thread IDs in the matching messages, a significantly
heavier operation than notmuch_query_count_messages().
Signed-off-by: Jani Nikula <jani@nikula.org>
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.
Based on discussions with amdragon, tschwinge, and others on IRC, I concluded that
1) symbol versioning was probably overkill for libnotmuch
2) It was also probably GNU ld specific
3) Most importantly, nobody could tell me on short notice how exactly it works.
So since the change to the notmuch_database_find_message breaks the
previous ABI, we need to bump the SONAME.
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()
State up front that these functions may add a filename to an existing
message or remove only a filename (and not the message), respectively.
Previously, this key information was buried in return value
documentation or in "notes", which made it seem secondary to these
functions' semantics.
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.
Previously, this function would synchronize the folder list even if
removing the file name failed. Now it returns immediately if removing
the file name fails.
