ef/8761481f353d3728c3bd3f7f2b8636277d349e

   1 Return-Path: <amdragon@mit.edu>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 37F54429E45\r
   6         for <notmuch@notmuchmail.org>; Thu, 24 Oct 2013 08:19:50 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id Sa26Z-GJxz2p for <notmuch@notmuchmail.org>;\r
  16         Thu, 24 Oct 2013 08:19:43 -0700 (PDT)\r
  17 Received: from dmz-mailsec-scanner-5.mit.edu (dmz-mailsec-scanner-5.mit.edu\r
  18         [18.7.68.34])\r
  19         by olra.theworths.org (Postfix) with ESMTP id 323C4429E4E\r
  20         for <notmuch@notmuchmail.org>; Thu, 24 Oct 2013 08:19:25 -0700 (PDT)\r
  21 X-AuditID: 12074422-b7f5a8e000000a34-f4-52693a7cc45b\r
  22 Received: from mailhub-auth-3.mit.edu ( [18.9.21.43])\r
  23         by dmz-mailsec-scanner-5.mit.edu (Symantec Messaging Gateway) with SMTP\r
  24         id 6F.56.02612.C7A39625; Thu, 24 Oct 2013 11:19:24 -0400 (EDT)\r
  25 Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11])\r
  26         by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id r9OFJJha030444; \r
  27         Thu, 24 Oct 2013 11:19:20 -0400\r
  28 Received: from drake.dyndns.org\r
  29         (216-15-114-40.c3-0.arl-ubr1.sbo-arl.ma.cable.rcn.com\r
  30         [216.15.114.40]) (authenticated bits=0)\r
  31         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
  32         by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id r9OFJDDw012929\r
  33         (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT);\r
  34         Thu, 24 Oct 2013 11:19:19 -0400\r
  35 Received: from amthrax by drake.dyndns.org with local (Exim 4.77)\r
  36         (envelope-from <amdragon@mit.edu>)\r
  37         id 1VZMgq-0006eB-U8; Thu, 24 Oct 2013 11:19:12 -0400\r
  38 From: Austin Clements <amdragon@MIT.EDU>\r
  39 To: notmuch@notmuchmail.org\r
  40 Subject: [PATCH v2 11/11] emacs: Fix search tagging races\r
  41 Date: Thu, 24 Oct 2013 11:19:11 -0400\r
  42 Message-Id: <1382627951-25252-12-git-send-email-amdragon@mit.edu>\r
  43 X-Mailer: git-send-email 1.8.4.rc3\r
  44 In-Reply-To: <1382627951-25252-1-git-send-email-amdragon@mit.edu>\r
  45 References: <1382627951-25252-1-git-send-email-amdragon@mit.edu>\r
  46 X-Brightmail-Tracker:\r
  47  H4sIAAAAAAAAA+NgFjrGIsWRmVeSWpSXmKPExsUixCmqrVtjlRlkcOSztkXTdGeL1XN5LK7f\r
  48         nMnswOyxc9Zddo9b91+zezxbdYs5gDmKyyYlNSezLLVI3y6BK+PH1GamgouGFdM/trA1MPZr\r
  49         djFyckgImEgs3dDKCGGLSVy4t56ti5GLQ0hgH6PEulkP2CGcjYwSVzunMkI4d5gkpr7sYYZw\r
  50         5jJKnJ78igmkn01AQ2Lb/uVgs0QEpCV23p3NCmIzC0RLHLk8gw3EFhawlDjVs5EFxGYRUJXo\r
  51         3/+dGcTmFXCUOPXzFDPEHUoSC09tA+vlBIr/fH0NbKaQgIPEr99nWCYw8i9gZFjFKJuSW6Wb\r
  52         m5iZU5yarFucnJiXl1qka6qXm1mil5pSuokRFFzsLko7GH8eVDrEKMDBqMTDq/EhPUiINbGs\r
  53         uDL3EKMkB5OSKO8908wgIb6k/JTKjMTijPii0pzU4kOMEhzMSiK80/SAcrwpiZVVqUX5MClp\r
  54         DhYlcd5bHPZBQgLpiSWp2ampBalFMFkZDg4lCV5/S6BGwaLU9NSKtMycEoQ0EwcnyHAeoOHp\r
  55         IDW8xQWJucWZ6RD5U4yKUuK8SSAJAZBERmkeXC8s+l8xigO9IsxbDlLFA0wccN2vgAYzAQ2e\r
  56         siQNZHBJIkJKqoFRrfKAl6dM0HkmtqSpfd07TU4VVvy1iNghu1sijWnprqz628/eqycs3H3p\r
  57         jkacf7btT+6kv5aGd2ovWhittdpzX1mHuThs8414dd/rM5Y8uXZNbPe31aH+8199Xsy0SXqJ\r
  58         9k2eKIFtn0Kt36flPfaXUQmVkZ42Qa/+YaflN/YM6w2FMx3Wzb2uxFKckWioxVxUnAgA8FCt\r
  59         ktkCAAA=\r
  60 X-BeenThere: notmuch@notmuchmail.org\r
  61 X-Mailman-Version: 2.1.13\r
  62 Precedence: list\r
  63 List-Id: "Use and development of the notmuch mail system."\r
  64         <notmuch.notmuchmail.org>\r
  65 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  66         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  67 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  68 List-Post: <mailto:notmuch@notmuchmail.org>\r
  69 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  70 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  71         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  72 X-List-Received-Date: Thu, 24 Oct 2013 15:19:50 -0000\r
  73 \r
  74 This fixes races in thread-local and global tagging in notmuch-search\r
  75 (e.g., "+", "-", "a", "*", etc.).  Previously, these would modify tags\r
  76 of new messages that arrived after the search.  Now they only operate\r
  77 on the messages that were in the threads when the search was\r
  78 performed.  This prevents surprises like archiving messages that\r
  79 arrived in a thread after the search results were shown.\r
  80 \r
  81 This eliminates `notmuch-search-find-thread-id-region(-search)'\r
  82 because these functions strongly encouraged racy usage.\r
  83 \r
  84 This fixes the two broken tests added by the previous patch.\r
  85 ---\r
  86  devel/TODO       |  5 -----\r
  87  emacs/notmuch.el | 40 +++++++++++++++++++++++++++-------------\r
  88  test/emacs       |  4 +---\r
  89  3 files changed, 28 insertions(+), 21 deletions(-)\r
  90 \r
  91 diff --git a/devel/TODO b/devel/TODO\r
  92 index f212483..1cf4089 100644\r
  93 --- a/devel/TODO\r
  94 +++ b/devel/TODO\r
  95 @@ -14,11 +14,6 @@ sender's name containing ';' which causes emacs to drop a search\r
  96  result.) This may require removing the outer array from the current\r
  97  "notmuch search --format=json" results.\r
  98  \r
  99 -Fix '*' to work by simply calling '+' or '-' on a region consisting of\r
 100 -the entire buffer, (this would avoid one race condition---while still\r
 101 -leaving other race conditions---but could also potentially make '*' a\r
 102 -very expensive operation).\r
 103 -\r
 104  Add a global keybinding table for notmuch, and then view-specific\r
 105  tables that add to it.\r
 106         \r
 107 diff --git a/emacs/notmuch.el b/emacs/notmuch.el\r
 108 index 496c8ec..fea1a93 100644\r
 109 --- a/emacs/notmuch.el\r
 110 +++ b/emacs/notmuch.el\r
 111 @@ -485,14 +485,25 @@ If BARE is set then do not prefix with \"thread:\""\r
 112    (let ((thread (plist-get (notmuch-search-get-result) :thread)))\r
 113      (when thread (concat (unless bare "thread:") thread))))\r
 114  \r
 115 -(defun notmuch-search-find-thread-id-region (beg end)\r
 116 -  "Return a list of threads for the current region"\r
 117 -  (mapcar (lambda (thread) (concat "thread:" thread))\r
 118 -         (notmuch-search-properties-in-region :thread beg end)))\r
 119 -\r
 120 -(defun notmuch-search-find-thread-id-region-search (beg end)\r
 121 -  "Return a search string for threads for the current region"\r
 122 -  (mapconcat 'identity (notmuch-search-find-thread-id-region beg end) " or "))\r
 123 +(defun notmuch-search-find-stable-query ()\r
 124 +  "Return the stable queries for the current thread.\r
 125 +\r
 126 +This returns a list (MATCHED-QUERY UNMATCHED-QUERY) for the\r
 127 +matched and unmatched messages in the current thread."\r
 128 +  (plist-get (notmuch-search-get-result) :query))\r
 129 +\r
 130 +(defun notmuch-search-find-stable-query-region (beg end &optional only-matched)\r
 131 +  "Return the stable query for the current region.\r
 132 +\r
 133 +If ONLY-MATCHED is non-nil, include only matched messages.  If it\r
 134 +is nil, include both matched and unmatched messages."\r
 135 +  (let ((query-list nil) (all (not only-matched)))\r
 136 +    (dolist (queries (notmuch-search-properties-in-region :query beg end))\r
 137 +      (when (first queries)\r
 138 +       (push (first queries) query-list))\r
 139 +      (when (and all (second queries))\r
 140 +       (push (second queries) query-list)))\r
 141 +    (concat "(" (mapconcat 'identity query-list ") or (") ")")))\r
 142  \r
 143  (defun notmuch-search-find-authors ()\r
 144    "Return the authors for the current thread"\r
 145 @@ -569,17 +580,20 @@ Returns (TAG-CHANGES REGION-BEGIN REGION-END)."\r
 146            (notmuch-search-get-tags-region beg end) prompt initial-input)\r
 147           region)))\r
 148  \r
 149 -(defun notmuch-search-tag (tag-changes &optional beg end)\r
 150 +(defun notmuch-search-tag (tag-changes &optional beg end only-matched)\r
 151    "Change tags for the currently selected thread or region.\r
 152  \r
 153  See `notmuch-tag' for information on the format of TAG-CHANGES.\r
 154  When called interactively, this uses the region if the region is\r
 155  active.  When called directly, BEG and END provide the region.\r
 156  If these are nil or not provided, this applies to the thread at\r
 157 -point."\r
 158 +point.\r
 159 +\r
 160 +If ONLY-MATCHED is non-nil, only tag matched messages."\r
 161    (interactive (notmuch-search-interactive-tag-changes))\r
 162    (unless (and beg end) (setq beg (point) end (point)))\r
 163 -  (let ((search-string (notmuch-search-find-thread-id-region-search beg end)))\r
 164 +  (let ((search-string (notmuch-search-find-stable-query-region\r
 165 +                       beg end only-matched)))\r
 166      (notmuch-tag search-string tag-changes)\r
 167      (notmuch-search-foreach-result beg end\r
 168        (lambda (pos)\r
 169 @@ -847,7 +861,7 @@ See `notmuch-tag' for information on the format of TAG-CHANGES."\r
 170    (interactive\r
 171     (list (notmuch-read-tag-changes\r
 172           (notmuch-search-get-tags-region (point-min) (point-max)) "Tag all")))\r
 173 -  (notmuch-tag notmuch-search-query-string tag-changes))\r
 174 +  (notmuch-search-tag tag-changes (point-min) (point-max) t))\r
 175  \r
 176  (defun notmuch-search-buffer-title (query)\r
 177    "Returns the title for a buffer with notmuch search results."\r
 178 @@ -951,7 +965,7 @@ the configured default sort order."\r
 179        (save-excursion\r
 180         (let ((proc (notmuch-start-notmuch\r
 181                      "notmuch-search" buffer #'notmuch-search-process-sentinel\r
 182 -                    "search" "--format=sexp" "--format-version=1"\r
 183 +                    "search" "--format=sexp" "--format-version=2"\r
 184                      (if oldest-first\r
 185                          "--sort=oldest-first"\r
 186                        "--sort=newest-first")\r
 187 diff --git a/test/emacs b/test/emacs\r
 188 index 7503e96..3b3b14d 100755\r
 189 --- a/test/emacs\r
 190 +++ b/test/emacs\r
 191 @@ -889,7 +889,7 @@ $PWD/notmuch_fail exited with status 1 (see *Notmuch errors* for more details)\r
 192  ---\r
 193  [XXX]\r
 194  $PWD/notmuch_fail exited with status 1\r
 195 -command: $PWD/notmuch_fail search --format\=sexp --format-version\=1 --sort\=newest-first tag\:inbox\r
 196 +command: $PWD/notmuch_fail search --format\=sexp --format-version\=2 --sort\=newest-first tag\:inbox\r
 197  exit status: 1"\r
 198  \r
 199  test_begin_subtest "Search handles subprocess warnings"\r
 200 @@ -923,7 +923,6 @@ This is a warning\r
 201  This is another warning"\r
 202  \r
 203  test_begin_subtest "Search thread tag operations are race-free"\r
 204 -test_subtest_known_broken\r
 205  add_message '[subject]="Search race test"'\r
 206  gen_msg_id_1=$gen_msg_id\r
 207  generate_message '[in-reply-to]="<'$gen_msg_id_1'>"' \\r
 208 @@ -937,7 +936,6 @@ output=$(notmuch search --output=messages 'tag:search-thread-race-tag')\r
 209  test_expect_equal "$output" "id:$gen_msg_id_1"\r
 210  \r
 211  test_begin_subtest "Search global tag operations are race-free"\r
 212 -test_subtest_known_broken\r
 213  generate_message '[in-reply-to]="<'$gen_msg_id_1'>"' \\r
 214             '[references]="<'$gen_msg_id_1'>"' \\r
 215             '[subject]="Re: Search race test"'\r
 216 -- \r
 217 1.8.4.rc3\r
 218 \r