unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* non-deterministic behaviour of new.ignore (regexp) test
@ 2018-04-29  2:10 David Bremner
  2018-04-29  9:02 ` Jani Nikula
  0 siblings, 1 reply; 12+ messages in thread
From: David Bremner @ 2018-04-29  2:10 UTC (permalink / raw)
  To: notmuch


For me the following seems to consistently fail after between 30 and 500
attempts

    export NOTMUCH_TEST_QUIET=yes; count=0; while ./T050-new.sh; do (( count++ )); echo $count; done

The test failure looks like

T050-new: Testing "notmuch new" in several variations
 FAIL   Ignore files and directories specified in new.ignore (regexp)
	--- T050-new.25.expected	2018-04-29 02:07:13.055693999 +0000
	+++ T050-new.25.output	2018-04-29 02:07:13.055693999 +0000
	@@ -6,10 +6,6 @@
	 (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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: non-deterministic behaviour of new.ignore (regexp) test
  2018-04-29  2:10 non-deterministic behaviour of new.ignore (regexp) test David Bremner
@ 2018-04-29  9:02 ` Jani Nikula
  2018-04-29 11:48   ` [PATCH 1/2] CLI/new: add mtime-opt option David Bremner
  0 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2018-04-29  9:02 UTC (permalink / raw)
  To: David Bremner, notmuch

On Sat, 28 Apr 2018, David Bremner <david@tethera.net> wrote:
> For me the following seems to consistently fail after between 30 and 500
> attempts
>
>     export NOTMUCH_TEST_QUIET=yes; count=0; while ./T050-new.sh; do (( count++ )); echo $count; done

I believe this happens because the directory mtime is unchanged from the
previous scan in the test, and we skip the directory before we could
ignore the files. Quoting add_files():

    /* If the directory's modification time in the filesystem is the
     * same as what we recorded in the database the last time we
     * scanned it, then we can skip the second pass entirely.
     *
     * We test for strict equality here to avoid a bug that can happen
     * if the system clock jumps backward, (preventing new mail from
     * being discovered until the clock catches up and the directory
     * is modified again).
     */

I can't reproduce if I add this to the test:

diff --git a/test/T050-new.sh b/test/T050-new.sh
index 9025fa7aa63e..0db76f47130b 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -260,6 +260,7 @@ output=$(NOTMUCH_NEW 2>&1)
 test_expect_equal "$output" "No new mail."
 
 test_begin_subtest "Ignore files and directories specified in new.ignore (regexp)"
+touch "${MAIL_DIR}" # force rescan of the top level directory
 notmuch config set new.ignore ".git" "/^bro.*ink\$/" "/ignored.*file/"
 output=$(NOTMUCH_NEW --debug 2>&1 | sort)
 test_expect_equal "$output" \

---

However, I'm not sure even that is enough if all this happens in the
same second. I think the way notmuch new is written, it may skip as long
as it ensures a subsequent scan will catch the modified files:

    /* If the directory's mtime is the same as the wall-clock time
     * when we stat'ed the directory, we skip updating the mtime in
     * the database because a message could be delivered later in this
     * same second.  This may lead to unnecessary re-scans, but it
     * avoids overlooking messages. */

I think we can make the problem less likely with the touch, but as
everything gets faster, we might hit this more and more. One approach
might be a notmuch new --force option that would rescan all directories
regardless of mtimes. We could use this for testing (except when we're
testing the optimization).

BR,
Jani.

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 1/2] CLI/new: add mtime-opt option
  2018-04-29  9:02 ` Jani Nikula
@ 2018-04-29 11:48   ` David Bremner
  2018-04-29 11:48     ` [PATCH 2/2] test: use --no-mtime-opt in T050-new.sh David Bremner
  2018-04-29 16:20     ` [PATCH 1/2] CLI/new: add mtime-opt option Jani Nikula
  0 siblings, 2 replies; 12+ messages in thread
From: David Bremner @ 2018-04-29 11:48 UTC (permalink / raw)
  To: Jani Nikula, David Bremner, notmuch

This option, on by default, controls whether mtimes are used to
optimize the scanning of directories. The intent is to turn this
optimization off for certain tests.
---
 notmuch-new.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index c4345705..099bbbae 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -47,6 +47,7 @@ typedef struct {
     int output_is_a_tty;
     enum verbosity verbosity;
     bool debug;
+    bool mtime_opt;
     const char **new_tags;
     size_t new_tags_length;
     const char **ignore_verbatim;
@@ -527,7 +528,7 @@ add_files (notmuch_database_t *notmuch,
      * mistakenly return the total number of directory entries, since
      * that only inflates the count beyond 2.
      */
-    if (directory && fs_mtime == db_mtime && st.st_nlink == 2) {
+    if (directory && state->mtime_opt && fs_mtime == db_mtime && st.st_nlink == 2) {
 	/* There's one catch: pass 1 below considers symlinks to
 	 * directories to be directories, but these don't increase the
 	 * file system link count.  So, only bail early if the
@@ -618,7 +619,7 @@ add_files (notmuch_database_t *notmuch,
      * being discovered until the clock catches up and the directory
      * is modified again).
      */
-    if (directory && fs_mtime == db_mtime)
+    if (directory && state->mtime_opt && fs_mtime == db_mtime)
 	goto DONE;
 
     /* If the database has never seen this directory before, we can
@@ -771,7 +772,7 @@ add_files (notmuch_database_t *notmuch,
      * the database because a message could be delivered later in this
      * same second.  This may lead to unnecessary re-scans, but it
      * avoids overlooking messages. */
-    if (fs_mtime != stat_time)
+    if (state->mtime_opt && fs_mtime != stat_time)
 	_filename_list_add (state->directory_mtimes, path)->mtime = fs_mtime;
 
   DONE:
@@ -1053,6 +1054,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
     add_files_state_t add_files_state = {
 	.verbosity = VERBOSITY_NORMAL,
 	.debug = false,
+	.mtime_opt = true,
 	.output_is_a_tty = isatty (fileno (stdout)),
     };
     struct timeval tv_start;
@@ -1073,6 +1075,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
 	{ .opt_bool = &quiet, .name = "quiet" },
 	{ .opt_bool = &verbose, .name = "verbose" },
 	{ .opt_bool = &add_files_state.debug, .name = "debug" },
+	{ .opt_bool = &add_files_state.mtime_opt, .name = "mtime-opt" },
 	{ .opt_bool = &hooks, .name = "hooks" },
 	{ .opt_inherit = notmuch_shared_indexing_options },
 	{ .opt_inherit = notmuch_shared_options },
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] test: use --no-mtime-opt in T050-new.sh
  2018-04-29 11:48   ` [PATCH 1/2] CLI/new: add mtime-opt option David Bremner
@ 2018-04-29 11:48     ` David Bremner
  2018-04-29 16:24       ` Jani Nikula
  2018-04-29 16:20     ` [PATCH 1/2] CLI/new: add mtime-opt option Jani Nikula
  1 sibling, 1 reply; 12+ messages in thread
From: David Bremner @ 2018-04-29 11:48 UTC (permalink / raw)
  To: Jani Nikula, David Bremner, notmuch

Wherever the test relies on directories being scanned, this option
should be used to avoid skipping them due to mtimes on directories
matching the database.
---
 test/T050-new.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/test/T050-new.sh b/test/T050-new.sh
index 9025fa7a..12dba471 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -87,7 +87,7 @@ notmuch new > /dev/null
 
 mv "${MAIL_DIR}"/dir "${MAIL_DIR}"/dir-renamed
 
-output=$(NOTMUCH_NEW --debug)
+output=$(NOTMUCH_NEW --debug --no-mtime-opt)
 test_expect_equal "$output" "(D) add_files, pass 2: queuing passed directory ${MAIL_DIR}/dir for deletion from database
 No new mail. Detected 3 file renames."
 
@@ -95,7 +95,7 @@ No new mail. Detected 3 file renames."
 test_begin_subtest "Deleted directory"
 rm -rf "${MAIL_DIR}"/dir-renamed
 
-output=$(NOTMUCH_NEW --debug)
+output=$(NOTMUCH_NEW --debug --no-mtime-opt)
 test_expect_equal "$output" "(D) add_files, pass 2: queuing passed directory ${MAIL_DIR}/dir-renamed for deletion from database
 No new mail. Removed 3 messages."
 
@@ -114,7 +114,7 @@ test_begin_subtest "Deleted directory (end of list)"
 
 rm -rf "${MAIL_DIR}"/zzz
 
-output=$(NOTMUCH_NEW --debug)
+output=$(NOTMUCH_NEW --debug --no-mtime-opt)
 test_expect_equal "$output" "(D) add_files, pass 3: queuing leftover directory ${MAIL_DIR}/zzz for deletion from database
 No new mail. Removed 3 messages."
 
@@ -165,7 +165,7 @@ test_begin_subtest "Deleted two-level directory"
 
 rm -rf "${MAIL_DIR}"/two
 
-output=$(NOTMUCH_NEW --debug)
+output=$(NOTMUCH_NEW --debug --no-mtime-opt)
 test_expect_equal "$output" "(D) add_files, pass 3: queuing leftover directory ${MAIL_DIR}/two for deletion from database
 No new mail. Removed 3 messages."
 
@@ -211,7 +211,7 @@ Subject: Test mbox message 2
 
 Body 2.
 EOF
-output=$(NOTMUCH_NEW --debug 2>&1)
+output=$(NOTMUCH_NEW --debug --no-mtime-opt 2>&1)
 test_expect_equal "$output" \
 "Note: Ignoring non-mail file: ${MAIL_DIR}/.git/config
 Note: Ignoring non-mail file: ${MAIL_DIR}/.ignored_hidden_file
@@ -234,7 +234,7 @@ touch "${MAIL_DIR}"/.git # change .git's mtime for notmuch new to rescan.
 touch "${MAIL_DIR}"      # likewise for MAIL_DIR
 mkdir -p "${MAIL_DIR}"/one/two/three/.git
 touch "${MAIL_DIR}"/{one,one/two,one/two/three}/ignored_file
-output=$(NOTMUCH_NEW --debug 2>&1 | sort)
+output=$(NOTMUCH_NEW --debug --no-mtime-opt 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
@@ -261,7 +261,7 @@ 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)
+output=$(NOTMUCH_NEW --debug --no-mtime-opt 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
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] CLI/new: add mtime-opt option
  2018-04-29 11:48   ` [PATCH 1/2] CLI/new: add mtime-opt option David Bremner
  2018-04-29 11:48     ` [PATCH 2/2] test: use --no-mtime-opt in T050-new.sh David Bremner
@ 2018-04-29 16:20     ` Jani Nikula
  2018-04-29 23:19       ` [PATCH 1/4] CLI/new: add full-scan option David Bremner
  1 sibling, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2018-04-29 16:20 UTC (permalink / raw)
  To: David Bremner, David Bremner, notmuch

On Sun, 29 Apr 2018, David Bremner <david@tethera.net> wrote:
> This option, on by default, controls whether mtimes are used to
> optimize the scanning of directories. The intent is to turn this
> optimization off for certain tests.
> ---
>  notmuch-new.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/notmuch-new.c b/notmuch-new.c
> index c4345705..099bbbae 100644
> --- a/notmuch-new.c
> +++ b/notmuch-new.c
> @@ -47,6 +47,7 @@ typedef struct {
>      int output_is_a_tty;
>      enum verbosity verbosity;
>      bool debug;
> +    bool mtime_opt;
>      const char **new_tags;
>      size_t new_tags_length;
>      const char **ignore_verbatim;
> @@ -527,7 +528,7 @@ add_files (notmuch_database_t *notmuch,
>       * mistakenly return the total number of directory entries, since
>       * that only inflates the count beyond 2.
>       */
> -    if (directory && fs_mtime == db_mtime && st.st_nlink == 2) {
> +    if (directory && state->mtime_opt && fs_mtime == db_mtime && st.st_nlink == 2) {
>  	/* There's one catch: pass 1 below considers symlinks to
>  	 * directories to be directories, but these don't increase the
>  	 * file system link count.  So, only bail early if the
> @@ -618,7 +619,7 @@ add_files (notmuch_database_t *notmuch,
>       * being discovered until the clock catches up and the directory
>       * is modified again).
>       */
> -    if (directory && fs_mtime == db_mtime)
> +    if (directory && state->mtime_opt && fs_mtime == db_mtime)
>  	goto DONE;
>  
>      /* If the database has never seen this directory before, we can
> @@ -771,7 +772,7 @@ add_files (notmuch_database_t *notmuch,
>       * the database because a message could be delivered later in this
>       * same second.  This may lead to unnecessary re-scans, but it
>       * avoids overlooking messages. */
> -    if (fs_mtime != stat_time)
> +    if (state->mtime_opt && fs_mtime != stat_time)
>  	_filename_list_add (state->directory_mtimes, path)->mtime = fs_mtime;

I don't think we should skip this part. We've done a full scan now, so
we should record that in the database so a subsequent scan doesn't have
to.

>  
>    DONE:
> @@ -1053,6 +1054,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
>      add_files_state_t add_files_state = {
>  	.verbosity = VERBOSITY_NORMAL,
>  	.debug = false,
> +	.mtime_opt = true,
>  	.output_is_a_tty = isatty (fileno (stdout)),
>      };
>      struct timeval tv_start;
> @@ -1073,6 +1075,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
>  	{ .opt_bool = &quiet, .name = "quiet" },
>  	{ .opt_bool = &verbose, .name = "verbose" },
>  	{ .opt_bool = &add_files_state.debug, .name = "debug" },
> +	{ .opt_bool = &add_files_state.mtime_opt, .name = "mtime-opt" },

--full-scan? --force? --force-scan?

I think we've had people ask how they can have notmuch scan some
directories again, for some reason or another. Maybe to test their
ignores, and they can't have notmuch rescan the directory without
touching it. IMHO --no-mtime-opt doesn't sound very intuitive for that
purpose.

Otherwise, seems like a thing we should add.

BR,
Jani.

>  	{ .opt_bool = &hooks, .name = "hooks" },
>  	{ .opt_inherit = notmuch_shared_indexing_options },
>  	{ .opt_inherit = notmuch_shared_options },
> -- 
> 2.17.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] test: use --no-mtime-opt in T050-new.sh
  2018-04-29 11:48     ` [PATCH 2/2] test: use --no-mtime-opt in T050-new.sh David Bremner
@ 2018-04-29 16:24       ` Jani Nikula
  0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2018-04-29 16:24 UTC (permalink / raw)
  To: David Bremner, David Bremner, notmuch

On Sun, 29 Apr 2018, David Bremner <david@tethera.net> wrote:
> Wherever the test relies on directories being scanned, this option
> should be used to avoid skipping them due to mtimes on directories
> matching the database.

I think you could additionally remove a few touch calls in the
test. Some of them do actually create empty files, but some of them just
touch directories to force rescans.

Otherwise, LGTM.

BR,
Jani.

> ---
>  test/T050-new.sh | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/test/T050-new.sh b/test/T050-new.sh
> index 9025fa7a..12dba471 100755
> --- a/test/T050-new.sh
> +++ b/test/T050-new.sh
> @@ -87,7 +87,7 @@ notmuch new > /dev/null
>  
>  mv "${MAIL_DIR}"/dir "${MAIL_DIR}"/dir-renamed
>  
> -output=$(NOTMUCH_NEW --debug)
> +output=$(NOTMUCH_NEW --debug --no-mtime-opt)
>  test_expect_equal "$output" "(D) add_files, pass 2: queuing passed directory ${MAIL_DIR}/dir for deletion from database
>  No new mail. Detected 3 file renames."
>  
> @@ -95,7 +95,7 @@ No new mail. Detected 3 file renames."
>  test_begin_subtest "Deleted directory"
>  rm -rf "${MAIL_DIR}"/dir-renamed
>  
> -output=$(NOTMUCH_NEW --debug)
> +output=$(NOTMUCH_NEW --debug --no-mtime-opt)
>  test_expect_equal "$output" "(D) add_files, pass 2: queuing passed directory ${MAIL_DIR}/dir-renamed for deletion from database
>  No new mail. Removed 3 messages."
>  
> @@ -114,7 +114,7 @@ test_begin_subtest "Deleted directory (end of list)"
>  
>  rm -rf "${MAIL_DIR}"/zzz
>  
> -output=$(NOTMUCH_NEW --debug)
> +output=$(NOTMUCH_NEW --debug --no-mtime-opt)
>  test_expect_equal "$output" "(D) add_files, pass 3: queuing leftover directory ${MAIL_DIR}/zzz for deletion from database
>  No new mail. Removed 3 messages."
>  
> @@ -165,7 +165,7 @@ test_begin_subtest "Deleted two-level directory"
>  
>  rm -rf "${MAIL_DIR}"/two
>  
> -output=$(NOTMUCH_NEW --debug)
> +output=$(NOTMUCH_NEW --debug --no-mtime-opt)
>  test_expect_equal "$output" "(D) add_files, pass 3: queuing leftover directory ${MAIL_DIR}/two for deletion from database
>  No new mail. Removed 3 messages."
>  
> @@ -211,7 +211,7 @@ Subject: Test mbox message 2
>  
>  Body 2.
>  EOF
> -output=$(NOTMUCH_NEW --debug 2>&1)
> +output=$(NOTMUCH_NEW --debug --no-mtime-opt 2>&1)
>  test_expect_equal "$output" \
>  "Note: Ignoring non-mail file: ${MAIL_DIR}/.git/config
>  Note: Ignoring non-mail file: ${MAIL_DIR}/.ignored_hidden_file
> @@ -234,7 +234,7 @@ touch "${MAIL_DIR}"/.git # change .git's mtime for notmuch new to rescan.
>  touch "${MAIL_DIR}"      # likewise for MAIL_DIR
>  mkdir -p "${MAIL_DIR}"/one/two/three/.git
>  touch "${MAIL_DIR}"/{one,one/two,one/two/three}/ignored_file
> -output=$(NOTMUCH_NEW --debug 2>&1 | sort)
> +output=$(NOTMUCH_NEW --debug --no-mtime-opt 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
> @@ -261,7 +261,7 @@ 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)
> +output=$(NOTMUCH_NEW --debug --no-mtime-opt 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
> -- 
> 2.17.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/4] CLI/new: add full-scan option
  2018-04-29 16:20     ` [PATCH 1/2] CLI/new: add mtime-opt option Jani Nikula
@ 2018-04-29 23:19       ` David Bremner
  2018-04-29 23:19         ` [PATCH 2/4] test: add tests for notmuch new --full-scan David Bremner
                           ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: David Bremner @ 2018-04-29 23:19 UTC (permalink / raw)
  To: Jani Nikula, David Bremner, notmuch

By default notmuch-new uses directory mtimes to optimize the scanning of
directories for new mail. This option allows turning that optimization
off e.g. for testing or debugging.
---
 notmuch-new.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index c4345705..6a54a1a1 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -47,6 +47,7 @@ typedef struct {
     int output_is_a_tty;
     enum verbosity verbosity;
     bool debug;
+    bool full_scan;
     const char **new_tags;
     size_t new_tags_length;
     const char **ignore_verbatim;
@@ -527,7 +528,7 @@ add_files (notmuch_database_t *notmuch,
      * mistakenly return the total number of directory entries, since
      * that only inflates the count beyond 2.
      */
-    if (directory && fs_mtime == db_mtime && st.st_nlink == 2) {
+    if (directory && (! state->full_scan) && fs_mtime == db_mtime && st.st_nlink == 2) {
 	/* There's one catch: pass 1 below considers symlinks to
 	 * directories to be directories, but these don't increase the
 	 * file system link count.  So, only bail early if the
@@ -618,7 +619,7 @@ add_files (notmuch_database_t *notmuch,
      * being discovered until the clock catches up and the directory
      * is modified again).
      */
-    if (directory && fs_mtime == db_mtime)
+    if (directory && (! state->full_scan) && fs_mtime == db_mtime)
 	goto DONE;
 
     /* If the database has never seen this directory before, we can
@@ -1053,6 +1054,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
     add_files_state_t add_files_state = {
 	.verbosity = VERBOSITY_NORMAL,
 	.debug = false,
+	.full_scan = false,
 	.output_is_a_tty = isatty (fileno (stdout)),
     };
     struct timeval tv_start;
@@ -1073,6 +1075,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
 	{ .opt_bool = &quiet, .name = "quiet" },
 	{ .opt_bool = &verbose, .name = "verbose" },
 	{ .opt_bool = &add_files_state.debug, .name = "debug" },
+	{ .opt_bool = &add_files_state.full_scan, .name = "full-scan" },
 	{ .opt_bool = &hooks, .name = "hooks" },
 	{ .opt_inherit = notmuch_shared_indexing_options },
 	{ .opt_inherit = notmuch_shared_options },
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/4] test: add tests for notmuch new --full-scan
  2018-04-29 23:19       ` [PATCH 1/4] CLI/new: add full-scan option David Bremner
@ 2018-04-29 23:19         ` David Bremner
  2018-04-29 23:19         ` [PATCH 3/4] test: use --full-scan in T050-new.sh David Bremner
  2018-04-29 23:19         ` [PATCH 4/4] doc: document notmuch new --full-scan David Bremner
  2 siblings, 0 replies; 12+ messages in thread
From: David Bremner @ 2018-04-29 23:19 UTC (permalink / raw)
  To: Jani Nikula, David Bremner, notmuch

Most of these just check that adding the flag does not break existing
functionality. The one test that does check the full-scan
functionality had to be rewritten to output debugging info.
---
 test/T050-new.sh | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/test/T050-new.sh b/test/T050-new.sh
index 9025fa7a..1f111142 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -12,6 +12,10 @@ generate_message
 output=$(NOTMUCH_NEW --debug)
 test_expect_equal "$output" "Added 1 new message to the database."
 
+test_begin_subtest "Single message (full-scan)"
+generate_message
+output=$(NOTMUCH_NEW --debug --full-scan 2>&1)
+test_expect_equal "$output" "Added 1 new message to the database."
 
 test_begin_subtest "Multiple new messages"
 generate_message
@@ -19,11 +23,19 @@ generate_message
 output=$(NOTMUCH_NEW --debug)
 test_expect_equal "$output" "Added 2 new messages to the database."
 
+test_begin_subtest "Multiple new messages (full-scan)"
+generate_message
+generate_message
+output=$(NOTMUCH_NEW --debug --full-scan 2>&1)
+test_expect_equal "$output" "Added 2 new messages to the database."
 
 test_begin_subtest "No new messages (non-empty DB)"
 output=$(NOTMUCH_NEW --debug)
 test_expect_equal "$output" "No new mail."
 
+test_begin_subtest "No new messages (full-scan)"
+output=$(NOTMUCH_NEW --debug --full-scan 2>&1)
+test_expect_equal "$output" "No new mail."
 
 test_begin_subtest "New directories"
 rm -rf "${MAIL_DIR}"/* "${MAIL_DIR}"/.notmuch
@@ -224,8 +236,24 @@ test_begin_subtest "Ignore files and directories specified in new.ignore"
 generate_message
 notmuch config set new.ignore .git ignored_file .ignored_hidden_file
 touch "${MAIL_DIR}"/.git # change .git's mtime for notmuch new to rescan.
-output=$(NOTMUCH_NEW 2>&1)
-test_expect_equal "$output" "Added 1 new message to the database."
+NOTMUCH_NEW --debug 2>&1 | sort > OUTPUT
+cat <<EOF > EXPECTED
+(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/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/ignored_file
+Added 1 new message to the database.
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "Ignore files and directories specified in new.ignore (full-scan)"
+generate_message
+notmuch config set new.ignore .git ignored_file .ignored_hidden_file
+NOTMUCH_NEW --debug --full-scan 2>&1 | sort > OUTPUT
+# reuse EXPECTED from previous test
+test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "Ignore files and directories specified in new.ignore (multiple occurrences)"
 notmuch config set new.ignore .git ignored_file .ignored_hidden_file
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/4] test: use --full-scan in T050-new.sh
  2018-04-29 23:19       ` [PATCH 1/4] CLI/new: add full-scan option David Bremner
  2018-04-29 23:19         ` [PATCH 2/4] test: add tests for notmuch new --full-scan David Bremner
@ 2018-04-29 23:19         ` David Bremner
  2018-04-29 23:19         ` [PATCH 4/4] doc: document notmuch new --full-scan David Bremner
  2 siblings, 0 replies; 12+ messages in thread
From: David Bremner @ 2018-04-29 23:19 UTC (permalink / raw)
  To: Jani Nikula, David Bremner, notmuch

Wherever the test relies on directories being scanned, this option
should be used to avoid skipping them due to mtimes on directories
matching the database.
---
 test/T050-new.sh | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/test/T050-new.sh b/test/T050-new.sh
index 1f111142..40c5e6fa 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -99,7 +99,7 @@ notmuch new > /dev/null
 
 mv "${MAIL_DIR}"/dir "${MAIL_DIR}"/dir-renamed
 
-output=$(NOTMUCH_NEW --debug)
+output=$(NOTMUCH_NEW --debug --full-scan)
 test_expect_equal "$output" "(D) add_files, pass 2: queuing passed directory ${MAIL_DIR}/dir for deletion from database
 No new mail. Detected 3 file renames."
 
@@ -107,7 +107,7 @@ No new mail. Detected 3 file renames."
 test_begin_subtest "Deleted directory"
 rm -rf "${MAIL_DIR}"/dir-renamed
 
-output=$(NOTMUCH_NEW --debug)
+output=$(NOTMUCH_NEW --debug --full-scan)
 test_expect_equal "$output" "(D) add_files, pass 2: queuing passed directory ${MAIL_DIR}/dir-renamed for deletion from database
 No new mail. Removed 3 messages."
 
@@ -126,7 +126,7 @@ test_begin_subtest "Deleted directory (end of list)"
 
 rm -rf "${MAIL_DIR}"/zzz
 
-output=$(NOTMUCH_NEW --debug)
+output=$(NOTMUCH_NEW --debug --full-scan)
 test_expect_equal "$output" "(D) add_files, pass 3: queuing leftover directory ${MAIL_DIR}/zzz for deletion from database
 No new mail. Removed 3 messages."
 
@@ -177,7 +177,7 @@ test_begin_subtest "Deleted two-level directory"
 
 rm -rf "${MAIL_DIR}"/two
 
-output=$(NOTMUCH_NEW --debug)
+output=$(NOTMUCH_NEW --debug --full-scan)
 test_expect_equal "$output" "(D) add_files, pass 3: queuing leftover directory ${MAIL_DIR}/two for deletion from database
 No new mail. Removed 3 messages."
 
@@ -223,7 +223,7 @@ Subject: Test mbox message 2
 
 Body 2.
 EOF
-output=$(NOTMUCH_NEW --debug 2>&1)
+output=$(NOTMUCH_NEW --debug --full-scan 2>&1)
 test_expect_equal "$output" \
 "Note: Ignoring non-mail file: ${MAIL_DIR}/.git/config
 Note: Ignoring non-mail file: ${MAIL_DIR}/.ignored_hidden_file
@@ -258,11 +258,9 @@ test_expect_equal_file EXPECTED OUTPUT
 test_begin_subtest "Ignore files and directories specified in new.ignore (multiple occurrences)"
 notmuch config set new.ignore .git ignored_file .ignored_hidden_file
 notmuch new > /dev/null # ensure that files/folders will be printed in ASCII order.
-touch "${MAIL_DIR}"/.git # change .git's mtime for notmuch new to rescan.
-touch "${MAIL_DIR}"      # likewise for MAIL_DIR
 mkdir -p "${MAIL_DIR}"/one/two/three/.git
 touch "${MAIL_DIR}"/{one,one/two,one/two/three}/ignored_file
-output=$(NOTMUCH_NEW --debug 2>&1 | sort)
+output=$(NOTMUCH_NEW --debug --full-scan 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
@@ -289,7 +287,7 @@ 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)
+output=$(NOTMUCH_NEW --debug --full-scan 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
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/4] doc: document notmuch new --full-scan
  2018-04-29 23:19       ` [PATCH 1/4] CLI/new: add full-scan option David Bremner
  2018-04-29 23:19         ` [PATCH 2/4] test: add tests for notmuch new --full-scan David Bremner
  2018-04-29 23:19         ` [PATCH 3/4] test: use --full-scan in T050-new.sh David Bremner
@ 2018-04-29 23:19         ` David Bremner
  2018-05-01 19:58           ` Tomi Ollila
  2 siblings, 1 reply; 12+ messages in thread
From: David Bremner @ 2018-04-29 23:19 UTC (permalink / raw)
  To: Jani Nikula, David Bremner, notmuch

---
 doc/man1/notmuch-new.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/doc/man1/notmuch-new.rst b/doc/man1/notmuch-new.rst
index 16af5492..b0016ccc 100644
--- a/doc/man1/notmuch-new.rst
+++ b/doc/man1/notmuch-new.rst
@@ -59,6 +59,12 @@ Supported options for **new** include
 
     See also ``index.decrypt`` in **notmuch-config(1)**.
 
+``--full-scan``
+    By default notmuch-new uses directory modification times (mtimes)
+    to optimize the scanning of directories for new mail. This option
+    allows turning that optimization off e.g. for testing or
+    debugging.
+
 EXIT STATUS
 ===========
 
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] doc: document notmuch new --full-scan
  2018-04-29 23:19         ` [PATCH 4/4] doc: document notmuch new --full-scan David Bremner
@ 2018-05-01 19:58           ` Tomi Ollila
  2018-05-22 16:46             ` David Bremner
  0 siblings, 1 reply; 12+ messages in thread
From: Tomi Ollila @ 2018-05-01 19:58 UTC (permalink / raw)
  To: David Bremner, Jani Nikula, David Bremner, notmuch

On Sun, Apr 29 2018, David Bremner wrote:

> ---
>  doc/man1/notmuch-new.rst | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/doc/man1/notmuch-new.rst b/doc/man1/notmuch-new.rst
> index 16af5492..b0016ccc 100644
> --- a/doc/man1/notmuch-new.rst
> +++ b/doc/man1/notmuch-new.rst
> @@ -59,6 +59,12 @@ Supported options for **new** include
>  
>      See also ``index.decrypt`` in **notmuch-config(1)**.
>  
> +``--full-scan``
> +    By default notmuch-new uses directory modification times (mtimes)
> +    to optimize the scanning of directories for new mail. This option
> +    allows turning that optimization off e.g. for testing or
> +    debugging.
> +

Perhaps just (the last sentence): "This option turns that optimization off."

Tomi


>  EXIT STATUS
>  ===========
>  
> -- 
> 2.17.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] doc: document notmuch new --full-scan
  2018-05-01 19:58           ` Tomi Ollila
@ 2018-05-22 16:46             ` David Bremner
  0 siblings, 0 replies; 12+ messages in thread
From: David Bremner @ 2018-05-22 16:46 UTC (permalink / raw)
  To: Tomi Ollila, Jani Nikula, notmuch

Tomi Ollila <tomi.ollila@iki.fi> writes:

>> +
>
> Perhaps just (the last sentence): "This option turns that optimization off."
>
> Tomi

pushed to master, with this change.

d

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-05-22 16:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-29  2:10 non-deterministic behaviour of new.ignore (regexp) test David Bremner
2018-04-29  9:02 ` Jani Nikula
2018-04-29 11:48   ` [PATCH 1/2] CLI/new: add mtime-opt option David Bremner
2018-04-29 11:48     ` [PATCH 2/2] test: use --no-mtime-opt in T050-new.sh David Bremner
2018-04-29 16:24       ` Jani Nikula
2018-04-29 16:20     ` [PATCH 1/2] CLI/new: add mtime-opt option Jani Nikula
2018-04-29 23:19       ` [PATCH 1/4] CLI/new: add full-scan option David Bremner
2018-04-29 23:19         ` [PATCH 2/4] test: add tests for notmuch new --full-scan David Bremner
2018-04-29 23:19         ` [PATCH 3/4] test: use --full-scan in T050-new.sh David Bremner
2018-04-29 23:19         ` [PATCH 4/4] doc: document notmuch new --full-scan David Bremner
2018-05-01 19:58           ` Tomi Ollila
2018-05-22 16:46             ` David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).