C99 stdbool turned 18 this year. There really is no reason to use our
own, except in the library interface for backward
compatibility. Convert the lib internally to stdbool.
This is a logical followup to "lib: index the content type of
signature parts", which will make it easier to record the message
structure of all messages.
It's useful (*) to be able to easily find messages with certain types
of signatures. Having the mimetype: prefix searches fail for some
content types is also genuinely surprising (*). Index the content type
of signature parts.
While at it, switch to the gmime convenience constants for content and
signature part indexes.
*) At least for developers of email software!
'g_object_newv' is deprecated, and prints annoying warnings. The
warnings suggest using 'g_object_new_with_properties', but that's only
available since glib 2.55 (i.e. a month ago as of this writing).
Since we don't actuall pass any properties, it seems we can just call
'g_object_new'.
I considered a higher level interface where the caller passes a tag
name rather than a flag character, but the role of the "unread" tag is
particularly confusing with such an interface.
There are at least three places in notmuch that can trigger an
indexing action:
* notmuch new
* notmuch insert
* notmuch reindex
I have plans to add some indexing options (e.g. indexing the cleartext
of encrypted parts, external filters, automated property injection)
that should properly be available in all places where indexing
happens.
I also want those indexing options to be exposed by (and constrained
by) the libnotmuch C API.
This isn't yet an API break because we've never made a release with
notmuch_param_t.
These indexing options are relevant in the listed places (and in the
libnotmuch analogues), but they aren't relevant in the other kinds of
functionality that notmuch offers (e.g. dump/restore, tagging, search,
show, reply).
So i think a generic "param" object isn't well-suited for this case.
In particular:
* a param object sounds like it could contain parameters for some
other (non-indexing) operation. This sounds confusing -- why would
i pass non-indexing parameters to a function that only does
indexing?
* bremner suggests online a generic param object would actually be
passed as a list of param objects, argv-style. In this case (at
least in the obvious argv implementation), the params might be some
sort of generic string. This introduces a problem where the API of
the library doesn't grow as new options are added, which means that
when code outside the library tries to use a feature, it first has
to test for it, and have code to handle it not being available.
The indexopts approach proposed here instead makes it clear at
compile time and at dynamic link time that there is an explicit
dependency on that feature, which allows automated tools to keep
track of what's needed and keeps the actual code simple.
My proposal adds the notmuch_indexopts_t as an opaque struct, so that
we can extend the list of options without causing ABI breakage.
The cost of this proposal appears to be that the "boilerplate" API
increases a little bit, with a generic constructor and destructor
function for the indexopts struct.
More patches will follow that make use of this indexopts approach.
We need a way to pass parameters to the indexing functionality on the
first index, not just on reindexing. The obvious place is in
notmuch_database_add_message. But since modifying the argument list
would break both API and ABI, we needed a new name.
I considered notmuch_database_add_message_with_params(), but the
functionality we're talking about doesn't always add a message. It
tries to index a specific file, possibly adding a message, but
possibly doing other things, like adding terms to an existing message,
or failing to deal with message objects entirely (e.g. because the
file didn't contain a message).
So i chose the function name notmuch_database_index_file.
I confess i'm a little concerned about confusing future notmuch
developers with the new name, since we already have a private
_notmuch_message_index_file function, and the two do rather different
things. But i think the added clarity for people linking against the
future libnotmuch and the capacity for using index parameters makes
this a worthwhile tradeoff. (that said, if anyone has another name
that they strongly prefer, i'd be happy to go with it)
This changeset also adjusts the tests so that we test whether the new,
preferred function returns bad values (since the deprecated function
just calls the new one).
We can keep the deprecated n_d_add_message function around as long as
we like, but at the next place where we're forced to break API or ABI
we can probably choose to drop the name relatively safely.
NOTE: there is probably more cleanup to do in the ruby and go bindings
to complete the deprecation directly. I don't know those languages
well enough to attempt a fix; i don't know how to test them; and i
don't know the culture around those languages about API additions or
deprecations.
Stripping trailing character is not that uncommon
operation. Particularly, the next patch has to perform it as
well. Lets move it to the separate function to avoid code duplication.
Also the new function has a little improvement: if the character to
strip is repeated several times in the end of a string, function
strips them all.
Signed-off-by: Yuri Volchkov <yuri.volchkov@gmail.com>
Since we're accumulating the index when we add a new file to the
message, the semantics have slightly changed. This tries to align the
documentation with the actual functionality.
This new function asks the database to reindex a given message.
The parameter `indexopts` is currently ignored, but is intended to
provide an extensible API to support e.g. changing the encryption or
filtering status (e.g. whether and how certain non-plaintext parts are
indexed).
This operation is relatively inexpensive, as the needed metadata is
already computed by our lazy metadata fetching. The goal is to support
better UI for messages with multipile files.
The corresponding xapian document just gets more terms added to it,
but this doesn't seem to break anything. Values on the other hand get
overwritten, which is a bit annoying, but arguably it is not worse to
take the values (from, subject, date) from the last file indexed
rather than the first.
This is really pure C string parsing, and doesn't need to be mixed in
with the Xapian/C++ layer. Although not strictly necessary, it also
makes it a bit more natural to call _parse_message_id from multiple
compilation units.
The switch is easier to understand than the side effects in the if
test. It also potentially allows us more flexibility in breaking up
this function into smaller pieces, since passing private_status around
is icky.
'database.cc' is becoming a monster, and it's hard to follow what the
various static functions are used for. It turns out that about 1/3 of
this file notmuch_database_add_message and helper functions not used
by any other function. This commit isolates this code into it's own
file.
Some side effects of this refactoring:
- find_doc_ids becomes the non-static (but still private)
_notmuch_database_find_doc_ids
- a few instances of 'string' have 'std::' prepended, avoiding the
need for 'using namespace std;' in the new file.
We need to rewrite the loop for gmime-3.0; move the loop body to its
own function to avoid code duplication. Keep the common exit via
"goto DONE" to make this pure code movement. It's important to note
that the existing exit path only deallocates the iterator.
We want to reuse the scanner definition with a different table. This
is mainly code movement, and making the state table part of the filter
struct/class.
Commit d5523ead90 ("Mark some structures in the library interface
with visibility=default attribute.") fixed some mixed visibility
issues with structs. With the symbol default visibility reversed, this
is no longer a problem.
The dynamic generation of the linker version script for libnotmuch
exports has grown rather complicated.
Reverse the visibility control by hiding symbols by default using
-fvisibility=hidden, and explicitly exporting symbols in notmuch.h
using #pragma GCC visibility. (We could also use __attribute__
((visibility ("default"))) for each exported function, but the pragma
is more convenient.)
The above is not quite enough alone, as it would "leak" a number of
weak symbols from Xapian and C++ standard library. Combine it with a
small static version script that filters out everything except the
notmuch_* symbols that we explicitly exposed, and the C++ RTTI
typeinfo symbols for exception handling.
Finally, as the symbol hiding test can no longer look at the generated
symbol table, switch the test to parse the functions from notmuch.h.
Commits 9db2145272 ("lib/gen-version-script.h: add getline and
getdelim to notmuch.sym if needed") and 3242e29e57 ("build: add
canonicalize_file_name to symbols exported from libnotmuch.so")
started exporting compat functions from libnotmuch so that the cli
could use them. But we shouldn't export such functions from the
library. They are not part of our ABI. Instead, the cli should include
its own copies of the compat functions.
From a UI perspective this looks similar to what was already provided
for from, subject, and mid, but the implementation is quite
different. It uses the database's list of terms to construct a term
based query equivalent to the passed regular expression.
The index(3) function has been deprecated in POSIX since 2001 and
removed in 2008, and most code in notmuch already calls strchr(3).
This fixes a compilation error on Android whose libc does not have
index(3).
The non-field processor behaviour is is convert the corresponding
queries into a search for the unprefixed terms. This yields pretty
surprising results so I decided to generate a query that would match
the terms (i.e. none with that prefix) generated for an empty header.
The argument is that if the string passed to the field processor has
no spaces, then the added quotes won't have any benefit except for
disabling wildcards. But disabling wildcards doesn't seem very useful
in the normal Xapian query parser, since they're stripped before
generating terms anyway. It does mean that the query 'from:"foo*"' will
not be precisely equivalent to 'from:foo' as it is for the non
field-processor version.
This function was deprecated in notmuch 0.21. We re-use the name for
a status returning version, and deprecate the _st name. One or two
remaining uses of the (removed) non-status returning version fixed at
the same time
This function was deprecated in notmuch 0.21. We finally remove the
deprecated API, and rename the status returning version to the simpler
name. The status returning is kept as a deprecated alias.
Apparently some systems (MacOS?) have a system library called libutil
and the name conflict causes problems. Since this library is quite
notmuch specific, rename it to something less generic.
The object where pointer to `data` was received was deleted before
it was used in _notmuch_string_list_append().
Relevant Coverity messages follow:
3: extract
Assigning: data = std::__cxx11::string(message->doc.()).c_str(),
which extracts wrapped state from temporary of type std::__cxx11::string.
4: dtor_free
The internal representation of temporary of type std::__cxx11::string
is freed by its destructor.
5: use after free:
Wrapper object use after free (WRAPPER_ESCAPE)
Using internal representation of destroyed object local data.
For reasons not completely understood at this time, gmime (as of
2.6.22) is returning a date before 1900 on bad date input. Since this
confuses some other software, we clamp such dates to 0,
i.e. 1970-01-01.
Remove incorrect skipping to first match from init(), and add explicit
skip_to() and check() methods to work around xapian-core bug (the
check() method will also improve speed when filtering by one of
these).
We filter added exclude at add time, rather than modifying the query by
count search. As noted in the comments, there are several ignored
conditions here.
The main goal is to prepare the way for non-destructive (or at least
less destructive) exclude tag handling. It does this by having a
pre-parsed query available for further processing. This also allows us
to provide slightly more precise error messages.
Fix warning caught by clang:
lib/regexp-fields.cc:41:2: warning: 'delete' applied to a pointer that was allocated
with 'new[]'; did you mean 'delete[]'? [-Wmismatched-new-delete]
delete buffer;
^
[]
lib/regexp-fields.cc:37:17: note: allocated with 'new[]' here
char *buffer = new char[len];
^
mid: is the url scheme suggested by URL 2392. We also plan to
introduce more flexible searches for mid: than are possible with
id: (in order not to break assumptions about the special behaviour of
id:, e.g. identifying at most one message).
the idea is that you can run
% notmuch search subject:/<your-favourite-regexp>/
% notmuch search from:/<your-favourite-regexp>/
or
% notmuch search subject:"your usual phrase search"
% notmuch search from:"usual phrase search"
This feature is only available with recent Xapian, specifically
support for field processors is needed.
It should work with bindings, since it extends the query parser.
This is easy to extend for other value slots, but currently the only
value slots are date, message_id, from, subject, and last_mod. Date is
already searchable; message_id is left for a followup commit.
This was originally written by Austin Clements, and ported to Xapian
field processors (from Austin's custom query parser) by yours truly.
The retries are hardcoded to a small number, and error handling aborts
than propagating errors from notmuch_database_reopen. These are both
somewhat justified by the assumption that most things that can go
wrong in Xapian::Database::reopen are rare and fatal. Here's the brief
discussion with Xapian upstream:
24-02-2017 08:12:57 < bremner> any intuition about how likely
Xapian::Database::reopen is to fail? I'm catching a
DatabaseModifiedError somewhere where handling any further errors is
tricky, and wondering about treating a failed reopen as as "the
impossible happened, stopping"
24-02-2017 16:22:34 < olly> bremner: there should not be much scope for
failure - stuff like out of memory or disk errors, which are probably a
good enough excuse to stop
The two g_hash_table functions (insert, add) have different behaviour
with respect to existing keys. g_hash_table_insert frees the new key,
while g_hash_table_add (which is really g_hash_table_replace in
disguise) frees the existing key. With this change 'ref' is live until
the end of the function (assuming single-threaded access to
'hash'). We can't guarantee it will continue to be live in the
future (i.e. there may be a future key duplication) so we copy it with
the allocation context passed to parse_references (in practice this is
the notmuch_message_t object whose parents we are finding).
Thanks to Tomi for the simpler approach to the problem based on
reading the fine glib manual.
Replace multiple tables with some flags in a single table. This makes
the code in notmuch_database_open_verbose a bit shorter, and it should
also make it easier to add other options to fields, e.g. regexp
searching.
From #xapian
olly> bremner: btw, i noticed notmuch count see ms to request all the documents and then ignores them
bremner> hmm. There's something funny about the way that notmuch uses matches in general iirc
olly> it should be able to do: mset = enquire.get_mset (0, 0, notmuch->xapian_db->get_doccount ());
...
olly> get_matches_estimated() will be exact because check_at_least is the size of the database
We already depend on glib both directly and indirectly (via gmime). We
might as well make use of its facilities. Drop the embedded libsha1
and use glib for sha1 digests.
The todo comment got separated from the status it's related to at
commit 3f32fd8a1c ("Add missing comment for
NOTMUCH_STATUS_READONLY_DATABASE."). Later, commit b65ca8e0ba ("lib:
modify notmuch.h for automatic document generation") moved it, but to
the wrong place. Fix the location.
It seems that no-one tried to compile without Xapian compact support
since March of 2015, since that's when I introduced a syntax error in
that branch of the ifdef.
Given the choice of maintaining this underused branch of code, or
bumping the Xapian dependency to a version from 2011, it seems
reasonable to do the latter.
This should not change the SONAME, and therefore won't change the
dynamic linking behaviour, but it may help some users debug missing
symbols in case their libnotmuch is too old.
This is needed so that when the map is modified during traversal, and
thus unlinked by the database code, the map is not disposed of until the
iterator is done with it.
We want to be able to query the properties directly, like:
notmuch count property:foo=bar
which should return a count of messages where the property with key
"foo" has value equal to "bar".
I believe the current one is misleading, because in my experiments
Xapian did not add : when prefix and term were both upper case. Indeed,
it's hard to see how it could, because prefixes are added at a layer
above Xapian in our code. See _notmuch_message_add_term for an example.
Also try to explain why this is a good idea. As far as I can ascertain,
this is more of an issue for a system trying to work with an unknown set
of prefixes. Since notmuch has a fixed set of prefixes, and we can
hopefully be trusted not to add XGOLD and XGOLDEN as prefixes, it is
harder for problems to arise.
_notmuch_database_log clears the log buffer each time. Rather than
introducing more complicated semantics about for this function, provide
a second function that does not clear the buffer. This is mainly a
convenience function for callers constructing complex or multi-line log
messages.
The changes to query.cc are to make sure that the common code path of
the new function is tested.
This support will be present only if the appropriate version of xapian
is available _and_ the user did not disable the feature when
building. So there really needs to be some way for the user to check.
Xapian 1.3 has introduced the DB_RETRY_LOCK flag (Xapian bug
275). Detect it in configure and optionally use it. With this flag
commands that need the write lock will wait for their turn instead of
aborting when it's not immediately available.
Amended by db: allow disabling in configure
Fix bug reported in id:20160606124522.g2y2eazhhrwjsa4h@flatcap.org
Although the C99 standard 6.10 is a little non-obvious on this point,
the docs for e.g. gcc are unambiguous. And indeed in practice with the
extra space, this code fails
#include <stdio.h>
#define foo (x) (x+1)
int main(int argc, char **argv){
printf("%d\n",foo(1));
}
Many of the external links found in the notmuch source can be resolved
using https instead of http. This changeset addresses as many as i
could find, without touching the e-mail corpus or expected outputs
found in tests.
Cleaned the following whitespace in lib/* files:
lib/index.cc: 1 line: trailing whitespace
lib/database.cc 5 lines: 8 spaces at the beginning of line
lib/notmuch-private.h: 4 lines: 8 spaces at the beginning of line
lib/message.cc: 1 line: trailing whitespace
lib/sha1.c: 1 line: empty lines at the end of file
lib/query.cc: 2 lines: 8 spaces at the beginning of line
lib/gen-version-script.sh: 1 line: trailing whitespace
It's already kindof gross that this is hardcoded in two different
places. We will also need these later in field processors calling back
into the query parser.
Since xapian provides the ability to restrict the iterator to a given
prefix, we expose this ability to the user. Otherwise we mimic the other
iterator interfances in notmuch (e.g. tags.c).
This is a thin wrapper around the Xapian metadata API. The job of this
layer is to keep the config key value pairs from colliding with other
metadata by transparently prefixing the keys, along with the usual glue
to provide a C interface.
The split of _get_config into two functions is to allow returning of the
return value with different memory ownership semantics.
To fully complete the ghost-on-removal-when-shared-thread-exists
proposal, we need to clear all ghost messages when the last active
message is removed from a thread.
Amended by db: Remove the last test of T530, as it no longer makes sense
if we are garbage collecting ghost messages.
There is no need to add a ghost message upon deletion if there are no
other active messages in the thread.
Also, if the message being deleted was a ghost already, we can just go
ahead and delete it.
Publicly we are only exposing the non-ghost documents (of "type"
"mail"). But internally we might want to inspect the ghost messages
as well.
This changeset adds two new private interfaces to queries to recover
information about alternate document types.
implement ghost-on-removal, the solution to T590-thread-breakage.sh
that just adds a ghost message after removing each message.
It leaks information about whether we've ever seen a given message id,
but it's a fairly simple implementation.
Note that _resolve_message_id_to_thread_id already introduces new
message_ids to the database, so i think just searching for a given
message ID may introduce the same metadata leakage.
The code to skip multiple slashes in _notmuch_database_split_path()
skips back one character too much. This is compensated by a +1 in the
length parameter to the strndup() call. Mostly this works fine, but if
the path is to a file under a top level directory with one character
long name, the directory part is mistaken to be part of the file name
(slash == path in code). The returned directory name will be the empty
string and the basename will be the full path, breaking the indexing
logic in notmuch new.
Fix the multiple slash skipping to keep the slash variable pointing at
the last slash, and adjust strndup() accordingly.
The bug was introduced in
commit e890b0cf40
Author: Carl Worth <cworth@cworth.org>
Date: Sat Dec 19 13:20:26 2009 -0800
database: Store the parent ID for each directory document.
just a little over two months after the initial commit in the Notmuch
code history, making this the longest living bug in Notmuch to date.
Some compilers (older than gcc 4.5 and clang 2.9) do support
__attribute__ ((deprecated)) but not
__attribute__ ((deprecated("message"))).
Check if clang version is at least 3.0, or gcc version
is at least 4.5 to define NOTMUCH_DEPRECATED as the
latter variant above. Otherwise define NOTMUCH_DEPRECATED
as the former variant above.
For a bit simpler implementation clang 2.9 is not included
to use the newer variant. It is just one release, and the
older one works fine. Clang 3.0 was released around 2011-11
and gcc 5.1 2015-04-22 (therefore newer macro for gcc 4.5+)
We can't (but currently do) allow upgrades within transactions because
upgrades need their own transactions. We don't want to re-use the
current transaction because bailing out of an upgrade would mean loosing
all previous changes (because our "atomic" transactions don't commit
before hand). This gives us two options:
1. Fail at the beginning of upgrade (tell the user to end the
transaction, upgrade, and start over).
2. Don't allow the user to start the transaction.
I went with the latter because:
1. There is no reason to call `begin_atomic` unless you intend to to
write to the database and anyone intending to write to the database
should upgrade it first.
2. This means that nothing inside an atomic transaction can ever fail
with NOTMUCH_STATUS_UPGRADE_REQUIRED.
Per RFC 2183, the values for Content-Disposition values are not
case-sensitive. While at it, use the gmime function for getting at the
disposition string instead of referencing the field directly.
This fixes "attachment" tagging and filename term generation for
attachments while indexing.
Although I think it's a pretty bad idea to continue using the old API,
this allows both a more gentle transition for clients of the library,
and allows us to break one monolithic change into a series
It doesn't seem likely we can support simple date:<expr> expanding to
date:<expr>..<expr> any time soon. (This can be done with a future
version of Xapian, or with a custom query query parser.) In the mean
time, provide shorthand date:<expr>..! to mean the same. This is
useful, as the expansion takes place before interpetation, and we can
use, for example, date:yesterday..! to match from beginning of
yesterday to end of yesterday.
Idea from Mark Walters <markwalters1009@gmail.com>.
These functions are all just accessors, and it's pretty clear they don't
modify the query struct. This also fixes one warning I created when I
introduced status.c.
This exposes the committed database revision to library users along
with a UUID that can be used to detect when revision numbers are no
longer comparable (e.g., because the database has been replaced).
This adds a new document value that stores the revision of the last
modification to message metadata, where the revision number increases
monotonically with each database commit.
An alternative would be to store the wall-clock time of the last
modification of each message. In principle this is simpler and has
the advantage that any process can determine the current timestamp
without support from libnotmuch. However, even assuming a computer's
clock never goes backward and ignoring clock skew in networked
environments, this has a fatal flaw. Xapian uses (optimistic)
snapshot isolation, which means reads can be concurrent with writes.
Given this, consider the following time line with a write and two read
transactions:
write |-X-A--------------|
read 1 |---B---|
read 2 |---|
The write transaction modifies message X and records the wall-clock
time of the modification at A. The writer hangs around for a while
and later commits its change. Read 1 is concurrent with the write, so
it doesn't see the change to X. It does some query and records the
wall-clock time of its results at B. Transaction read 2 later starts
after the write commits and queries for changes since wall-clock time
B (say the reads are performing an incremental backup). Even though
read 1 could not see the change to X, read 2 is told (correctly) that
X has not changed since B, the time of the last read. In fact, X
changed before wall-clock time A, but the change was not visible until
*after* wall-clock time B, so read 2 misses the change to X.
This is tricky to solve in full-blown snapshot isolation, but because
Xapian serializes writes, we can use a simple, monotonically
increasing database revision number. Furthermore, maintaining this
revision number requires no more IO than a wall-clock time solution
because Xapian already maintains statistics on the upper (and lower)
bound of each value stream.
- Make lib/notmuch.h the canonical location for the library versioning
information.
- Since the release-check should never fail now, remove it to reduce
complexity.
- Make the version numbers in notmuch.h consistent with the (now
deleted) ones in lib/Makefile.local
The CLI (and bindings) code should really be updated to use the new
status-code-returning versions. Here are some warnings to prod us (and
other clients) to do so.
Previously, we updated the database copy of a message on every call to
_notmuch_message_sync, even if nothing had changed. In particular,
this always happens on a thaw, so a freeze/thaw pair with no
modifications between still caused a database update.
We only modify message documents in a handful of places, so keep track
of whether the document has been modified and only sync it when
necessary. This will be particularly important when we add message
revision tracking.
It turns out that on certain systems like FreeBSD, c++filt is not
installed by default. It's basically OK if we fail the build in that
case, but what's really not OK is for the build to continue and
generate bad binaries.
There are many places in the notmuch code where the path is assumed to be absolute. If someone (TM) wants a project, one could remove these assumptions. In the mean time, prevent users from shooting themselves in the foot.
Update test suite mark tests for this error as no longer broken, and
also convert some tests that used relative paths for nonexistent
directories.
The difference with FILE_ERROR is that this is for things that are
wrong with the path before looking at the disk.
Add some 3 tests; two broken as a reminder to actually use this new
code.
You may wonder why _notmuch_message_file_open_ctx has two parameters.
This is because we need sometime to use a ctx which is a
notmuch_message_t. While we could get the database from this, there is
no easy way in C to tell type we are getting.
This is not supposed to change any functionality from an end user
point of view. Note that it will eliminate some output to stderr. The
query debugging output is left as is; it doesn't really fit with the
current primitive logging model. The remaining "bad" fprintf will need
an internal API change.
The compatibility wrapper ensures that clients calling
notmuch_database_open will receive consistent output for now.
The changes to notmuch-{new,search} and test/symbol-test are just to
make the test suite pass.
The use of IGNORE_RESULT is justified by two things. 1) I don't know
what else to do. 2) asprintf guarantees the output string is NULL if
an error occurs, so at least we are not passing garbage back.
