The bug here was that we would see that the database did not know
anything about a directory so would get results from the filesystem in
inode rather than strcmp order.
However, we wouldn't actually ask for the list of files from the
database until after recursing into the sub-directories. So by the
time we traverse the filenames looking for deletions, the database
*does* have entries and we end up detecting erroneous deletions
because our filename list from the filesystem isn't in strcmp order.
So ask for the list of names from the database before doing any
additions to avoid this problem.
Previously we only scanned the list of filenames in the filesystem and
detected a deletion whenever that scan skipped a name that existed in
the database. That much was fine, but we *also* need to continue
walking the list of names from the database when the filesystem list
is exhausted.
Without this, removing the last file or directory within any
particular directory would go undetected.
As described in the previous commit message, we introduced multiple
symlink-based regressions in commit
3df737bc4addfce71c647792ee668725e5221a98
Here, we fix the case of symlinks to regular files by doing an extra
stat of any DT_LNK files to determine if they do, in fact, link to
regular files.
In commit 3df737bc4addfce71c647792ee668725e5221a98 we switched from
using stat() to using the d_type field in the result of scandir() to
determine whether a filename is a regular file or a directory. This
change introduced a regression in that the recursion would no longer
traverse through a symlink to a directory. (Since stat() would resolve
the symlink but with scandir() we see a distinct DT_LNK value in
d_type).
We fix this for directories by allowing both DT_DIR and DT_LNK values
to recurse, and then downgrading the existing not-a-directory check
within the recursion to not be an error. We also add a new
not-a-directory check outside the recursion that is an error.
The "notmuch new" command will now efficiently notice if any files or
directories have been removed from the mail store and will
appropriately update its database.
Any given mail message (as determined by the message ID) may have
multiple corresponding filenames, and notmuch will return one of
them. When a filen is deleted, the corresponding filename will be
removed from the message in the database. When the last filename is
removed from a message, that message will be entirely removed from the
database.
All file additions are handled before any file removals so that rename
is supported properly.
It is essential to defer the actual removal of any filenames from the
database until we are entirely done adding any new files. This is to
avoid any information loss from the database in the case of a renamed
file or directory.
Note that we're *still* not actually doing any removal---still just
printing messages indicating the filenames that were detected as
removed. But we're at least now printing those messages at a time when
we actually *can* do the actual removal.
This takes advantage of the notmuch_directory_t interfaces added
recently (with cooresponding storage of directory documents in the
database) to detect when files or entire directories are deleted or
renamed within the mail store.
This also fixes the recent regression where *all* files would be
processed by every run of "notmuch new", (now only new files are
processed once again).
The deleted files and directories are only detected so far. They
aren't properly removed from the database.
Previously, we were re-scanning the entire list of entries for every
directory entry. Instead, we can simply check if the entries look like
a maildir once, up-front.
We now do two scans over the entries returned from scandir. The first
scan is looking for directories (and making the recursive call). The
second scan is looking for new files to add to the database.
This is easier to read than the previous code which had a single loop
and some if statements with ridiculously long bodies. It also has the
advantage that once the directory scan is complete we can do a single
comparison of the filesystem and database mtimes and entirely skip the
second scan if it's not needed.
Previously we had an array named "namelist" and its count named
"num_entries". We now use an array name of "fs_entries" and a count
named "num_fs_entries" to try to preserve sanity.
We were previousl using the stat for two reasons. One was to obtain
the mtime of the file. This usage was removed in the previous commit,
(since the mtime is unreliable in the case of a file being moved into
the mail store).
The second reason was to identify regular and directory file
types. But this information is already available in the result we get
from scandir.
What's left is simply a stat for each directory in the mailstore,
(which we are still using to compare filesystem mtime with the mtime
stored in the database).
This check was buggy in that moving a pre-existing file into the mail
store, (where the file existed before the last run of "notmuch new"),
does not update the mtime of the file. So the message would never be
added to the database.
The fix here is not practical in the long run, (since it causes *all*
files in the mail store to be processed in every run of "notmuch new"
(!)). But this change will let us drop a stat() call that we don't
otherwise need and will help move us toward proper database-backed
detection of new files, (which will fix the bug without the
performance impact of the current fix).
The previous name of "path_mtime" was very ambiguous. The new names
are much more obvious (fs_mtime is the mtime from the filesystem and
db_mtime is the mtime from the database).
This was a very dangerous bug. An interrupted "notmuch new" session
would still update the timestamp for the directory in the
database. This would result in mail files that were not processed due
to the original interruption *never* being picked up by future runs of
"notmuch new". Yikes!
This new directory ojbect provides all the infrastructure needed to
detect when files or directories are deleted or renamed. There's still
code needed on top of this (within "notmuch new") to actually do that
detection.
This was really the last thing keeping the initial run of "notmuch
new" being different from all other runs. And I'm taking a fresh
look at the performance of "notmuch new" anyway, so I think we can
safely drop this optimization.
Several people complained that the humor wore thin very quickly. The
most significant case of "not much mail" is when counting the user's
initial mail collection. We've promised on the web page that no matter
how much mail the user has, notmuch will consider it to be "not much"
so let's say so. (This message was in place very early on, but was
inadvertently dropped at some point.)
Glibc (at least) provides the warn_unused_result attribute on write,
(if optimizing and _FORTIFY_SOURCE is defined). So we explicitly
ignore the return value in our signal handler, where we couldn't do
anything anyway.
Compile with:
make CFLAGS="-O -D_FORTIFY_SOURCE"
before this commit to see the warning.
Currently we assume that all errors on stat() a dname is fatal (but
continue anyway and report the error at the end). However, some errors
reported by stat() such as a missing file or insufficient privilege,
we can simply ignore and skip the file. For the others, such as a fault
(unlikely!) or out-of-memory, we handle like the other fatal errors by
jumping to the end.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
'notmuch new' skips directory entries with the name 'tmp'. This is to
prevent notmuch from processing possibly incomplete Maildir messages
stored in that directory.
This patch attempts to refine the feature. If "tmp" entry is found,
it first checks if the containing directory looks like a Maildir
directory. This is done by searching for other common Maildir
subdirectories. If they exist and if the entry "tmp" is a directory
then it is skipped.
Files and subdirectories with the name "tmp" that do not look like
Maildir will still be processed by 'notmuch new'.
Signed-off-by: Jan Janak <jan@ryngle.com>
We look at the modified time of the database and the directory
to decide whether we need to look at only the subdirectories.
ie, if directory modified time is < database modified time
then we have already looking at all the files withing the
directory. So we just need to iterate through the subdirectories
But with symlinks we need to make sure we follow them even if
the directory modified time is less than database modified time
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
When running "notmuch new --verbose", ANSI escapes are used. This may not be
desirable when the output of the command is *not* being sent to a terminal
(e.g. when piping output into another command). In that case each file
processed is printed in a new line and ANSI escapes are not used at all.
For very large mail boxes, it is desirable to know which files are being
processed e.g. when a crash occurs to know which one was the cause. Also,
it may be interesting to have a better idea of how the operation is
progressing when processing mailboxes with big messages.
This patch adds support for printing messages as they are processed by
"notmuch new":
* The "new" command now supports a "--verbose" flag.
* When running in verbose mode, the file path of the message about to be
processed is printed in the following format:
current/total: /path/to/message/file
Where "current" is the number of messages processed so far and "total" is
the total count of files to be processed.
The status line is erased using an ANSI sequence "\033[K" (erase current
line from the cursor to the end of line) each time it is refreshed. This
should not pose a problem because nearly every terminal supports it.
* The signal handler for SIGALRM and the timer are not enabled when running
in verbose mode, because we are already printing progress with each file,
periodical reports are not neccessary.
Check that the stdout is connected to an interactive terminal with
isatty() before installing the periodic timer to print progress reports.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
I felt sorry for Carl trying to step through an exception from xapian
and suffering from the SIGALARMs..
We can detect if the user launched notmuch under a debugger by either
checking our cmdline for the presence of the gdb string or querying if
valgrind is controlling our process. For the latter we need to add a
compile time check for the valgrind development library, and so add the
initial support to build Makefile.config from configure.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Carl Worth <cworth@cworth.org>
[ickle: And do not install the timer when under the debugger]
We only rarely need to actually open the database for writing, but we
always create a Xapian::WritableDatabase. This has the effect of
preventing searches and like whilst updating the index.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Carl Worth <cworth@cworth.org>
This reverts commit 9794f19017.
The feature makes a lot of sense for the initial import, but it's not
as clear whether it makes sense for ongoing "notmuch new" runs. We
might need to make this opt-in by configuration.
This patch adds maildir directory name as the tag name for
messages. This helps in adding tags using filtering already
provided by procmail.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Carl says: This has similar performance benefits as the previous
patch, and I fixed similar style issues here as well, (including
missing more of a commit message than the one-line summary).
This gives a rather decent reduction in number of seeks required when
reading a Maildir that isn't in pagecache.
Most filesystems give some locality on disk based on inode numbers.
In ext[234] this is the inode tables, in XFS groups of sequential inode
numbers are together on disk and the most significant bits indicate
allocation group (i.e inode 1,000,000 is always after inode 1,000).
With this patch, we read in the whole directory, sort by inode number
before stat()ing the contents.
Ideally, directory is sequential and then we make one scan through the
file system stat()ing.
Since the universe is not ideal, we'll probably seek during reading the
directory and a fair bit while reading the inodes themselves.
However... with readahead, and stat()ing in inode order, we should be
in the best place possible to hit the cache.
In a (not very good) benchmark of "how long does it take to find the first
15,000 messages in my Maildir after 'echo 3 > /proc/sys/vm/drop_caches'",
this patch consistently cut at least 8 seconds off the scan time.
Without patch: 50 seconds
With patch: 38-42 seconds.
(I did this in a previous maildir reading project and saw large improvements too)
By installing a signal handler for SIGINT we can ensure that no work
that is already complete will be lost if the user interrupts a
"notmuch new" run with Control-C.
I recently discovered that mb2md has the annoying bug of creating
files with mtime of 0, and notmuch then promptly ignored them,
(thinking that its timestamps initialized to 0 were just as new).
We fix notmuch to not exclude messages based on a database timestamp
of 0.
Leaving this variable uninitialized caused notmuch to display a random
number while counting files for the new database.
Signed-off-by: Keith Packard <keithp@keithp.com>
Now that the client sources are alone here in their own directory,
(with all the library sources down inside the lib directory), we can
break the client up into multiple files without mixing the files up.
The hope is that these smaller files will be easier to manage and
maintain.
