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.
The default is actually exact if no checkatleast parameter is
specified. This change makes that explicit, mainly for documentation,
but also to be safe in the unlikely event of a change of default.
[ commit message rewritten by db based on id:87lho0nlkk.fsf@nikula.org
]
The install_name of libnotmuch.dylib on Mac OS X is what is written
into a program that links against it. If it is just the name of the
shared library file, as opposed to the full path, the program won't be
able to find it when it runs and will abort. Instead, the install_name
should be the full path to the shared library (in its final installed
location).
Why does Notmuch work without this patch when installed via Homebrew?
The answer is twofold. One, /usr/local/lib is a special location in
which the dynamic linker will look by default to find shared libraries.
Homebrew highly recommends installing to /usr/local, and, assuming it
has been configured this way, the Notmuch library will end up installed
in /usr/local/lib, and the dynamic linker will find it. Two, Homebrew
globally corrects all install names in dynamically shared libraries and
binaries for each package it installs. So, even if the install names in
a package's binaries and libraries are incorrect, Homebrew corrects them
automatically, and no one ever knows.
Why does Notmuch work without this patch when installed via MacPorts?
The answer is that MacPorts applies a patch just like this patch to fix
the same problem.
This indicates upwardly compatible changes, namely adding new symbols.
Although we don't formally need to do this until the next release,
there is no hard in doing it now, as long as we don't bump the minor
version for every addition between now and the release.
This at least allows distinguishing between out of memory and Xapian
exceptions. Adding finer grained status codes would allow different
Xapian exceptions to be preserved.
Adding wrappers allows people to transition gradually to the new API,
at the cost of bloating the library API a bit.
This adds the indexing support for the "mimetype:" term and removes
the broken test flag. The indexing is probablistic in Xapian terms,
which gives a better experience to end users. Standard content-types
of the form "foo/bar" are automatically interpreted as phrases in
Xapian due to the embedded slash.
Assume, separate messages with application/pdf and application/x-pdf
are indexed, then:
- mimetype:application/x-pdf will find only the application/x-pdf
- mimetype:application/pdf will find only the application/pdf
- mimetype:pdf will find both of the messages
This feature will exist in all newly created databases, but there is
no upgrade provided for it. If this flag exists, it indicates that
the database was created after the indexed MIME-types feature was
added.
_thread_set_subject_from_message sometimes replaces the subject, making the
cur_subject point to free'd memory
==6550== ERROR: AddressSanitizer: heap-use-after-free on address 0x601a0000bec0 at pc 0x4464a4 bp 0x7fffa40be910 sp 0x7fffa40be908
READ of size 1 at 0x601a0000bec0 thread T0
#0 0x4464a3 in _thread_add_matched_message /home/todd/.apps/notmuch/lib/thread.cc:369
#1 0x443c2c in notmuch_threads_get /home/todd/.apps/notmuch/lib/query.cc:496
#2 0x41d947 in do_search_threads /home/todd/.apps/notmuch/notmuch-search.c:131
#3 0x40a3fe in main /home/todd/.apps/notmuch/notmuch.c:345
#4 0x7f4e535b4ec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287
#5 0x40abe6 in _start ??:?
0x601a0000bec0 is located 96 bytes inside of 134-byte region [0x601a0000be60,0x601a0000bee6)
freed by thread T0 here:
#0 0x7f4e54e6933a in __interceptor_free ??:?
#1 0x7f4e54482fab in _talloc_free ??:?
previously allocated by thread T0 here:
#0 0x7f4e54e6941a in malloc ??:?
#1 0x7f4e54485b5d in talloc_strdup ??:?
==22884== ERROR: AddressSanitizer: heap-buffer-overflow on address 0x601600008291 at pc 0x7ff6295680e5 bp 0x7fff4ab9aa40 sp 0x7fff4ab9aa08
READ of size 1 at 0x601600008291 thread T0
#0 0x7ff6295680e4 in __interceptor_strcmp ??:?
#1 0x44763b in _thread_add_message /home/todd/.apps/notmuch/lib/thread.cc:255
#2 0x4459e8 in notmuch_threads_get /home/todd/.apps/notmuch/lib/query.cc:496
#3 0x41e2a7 in do_search_threads /home/todd/.apps/notmuch/notmuch-search.c:131
#4 0x40a408 in main /home/todd/.apps/notmuch/notmuch.c:345
#5 0x7ff627cb9ec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287
#6 0x40abf3 in _start ??:?
0x601600008291 is located 0 bytes to the right of 97-byte region [0x601600008230,0x601600008291)
allocated by thread T0 here:
#0 0x7ff62956e41a in malloc ??:?
#1 0x7ff628b8ab5d in talloc_strdup ??:?
Currently the thread is named based on either the oldest or newest
matching message (depending on the search order). If this message has
an empty subject, though, the thread will show up with an empty
subject in the search results. (See the thread starting with
`id:1412371140-21051-1-git-send-email-david@tethera.net` for an
example.)
This changes the behavior so it will use a non-empty name for the
thread if possible. We name threads based on (a) non-empty matches for
the query, and (b) the search order. If the search order is
oldest-first (as in the default inbox) it chooses the oldest matching
non-empty message as the subject. If the search order is newest-first
it chooses the newest one.
Tamas Szakaly points out [1] that the bug fixed in 51b073c still
exists in at least one place. This change follows the suggestion of
[2] and creates a block scope temporary std::string to avoid the rules
of iterators temporaries.
[1]: id:20141226113755.GA64154@pamparam
[2]: id:20141226230655.GA41992@pamparam
We generally do not support mbox files, but for historical reasons
we've supported single-message mbox files, with a deprecation
message. We've tried dropping the support altogether, but backed out
of it because we'd need to stop indexing them, while keeping support
for previously indexed files. This would be more complicated than
simply supporting single-message mbox files. Therefore, drop the
deprecation message, and just silently accept single-message mboxes.
Currently, if a From-header is of the form:
"" <address@example.com>
the empty string will be treated as a valid real-name, and the entry
in the search results will be empty.
The new behavior here is that we treat an empty real-name field as if
it were null, so that the email address will be used in the search
results instead.
Signed-off-by: Jesse Rosenthal <jrosenthal@jhu.edu>
Previously, it was necessary to link new messages to children to work
around some (though not all) problems with the old metadata-based
approach to stored thread IDs. With ghost messages, this is no longer
necessary, so don't bother with child linking when ghost messages are
in use.
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
