[PATCH v2 00/11] Fix search tagging races

[PATCH v2 00/11] Fix search tagging races

[Austin Clements]

This is v2 of id:1381185201-25197-1-git-send-email-amdragon@mit.edu.
It fixes several comments from Mark and Jani.  This has been rebased
on top of the tag completion changes, so doing * from a large search
buffer will no longer crash.  Hence, this series depends on the
(currently pending) series in
id:1382487721-31776-1-git-send-email-amdragon@mit.edu.

This version does not address what happens when you * on a search
buffer that's still filling.  With this series, it will apply to all
messages that have appeared when the user finishes entering tag
changes.  This isn't ideal, but this seems pretty obscure and I'm not
sure what the right answer is, so I'm punting it to the future.

Another thing that may be worth changing in a follow-up is what
messages * applies to.  Currently, * applies *only* to matched
messages, not all threads in the search, which I think was an accident
of implementation.  This series retains that behavior, but opens up
the possibility of applying to all threads instead.  I think that
would be much more consistent and less surprising behavior.

An approximate diff from v1 is below.  This diff is prior to rebasing,
since the post-rebase diff is not useful.

diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index a4eec14..36937fb 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -242,7 +242,11 @@ from TAGS if present."
 	   (error "Changed tag must be of the form `+this_tag' or `-that_tag'")))))
     (sort result-tags 'string<)))
 
-(defconst notmuch-tag-argument-limit 1000)
+(defconst notmuch-tag-argument-limit 1000
+  "Use batch tagging if the tagging query is longer than this.
+
+This limits the length of arguments passed to the notmuch CLI to
+avoid system argument length limits and performance problems.")
 
 (defun notmuch-tag (query &optional tag-changes)
   "Add/remove tags in TAG-CHANGES to messages matching QUERY.
@@ -276,7 +280,6 @@ notmuch-after-tag-hook will be run."
       ;; Use batch tag mode to avoid argument length limitations
       (let ((batch-op (concat (mapconcat #'notmuch-hex-encode tag-changes " ")
 			      " -- " query)))
-	(message "Batch tagging with %s" batch-op)
 	(notmuch-call-notmuch-process :stdin-string batch-op "tag" "--batch")))
     (run-hooks 'notmuch-after-tag-hook))
   ;; in all cases we return tag-changes as a list
diff --git a/notmuch-client.h b/notmuch-client.h
index 1b14910..8bc1a2a 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -144,7 +144,8 @@ chomp_newline (char *str)
 #define NOTMUCH_FORMAT_MIN 1
 /* The minimum non-deprecated structured output format version.
  * Requests for format versions below this will print a stern warning.
- * Must be >= NOTMUCH_FORMAT_MIN and < NOTMUCH_FORMAT_CUR.
+ * Must be between NOTMUCH_FORMAT_MIN and NOTMUCH_FORMAT_CUR,
+ * inclusive.
  */
 #define NOTMUCH_FORMAT_MIN_ACTIVE 1
 
diff --git a/notmuch-search.c b/notmuch-search.c
index 1d14651..7c973b3 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -53,13 +53,13 @@ sanitize_string (const void *ctx, const char *str)
  * NULL. */
 static int
 get_thread_query (notmuch_thread_t *thread,
-		  char **matched_out, char **unmached_out)
+		  char **matched_out, char **unmatched_out)
 {
     notmuch_messages_t *messages;
     char *escaped = NULL;
     size_t escaped_len = 0;
 
-    *matched_out = *unmached_out = NULL;
+    *matched_out = *unmatched_out = NULL;
 
     for (messages = notmuch_thread_get_messages (thread);
 	 notmuch_messages_valid (messages);
@@ -69,17 +69,16 @@ get_thread_query (notmuch_thread_t *thread,
 	const char *mid = notmuch_message_get_message_id (message);
 	/* Determine which query buffer to extend */
 	char **buf = notmuch_message_get_flag (
-	    message, NOTMUCH_MESSAGE_FLAG_MATCH) ? matched_out : unmached_out;
-	/* Allocate the query buffer is this is the first message */
-	if (!*buf && (*buf = talloc_strdup (thread, "")) == NULL)
-	    return -1;
+	    message, NOTMUCH_MESSAGE_FLAG_MATCH) ? matched_out : unmatched_out;
 	/* Add this message's id: query.  Since "id" is an exclusive
 	 * prefix, it is implicitly 'or'd together, so we only need to
 	 * join queries with a space. */
 	if (make_boolean_term (thread, "id", mid, &escaped, &escaped_len) < 0)
 	    return -1;
-	*buf = talloc_asprintf_append_buffer (
-	    *buf, "%s%s", **buf ? " " : "", escaped);
+	if (*buf)
+	    *buf = talloc_asprintf_append_buffer (*buf, " %s", escaped);
+	else
+	    *buf = talloc_strdup (thread, escaped);
 	if (!*buf)
 	    return -1;
     }

[PATCH v2 01/11] schemata: Disambiguate non-terminal names

[Austin Clements]

Previously, the show schema and the search schema used different
"thread" non-terminals.  While these schemata don't interact, this is
still confusing, so rename search's "thread" to "thread_summary".  To
further limit confusion, prefix all top-level search non-terminals now
begin with "search_".
---
 devel/schemata | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/devel/schemata b/devel/schemata
index 2405756..cdd0e43 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -122,21 +122,21 @@ notmuch search schema
 ---------------------
 
 # --output=summary
-summary = [thread*]
+search_summary = [thread_summary*]
 
 # --output=threads
-threads = [threadid*]
+search_threads = [threadid*]
 
 # --output=messages
-messages = [messageid*]
+search_messages = [messageid*]
 
 # --output=files
-files = [string*]
+search_files = [string*]
 
 # --output=tags
-tags = [string*]
+search_tags = [string*]
 
-thread = {
+thread_summary = {
     thread:         threadid,
     timestamp:      unix_time,
     date_relative:  string,   # user-friendly timestamp
-- 
1.8.4.rc3

[PATCH v2 02/11] cli: Separate current and deprecated format version

[Austin Clements]

Previously, the CLI would print a deprecation warning if a client
requested any format version other than the current one.  However, if
we add fields that are backwards-compatible, but want clients to be
able to depend on, we need to bump the version, but that doesn't make
the older version deprecated.

Hence, separate out the "minimum active" version and only print a
warning for requests below this version number.
---
 notmuch-client.h | 6 ++++++
 notmuch.c        | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 0bfa4da..4ecb3ae 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -142,6 +142,12 @@ chomp_newline (char *str)
 /* The minimum supported structured output format version.  Requests
  * for format versions below this will return an error. */
 #define NOTMUCH_FORMAT_MIN 1
+/* The minimum non-deprecated structured output format version.
+ * Requests for format versions below this will print a stern warning.
+ * Must be between NOTMUCH_FORMAT_MIN and NOTMUCH_FORMAT_CUR,
+ * inclusive.
+ */
+#define NOTMUCH_FORMAT_MIN_ACTIVE 1
 
 /* The output format version requested by the caller on the command
  * line.  If no format version is requested, this will be set to
diff --git a/notmuch.c b/notmuch.c
index 8d303a1..54f46c6 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -127,7 +127,7 @@ by the notmuch CLI (it requires at least version %d).  You may need to\n\
 upgrade your notmuch front-end.\n",
 		 notmuch_format_version, NOTMUCH_FORMAT_MIN);
 	exit (NOTMUCH_EXIT_FORMAT_TOO_OLD);
-    } else if (notmuch_format_version != NOTMUCH_FORMAT_CUR) {
+    } else if (notmuch_format_version < NOTMUCH_FORMAT_MIN_ACTIVE) {
 	/* Warn about old version requests so compatibility issues are
 	 * less likely when we drop support for a deprecated format
 	 * versions. */
-- 
1.8.4.rc3

[PATCH v2 03/11] lib: Document extent of some return values

[Austin Clements]

This documents the extent of the notmuch_messages_t* pointers returned
by notmuch_thread_get_toplevel_messages and
notmuch_thread_get_messages.
---
 lib/notmuch.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 9dab555..0fb7121 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -765,12 +765,16 @@ notmuch_thread_get_total_messages (notmuch_thread_t *thread);
  * This iterator will not necessarily iterate over all of the messages
  * in the thread. It will only iterate over the messages in the thread
  * which are not replies to other messages in the thread.
+ *
+ * The returned list will be destroyed when the thread is destroyed.
  */
 notmuch_messages_t *
 notmuch_thread_get_toplevel_messages (notmuch_thread_t *thread);
 
 /* Get a notmuch_thread_t iterator for all messages in 'thread' in
  * oldest-first order.
+ *
+ * The returned list will be destroyed when the thread is destroyed.
  */
 notmuch_messages_t *
 notmuch_thread_get_messages (notmuch_thread_t *thread);
-- 
1.8.4.rc3

[PATCH v2 04/11] test: Fix missing erase-buffer in emacs test

[Austin Clements]

The first subprocess error exit code test assumed the *Notmuch errors*
buffer would be empty.  Rather than assuming, make it so.
---
 test/emacs | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/test/emacs b/test/emacs
index 5bc3efc..05295af 100755
--- a/test/emacs
+++ b/test/emacs
@@ -864,6 +864,8 @@ EOF
 chmod a+x notmuch_fail
 test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\"))
 	       (with-current-buffer \"*Messages*\" (erase-buffer))
+	       (with-current-buffer (get-buffer-create \"*Notmuch errors*\")
+                 (erase-buffer))
 	       (notmuch-search \"tag:inbox\")
 	       (notmuch-test-wait)
 	       (with-current-buffer \"*Messages*\"
@@ -893,7 +895,8 @@ EOF
 chmod a+x notmuch_fail
 test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\"))
 	       (with-current-buffer \"*Messages*\" (erase-buffer))
-	       (with-current-buffer \"*Notmuch errors*\" (erase-buffer))
+	       (with-current-buffer (get-buffer-create \"*Notmuch errors*\")
+                 (erase-buffer))
 	       (notmuch-search \"tag:inbox\")
 	       (notmuch-test-wait)
 	       (with-current-buffer \"*Messages*\"
-- 
1.8.4.rc3

[PATCH v2 05/11] emacs: Move `notmuch-call-notmuch-process' to notmuch-lib

[Austin Clements]

Previously, this was in notmuch.el, but all of the other notmuch call
wrappers were in notmuch-lib.el.  Move `notmuch-call-notmuch-process'
to live with its friends.  This happens to fix a missing dependency
from notmuch-tag.el, which required notmuch-lib, but not notmuch.
---
 emacs/notmuch-lib.el | 11 +++++++++++
 emacs/notmuch.el     | 11 -----------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 58f3313..4e0604e 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -534,6 +534,17 @@ You may need to restart Emacs or upgrade your notmuch package."))
 	;; `notmuch-logged-error' does not return.
 	))))
 
+(defun notmuch-call-notmuch-process (&rest args)
+  "Synchronously invoke \"notmuch\" with the given list of arguments.
+
+If notmuch exits with a non-zero status, output from the process
+will appear in a buffer named \"*Notmuch errors*\" and an error
+will be signaled."
+  (with-temp-buffer
+    (let ((status (apply #'call-process notmuch-command nil t nil args)))
+      (notmuch-check-exit-status status (cons notmuch-command args)
+				 (buffer-string)))))
+
 (defun notmuch-call-notmuch-sexp (&rest args)
   "Invoke `notmuch-command' with ARGS and return the parsed S-exp output.
 
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 53e9826..496c8ec 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -535,17 +535,6 @@ If BARE is set then do not prefix with \"thread:\""
   (let ((message-id (notmuch-search-find-thread-id)))
     (notmuch-mua-new-reply message-id prompt-for-sender nil)))
 
-(defun notmuch-call-notmuch-process (&rest args)
-  "Synchronously invoke \"notmuch\" with the given list of arguments.
-
-If notmuch exits with a non-zero status, output from the process
-will appear in a buffer named \"*Notmuch errors*\" and an error
-will be signaled."
-  (with-temp-buffer
-    (let ((status (apply #'call-process notmuch-command nil t nil args)))
-      (notmuch-check-exit-status status (cons notmuch-command args)
-				 (buffer-string)))))
-
 (defun notmuch-search-set-tags (tags &optional pos)
   (let ((new-result (plist-put (notmuch-search-get-result pos) :tags tags)))
     (notmuch-search-update-result new-result pos)))
-- 
1.8.4.rc3

[PATCH v2 06/11] emacs: Support passing input via `notmuch-call-notmuch-*'

[Austin Clements]

This adds support for passing a string to write to notmuch's stdin to
`notmuch-call-notmuch-process' and `notmuch-call-notmuch-sexp'.  Since
this makes both interfaces a little more complicated, it also unifies
their documentation better.
---
 emacs/notmuch-lib.el | 39 +++++++++++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 4e0604e..22156f1 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -534,28 +534,55 @@ You may need to restart Emacs or upgrade your notmuch package."))
 	;; `notmuch-logged-error' does not return.
 	))))
 
+(defun notmuch-call-notmuch--helper (destination args)
+  "Helper for synchronous notmuch invocation commands.
+
+This wraps `call-process'.  DESTINATION has the same meaning as
+for `call-process'.  ARGS is as described for
+`notmuch-call-notmuch-process'."
+
+  (let (stdin-string)
+    (while (keywordp (car args))
+      (case (car args)
+	(:stdin-string (setq stdin-string (cadr args)
+			     args (cddr args)))
+	(otherwise
+	 (error "Unknown keyword argument: %s" (car args)))))
+    (if (null stdin-string)
+	(apply #'call-process notmuch-command nil destination nil args)
+      (insert stdin-string)
+      (apply #'call-process-region (point-min) (point-max)
+	     notmuch-command t destination nil args))))
+
 (defun notmuch-call-notmuch-process (&rest args)
-  "Synchronously invoke \"notmuch\" with the given list of arguments.
+  "Synchronously invoke `notmuch-command' with ARGS.
+
+The caller may provide keyword arguments before ARGS.  Currently
+supported keyword arguments are:
+
+  :stdin-string STRING - Write STRING to stdin
 
 If notmuch exits with a non-zero status, output from the process
 will appear in a buffer named \"*Notmuch errors*\" and an error
 will be signaled."
   (with-temp-buffer
-    (let ((status (apply #'call-process notmuch-command nil t nil args)))
+    (let ((status (notmuch-call-notmuch--helper t args)))
       (notmuch-check-exit-status status (cons notmuch-command args)
 				 (buffer-string)))))
 
 (defun notmuch-call-notmuch-sexp (&rest args)
   "Invoke `notmuch-command' with ARGS and return the parsed S-exp output.
 
-If notmuch exits with a non-zero status, this will pop up a
-buffer containing notmuch's output and signal an error."
+This is equivalent to `notmuch-call-notmuch-process', but parses
+notmuch's output as an S-expression and returns the parsed value.
+Like `notmuch-call-notmuch-process', if notmuch exits with a
+non-zero status, this will report its output and signal an
+error."
 
   (with-temp-buffer
     (let ((err-file (make-temp-file "nmerr")))
       (unwind-protect
-	  (let ((status (apply #'call-process
-			       notmuch-command nil (list t err-file) nil args)))
+	  (let ((status (notmuch-call-notmuch--helper (list t err-file) args)))
 	    (notmuch-check-exit-status status (cons notmuch-command args)
 				       (buffer-string) err-file)
 	    (goto-char (point-min))
-- 
1.8.4.rc3

[PATCH v2 07/11] emacs: Use notmuch tag --batch for large tag queries

[Austin Clements]

(Unfortunately, it's difficult to first demonstrate this problem with
a known-broken test because modern Linux kernels have argument length
limits in the megabytes, which makes Emacs really slow!)
---
 emacs/notmuch-lib.el |  8 ++++++++
 emacs/notmuch-tag.el | 15 +++++++++++++--
 test/emacs           |  8 ++++++++
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 22156f1..348112b 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -261,6 +261,14 @@ user-friendly queries."
   "Return a query that matches the message with id ID."
   (concat "id:" (notmuch-escape-boolean-term id)))
 
+(defun notmuch-hex-encode (str)
+  "Hex-encode STR (e.g., as used by batch tagging).
+
+This replaces spaces, percents, and double quotes in STR with
+%NN where NN is the hexadecimal value of the character."
+  (replace-regexp-in-string
+   "[ %\"]" (lambda (match) (format "%%%02x" (aref match 0))) str))
+
 ;;
 
 (defun notmuch-common-do-stash (text)
diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index 7b21006..cbb935c 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -252,6 +252,12 @@ from TAGS if present."
 	   (error "Changed tag must be of the form `+this_tag' or `-that_tag'")))))
     (sort result-tags 'string<)))
 
+(defconst notmuch-tag-argument-limit 1000
+  "Use batch tagging if the tagging query is longer than this.
+
+This limits the length of arguments passed to the notmuch CLI to
+avoid system argument length limits and performance problems.")
+
 (defun notmuch-tag (query tag-changes)
   "Add/remove tags in TAG-CHANGES to messages matching QUERY.
 
@@ -270,8 +276,13 @@ notmuch-after-tag-hook will be run."
 	tag-changes)
   (unless (null tag-changes)
     (run-hooks 'notmuch-before-tag-hook)
-    (apply 'notmuch-call-notmuch-process "tag"
-	   (append tag-changes (list "--" query)))
+    (if (<= (length query) notmuch-tag-argument-limit)
+	(apply 'notmuch-call-notmuch-process "tag"
+	       (append tag-changes (list "--" query)))
+      ;; Use batch tag mode to avoid argument length limitations
+      (let ((batch-op (concat (mapconcat #'notmuch-hex-encode tag-changes " ")
+			      " -- " query)))
+	(notmuch-call-notmuch-process :stdin-string batch-op "tag" "--batch")))
     (run-hooks 'notmuch-after-tag-hook)))
 
 (defun notmuch-tag-change-list (tags &optional reverse)
diff --git a/test/emacs b/test/emacs
index 05295af..5fbee12 100755
--- a/test/emacs
+++ b/test/emacs
@@ -122,6 +122,14 @@ test_emacs "(notmuch-search \"$os_x_darwin_thread\")
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
 
+test_begin_subtest "Add tag (large query)"
+# We use a long query to force us into batch mode and use a funny tag
+# that requires escaping for batch tagging.
+test_emacs "(notmuch-tag (concat \"$os_x_darwin_thread\" \" or \" (make-string notmuch-tag-argument-limit ?x)) (list \"+tag-from-%-large-query\"))"
+output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox tag-from-%-large-query unread)"
+notmuch tag -tag-from-%-large-query $os_x_darwin_thread
+
 test_begin_subtest "notmuch-show: add single tag to single message"
 test_emacs "(notmuch-show \"$os_x_darwin_thread\")
 	    (execute-kbd-macro \"+tag-from-show-view\")"
-- 
1.8.4.rc3

[PATCH v2 08/11] search: Add stable queries to thread search results

[Austin Clements]

These queries will match exactly the set of messages currently in the
thread, even if more messages later arrive.  Two queries are provided:
one for matched messages and one for unmatched messages.

This can be used to fix race conditions with tagging threads from
search results.  While tagging based on a thread: query can affect
messages that arrived after the search, tagging based on stable
queries affects only the messages the user was shown in the search UI.

Since we want clients to be able to depend on the presence of these
queries, this ushers in schema version 2.
---
 devel/schemata       | 22 ++++++++++++++++++--
 notmuch-client.h     |  2 +-
 notmuch-search.c     | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 test/json            |  2 ++
 test/missing-headers |  6 ++++--
 test/sexp            |  4 ++--
 6 files changed, 88 insertions(+), 7 deletions(-)

diff --git a/devel/schemata b/devel/schemata
index cdd0e43..41dc4a6 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -14,7 +14,17 @@ are interleaved. Keys are printed as keywords (symbols preceded by a
 colon), e.g. (:id "123" :time 54321 :from "foobar"). Null is printed as
 nil, true as t and false as nil.
 
-This is version 1 of the structured output format.
+This is version 2 of the structured output format.
+
+Version history
+---------------
+
+v1
+- First versioned schema release.
+- Added part.content-length and part.content-transfer-encoding fields.
+
+v2
+- Added the thread_summary.query field.
 
 Common non-terminals
 --------------------
@@ -145,7 +155,15 @@ thread_summary = {
     authors:        string,   # comma-separated names with | between
                               # matched and unmatched
     subject:        string,
-    tags:           [string*]
+    tags:           [string*],
+
+    # Two stable query strings identifying exactly the matched and
+    # unmatched messages currently in this thread.  The messages
+    # matched by these queries will not change even if more messages
+    # arrive in the thread.  If there are no matched or unmatched
+    # messages, the corresponding query will be null (there is no
+    # query that matches nothing).  (Added in schema version 2.)
+    query:          [string|null, string|null],
 }
 
 notmuch reply schema
diff --git a/notmuch-client.h b/notmuch-client.h
index 4ecb3ae..278b498 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -138,7 +138,7 @@ chomp_newline (char *str)
  * this.  New (required) map fields can be added without increasing
  * this.
  */
-#define NOTMUCH_FORMAT_CUR 1
+#define NOTMUCH_FORMAT_CUR 2
 /* The minimum supported structured output format version.  Requests
  * for format versions below this will return an error. */
 #define NOTMUCH_FORMAT_MIN 1
diff --git a/notmuch-search.c b/notmuch-search.c
index d9d39ec..7c973b3 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -20,6 +20,7 @@
 
 #include "notmuch-client.h"
 #include "sprinter.h"
+#include "string-util.h"
 
 typedef enum {
     OUTPUT_SUMMARY,
@@ -46,6 +47,45 @@ sanitize_string (const void *ctx, const char *str)
     return out;
 }
 
+/* Return two stable query strings that identify exactly the matched
+ * and unmatched messages currently in thread.  If there are no
+ * matched or unmatched messages, the returned buffers will be
+ * NULL. */
+static int
+get_thread_query (notmuch_thread_t *thread,
+		  char **matched_out, char **unmatched_out)
+{
+    notmuch_messages_t *messages;
+    char *escaped = NULL;
+    size_t escaped_len = 0;
+
+    *matched_out = *unmatched_out = NULL;
+
+    for (messages = notmuch_thread_get_messages (thread);
+	 notmuch_messages_valid (messages);
+	 notmuch_messages_move_to_next (messages))
+    {
+	notmuch_message_t *message = notmuch_messages_get (messages);
+	const char *mid = notmuch_message_get_message_id (message);
+	/* Determine which query buffer to extend */
+	char **buf = notmuch_message_get_flag (
+	    message, NOTMUCH_MESSAGE_FLAG_MATCH) ? matched_out : unmatched_out;
+	/* Add this message's id: query.  Since "id" is an exclusive
+	 * prefix, it is implicitly 'or'd together, so we only need to
+	 * join queries with a space. */
+	if (make_boolean_term (thread, "id", mid, &escaped, &escaped_len) < 0)
+	    return -1;
+	if (*buf)
+	    *buf = talloc_asprintf_append_buffer (*buf, " %s", escaped);
+	else
+	    *buf = talloc_strdup (thread, escaped);
+	if (!*buf)
+	    return -1;
+    }
+    talloc_free (escaped);
+    return 0;
+}
+
 static int
 do_search_threads (sprinter_t *format,
 		   notmuch_query_t *query,
@@ -131,6 +171,25 @@ do_search_threads (sprinter_t *format,
 		format->string (format, authors);
 		format->map_key (format, "subject");
 		format->string (format, subject);
+		if (notmuch_format_version >= 2) {
+		    char *matched_query, *unmatched_query;
+		    if (get_thread_query (thread, &matched_query,
+					  &unmatched_query) < 0) {
+			fprintf (stderr, "Out of memory\n");
+			return 1;
+		    }
+		    format->map_key (format, "query");
+		    format->begin_list (format);
+		    if (matched_query)
+			format->string (format, matched_query);
+		    else
+			format->null (format);
+		    if (unmatched_query)
+			format->string (format, unmatched_query);
+		    else
+			format->null (format);
+		    format->end (format);
+		}
 	    }
 
 	    talloc_free (ctx_quote);
diff --git a/test/json b/test/json
index b87b7f6..e07a290 100755
--- a/test/json
+++ b/test/json
@@ -26,6 +26,7 @@ test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
  \"total\": 1,
  \"authors\": \"Notmuch Test Suite\",
  \"subject\": \"json-search-subject\",
+ \"query\": [\"id:$gen_msg_id\", null],
  \"tags\": [\"inbox\",
  \"unread\"]}]"
 
@@ -59,6 +60,7 @@ test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
  \"total\": 1,
  \"authors\": \"Notmuch Test Suite\",
  \"subject\": \"json-search-utf8-body-sübjéct\",
+ \"query\": [\"id:$gen_msg_id\", null],
  \"tags\": [\"inbox\",
  \"unread\"]}]"
 
diff --git a/test/missing-headers b/test/missing-headers
index f14b878..43e861b 100755
--- a/test/missing-headers
+++ b/test/missing-headers
@@ -43,7 +43,8 @@ test_expect_equal_json "$output" '
         ],
         "thread": "XXX",
         "timestamp": 978709437,
-        "total": 1
+        "total": 1,
+        "query": ["id:notmuch-sha1-7a6e4eac383ef958fcd3ebf2143db71b8ff01161", null]
     },
     {
         "authors": "Notmuch Test Suite",
@@ -56,7 +57,8 @@ test_expect_equal_json "$output" '
         ],
         "thread": "XXX",
         "timestamp": 0,
-        "total": 1
+        "total": 1,
+        "query": ["id:notmuch-sha1-ca55943aff7a72baf2ab21fa74fab3d632401334", null]
     }
 ]'
 
diff --git a/test/sexp b/test/sexp
index 492a82f..be815e1 100755
--- a/test/sexp
+++ b/test/sexp
@@ -19,7 +19,7 @@ test_expect_equal "$output" "((((:id \"${gen_msg_id}\" :match t :excluded nil :f
 test_begin_subtest "Search message: sexp"
 add_message "[subject]=\"sexp-search-subject\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"sexp-search-message\""
 output=$(notmuch search --format=sexp "sexp-search-message" | notmuch_search_sanitize)
-test_expect_equal "$output" "((:thread \"0000000000000002\" :timestamp 946728000 :date_relative \"2000-01-01\" :matched 1 :total 1 :authors \"Notmuch Test Suite\" :subject \"sexp-search-subject\" :tags (\"inbox\" \"unread\")))"
+test_expect_equal "$output" "((:thread \"0000000000000002\" :timestamp 946728000 :date_relative \"2000-01-01\" :matched 1 :total 1 :authors \"Notmuch Test Suite\" :subject \"sexp-search-subject\" :query (\"id:$gen_msg_id\" nil) :tags (\"inbox\" \"unread\")))"
 
 test_begin_subtest "Show message: sexp, utf-8"
 add_message "[subject]=\"sexp-show-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-show-méssage\""
@@ -44,7 +44,7 @@ test_expect_equal "$output" "((((:id \"$id\" :match t :excluded nil :filename \"
 test_begin_subtest "Search message: sexp, utf-8"
 add_message "[subject]=\"sexp-search-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-search-méssage\""
 output=$(notmuch search --format=sexp "jsön-search-méssage" | notmuch_search_sanitize)
-test_expect_equal "$output" "((:thread \"0000000000000005\" :timestamp 946728000 :date_relative \"2000-01-01\" :matched 1 :total 1 :authors \"Notmuch Test Suite\" :subject \"sexp-search-utf8-body-sübjéct\" :tags (\"inbox\" \"unread\")))"
+test_expect_equal "$output" "((:thread \"0000000000000005\" :timestamp 946728000 :date_relative \"2000-01-01\" :matched 1 :total 1 :authors \"Notmuch Test Suite\" :subject \"sexp-search-utf8-body-sübjéct\" :query (\"id:$gen_msg_id\" nil) :tags (\"inbox\" \"unread\")))"
 
 
 test_done
-- 
1.8.4.rc3

[PATCH v2 09/11] Add TODO about more efficient stable thread queries

[Austin Clements]

---
 devel/TODO | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/devel/TODO b/devel/TODO
index 844555e..f212483 100644
--- a/devel/TODO
+++ b/devel/TODO
@@ -114,6 +114,12 @@ are multiple ideas that might make sense. Some consensus needs to be
 reached on this issue, and then both reply formats should be updated
 to be consistent.
 
+Return docid-based queries in thread search results, instead of
+message-id-based queries.  These are much more compact and much faster
+to later query.  This will require support from the library, both for
+retrieving docids (probably as an opaque "get a query that identifies
+this message") and for docid queries.
+
 notmuch library
 ---------------
 Add support for custom flag<->tag mappings. In the notmuch
-- 
1.8.4.rc3

[PATCH v2 10/11] emacs: Add known-broken tests for search tagging races

[Austin Clements]

These tests check that both thread-local and global search tagging
operations are race-free.  They are currently known-broken because
they aren't race-free.
---
 test/emacs | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/test/emacs b/test/emacs
index 5fbee12..7503e96 100755
--- a/test/emacs
+++ b/test/emacs
@@ -922,4 +922,30 @@ This is a warning (see *Notmuch errors* for more details)
 This is a warning
 This is another warning"
 
+test_begin_subtest "Search thread tag operations are race-free"
+test_subtest_known_broken
+add_message '[subject]="Search race test"'
+gen_msg_id_1=$gen_msg_id
+generate_message '[in-reply-to]="<'$gen_msg_id_1'>"' \
+	    '[references]="<'$gen_msg_id_1'>"' \
+	    '[subject]="Search race test two"'
+test_emacs '(notmuch-search "subject:\"search race test\"")
+	    (notmuch-test-wait)
+	    (notmuch-poll)
+	    (execute-kbd-macro "+search-thread-race-tag")'
+output=$(notmuch search --output=messages 'tag:search-thread-race-tag')
+test_expect_equal "$output" "id:$gen_msg_id_1"
+
+test_begin_subtest "Search global tag operations are race-free"
+test_subtest_known_broken
+generate_message '[in-reply-to]="<'$gen_msg_id_1'>"' \
+	    '[references]="<'$gen_msg_id_1'>"' \
+	    '[subject]="Re: Search race test"'
+test_emacs '(notmuch-search "subject:\"search race test\" -subject:two")
+	    (notmuch-test-wait)
+	    (notmuch-poll)
+	    (execute-kbd-macro "*+search-global-race-tag")'
+output=$(notmuch search --output=messages 'tag:search-global-race-tag')
+test_expect_equal "$output" "id:$gen_msg_id_1"
+
 test_done
-- 
1.8.4.rc3

[PATCH v2 11/11] emacs: Fix search tagging races

[Austin Clements]

This fixes races in thread-local and global tagging in notmuch-search
(e.g., "+", "-", "a", "*", etc.).  Previously, these would modify tags
of new messages that arrived after the search.  Now they only operate
on the messages that were in the threads when the search was
performed.  This prevents surprises like archiving messages that
arrived in a thread after the search results were shown.

This eliminates `notmuch-search-find-thread-id-region(-search)'
because these functions strongly encouraged racy usage.

This fixes the two broken tests added by the previous patch.
---
 devel/TODO       |  5 -----
 emacs/notmuch.el | 40 +++++++++++++++++++++++++++-------------
 test/emacs       |  4 +---
 3 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/devel/TODO b/devel/TODO
index f212483..1cf4089 100644
--- a/devel/TODO
+++ b/devel/TODO
@@ -14,11 +14,6 @@ sender's name containing ';' which causes emacs to drop a search
 result.) This may require removing the outer array from the current
 "notmuch search --format=json" results.
 
-Fix '*' to work by simply calling '+' or '-' on a region consisting of
-the entire buffer, (this would avoid one race condition---while still
-leaving other race conditions---but could also potentially make '*' a
-very expensive operation).
-
 Add a global keybinding table for notmuch, and then view-specific
 tables that add to it.
 	
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 496c8ec..fea1a93 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -485,14 +485,25 @@ If BARE is set then do not prefix with \"thread:\""
   (let ((thread (plist-get (notmuch-search-get-result) :thread)))
     (when thread (concat (unless bare "thread:") thread))))
 
-(defun notmuch-search-find-thread-id-region (beg end)
-  "Return a list of threads for the current region"
-  (mapcar (lambda (thread) (concat "thread:" thread))
-	  (notmuch-search-properties-in-region :thread beg end)))
-
-(defun notmuch-search-find-thread-id-region-search (beg end)
-  "Return a search string for threads for the current region"
-  (mapconcat 'identity (notmuch-search-find-thread-id-region beg end) " or "))
+(defun notmuch-search-find-stable-query ()
+  "Return the stable queries for the current thread.
+
+This returns a list (MATCHED-QUERY UNMATCHED-QUERY) for the
+matched and unmatched messages in the current thread."
+  (plist-get (notmuch-search-get-result) :query))
+
+(defun notmuch-search-find-stable-query-region (beg end &optional only-matched)
+  "Return the stable query for the current region.
+
+If ONLY-MATCHED is non-nil, include only matched messages.  If it
+is nil, include both matched and unmatched messages."
+  (let ((query-list nil) (all (not only-matched)))
+    (dolist (queries (notmuch-search-properties-in-region :query beg end))
+      (when (first queries)
+	(push (first queries) query-list))
+      (when (and all (second queries))
+	(push (second queries) query-list)))
+    (concat "(" (mapconcat 'identity query-list ") or (") ")")))
 
 (defun notmuch-search-find-authors ()
   "Return the authors for the current thread"
@@ -569,17 +580,20 @@ Returns (TAG-CHANGES REGION-BEGIN REGION-END)."
 	   (notmuch-search-get-tags-region beg end) prompt initial-input)
 	  region)))
 
-(defun notmuch-search-tag (tag-changes &optional beg end)
+(defun notmuch-search-tag (tag-changes &optional beg end only-matched)
   "Change tags for the currently selected thread or region.
 
 See `notmuch-tag' for information on the format of TAG-CHANGES.
 When called interactively, this uses the region if the region is
 active.  When called directly, BEG and END provide the region.
 If these are nil or not provided, this applies to the thread at
-point."
+point.
+
+If ONLY-MATCHED is non-nil, only tag matched messages."
   (interactive (notmuch-search-interactive-tag-changes))
   (unless (and beg end) (setq beg (point) end (point)))
-  (let ((search-string (notmuch-search-find-thread-id-region-search beg end)))
+  (let ((search-string (notmuch-search-find-stable-query-region
+			beg end only-matched)))
     (notmuch-tag search-string tag-changes)
     (notmuch-search-foreach-result beg end
       (lambda (pos)
@@ -847,7 +861,7 @@ See `notmuch-tag' for information on the format of TAG-CHANGES."
   (interactive
    (list (notmuch-read-tag-changes
 	  (notmuch-search-get-tags-region (point-min) (point-max)) "Tag all")))
-  (notmuch-tag notmuch-search-query-string tag-changes))
+  (notmuch-search-tag tag-changes (point-min) (point-max) t))
 
 (defun notmuch-search-buffer-title (query)
   "Returns the title for a buffer with notmuch search results."
@@ -951,7 +965,7 @@ the configured default sort order."
       (save-excursion
 	(let ((proc (notmuch-start-notmuch
 		     "notmuch-search" buffer #'notmuch-search-process-sentinel
-		     "search" "--format=sexp" "--format-version=1"
+		     "search" "--format=sexp" "--format-version=2"
 		     (if oldest-first
 			 "--sort=oldest-first"
 		       "--sort=newest-first")
diff --git a/test/emacs b/test/emacs
index 7503e96..3b3b14d 100755
--- a/test/emacs
+++ b/test/emacs
@@ -889,7 +889,7 @@ $PWD/notmuch_fail exited with status 1 (see *Notmuch errors* for more details)
 ---
 [XXX]
 $PWD/notmuch_fail exited with status 1
-command: $PWD/notmuch_fail search --format\=sexp --format-version\=1 --sort\=newest-first tag\:inbox
+command: $PWD/notmuch_fail search --format\=sexp --format-version\=2 --sort\=newest-first tag\:inbox
 exit status: 1"
 
 test_begin_subtest "Search handles subprocess warnings"
@@ -923,7 +923,6 @@ This is a warning
 This is another warning"
 
 test_begin_subtest "Search thread tag operations are race-free"
-test_subtest_known_broken
 add_message '[subject]="Search race test"'
 gen_msg_id_1=$gen_msg_id
 generate_message '[in-reply-to]="<'$gen_msg_id_1'>"' \
@@ -937,7 +936,6 @@ output=$(notmuch search --output=messages 'tag:search-thread-race-tag')
 test_expect_equal "$output" "id:$gen_msg_id_1"
 
 test_begin_subtest "Search global tag operations are race-free"
-test_subtest_known_broken
 generate_message '[in-reply-to]="<'$gen_msg_id_1'>"' \
 	    '[references]="<'$gen_msg_id_1'>"' \
 	    '[subject]="Re: Search race test"'
-- 
1.8.4.rc3

Re: [PATCH v2 00/11] Fix search tagging races

[Mark Walters]

On Thu, 24 Oct 2013, Austin Clements <amdragon@MIT.EDU> wrote:
> This is v2 of id:1381185201-25197-1-git-send-email-amdragon@mit.edu.
> It fixes several comments from Mark and Jani.  This has been rebased
> on top of the tag completion changes, so doing * from a large search
> buffer will no longer crash.  Hence, this series depends on the
> (currently pending) series in
> id:1382487721-31776-1-git-send-email-amdragon@mit.edu.

This looks good to me +1

Mark

>
> This version does not address what happens when you * on a search
> buffer that's still filling.  With this series, it will apply to all
> messages that have appeared when the user finishes entering tag
> changes.  This isn't ideal, but this seems pretty obscure and I'm not
> sure what the right answer is, so I'm punting it to the future.
>
> Another thing that may be worth changing in a follow-up is what
> messages * applies to.  Currently, * applies *only* to matched
> messages, not all threads in the search, which I think was an accident
> of implementation.  This series retains that behavior, but opens up
> the possibility of applying to all threads instead.  I think that
> would be much more consistent and less surprising behavior.
>
> An approximate diff from v1 is below.  This diff is prior to rebasing,
> since the post-rebase diff is not useful.
>
> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
> index a4eec14..36937fb 100644
> --- a/emacs/notmuch-tag.el
> +++ b/emacs/notmuch-tag.el
> @@ -242,7 +242,11 @@ from TAGS if present."
>  	   (error "Changed tag must be of the form `+this_tag' or `-that_tag'")))))
>      (sort result-tags 'string<)))
>  
> -(defconst notmuch-tag-argument-limit 1000)
> +(defconst notmuch-tag-argument-limit 1000
> +  "Use batch tagging if the tagging query is longer than this.
> +
> +This limits the length of arguments passed to the notmuch CLI to
> +avoid system argument length limits and performance problems.")
>  
>  (defun notmuch-tag (query &optional tag-changes)
>    "Add/remove tags in TAG-CHANGES to messages matching QUERY.
> @@ -276,7 +280,6 @@ notmuch-after-tag-hook will be run."
>        ;; Use batch tag mode to avoid argument length limitations
>        (let ((batch-op (concat (mapconcat #'notmuch-hex-encode tag-changes " ")
>  			      " -- " query)))
> -	(message "Batch tagging with %s" batch-op)
>  	(notmuch-call-notmuch-process :stdin-string batch-op "tag" "--batch")))
>      (run-hooks 'notmuch-after-tag-hook))
>    ;; in all cases we return tag-changes as a list
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 1b14910..8bc1a2a 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -144,7 +144,8 @@ chomp_newline (char *str)
>  #define NOTMUCH_FORMAT_MIN 1
>  /* The minimum non-deprecated structured output format version.
>   * Requests for format versions below this will print a stern warning.
> - * Must be >= NOTMUCH_FORMAT_MIN and < NOTMUCH_FORMAT_CUR.
> + * Must be between NOTMUCH_FORMAT_MIN and NOTMUCH_FORMAT_CUR,
> + * inclusive.
>   */
>  #define NOTMUCH_FORMAT_MIN_ACTIVE 1
>  
> diff --git a/notmuch-search.c b/notmuch-search.c
> index 1d14651..7c973b3 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -53,13 +53,13 @@ sanitize_string (const void *ctx, const char *str)
>   * NULL. */
>  static int
>  get_thread_query (notmuch_thread_t *thread,
> -		  char **matched_out, char **unmached_out)
> +		  char **matched_out, char **unmatched_out)
>  {
>      notmuch_messages_t *messages;
>      char *escaped = NULL;
>      size_t escaped_len = 0;
>  
> -    *matched_out = *unmached_out = NULL;
> +    *matched_out = *unmatched_out = NULL;
>  
>      for (messages = notmuch_thread_get_messages (thread);
>  	 notmuch_messages_valid (messages);
> @@ -69,17 +69,16 @@ get_thread_query (notmuch_thread_t *thread,
>  	const char *mid = notmuch_message_get_message_id (message);
>  	/* Determine which query buffer to extend */
>  	char **buf = notmuch_message_get_flag (
> -	    message, NOTMUCH_MESSAGE_FLAG_MATCH) ? matched_out : unmached_out;
> -	/* Allocate the query buffer is this is the first message */
> -	if (!*buf && (*buf = talloc_strdup (thread, "")) == NULL)
> -	    return -1;
> +	    message, NOTMUCH_MESSAGE_FLAG_MATCH) ? matched_out : unmatched_out;
>  	/* Add this message's id: query.  Since "id" is an exclusive
>  	 * prefix, it is implicitly 'or'd together, so we only need to
>  	 * join queries with a space. */
>  	if (make_boolean_term (thread, "id", mid, &escaped, &escaped_len) < 0)
>  	    return -1;
> -	*buf = talloc_asprintf_append_buffer (
> -	    *buf, "%s%s", **buf ? " " : "", escaped);
> +	if (*buf)
> +	    *buf = talloc_asprintf_append_buffer (*buf, " %s", escaped);
> +	else
> +	    *buf = talloc_strdup (thread, escaped);
>  	if (!*buf)
>  	    return -1;
>      }

Re: [PATCH v2 00/11] Fix search tagging races

[Tomi Ollila]

On Thu, Oct 24 2013, Austin Clements <amdragon@MIT.EDU> wrote:

> This is v2 of id:1381185201-25197-1-git-send-email-amdragon@mit.edu.
> It fixes several comments from Mark and Jani.  This has been rebased
> on top of the tag completion changes, so doing * from a large search
> buffer will no longer crash.  Hence, this series depends on the
> (currently pending) series in
> id:1382487721-31776-1-git-send-email-amdragon@mit.edu.
>
> This version does not address what happens when you * on a search
> buffer that's still filling.  With this series, it will apply to all
> messages that have appeared when the user finishes entering tag
> changes.  This isn't ideal, but this seems pretty obscure and I'm not
> sure what the right answer is, so I'm punting it to the future.
>
> Another thing that may be worth changing in a follow-up is what
> messages * applies to.  Currently, * applies *only* to matched
> messages, not all threads in the search, which I think was an accident
> of implementation.  This series retains that behavior, but opens up
> the possibility of applying to all threads instead.  I think that
> would be much more consistent and less surprising behavior.

LGTM. applies cleanly and tests pass

Tomi


>
> An approximate diff from v1 is below.  This diff is prior to rebasing,
> since the post-rebase diff is not useful.
>
> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
> index a4eec14..36937fb 100644
> --- a/emacs/notmuch-tag.el
> +++ b/emacs/notmuch-tag.el
> @@ -242,7 +242,11 @@ from TAGS if present."
>  	   (error "Changed tag must be of the form `+this_tag' or `-that_tag'")))))
>      (sort result-tags 'string<)))
>  
> -(defconst notmuch-tag-argument-limit 1000)
> +(defconst notmuch-tag-argument-limit 1000
> +  "Use batch tagging if the tagging query is longer than this.
> +
> +This limits the length of arguments passed to the notmuch CLI to
> +avoid system argument length limits and performance problems.")
>  
>  (defun notmuch-tag (query &optional tag-changes)
>    "Add/remove tags in TAG-CHANGES to messages matching QUERY.
> @@ -276,7 +280,6 @@ notmuch-after-tag-hook will be run."
>        ;; Use batch tag mode to avoid argument length limitations
>        (let ((batch-op (concat (mapconcat #'notmuch-hex-encode tag-changes " ")
>  			      " -- " query)))
> -	(message "Batch tagging with %s" batch-op)
>  	(notmuch-call-notmuch-process :stdin-string batch-op "tag" "--batch")))
>      (run-hooks 'notmuch-after-tag-hook))
>    ;; in all cases we return tag-changes as a list
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 1b14910..8bc1a2a 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -144,7 +144,8 @@ chomp_newline (char *str)
>  #define NOTMUCH_FORMAT_MIN 1
>  /* The minimum non-deprecated structured output format version.
>   * Requests for format versions below this will print a stern warning.
> - * Must be >= NOTMUCH_FORMAT_MIN and < NOTMUCH_FORMAT_CUR.
> + * Must be between NOTMUCH_FORMAT_MIN and NOTMUCH_FORMAT_CUR,
> + * inclusive.
>   */
>  #define NOTMUCH_FORMAT_MIN_ACTIVE 1
>  
> diff --git a/notmuch-search.c b/notmuch-search.c
> index 1d14651..7c973b3 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -53,13 +53,13 @@ sanitize_string (const void *ctx, const char *str)
>   * NULL. */
>  static int
>  get_thread_query (notmuch_thread_t *thread,
> -		  char **matched_out, char **unmached_out)
> +		  char **matched_out, char **unmatched_out)
>  {
>      notmuch_messages_t *messages;
>      char *escaped = NULL;
>      size_t escaped_len = 0;
>  
> -    *matched_out = *unmached_out = NULL;
> +    *matched_out = *unmatched_out = NULL;
>  
>      for (messages = notmuch_thread_get_messages (thread);
>  	 notmuch_messages_valid (messages);
> @@ -69,17 +69,16 @@ get_thread_query (notmuch_thread_t *thread,
>  	const char *mid = notmuch_message_get_message_id (message);
>  	/* Determine which query buffer to extend */
>  	char **buf = notmuch_message_get_flag (
> -	    message, NOTMUCH_MESSAGE_FLAG_MATCH) ? matched_out : unmached_out;
> -	/* Allocate the query buffer is this is the first message */
> -	if (!*buf && (*buf = talloc_strdup (thread, "")) == NULL)
> -	    return -1;
> +	    message, NOTMUCH_MESSAGE_FLAG_MATCH) ? matched_out : unmatched_out;
>  	/* Add this message's id: query.  Since "id" is an exclusive
>  	 * prefix, it is implicitly 'or'd together, so we only need to
>  	 * join queries with a space. */
>  	if (make_boolean_term (thread, "id", mid, &escaped, &escaped_len) < 0)
>  	    return -1;
> -	*buf = talloc_asprintf_append_buffer (
> -	    *buf, "%s%s", **buf ? " " : "", escaped);
> +	if (*buf)
> +	    *buf = talloc_asprintf_append_buffer (*buf, " %s", escaped);
> +	else
> +	    *buf = talloc_strdup (thread, escaped);
>  	if (!*buf)
>  	    return -1;
>      }
>
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 00/11] Fix search tagging races

[David Bremner]

Austin Clements <amdragon@MIT.EDU> writes:

> This is v2 of id:1381185201-25197-1-git-send-email-amdragon@mit.edu.
> It fixes several comments from Mark and Jani.  This has been rebased
> on top of the tag completion changes, so doing * from a large search
> buffer will no longer crash.  Hence, this series depends on the
> (currently pending) series in
> id:1382487721-31776-1-git-send-email-amdragon@mit.edu.
>

pushed.

d