[PATCH 0/6] cli: support regex in new.ignore

[PATCH 0/6] cli: support regex in new.ignore

[Jani Nikula]

I dusted off some of my old patches to add support for /regex/ in
new.ignore. Regex instead of globbing because we now have prefix
searches with regex, so this is perhaps more in line. Some cleanups for
starters.

BR,
Jani.


Jani Nikula (6):
  cli/new: use the same style for fs entry loops
  cli/new: abstract special directory check
  cli/new: check for special directories earlier in pass 1
  cli/new: ignore special directories also in pass 2
  cli/new: support /<regex>/ in new.ignore
  test: test regexp based new.ignore

 doc/man1/notmuch-config.rst |  20 ++++--
 notmuch-new.c               | 144 ++++++++++++++++++++++++++++++++++++--------
 test/T050-new.sh            |  22 +++++++
 3 files changed, 155 insertions(+), 31 deletions(-)

-- 
2.11.0

[PATCH 1/6] cli/new: use the same style for fs entry loops

[Jani Nikula]

Just to please the eyes. No functional changes.
---
 notmuch-new.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index e011788da590..c6a741fefa03 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -443,10 +443,7 @@ add_files (notmuch_database_t *notmuch,
     /* Pass 1: Recurse into all sub-directories. */
     is_maildir = _entries_resemble_maildir (path, fs_entries, num_fs_entries);
 
-    for (i = 0; i < num_fs_entries; i++) {
-	if (interrupted)
-	    break;
-
+    for (i = 0; i < num_fs_entries && ! interrupted; i++) {
 	entry = fs_entries[i];
 
 	/* Ignore any files/directories the user has configured to
@@ -514,11 +511,7 @@ add_files (notmuch_database_t *notmuch,
     }
 
     /* Pass 2: Scan for new files, removed files, and removed directories. */
-    for (i = 0; i < num_fs_entries; i++)
-    {
-	if (interrupted)
-	    break;
-
+    for (i = 0; i < num_fs_entries && ! interrupted; i++) {
         entry = fs_entries[i];
 
 	/* Ignore files & directories user has configured to be ignored */
-- 
2.11.0

[PATCH 2/6] cli/new: abstract special directory check

[Jani Nikula]

Add an abstraction for . and .. directory checks. No functional
changes.
---
 notmuch-new.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index c6a741fefa03..faeb8f0a5896 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -234,6 +234,12 @@ _entries_resemble_maildir (const char *path, struct dirent **entries, int count)
     return 0;
 }
 
+static notmuch_bool_t
+_special_directory (const char *entry)
+{
+    return strcmp (entry, ".") == 0 || strcmp (entry, "..") == 0;
+}
+
 /* Test if the file/directory is to be ignored.
  */
 static notmuch_bool_t
@@ -475,8 +481,7 @@ add_files (notmuch_database_t *notmuch,
 	 * Also ignore the .notmuch directory and any "tmp" directory
 	 * that appears within a maildir.
 	 */
-	if (strcmp (entry->d_name, ".") == 0 ||
-	    strcmp (entry->d_name, "..") == 0 ||
+	if (_special_directory (entry->d_name) ||
 	    (is_maildir && strcmp (entry->d_name, "tmp") == 0) ||
 	    strcmp (entry->d_name, ".notmuch") == 0)
 	    continue;
@@ -738,8 +743,7 @@ count_files (const char *path, int *count, add_files_state_t *state)
 	/* Ignore special directories to avoid infinite recursion.
 	 * Also ignore the .notmuch directory.
 	 */
-	if (strcmp (entry->d_name, ".") == 0 ||
-	    strcmp (entry->d_name, "..") == 0 ||
+	if (_special_directory (entry->d_name) ||
 	    strcmp (entry->d_name, ".notmuch") == 0)
 	    continue;
 
-- 
2.11.0

[PATCH 3/6] cli/new: check for special directories earlier in pass 1

[Jani Nikula]

Avoid passing . and .. to ignore check. We don't need to check their
dirent type either.
---
 notmuch-new.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index faeb8f0a5896..378bf4c2a15a 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -452,6 +452,10 @@ add_files (notmuch_database_t *notmuch,
     for (i = 0; i < num_fs_entries && ! interrupted; i++) {
 	entry = fs_entries[i];
 
+	/* Ignore special directories to avoid infinite recursion. */
+	if (_special_directory (entry->d_name))
+	    continue;
+
 	/* Ignore any files/directories the user has configured to
 	 * ignore.  We do this before dirent_type both for performance
 	 * and because we don't care if dirent_type fails on entries
@@ -477,12 +481,10 @@ add_files (notmuch_database_t *notmuch,
 	    continue;
 	}
 
-	/* Ignore special directories to avoid infinite recursion.
-	 * Also ignore the .notmuch directory and any "tmp" directory
+	/* Ignore the .notmuch directory and any "tmp" directory
 	 * that appears within a maildir.
 	 */
-	if (_special_directory (entry->d_name) ||
-	    (is_maildir && strcmp (entry->d_name, "tmp") == 0) ||
+	if ((is_maildir && strcmp (entry->d_name, "tmp") == 0) ||
 	    strcmp (entry->d_name, ".notmuch") == 0)
 	    continue;
 
-- 
2.11.0

[PATCH 4/6] cli/new: ignore special directories also in pass 2

[Jani Nikula]

Avoid passing . and .. to ignore check. We also don't need to check
their dirent type either.
---
 notmuch-new.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/notmuch-new.c b/notmuch-new.c
index 378bf4c2a15a..2ce3af872f0e 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -521,6 +521,10 @@ add_files (notmuch_database_t *notmuch,
     for (i = 0; i < num_fs_entries && ! interrupted; i++) {
         entry = fs_entries[i];
 
+	/* Ignore special directories early. */
+	if (_special_directory (entry->d_name))
+	    continue;
+
 	/* Ignore files & directories user has configured to be ignored */
 	if (_entry_in_ignore_list (entry->d_name, state)) {
 	    if (state->debug)
-- 
2.11.0

[PATCH 5/6] cli/new: support /<regex>/ in new.ignore

[Jani Nikula]

Add support for using /<regex>/ style regular expressions in
new.ignore, mixed with the old style verbatim file and directory
basenames. The regex is matched against the relative path from the
database path.
---
 doc/man1/notmuch-config.rst |  20 ++++++--
 notmuch-new.c               | 109 ++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 114 insertions(+), 15 deletions(-)

diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index 6a51e64f1517..aab0676b1578 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -75,11 +75,21 @@ The available configuration items are described below.
         Default: ``unread;inbox``.
 
     **new.ignore**
-        A list of file and directory names, without path, that will not
-        be searched for messages by **notmuch new**. All the files and
-        directories matching any of the names specified here will be
-        ignored, regardless of the location in the mail store directory
-        hierarchy.
+        A list to specify files and directories that will not be
+        searched for messages by **notmuch new**. Each entry in the
+        list is either:
+
+	  A file or a directory name, without path, that will be
+	  ignored, regardless of the location in the mail store
+	  directory hierarchy.
+
+	Or:
+
+	  A regular expression delimited with // that will be matched
+	  against the path of the file or directory relative to the
+	  database path. The beginning and end of string must be
+	  explictly anchored. For example, /.*/foo$/ would match
+	  "bar/foo" and "bar/baz/foo", but not "foo" or "bar/foobar".
 
         Default: empty list.
 
diff --git a/notmuch-new.c b/notmuch-new.c
index 2ce3af872f0e..8146c99413b8 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -42,13 +42,17 @@ enum verbosity {
 };
 
 typedef struct {
+    const char *db_path;
+
     int output_is_a_tty;
     enum verbosity verbosity;
     notmuch_bool_t debug;
     const char **new_tags;
     size_t new_tags_length;
-    const char **new_ignore;
-    size_t new_ignore_length;
+    const char **ignore_verbatim;
+    size_t ignore_verbatim_length;
+    regex_t *ignore_regex;
+    size_t ignore_regex_length;
 
     int total_files;
     int processed_files;
@@ -240,18 +244,102 @@ _special_directory (const char *entry)
     return strcmp (entry, ".") == 0 || strcmp (entry, "..") == 0;
 }
 
+static notmuch_bool_t
+_setup_ignore (notmuch_config_t *config, add_files_state_t *state)
+{
+    const char **ignore_list, **ignore;
+    int nregex = 0, nverbatim = 0;
+    const char **verbatim = NULL;
+    regex_t *regex = NULL;
+
+    ignore_list = notmuch_config_get_new_ignore (config, NULL);
+    if (! ignore_list)
+	return TRUE;
+
+    for (ignore = ignore_list; *ignore; ignore++) {
+	const char *s = *ignore;
+	size_t len = strlen (s);
+
+	if (len > 2 && s[0] == '/' && s[len - 1] == '/') {
+	    regex_t *preg;
+	    char *r = talloc_strndup (config, s + 1, len - 2);
+
+	    regex = talloc_realloc (config, regex, regex_t, nregex + 1);
+	    preg = &regex[nregex];
+
+	    if (xregcomp (preg, r, REG_EXTENDED | REG_NOSUB) == 0)
+		nregex++;
+
+	    talloc_free (r);
+	} else {
+	    verbatim = talloc_realloc (config, verbatim, const char *,
+				       nverbatim + 1);
+	    verbatim[nverbatim++] = s;
+	}
+    }
+
+    state->ignore_regex = regex;
+    state->ignore_regex_length = nregex;
+    state->ignore_verbatim = verbatim;
+    state->ignore_verbatim_length = nverbatim;
+
+    return TRUE;
+}
+
+static char *
+_get_relative_path (const char *db_path, const char *dirpath, const char *entry)
+{
+    size_t db_path_len = strlen (db_path);
+
+    /* paranoia? */
+    if (strncmp (dirpath, db_path, db_path_len) != 0) {
+	fprintf (stderr, "Warning: '%s' is not a subdirectory of '%s'\n",
+		 dirpath, db_path);
+	return NULL;
+    }
+
+    dirpath += db_path_len;
+    while (*dirpath == '/')
+	dirpath++;
+
+    if (*dirpath)
+	return talloc_asprintf (NULL, "%s/%s", dirpath, entry);
+    else
+	return talloc_strdup (NULL, entry);
+}
+
 /* Test if the file/directory is to be ignored.
  */
 static notmuch_bool_t
-_entry_in_ignore_list (const char *entry, add_files_state_t *state)
+_entry_in_ignore_list (add_files_state_t *state, const char *dirpath,
+		       const char *entry)
 {
+    notmuch_bool_t ret = FALSE;
     size_t i;
+    char *path;
 
-    for (i = 0; i < state->new_ignore_length; i++)
-	if (strcmp (entry, state->new_ignore[i]) == 0)
+    for (i = 0; i < state->ignore_verbatim_length; i++) {
+	if (strcmp (entry, state->ignore_verbatim[i]) == 0)
 	    return TRUE;
+    }
+
+    if (! state->ignore_regex_length)
+	return FALSE;
 
-    return FALSE;
+    path = _get_relative_path (state->db_path, dirpath, entry);
+    if (! path)
+	return FALSE;
+
+    for (i = 0; i < state->ignore_regex_length; i++) {
+	if (regexec (&state->ignore_regex[i], path, 0, NULL, 0) == 0) {
+	    ret = TRUE;
+	    break;
+	}
+    }
+
+    talloc_free (path);
+
+    return ret;
 }
 
 /* Add a single file to the database. */
@@ -461,7 +549,7 @@ add_files (notmuch_database_t *notmuch,
 	 * and because we don't care if dirent_type fails on entries
 	 * that are explicitly ignored.
 	 */
-	if (_entry_in_ignore_list (entry->d_name, state)) {
+	if (_entry_in_ignore_list (state, path, entry->d_name)) {
 	    if (state->debug)
 		printf ("(D) add_files, pass 1: explicitly ignoring %s/%s\n",
 			path, entry->d_name);
@@ -526,7 +614,7 @@ add_files (notmuch_database_t *notmuch,
 	    continue;
 
 	/* Ignore files & directories user has configured to be ignored */
-	if (_entry_in_ignore_list (entry->d_name, state)) {
+	if (_entry_in_ignore_list (state, path, entry->d_name)) {
 	    if (state->debug)
 		printf ("(D) add_files, pass 2: explicitly ignoring %s/%s\n",
 			path, entry->d_name);
@@ -756,7 +844,7 @@ count_files (const char *path, int *count, add_files_state_t *state)
 	/* Ignore any files/directories the user has configured to be
 	 * ignored
 	 */
-	if (_entry_in_ignore_list (entry->d_name, state)) {
+	if (_entry_in_ignore_list (state, path, entry->d_name)) {
 	    if (state->debug)
 		printf ("(D) count_files: explicitly ignoring %s/%s\n",
 			path, entry->d_name);
@@ -980,9 +1068,10 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
 	add_files_state.verbosity = VERBOSITY_VERBOSE;
 
     add_files_state.new_tags = notmuch_config_get_new_tags (config, &add_files_state.new_tags_length);
-    add_files_state.new_ignore = notmuch_config_get_new_ignore (config, &add_files_state.new_ignore_length);
     add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
     db_path = notmuch_config_get_database_path (config);
+    add_files_state.db_path = db_path;
+    _setup_ignore (config, &add_files_state);
 
     for (i = 0; i < add_files_state.new_tags_length; i++) {
 	const char *error_msg;
-- 
2.11.0

[PATCH 6/6] test: test regexp based new.ignore

[Jani Nikula]

Just some basics.
---
 test/T050-new.sh | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/test/T050-new.sh b/test/T050-new.sh
index 272ed417aa2e..ee9ce08c8e86 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -259,6 +259,28 @@ ln -s i_do_not_exist "${MAIL_DIR}"/broken_link
 output=$(NOTMUCH_NEW 2>&1)
 test_expect_equal "$output" "No new mail."
 
+test_begin_subtest "Ignore files and directories specified in new.ignore (regexp)"
+notmuch config set new.ignore ".git" "/^bro.*ink\$/" "/ignored.*file/"
+output=$(NOTMUCH_NEW --debug 2>&1 | sort)
+test_expect_equal "$output" \
+"(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/.git
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/.ignored_hidden_file
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/broken_link
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/ignored_file
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/one/ignored_file
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/one/two/ignored_file
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/one/two/three/.git
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/one/two/three/ignored_file
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/.git
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/.ignored_hidden_file
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/broken_link
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/ignored_file
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/one/ignored_file
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/one/two/ignored_file
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/one/two/three/.git
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/one/two/three/ignored_file
+No new mail."
+
 test_begin_subtest "Quiet: No new mail."
 output=$(NOTMUCH_NEW --quiet)
 test_expect_equal "$output" ""
-- 
2.11.0

Re: [PATCH 1/6] cli/new: use the same style for fs entry loops

[David Bremner]

Jani Nikula <jani@nikula.org> writes:

> Just to please the eyes. No functional changes.

first two patches in the series pushed to master

d

Re: [PATCH 5/6] cli/new: support /<regex>/ in new.ignore

[David Bremner]

Jani Nikula <jani@nikula.org> writes:

> +	  A regular expression delimited with // that will be matched
> +	  against the path of the file or directory relative to the
> +	  database path. The beginning and end of string must be
> +	  explictly anchored. For example, /.*/foo$/ would match
> +	  "bar/foo" and "bar/baz/foo", but not "foo" or "bar/foobar".

Is it worth remarking that '/' does not need to be escaped? or more
interestingly, what happens if it is escaped, do things break?
>  
> +static notmuch_bool_t
> +_setup_ignore (notmuch_config_t *config, add_files_state_t *state)
> +{

Would be nice to document what this return value means.

> +    const char **ignore_list, **ignore;
> +    int nregex = 0, nverbatim = 0;
> +    const char **verbatim = NULL;
> +    regex_t *regex = NULL;
> +
> +    ignore_list = notmuch_config_get_new_ignore (config, NULL);
> +    if (! ignore_list)
> +	return TRUE;
> +
> +    for (ignore = ignore_list; *ignore; ignore++) {
> +	const char *s = *ignore;
> +	size_t len = strlen (s);
> +
> +	if (len > 2 && s[0] == '/' && s[len - 1] == '/') {

One thing we eventually settled on in the query parser is that an
opening '/' without a trailing '/' is an errror. But perhaps it's fine
to take a more permissive approach here.

> +
> +    if (! state->ignore_regex_length)
> +	return FALSE;

It's a nitpick, even by the standards of this review, but I'd prefer an
explicit '> 0' check.

Re: [PATCH 5/6] cli/new: support /<regex>/ in new.ignore

[Jani Nikula]

On Mon, 25 Sep 2017, David Bremner <david@tethera.net> wrote:
> Jani Nikula <jani@nikula.org> writes:
>
>> +	  A regular expression delimited with // that will be matched
>> +	  against the path of the file or directory relative to the
>> +	  database path. The beginning and end of string must be
>> +	  explictly anchored. For example, /.*/foo$/ would match
>> +	  "bar/foo" and "bar/baz/foo", but not "foo" or "bar/foobar".
>
> Is it worth remarking that '/' does not need to be escaped? or more
> interestingly, what happens if it is escaped, do things break?

It just gets passed down to regcomp() with the escape and all. I'm not
sure it's worth trying to exhaustively explain everything.

>>  
>> +static notmuch_bool_t
>> +_setup_ignore (notmuch_config_t *config, add_files_state_t *state)
>> +{
>
> Would be nice to document what this return value means.

Something like:

/* Jani forgot to do anything useful with the return value */

I think it was originally meant to return false on illegal input
(e.g. opening '/' without closing '/') or regcomp() failing, but then I
opted for the more lax approach. xregcomp() warns about failures though.

>
>> +    const char **ignore_list, **ignore;
>> +    int nregex = 0, nverbatim = 0;
>> +    const char **verbatim = NULL;
>> +    regex_t *regex = NULL;
>> +
>> +    ignore_list = notmuch_config_get_new_ignore (config, NULL);
>> +    if (! ignore_list)
>> +	return TRUE;
>> +
>> +    for (ignore = ignore_list; *ignore; ignore++) {
>> +	const char *s = *ignore;
>> +	size_t len = strlen (s);
>> +
>> +	if (len > 2 && s[0] == '/' && s[len - 1] == '/') {
>
> One thing we eventually settled on in the query parser is that an
> opening '/' without a trailing '/' is an errror. But perhaps it's fine
> to take a more permissive approach here.

I'm fine either way, I just chose to be permissive.

So do I make the function void and drop the return values, or make it
check and return errors?

>
>> +
>> +    if (! state->ignore_regex_length)
>> +	return FALSE;
>
> It's a nitpick, even by the standards of this review, but I'd prefer an
> explicit '> 0' check.

ITYM (state->ignore_regex_length == 0) but ack.

BR,
Jani.

Re: [PATCH 6/6] test: test regexp based new.ignore

[David Bremner]

[-- Attachment #1: Type: text/plain, Size: 263 bytes --]

Jani Nikula <jani@nikula.org> writes:

> Just some basics.
> ---
>  test/T050-new.sh | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)

Tests fail after applying this patch (output attached).

It sortof looks like the regexp ignore is not working?


[-- Attachment #2: T50.out --]
[-- Type: application/octet-stream, Size: 4324 bytes --]


T050-new: Testing "notmuch new" in several variations
 PASS   No new messages
 PASS   Single new message
 PASS   Multiple new messages
 PASS   No new messages (non-empty DB)
 PASS   New directories
 PASS   Alternate inode order
 PASS   Message moved in
 PASS   Renamed message
 PASS   Deleted message
 PASS   Renamed directory
 PASS   Deleted directory
 PASS   New directory (at end of list)
 PASS   Deleted directory (end of list)
 PASS   New symlink to directory
 PASS   New symlink to a file
 PASS   Broken symlink aborts
 PASS   New two-level directory
 PASS   Deleted two-level directory
 PASS   One character directory at top level
 PASS   Support single-message mbox
 PASS   Skip and report non-mail files
 PASS   Ignore files and directories specified in new.ignore
 PASS   Ignore files and directories specified in new.ignore (multiple occurrences)
 PASS   Don't stop for ignored broken symlinks
 FAIL   Ignore files and directories specified in new.ignore (regexp)
	--- T050-new.25.expected	2017-10-02 00:53:44.077523808 +0000
	+++ T050-new.25.output	2017-10-02 00:53:44.077523808 +0000
	@@ -1,17 +1,4 @@
	 (D) add_files, pass 1: explicitly ignoring /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/.git
	-(D) add_files, pass 1: explicitly ignoring /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/.ignored_hidden_file
	-(D) add_files, pass 1: explicitly ignoring /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/broken_link
	-(D) add_files, pass 1: explicitly ignoring /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/ignored_file
	-(D) add_files, pass 1: explicitly ignoring /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/one/ignored_file
	-(D) add_files, pass 1: explicitly ignoring /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/one/two/ignored_file
	-(D) add_files, pass 1: explicitly ignoring /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/one/two/three/.git
	-(D) add_files, pass 1: explicitly ignoring /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/one/two/three/ignored_file
	-(D) add_files, pass 2: explicitly ignoring /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/.git
	-(D) add_files, pass 2: explicitly ignoring /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/.ignored_hidden_file
	-(D) add_files, pass 2: explicitly ignoring /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/broken_link
	-(D) add_files, pass 2: explicitly ignoring /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/ignored_file
	-(D) add_files, pass 2: explicitly ignoring /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/one/ignored_file
	-(D) add_files, pass 2: explicitly ignoring /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/one/two/ignored_file
	-(D) add_files, pass 2: explicitly ignoring /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/one/two/three/.git
	-(D) add_files, pass 2: explicitly ignoring /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/one/two/three/ignored_file
	+Error reading file /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/broken_link: No such file or directory
	 No new mail.
	+Note: A fatal error was encountered: Something went wrong trying to read or write a file
 PASS   Quiet: No new mail.
 PASS   Quiet: new, removed and renamed messages.
 PASS   Empty tags in new.tags are forbidden
 PASS   Tags starting with '-' in new.tags are forbidden
 PASS   Invalid tags set exit code
 PASS   Xapian exception: read only files
 FAIL   Handle files vanishing between scandir and add_file
	--- T050-new.32.EXPECTED	2017-10-02 00:53:44.721523264 +0000
	+++ T050-new.32.OUTPUT	2017-10-02 00:53:44.725523262 +0000
	@@ -1,4 +1,3 @@
	-Unexpected error with file /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/vanish
	-add_file: Something went wrong trying to read or write a file
	-Error opening /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/vanish: No such file or directory
	-exit status: 75
	+Error reading file /home/bremner/software/upstream/notmuch/test/tmp.T050-new/mail/broken_link: No such file or directory
	+Note: A fatal error was encountered: Something went wrong trying to read or write a file
	+exit status: 1

Re: [PATCH 5/6] cli/new: support /<regex>/ in new.ignore

[David Bremner]

Jani Nikula <jani@nikula.org> writes:
>>
>> One thing we eventually settled on in the query parser is that an
>> opening '/' without a trailing '/' is an errror. But perhaps it's fine
>> to take a more permissive approach here.
>
> I'm fine either way, I just chose to be permissive.
>
> So do I make the function void and drop the return values, or make it
> check and return errors?

I think I'd prefer to start strict, it's easier to become permissive later.

>
>>
>>> +
>>> +    if (! state->ignore_regex_length)
>>> +	return FALSE;
>>
>> It's a nitpick, even by the standards of this review, but I'd prefer an
>> explicit '> 0' check.
>
> ITYM (state->ignore_regex_length == 0) but ack.
>

yeah, thought of that after just after I sent it, but yes.

Re: [PATCH 6/6] test: test regexp based new.ignore

[David Bremner]

David Bremner <david@tethera.net> writes:

> Jani Nikula <jani@nikula.org> writes:
>
>> Just some basics.
>> ---
>>  test/T050-new.sh | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>
> Tests fail after applying this patch (output attached).
>
> It sortof looks like the regexp ignore is not working?

Oh, never mind, I out clevered myself managing patches and skipped the
one that actually enabled the regexp ignore handling.

d

Re: [PATCH 3/6] cli/new: check for special directories earlier in pass 1

[David Bremner]

Jani Nikula <jani@nikula.org> writes:

> Avoid passing . and .. to ignore check. We don't need to check their
> dirent type either.

pushed 3 and 4

d