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
Prior to 0.32, notmuch had the (undocumented) behaviour that it
expanded a relative value of database.path with respect to $HOME. In
0.32 this was special cased for database.path but broken for
database.mail_root, which causes problems for at least notmuch-new
when database.path is set to a relative path.
The change in T030-config.sh reflects a user visible, but hopefully
harmless behaviour change; the expanded form of the paths will now be
printed by notmuch config.
We always do test_expect_equal_file, so do it in test_ruby() directly.
The only subtest where we don't (get non-existent file) can be easily
modified.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Not much point in polluting the main library, and also will be useful to
modify it in tandem with the tests.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
test_emacs may update the external prereqs, in which case we want to
skip the test rather than fail.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
When the external prereqs are updated inside the body of the command
(e.g. test_emacs) the message in test_report_skip_ is wrong: it outputs
the body of the command instead of the subtest name.
We need to pass the same argument we pass to test_skip.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
My fqdn is 'natae.localdomain', however, socket.getfqdn() returns
'localhost'.
To fetch the true fqdn we need socket.getaddrinfo().
For more information see: https://stackoverflow.com/a/11580042/10474
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
If any of the variables is empty the output is completely messed up,
because replace("", "FOO") puts "FOO" before every single character.
I don't have my full name configured, and this is what I get:
USER_FULL_NAME=USER_FULL_NAME=USER_FULL_NAME USER_FULL_NAMEsUSER_FULL_NAMEtUSER_FULL_NAMEdUSER_FULL_NAMEoUSER_FULL_NAMEuUSER_FULL_NAMEtUSER_FULL_NAME USER_FULL_NAME=USER_FULL_NAME=USER_FULL_NAME
Let's check for empty strings before doing any replace.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
The lastest versions of GNU parallel no longer make mention of GNU
within their help output. This causes the test script to mistakenly use
the moreutils parallel execution. In order to fix this, while
maintaining compatibility with previous versions of GNU parallel,
--version should be used.
Signed-off-by: Tobias Backer Dirks <omgitsaheadcrab@gmail.com>
Apparently the -f option to hostname is not portable, and in fact it
does not seem to always behave reasonably in e.g. a chroot.
Python code originally due to Tomi [1], modified by yours truly.
[1]: id:m2lf9fbkug.fsf@guru.guru-group.fi
lib/open.cc:_load_key_file will only open xdg-config files in
$XDG_CONFIG_HOME if it's defined, $HOME/.config will be considered if
and only if XDG_CONFIG_HOME not defined.
Let's unset said variable before running the test.
Certain tools like the address-sanitizer fail if they are not the
first LD_PRELOADed library. It does not seem to matter for our shims,
as long as they are loaded before libnotmuch.
This will allow simplifying the subcommand interface.
Change the internal API to notmuch_config_open to not tie it to the
implementation of subcommands in notmuch.c.
It also fixes a previously broken test, since notmuch_config_open does
not understand the notion of the empty string as a config file name.
There are two small code changes. The main one is to retrieve the
possibly updated config file name found during the database opening
call. The second change is to allow empty config file names, as
a (currently broken) way of specifying that configuration should only
be taken from the database.
Eventually we want to do all opening of databases in the top
level (main function). This means that detection of missing databases
needs to move out of subcommands. It also requires updating the
library to use the new NO_DATABASE status code.
Previously the fact that some configuration options were only stored
in the database (and thus editing the config file had no effect) was a
source of user confusion. This was fixed with the series ending at
d9af0af164.
On the other hand, the underlying partition of config options into
those stored by default in the database and those stored in the config
file remained. This is also confusing, since now some invocations of
"notmuch config set" modify the config file, and others silently
modify the database instead.
With this commit, it is up to the user to decide which configuration
to modify. A new "--database" option is provided for notmuch config to
enable modifying the configuration information in the database;
otherwise the default is to update an external config file.
Use the database opened at the top level rather than opening another
notmuch_database_t.
Test output changes because keys are now listed in alphabetical order,
and because a missing database is no longer an error.
This commit starts the conversion of notmuch-config.c
functionality (as opposed to just interface) to the new config
framework.
The change to T030-config is because of the move of the
canonicalization database paths from the notmuch_config_t accessor to
the internal function _choose_database_path.
The layer of shims here seems a bit wasteful compared to just calling
the corresponding string map functions directly, but it allows control
over the API (calling with notmuch_database_t *) and flexibility for
future changes.
The stat is essentially replaced by the mkdir for error detection
purposes. This changes the default location for backups to make
things tidier, even in non-split configurations. Hopefully there is
not too many user scripts relying on the previous location.
Because the default location may not exist, replace the use of stat
for error detection with a call to mkdir.
Like the hook directory, we primarily need a way to communicate this
directory between various components, but we may as well let the user
configure it.
Most of the diff is generalizing choose_hook_dir to work for both
backup and hook directories.
Choose sibling directory of xapian database, as .notmuch may not
exist.
libgen.h is already used in debugger.c, so it is not a new dependency
/ potential portability problem.
The new test is in T055-path-config because it uses the helper
function split_config, and because it seems easier to put the
database path related tests in one place.
This changes some error reporting, either intentionally by reporting
the highest level missing directory, or by side effect from looking in
XDG locations when given null database location.
This adds new state variable for the mail root, and uses it most
places db_path was used. The notable exception is dumps from
backups. The latter will be dealt with properly in a future commit.
The main functionality will be tested when notmuch-new is converted to
support split configuration. Here only the somewhat odd case of split
mail root which is actually symlinked to the database path is tested.
Introduce a new configuration value for the mail root, and use it to
locate mail messages in preference to the database.path (which
previously implied the mail messages were also in this location.
Initially only a subset of the CLI is tested in a split
configuration. Further changes will be needed for the remainder of the
CLI to work in split configurations.
In order to open the database in main() for this command, we may need
to re-open it in the (possibly less common) case where crypto options
require write access.
This fixes a bug reported in [1]. In principle it could be possible
avoid one of these reopens, but it complicates the logic in main with
respect to creating new databases.
[1]: id:9C1993DF-84BD-4199-A9C8-BADA98498812@bubblegen.co.uk
Recent changes to configuration handling meant the pre-new hook was
run while the database was open read only, limiting what could be done
in the hook. Add some known broken tests for this problem, as well as
a regression test for write access from the post-new hook.
In the future Xapian will apparently support this more conveniently
for the cases other than READ_ONLY => READ_ONLY
Conceptually this function seems to fit better in lib/open.cc;
database.cc is still large enough that moving the function makes
sense.
In [1] Gregor Zattler explained the results of his hard work
tracking down a bug in notmuch with long directories. This test
duplicates the bug.
[1]: id:20210317194728.GB5561@no.workgroup
Fix use of $TEST_DIRECTORY ($NOTMUCH_BUILDDIR/test/) with use of
$TMP_DIRECTORY ($NOTMUCH_BUILDDIR/test/tmp.T020-compact/ in case
of T020-compact.sh) as root directory where to write test files
and directories.
The assignment of thread-ids is (apparently) non-deterministic in a
way that mostly seems to show up on multicore machines. In my tests
the number is different from that previously assumed by this test
about 15% of the time on a 50 thread (25 core) Xeon.
Since message id's are fixed, use a message known to be in the thread
of interest to pick out the correct thread-id.
This is the last bit of "python" left in the notmuch codebase.
https://www.python.org/dev/peps/pep-0394/#recommendation encourages
"third-party distributors" to use more-specific shebang lines. I'm
not certain that the notmuch project itself is a "third-party
contributor" but I think this is a safe way to encourage people to use
python3 when they're developing notmuch.
We already have python3 explicitly elsewhere in the codebase for
developers (in nmbug).
Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
This enables support for hooks outside the database directory.
It relies strongly on configuration information being usable between
closing the database and destroying it.
The hook directory configuration needs to be kept in synch with the
other configuration information, so add scaffolding to support this at
database opening time.
In addition to the same type of changes as converting other
subcommands, add the possibility of creating a database at the top
level. It would probably make sense to use this for insert as well.
This will need some cleanup when the transition completes, and we stop
passing notmuch_config_t structs to the subcommands.
Unlike the general case, we open the database in the subcommand, since
we don't know whether it should be opened read/write until we parse
the command line arguments.
Add a test to make sure passing config file on the command line is not
broken by these or future config related changes.
Since we are already passing a search context around as a kind of
parameter block, add a new talloc context to that to replace relying
on 'config'.
Convert notmuch-search and notmuch-address at the same time, because
they share some code.
Add a test to make sure we don't break passing configuration as a
command line argument.
The new talloc context is needed to run the hook at the very end of
the function. That in turn is needed so that this process gives up the
write lock on the database.
This conversion is trivial because the only configuration information
accessed by dump is that stored in the database (in order to dump
it). We do need to be careful to keep the write lock on the database
to ensure dump consistency.
The main effort is changing from the old argv style config list
iterators to the new more opaque ones provided by the library (and
backed by the database+file config cache).
By using an enum we can have better error detection than copy pasting
key strings around.
The question of what layer this belongs in is a bit
tricky. Historically most of the keys are defined by the CLI. On the
other hand features like excludes are supported in the
library/bindings, and it makes sense to configure them from the
library as well.
The somewhat long prefix for notmuch_config_t is to avoid collisions
with the existing usage in notmuch-client.h.
Fill in the remainder of the documented functionality for
n_d_open_with_config with respect to config file location. Similar
searching default locations of the database file still needs to be
added.
The main goal is to allow configuration information to be temporarily
overridden by a separate config file. That will require further
changes not in this commit.
The performance impact is unclear, and will depend on the balance
between number of queries and number of distinct metadata items read
on the first call to n_d_get_config.
In ee897cab8b the upgrade tests from pre v3 databases were
removed. The reasons for that are still valid, but we should still
test the code paths that do the upgrade, and it is relatively
straightforward to do that for v3 to v3 upgrades.
`outline-minor-mode' treats comments that begin with three or more
semicolons as headings. That makes it very convenient to navigate
code and to show/hide parts of a file.
Elips libraries typically have four top-level sections, e.g.:
;;; notmuch.el --- run notmuch within emacs...
;;; Commentary:...
;;; Code:...
;;; notmuch.el ends here
In this package many libraries lack a "Commentary:" section, which is
not optimal but okay for most libraries, except major entry points.
Depending on how one chooses to look at it, the "... ends here" line
is not really a heading that begins a section, because it should never
have a "section" body (after all it marks eof).
If the file is rather short, then I left "Code:" as the only section
that contains code. Otherwise I split the file into multiple sibling
sections. The "Code:" section continues to contain `require' and
`declare-function' forms and other such "front matter".
If and only if I have split the code into multiple sections anyway,
then I also added an additional section named just "_" before the
`provide' form and shortly before the "...end here" line. This
section could also be called "Back matter", but I feel it would be
distracting to be that explicit about it. (The IMO unnecessary but
unfortunately still obligatory "... ends here" line is already
distracting enough as far as I am concerned.)
Before this commit some libraries already uses section headings, some
of them consistently. When a library already had some headings, then
this commit often sticks to that style, even at the cost inconsistent
styling across all libraries.
A very limited number of variable and function definitions have to be
moved around because they would otherwise end up in sections they do
not belong into.
Sections, including but not limited to their heading, can and should
be further improved in the future.
readelf on (at least) ppc64le sometimes generates some extension to
the Ndx name inside '[]'. Remove this output to allow our simple
column based parsing to work.
It turns out that using nm -P isn't as portable as hoped. In
particular with some ELF ABIs (e.g. ppc64 ELFv1), the desired symbols
end up in the data section instead of text.
The test is currently only functional on ELF based architectures, so I
think it's legit to depend on readelf instead of nm.
The switch to readelf has the advantage that we can explicitely ask
for all of the symbols with global visibility, rather than grepping
for notmuch. That seems a more robust approach since it will catch any
strangely named global symbols.
notmuch insert does not currently support passing a filename for the
input, so all of these tests have an extra error in addition to the
one being tested for.
Currently this does not make a difference because the error being
tested for is caught before the error of an extra command line
argument. In the future it might make a difference, and in any case it
is confusing.
Previously in message-show mode message's first header line (From
header) was always indented, even if user had turned thread
indentation off with "<" (notmuch-show-toggle-thread-indentation)
command.
This change modifies notmuch-show-insert-headerline function so that
it doesn't indent the first header line if notmuch-show-indent-content
variable is nil.
This change also modifies tests so that they expect this new output
format:
test/emacs-show.expected-output/notmuch-show-indent-thread-content-off
Use `makefile-gmake-mode' instead of `makefile-mode' because the
former also highlights ifdef et al. while the latter does not.
"./Makefile.global" and one "Makefile.local" failed to specify any
major mode at all but doing so is necessary because Emacs does not
automatically figure out that these are Makefiles (of any flavor).
On some systems (notably, the one shipped with LibreSSL),
default fingerprint digest algorithm is SHA256.
On other systems, users can change default digest algorithm by changing
default_md in /etc/ssl/default_md.
Let's ask openssl to provide us specific algorithm to make the test
more deterministic.
Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
Inspired by the suggestion of
id:20200727154108.16269-3-congdanhqx@gmail.com
to drop the configuration test for the default backend. This version
is hopefully robust against backend changes.
This is similar to the case of toplevel messages. Currently everything
is cached, so no database access is necessary. This might change in
the future, but it should not crash in either case.
The goal of this abstraction was to save space. But that failed as
the result actually was that four trivial lines got replace with 15
fairly complicated lines. The opposite of what it was supposed to
do.
Also it made it harder to come up with the fix in the previous commit;
simply grepping for the relevant symbols did not work because they get
constructed at run-time instead of appearing in the source file.
Starting with Emacs 27 undeclared variables in evaluated interactive
code uses lexical scope. This includes code passed with '--eval' as
we do in the Emacs tests, which also happen to assume dynamic scope.
- This can affect variables defined by libraries that we use. We
let-bind such variables to change the behavior of functions which we
then call with these bindings in effect. If these libraries are not
loaded beforehand, then the bindings are lexical and fail to have
the effect we intended.
At this time only 'smtpmail' has to be loaded explicitly (for the
variables let-bound in emacs_deliver_message and emacs_fcc_message).
'message' doesn't have to be loaded explicitly, because loading
'notmuch' (in 'run_emacs') already takes care of that, indirectly.
- Our own testing-only variables also have to be declared explicitly.
We should have done that anyway, but because of how and where these
variables are used it was very easy to overlook that (i.e. it isn't
something the byte-compiler ever looks at). Not so in Emacs 27
anymore; here this oversight caused four tests to fail.
The numeric values of these variables get incremented by functions
that we add to hooks that are run by many tests, not just the tests
where we actually inspect the value and therefore take care to let-
bind the values to 0 before we begin. The global values therefore
have to be numeric values as well. I have chosen -100 instead of 0
as the default in case someone writes a test that inspects the value
but forgets to let-bind the value. I hope that the unusual negative
value that one is going to see in such a case will help debugging
the issue.
