This is a bit different than n_d_{open,create}_with_config, since
there are several non-zero status codes where we do want to return a
non-NULL database structure.
This code previously relied on _finish_open to free the notmuch struct
on errors (except for the case of database == NULL, which was a
potential double free). When we removed those frees from _finish_open,
we introduced a (small) memory leak.
In this commit, fix the memory leak, and harmonize the on-error
behaviour with n_d_open_with_config.
During refactoring for 0.32, the code that set notmuch=NULL on various
errors was moved into _finish_open. This meant that the the code which
relied on that to set *database to NULL on error was no longer
correct. It also introduced a potential double free, since the notmuch
struct was deallocated inside _finish_open (via n_d_destroy).
In this commit we revert to "allocator frees", and leave any cleanup
to the caller of _finish_open. This allows us to get back the
behaviour of setting *database to NULL with a small change. Other
callers of _finish_open will need free notmuch on errors.
The documentation claims that the database will be set to NULL in this
case, but it is currently not happening. Based on a reproducer [1]
from Austin Ray.
[1]: id:20211021190401.imirxau2ewke6e2m@athena
As reported in id:87h7pxiek3.fsf@tethera.net, the previous version of
the test is flaky. There is some so-far undebugged interaction between
openssl and gpgsm that causes the keys to fail to import. As a
potential workaround, use the key as exported by gpgsm, and eliminate
openssl from this particular pipeline.
As with notmuch--process-lines, initial purpose is to provide a safe
binding for default-directory. This is enough to make notmuch-hello
robust against non-existent or corrupt values default-directory, but
probably not other views.
Because of the way emacs reports errors, a test form can crash and not
change the main buffer. To work around this, capture both signalled
errors and any other messages.
As discussed in the thread starting at [1], the fully qualified domain
name is a bit tricky to get reproducibly, might reveal information
people prefer to keep private, and somewhat unlikely to provide
reliable mail routing.
The new approach of $current_username@localhost is better for the
first two considerations, and probably at least as good as a test mail
address.
[1]: id:87sfyibqhj.fsf@tethera.net
Based on the commit message in id:20210221151902.2301690-3-dme@dme.org
Add the function notmuch-test-result-flags to test-lib.el to avoid
repeating it in 3 T*.sh files.
We have to rewrite _optimize_tag_query here because it is generating
a query string in the infix Xapian syntax. Luckily this is easy to do
with the sexp query syntax.
The change in each case is to call notmuch_query_create_with_syntax,
relying on the already inherited shared options. As a bonus we get
improved error handling from the new query creation API.
The remaining subcommand is 'tag', which is a bit trickier.
Macros implement lazy evaluation and lexical scope. The former is
needed to make certain natural constructs work sensibly (e.g. (tag
,param)) but the latter is mainly future-proofing in case the DSL is
is extended to allow local bindings.
For technical background, see chapters 6 and 17 of [1] (or some other
intermediate programming languages textbook).
[1] http://cs.brown.edu/courses/cs173/2012/book/
It turns out there is not really much code in query-fp.cc useful for
supporting the new syntax. The code we could potentially factor out
amounts to calling notmuch_database_get_config; both the key
construction and the parsing of the results are specific to the query
syntax involved.
This commit does not enable using saved s-expression queries, only
saving and retrieving them from the config file or the database. Use
in queries will be enabled in a following commit.
This provides functionality analogous to query: in the Xapian
QueryParser based parser. Perhaps counterintuitively, the saved
queries currently have to be in the original query syntax (i.e. not
s-expressions).
One subtle aspect is the replacement of _find_prefix with
_notmuch_database_prefix, which understands user headers. Otherwise
the code mainly consists of creating a fake prefix record (since the
user prefixes are not in the prefix table) and error handling.
This is necessary so that programs can take infix syntax queries from
a user and use the sexp query syntax to construct e.g. a refinement of
that query.
At least to the degree that the Xapian QueryParser based parser
also supports them. Support short alias 'rx' as it seems to make more
complex queries nicer to read.
This is equivalent to adding the same field name "" for multiple
prefixes in the Xapian query parser, but we have to explicitely
construct the resulting query.
The many tests potentially overkill, but they could catch typos in the
prefixes table. As a simplifying assumption, for now we assume a
single argument to the wildcard operator, as this matches the Xapian
semantics. The name 'starts-with' is chosen to emphasize the supported
case of wildcards in currrent (1.4.x) Xapian.
We use "boolean" to describe fields that should generate terms
literally without stemming or phrase splitting. This terminology
might not be ideal but it is already enshrined in
notmuch-search-terms(7).
Anything that is quoted or not purely word characters is considered a
phrase. Phrases are not stemmed, because the stems do not have
positional information in the database. It is less efficient to scan
the term twice, but it avoids a second pass to add prefixes, so maybe
it balances out. In any case, it seems unlikely query parsing is very
often a bottleneck.
All operations and (Xapian) fields will eventually have an entry in
the prefixes table. The flags field is just a placeholder for now, but
will eventually distinguish between various kinds of prefixes.
There is not much of a parser here yet, but it already does some
useful error reporting. Most functionality sketched in the
documentation is not implemented yet; detailed documentation will
follow with the implementation.
The configure part is essentially the same as the other checks using
pkg-config. Since the optional inclusion of this feature changes what
options are available to the user, include it in the "built_with"
pseudo-configuration keys.
It turns out that now that we pass an open database into the
subcommands, it is easy to check any requested uuid against the
database at the same time as we process the other shared
arguments. This results in overall less boilerplate code, as well as
making a CLI scope function and variable file scope in notmuch.c.
There are at least 3 bugs present.
1) notmuch-show-insert-part-message/rfc822 assumes that message/rfc822
parts will have a ":content" property, but that turns out not to be
the case.
2) something deep in gnus wants gnus-newsgroup-charset, but that is
defined in gnus-sum, which is not loaded by default.
3) If gnus-sum is loaded, then the display of the message/rfc822 part
succeeds, but the buffer gets put into gnus-article-mode, which means
that, inter alia, notmuch text properties and keybindings get wiped.
When using notmuch-reply and guessing the From: address from
Delivered-To headers, I had the wrong address chosen today. This was
because the messages from the notmuch list contain these headers in this
order:
Delivered-To: hannu.hartikainen@gmail.com
...
Delivered-To: hannu@hrtk.in
In my .notmuch-config I have the following configuration:
primary_email=hannu@hrtk.inother_email=hannu.hartikainen@gmail.com;...
Before this change, notmuch-reply would guess From: @gmail.com because
that is the first Delivered-To header present. After the change, the
primary address is chosen as I would expect.
Add a known broken subtest for guessing From: correctly when there are
multiple Delivered-To: headers. The address configured as primary_email
should get picked.
This is a bit of a cheat, since the format does not actually
change. On the other hand it is fairly common to do something like
this to shared libary SONAMEs when the ABI changes in some subtle way.
It does rely on the format-version argument being early enough on the
command line to generate a sensible error message.
This is more robust against crashes when the expected output is also
generated by notmuch. In the case where the expected output is
explicit, it seems like overkill.
A common bug in tests is that the code used to generate the EXPECTED
file fails, generating no output. When the code generating the OUTPUT
file fails in the same way, the test passes, even though there is a
failure being hidden. Add a new test function that guards against
this.
We need to special case the config section "built_with" because it is
not (currently) handled by the library. This seems consist with the
other sub-sub-commands 'list' and 'set'.
This highlights a bug reported by several users, including
Mohsin Kaleem [1].
The inconsistent use of test_begin_subtest_known_broken is because
some of these tests pass even though the database cannot be
located. This problem is left for a future commit.
[1]: id:87bl9lx864.fsf@kisara.moe
Test numbers are a concise way to communicate about tests and to remeber
them. Currently, there is one pait of duplicates:
T590-libconfig.sh
T590-thread-breakage.sh
Renumber the latter one to 592 since this keeps the alphabetic order and
leaves room in between.
Signed-off-by: Michael J Gruber <git@grubix.eu>
This is more efficient than notmuch-show-only-matching-messages, since
we do not parse the potentially large thread structure to find a
single message.
This is only a partial fix for notmuch-tree view, because displaying
the thread structure in the tree-mode window still crashes on long
threads. It is however enough to make unthreaded view handle long
threads.
This change addresses two known issues with large sets of changes to
the database. The first is that as reported by Steven Allen [1],
notmuch commits are not "flushed" when they complete, which means that
if there is an open transaction when the database closes (or e.g. the
program crashes) then all changes since the last commit will be
discarded (nothing is irrecoverably lost for "notmuch new", as the
indexing process just restarts next time it is run). This does not
really "fix" the issue reported in [1]; that seems rather difficult
given how transactions work in Xapian. On the other hand, with the
default settings, this should mean one only loses less than a minutes
worth of work. The second issue is the occasionally reported "storm"
of disk writes when notmuch finishes. I don't yet have a test for
this, but I think committing as we go should reduce the amount of work
when finalizing the database.
[1]: id:20151025210215.GA3754@stebalien.com
When the certificate that signs a message is known to be valid, GMime
is capable of reporting on the e-mail address embedded in the
certificate.
We pass this information along to the caller of "notmuch show", as
often only the e-mail address of the certificate has actually been
checked/verified.
Furthermore, signature verification should probably at some point
compare the e-mail address of the caller against the sender address of
the message itself. Having to parse what gmime thinks is a "userid"
to extract an e-mail address seems clunky and unnecessary if gmime
already thinks it knows what the e-mail address is.
See id:878s41ax6t.fsf@fifthhorseman.net for more motivation and discussion.
Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
This is a fix for the test failures reported by Dan Čermák [1].
It is more robust to check for the prerequisite inside the function
that uses it, rather than in every test file that calls the function.
[1]: id:87k0n4fqgm.fsf@tethera.net
notmuch-test will now call aggregate-results.sh with file list
that it compiles based on the test ran, and aggregate-results
will report failure is any of the test files are missing.
With this notmuch-test no longer has to exit in non-parallel
run if some test fail to write its report file -- so it works
as parallel tests in this sense.
Changed test_done() in test-lib.sh write report file in one write(2),
so there is (even) less chance it being partially written. Also,
now it writes 'total' last and aggregate-results.sh expects this
line to exist in all report files for reporting to be successful.
Added 'set -eu' to notmuch-test and modified code to work with
these settings. That makes it harder to get mistakes slipped
into committed code.
Gmane web interface is long gone, remove it. Make MARC the new
default. Update LKML to Lore, where it already redirects anyway. Also
add Notmuch web archive.
Sourcing test-lib.sh will cd to TMP_DIRECTORY, so
relative path in $0 will not work in previous version
. $(dirname "$0")/test-lib-emacs.sh
Now individual test scripts -- e.g. ./test/T310-emacs.sh
will work.
Prior to 9ad19e4454 there was an unhandled Xapian exception when
reindexing after a large number of deletes. This test was used for
bisection, and will subsequently serve as a regression test.
say_color() used to call (builtin) printf (and tput(1) to stdout)
several times, which caused attempts to write messages with color
to have partial content (e.g. escape sequences) often intermixed
with other tests when parallel tests were run.
Now, with all output collected, then written out using one
printf, all strings with color print out correctly
((at least short) write(2)'s appear to write out "atomically").
While at it, used only one tput(1) execution to determine whether
color output works, and made bold/colors/sgr0 to tput(1) their
values once per test.
notmuch_passwd_sanitize() in test-lib.sh is too generic, it cannot
work in many cases...
The more specific version _libconfig_sanitize() replaces it in
T590-libconfig.sh and the code that uses it is modified to output
the keys (ascending numbers printed in hex) so the sanitizer knows
what to sanitize in which lines...
"@" + fqdn -> "@FQDN" replacement is used as fqdn could
-- in theory -- be substring of 'USERNAME'.
'user -> 'USER_FULL_NAME replacement to work in cases where user
is empty -- as only first ' is replaced that works as expected.
In addition to ".(none)" now also ".localdomain" is filtered from
USERNAME@FQDN.
/dev/fd/{n} is not defined in posix, but it is portable enough
(if it weren't it is easy to fix -- now code looks clearer).
This prevents the message document getting multiple thread-id terms
when there are multiple files with the same message-id.
This change shifts some thread ids, requiring adjustments to other tests.
According to my bijection, this bug has been present since commit
411675a6ce in 2017. It is not completely clear what harm it causes in
regulary use, but it (at least) makes notmuch crash when compiled with
-DDEBUG_DATABASE_SANITY.
In test-lib-emacs.sh line 20:
test_require_external_prereq ${TEST_EMACS} || ret=1
^-----------^ SC2086: Double quote to prevent globbing and word splitting.
Did you mean:
test_require_external_prereq "${TEST_EMACS}" || ret=1
In test-lib-emacs.sh line 21:
test_require_external_prereq ${TEST_EMACSCLIENT} || ret=1
^-----------------^ SC2086: Double quote to prevent globbing and word splitting.
Did you mean:
test_require_external_prereq "${TEST_EMACSCLIENT}" || ret=1
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Commit d59d9c81 (test: Make the emacsclient binary user-configurable,
2012-11-27) modified the prereq check for the configured emacsclient,
but we probably want to do the same for emacs itself.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Although this default worked for "notmuch config get", it didn't work
most other places. Restore the previous functionality, with the
wrinkle that XDG locations will shadow $HOME/mail if they exist.
This fixes a bug reported by Jack Kamm in id:87eeefdc8b.fsf@gmail.com
notmuch-before-tag-hook and notmuch-after-tag-hook are supposed to
have access to two dynamic variables, tag-changes and query, but these
were lost with the switch to lexical binding in fc4cda07 (emacs: use
lexical-bindings in all libraries, 2021-01-13).
Add a variant of Emacs's dlet (not available until Emacs 28) and use
it in notmuch-tag to expose tag-changes and query to the hooks.
Due to the change in the config system, notmuch keeps a notmuch database
open when it would not do so before. Consequently, it can miss changes
to the database which are done from a hook (while notmuch holds the
databse in read only mode). When notmuch itself writes to the database
after that it uses wrong assumptions about the last used doc id etc.
Demonstrate this by triggering an assertion. (This new test succeeds
with notmuch 0.31.4.)
Signed-off-by: Michael J Gruber <git@grubix.eu>
Amended-by: db. Check for both messages
