The exact error messages returned by regerror() aren't standardized;
relying on them isn't portable. Thus, add a a prefix to make clear that
the subsequent message is a regexp parsing error, and only look for this
prefix in the test suite, ignoring the rest of the message.
I can't figure out how checking the sign of a bool ever worked. The
following program demonstrates the problem (i.e. for me it prints 1).
#include <stdio.h>
#include <stdbool.h>
int main(int argc, char **argv) {
bool x;
x = -1;
printf("x = %d\n", x);
}
This seems to be mandated by the C99 standard 6.3.1.2.
Use explicit labels for GTypeInfo member initializers, rather than
relying on comments and ordering. This is both easier to read, and
harder to screw up. This also makes it clear that we're mis-casting
GObject class initializers for gcc.
Without this patch, g++ 8.2.0-7 produces this warning:
CXX -g -O2 lib/index.o
lib/index.cc: In function ‘GMimeFilter* notmuch_filter_discard_non_term_new(GMimeContentType*)’:
lib/index.cc:252:23: warning: cast between incompatible function types from ‘void (*)(NotmuchFilterDiscardNonTermClass*)’ {aka ‘void (*)(_NotmuchFilterDiscardNonTermClass*)’} to ‘GClassInitFunc’ {aka ‘void (*)(void*, void*)’} [-Wcast-function-type]
(GClassInitFunc) notmuch_filter_discard_non_term_class_init,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The definition of GClassInitFunc in
/usr/include/glib-2.0/gobject/gtype.h suggests that this function will
always be called with the class_data member of the GTypeInfo. We set
that value to NULL in both GObject definitions in notmuch. So we mark
it as explicitly unused.
There is no functional change here, just code cleanup.
As reported by Sean Whitton, there are mailers (in particular the
Debian Bug Tracking System) that have sensible In-Reply-To headers,
but un-useful-for-notmuch References (in particular with the BTS, the
oldest reference is last). I looked at a sample of about 200K
messages, and only about 0.5% these had something other than a single
message-id in In-Reply-To. On this basis, if we see a single
message-id in In-Reply-To, consider that as authoritative.
The idea is that if a message-id parses with this function, the MUA
generating it was probably sane, and in particular it's probably safe
to use the result as a parent from In-Reply-to.
We (finally) implement the XXX comment. It requires a bit of care not
to reparent all of the possible toplevel messages.
_notmuch_messages_has_next is not ready to be a public function yet,
since it punts on the mset case. We know in the one case it is called,
the notmuch_messages_t is just a regular list / iterator.
This is mainly to lay out the structure of the final code. The problem
isn't really solved yet, although some very simple cases are
better (hence the fixed test). We need two passes through the messages
because we need to be careful not to re-parent too many messages and
end up without any toplevel messages.
There is no public notmuch_message_list_t public interface, so to this
is added to the private API. We use it immediately in thread.cc;
future commits will use it further.
For non-root messages, this should not should anything currently, as
the messages are already added in date order. In the future we will
add some non-root messages in a second pass out of order and the
sorting will be useful. It does fix the order of multiple
root-messages (although it is overkill for that).
This is technically an API change, but it is not an ABI change, and
it's merely a statement that limits what the library can do.
This is in parallel to notmuch_query_get_database(), which also takes
a const pointer.
The user can already do this manually, of course, but (a) it's nice to
have a convenience function, and (b) exposing this interface means
that someone more clever with a _notmuch_string_map_t than i am can
write a more efficient version if they like, and it will just
accelerate the users of the convenience function.
We've had _notmuch_message_database() internally for a while, and it's
useful. It turns out to be useful on the other side of the library
interface as well (i'll use it later in this series for "notmuch
show"), so we expose it publicly now.
The observation is that we are only using the messages to get there
thread_id, which is kindof a pessimal access pattern for the current
notmuch_message_get_thread_id
This change allows queries of the form
thread:{from:me} and thread:{from:jian} and not thread:{from:dave}
This is still somewhat brute-force, but it's a big improvement over
both the shell script solution and the previous proposal [1], because it
does not build the whole thread structure just generate a
query. A further potential optimization is to replace the calls to
notmuch with more specialized Xapian code; in particular it's not
likely that reading all of the message metadata is a win here.
[1]: id:20170820213240.20526-1-david@tethera.net
Correct URLs that have crept into the notmuch codebase with http://
when https:// is possible.
As part of this conversion, this changeset also indicates the current
preferred upstream URLs for both gmime and sup. the new URLs are
https-enabled, the old ones are not.
This also fixes T310-emacs.sh, thanks to Bremner for catching it.
At least Fedora28 triggers this Xapian bug due to some toolchain change .
https://bugzilla.redhat.com/show_bug.cgi?id=1546162
The underlying bug is fixed in xapian commit f92e2a936c1592, and
should be fixed in Xapian 1.4.6
We added several new functions, at least
notmuch_database_get_default_indexopts
notmuch_database_index_file
notmuch_indexopts_destroy
notmuch_indexopts_get_decrypt_policy
notmuch_indexopts_set_decrypt_policy
notmuch_message_count_files
notmuch_message_has_maildir_flag
notmuch_message_reindex
notmuch_message_remove_all_properties_with_prefix
notmuch_thread_get_total_files
This change allows queries of the form
thread:{from:me} and thread:{from:jian} and not thread:{from:dave}
This is still somewhat brute-force, but it's a big improvement over
both the shell script solution and the previous proposal [1], because it
does not build the whole thread structure just generate a
query. A further potential optimization is to replace the calls to
notmuch with more specialized Xapian code; in particular it's not
likely that reading all of the message metadata is a win here.
[1]: id:20170820213240.20526-1-david@tethera.net
The current behaviour is at best under-documented. The modified test in
T470-missing-headers.sh previously relied on printf doing the right
thing with NULL, which seems icky.
The use of talloc_strdup here is probably overkill, but it avoids
having to enforce that thread->authors is never mutated outside
_resolve_thread_authors_string.
Here's the configuration choice for people who want a cleartext index,
but don't want stashed session keys.
Interestingly, this "nostash" decryption policy is actually the same
policy that should be used by "notmuch show" and "notmuch reply",
since they never modify the index or database when they are invoked
with --decrypt.
We take advantage of this parallel to tune the behavior of those
programs so that we're not requesting session keys from GnuPG during
"show" and "reply" that we would then otherwise just throw away.
If you're going to store the cleartext index of an encrypted message,
in most situations you might just as well store the session key.
Doing this storage has efficiency and recoverability advantages.
Combined with a schedule of regular OpenPGP subkey rotation and
destruction, this can also offer security benefits, like "deletable
e-mail", which is the store-and-forward analog to "forward secrecy".
But wait, i hear you saying, i have a special need to store cleartext
indexes but it's really bad for me to store session keys! Maybe
(let's imagine) i get lots of e-mails with incriminating photos
attached, and i want to be able to search for them by the text in the
e-mail, but i don't want someone with access to the index to be
actually able to see the photos themselves.
Fret not, the next patch in this series will support your wacky
uncommon use case.
There are some situations where the user wants to get rid of the
cleartext index of a message. For example, if they're indexing
encrypted messages normally, but suddenly they run across a message
that they really don't want any trace of in their index.
In that case, the natural thing to do is:
notmuch reindex --decrypt=false id:whatever@example.biz
But of course, clearing the cleartext index without clearing the
stashed session key is just silly. So we do the expected thing and
also destroy any stashed session keys while we're destroying the index
of the cleartext.
Note that stashed session keys are stored in the xapian database, but
xapian does not currently allow safe deletion (see
https://trac.xapian.org/ticket/742).
As a workaround, after removing session keys and cleartext material
from the database, the user probably should do something like "notmuch
compact" to try to purge whatever recoverable data is left in the
xapian freelist. This problem really needs to be addressed within
xapian, though, if we want it fixed right.
The new "auto" decryption policy is not only good for "notmuch show"
and "notmuch reindex". It's also useful for indexing messages --
there's no good reason to not try to go ahead and index the cleartext
of a message that we have a stashed session key for.
This change updates the defaults and tunes the test suite to make sure
that they have taken effect.
In our consolidation of _notmuch_crypto_decrypt, the callers lost
track a little bit of whether any actual decryption was attempted.
Now that we have the more-subtle "auto" policy, it's possible that
_notmuch_crypto_decrypt could be called without having any actual
decryption take place.
This change lets the callers be a little bit smarter about whether or
not any decryption was actually attempted.
This new automatic decryption policy should make it possible to
decrypt messages that we have stashed session keys for, without
incurring a call to the user's asymmetric keys.
Future patches in this series will introduce new policies; this merely
readies the way for them.
We also convert --try-decrypt to a keyword argument instead of a boolean.
the command-line interface for indexing (reindex, new, insert) used
--try-decrypt; and the configuration records used index.try_decrypt.
But by comparison with "show" and "reply", there doesn't seem to be
any reason for the "try" prefix.
This changeset adjusts the command-line interface and the
configuration interface.
For the moment, i've left indexopts_{set,get}_try_decrypt alone. The
subsequent changeset will address those.
When doing any decryption, if the notmuch database knows of any
session keys associated with the message in question, try them before
defaulting to using default symmetric crypto.
This changeset does the primary work in _notmuch_crypto_decrypt, which
grows some new parameters to handle it.
The primary advantage this patch offers is a significant speedup when
rendering large encrypted threads ("notmuch show") if session keys
happen to be cached.
Additionally, it permits message composition without access to
asymmetric secret keys ("notmuch reply"); and it permits recovering a
cleartext index when reindexing after a "notmuch restore" for those
messages that already have a session key stored.
Note that we may try multiple decryptions here (e.g. if there are
multiple session keys in the database), but we will ignore and throw
away all the GMime errors except for those that come from last
decryption attempt. Since we don't necessarily know at the time of
the decryption that this *is* the last decryption attempt, we'll ask
for the errors each time anyway.
This does nothing if no session keys are stashed in the database,
which is fine. Actually stashing session keys in the database will
come as a subsequent patch.
This flag should make it easier to write the code for session-key
handling.
Note that this only works for GMime 2.6.21 and later (the session key
interface wasn't available before then). It should be fine to build
the rest of notmuch if this functionality isn't available.
Note that this also adds the "session_key" built_with() aspect to
libnotmuch.
We will use this centralized function to consolidate the awkward
behavior around different gmime versions.
It's only invoked from two places: mime-node.c's
node_decrypt_and_verify() and lib/index.cc's
_index_encrypted_mime_part().
However, those two places have some markedly distinct logic, so the
interface for this _notmuch_crypto_decrypt function is going to get a
little bit clunky. It's worthwhile, though, for the sake of keeping
these #if directives reasonably well-contained.
By default, notmuch won't try to decrypt on indexing. With this
patch, we make it possible to indicate a per-database preference using
the config variable "index.try_decrypt", which by default will be
false.
At indexing time, the database needs some way to know its internal
defaults for how to index encrypted parts. It shouldn't be contingent
on an external config file (since that can't be retrieved from the
database object itself), so we store it in the database.
This behaves similarly to the query.* configurations, which are also
stored in the database itself, so we're not introducing any new
dependencies by requiring that it be stored in the database.
If we see index options that ask us to decrypt when indexing a
message, and we encounter an encrypted part, we'll try to descend into
it.
If we can decrypt, we add the property index.decryption=success.
If we can't decrypt (or recognize the encrypted type of mail), we add
the property index.decryption=failure.
Note that a single message may have both values of the
"index.decryption" property: "success" and "failure". For example,
consider a message that includes multiple layers of encryption. If we
manage to decrypt the outer layer ("index.decryption=success"), but
fail on the inner layer ("index.decryption=failure").
Because of the property name, this will be automatically cleared (and
possibly re-set) during re-indexing. This means it will subsequently
correspond to the actual semantics of the stored index.
This allows us to create new properties that will be automatically set
during indexing, and cleared during re-indexing, just by choice of
property name.
This is currently mostly a wrapper around _notmuch_crypto_t that keeps
its internals private and doesn't expose any of the GMime API.
However, non-crypto indexing options might also be added later
(e.g. filters or other transformations).
Subsequent patches will introduce a convention that properties whose
name starts with "index." will be stripped (and possibly re-added)
during re-indexing. This patch lays the groundwork for doing that.
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.
