f5/0480549b08bcd12b33b93f94ecbe35d2627db3

   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 BCE61431FC7\r
   6         for <notmuch@notmuchmail.org>; Fri, 24 Jan 2014 13:18:32 -0800 (PST)\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 nqJ8vBnXO3Or for <notmuch@notmuchmail.org>;\r
  16         Fri, 24 Jan 2014 13:18:26 -0800 (PST)\r
  17 Received: from dmz-mailsec-scanner-2.mit.edu (dmz-mailsec-scanner-2.mit.edu\r
  18         [18.9.25.13])\r
  19         (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
  20         (No client certificate requested)\r
  21         by olra.theworths.org (Postfix) with ESMTPS id 2BB4F431FC3\r
  22         for <notmuch@notmuchmail.org>; Fri, 24 Jan 2014 13:18:26 -0800 (PST)\r
  23 X-AuditID: 1209190d-f79776d000000ce9-5a-52e2d8a1ecc6\r
  24 Received: from mailhub-auth-3.mit.edu ( [18.9.21.43])\r
  25         (using TLS with cipher AES256-SHA (256/256 bits))\r
  26         (Client did not present a certificate)\r
  27         by dmz-mailsec-scanner-2.mit.edu (Symantec Messaging Gateway) with SMTP\r
  28         id D5.86.03305.1A8D2E25; Fri, 24 Jan 2014 16:18:25 -0500 (EST)\r
  29 Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11])\r
  30         by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id s0OLIOWs026329; \r
  31         Fri, 24 Jan 2014 16:18:24 -0500\r
  32 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
  33         (authenticated bits=0)\r
  34         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
  35         by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id s0OLINGu020312\r
  36         (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);\r
  37         Fri, 24 Jan 2014 16:18:24 -0500\r
  38 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80)\r
  39         (envelope-from <amdragon@mit.edu>)\r
  40         id 1W6o8s-0006Dj-UL; Fri, 24 Jan 2014 16:18:22 -0500\r
  41 From: Austin Clements <amdragon@MIT.EDU>\r
  42 To: Jani Nikula <jani@nikula.org>, notmuch@notmuchmail.org\r
  43 Subject: Re: [PATCH 1/5] lib: make folder: prefix literal\r
  44 In-Reply-To:\r
  45  <d735583dd1bb48a87530fe3d52a57abd4d17acb8.1389304779.git.jani@nikula.org>\r
  46 References: <cover.1389304779.git.jani@nikula.org>\r
  47         <d735583dd1bb48a87530fe3d52a57abd4d17acb8.1389304779.git.jani@nikula.org>\r
  48 User-Agent: Notmuch/0.17~rc2+14~g06f47e0 (http://notmuchmail.org) Emacs/23.4.1\r
  49         (i486-pc-linux-gnu)\r
  50 Date: Fri, 24 Jan 2014 16:18:22 -0500\r
  51 Message-ID: <87wqhpm63l.fsf@awakening.csail.mit.edu>\r
  52 MIME-Version: 1.0\r
  53 Content-Type: text/plain; charset=us-ascii\r
  54 X-Brightmail-Tracker:\r
  55  H4sIAAAAAAAAA+NgFrrPIsWRmVeSWpSXmKPExsUixCmqrbvwxqMgg8c7+SyapjtbXL85k9mB\r
  56         yePW/dfsHs9W3WIOYIrisklJzcksSy3St0vgyng78xRjQU9ExanNP5gbGB+5dDFyckgImEj8\r
  57         mPKbDcIWk7hwbz2YLSQwm0mic1JAFyMXkL2RUeLE5cmsEM5pJokTF56wQzhLGCXurJsM1sIm\r
  58         oCGxbf9yRhBbRMBK4tzD1ywgtrCApcSOU0tYQWxOgTCJP3e7oVbUSOz8c5kZxBYVSJI4PfU3\r
  59         UA0HB4uAqkTLZQeQMC/QdUvfb2OCsAUlTs58AjaSWUBL4sa/l0wTGAVmIUnNQpJawMi0ilE2\r
  60         JbdKNzcxM6c4NVm3ODkxLy+1SNdILzezRC81pXQTIzgcJXl3ML47qHSIUYCDUYmH90PwwyAh\r
  61         1sSy4srcQ4ySHExKorxTLz8KEuJLyk+pzEgszogvKs1JLT7EKMHBrCTCO3czUI43JbGyKrUo\r
  62         HyYlzcGiJM57k8M+SEggPbEkNTs1tSC1CCYrw8GhJMH79RpQo2BRanpqRVpmTglCmomDE2Q4\r
  63         D9DwSSA1vMUFibnFmekQ+VOMilLivAtAEgIgiYzSPLheWLp4xSgO9Iowr+F1oCoeYKqB634F\r
  64         NJgJaPCKsw9ABpckIqSkGhjX7+IPKXtiFzmvWdu1oDkj8dl+k/CftZMvXp6h8MZQ+MIUb73q\r
  65         NF8jjwzBbR0PeB7d+BdiNvPetnqO81b1e99wr7p7Qnv/zb2aX3N5Xuk5ntzTfW/W/1vvBB4a\r
  66         yfFFPn99TqL90B5+zYMcupWe5Zwn1DoEbz09ZeeovfF4pY7gIcejh/a+vrxEiaU4I9FQi7mo\r
  67         OBEAkcueSvICAAA=\r
  68 X-BeenThere: notmuch@notmuchmail.org\r
  69 X-Mailman-Version: 2.1.13\r
  70 Precedence: list\r
  71 List-Id: "Use and development of the notmuch mail system."\r
  72         <notmuch.notmuchmail.org>\r
  73 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  74         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  75 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  76 List-Post: <mailto:notmuch@notmuchmail.org>\r
  77 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  78 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  79         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  80 X-List-Received-Date: Fri, 24 Jan 2014 21:18:32 -0000\r
  81 \r
  82 On Thu, 09 Jan 2014, Jani Nikula <jani@nikula.org> wrote:\r
  83 > In xapian terms, convert folder: prefix from probabilistic to boolean\r
  84 > prefix. This change constitutes a database change: bump the database\r
  85 > version and add database upgrade support.\r
  86 > ---\r
  87 >  lib/database.cc       |  39 ++++++++++++-\r
  88 >  lib/message.cc        | 154 +++++++++++++++++++++++++-------------------------\r
  89 >  lib/notmuch-private.h |   3 +\r
  90 >  3 files changed, 117 insertions(+), 79 deletions(-)\r
  91 >\r
  92 > diff --git a/lib/database.cc b/lib/database.cc\r
  93 > index f395061..145fd66 100644\r
  94 > --- a/lib/database.cc\r
  95 > +++ b/lib/database.cc\r
  96 > @@ -42,7 +42,7 @@ typedef struct {\r
  97 >      const char *prefix;\r
  98 >  } prefix_t;\r
  99 >  \r
 100 > -#define NOTMUCH_DATABASE_VERSION 1\r
 101 > +#define NOTMUCH_DATABASE_VERSION 2\r
 102 >  \r
 103 >  #define STRINGIFY(s) _SUB_STRINGIFY(s)\r
 104 >  #define _SUB_STRINGIFY(s) #s\r
 105 > @@ -208,7 +208,8 @@ static prefix_t BOOLEAN_PREFIX_EXTERNAL[] = {\r
 106 >      { "thread",                      "G" },\r
 107 >      { "tag",                 "K" },\r
 108 >      { "is",                  "K" },\r
 109 > -    { "id",                  "Q" }\r
 110 > +    { "id",                  "Q" },\r
 111 > +    { "folder",                      "P" },\r
 112 >  };\r
 113 >  \r
 114 >  static prefix_t PROBABILISTIC_PREFIX[]= {\r
 115 > @@ -216,7 +217,6 @@ static prefix_t PROBABILISTIC_PREFIX[]= {\r
 116 >      { "to",                  "XTO" },\r
 117 >      { "attachment",          "XATTACHMENT" },\r
 118 >      { "subject",             "XSUBJECT"},\r
 119 > -    { "folder",                      "XFOLDER"}\r
 120 >  };\r
 121 >  \r
 122 >  const char *\r
 123 > @@ -1167,6 +1167,39 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,\r
 124 >       }\r
 125 >      }\r
 126 >  \r
 127 > +    /*\r
 128 > +     * Prior to version 2, the "folder:" prefix was probabilistic and\r
 129 > +     * stemmed. Change it to the current boolean prefix.\r
 130 > +     */\r
 131 > +    if (version < 2) {\r
 132 > +     notmuch_query_t *query = notmuch_query_create (notmuch, "");\r
 133 > +     notmuch_messages_t *messages;\r
 134 > +     notmuch_message_t *message;\r
 135 > +\r
 136 > +     count = 0;\r
 137 > +     total = notmuch_query_count_messages (query);\r
 138 > +\r
 139 > +     for (messages = notmuch_query_search_messages (query);\r
 140 > +          notmuch_messages_valid (messages);\r
 141 > +          notmuch_messages_move_to_next (messages)) {\r
 142 > +         if (do_progress_notify) {\r
 143 > +             progress_notify (closure, (double) count / total);\r
 144 > +             do_progress_notify = 0;\r
 145 > +         }\r
 146 > +\r
 147 > +         message = notmuch_messages_get (messages);\r
 148 > +\r
 149 > +         _notmuch_message_upgrade_folder (message);\r
 150 > +         _notmuch_message_sync (message);\r
 151 > +\r
 152 > +         notmuch_message_destroy (message);\r
 153 > +\r
 154 > +         count++;\r
 155 > +     }\r
 156 > +\r
 157 > +     notmuch_query_destroy (query);\r
 158 > +    }\r
 159 > +\r
 160 \r
 161 Unless I'm missing something, the upgrade isn't done atomically.  Should\r
 162 it be?  (The answer may be "no", since this process appears to be\r
 163 idempotent; though it may simply be easier to reason about if it is\r
 164 atomic.)\r
 165 \r
 166 Everything else in this patch LGTM (modulo my complaint about stripping\r
 167 cur/new).\r
 168 \r
 169 >      db->set_metadata ("version", STRINGIFY (NOTMUCH_DATABASE_VERSION));\r
 170 >      db->flush ();\r
 171 >  \r
 172 > diff --git a/lib/message.cc b/lib/message.cc\r
 173 > index 1b46379..500aa26 100644\r
 174 > --- a/lib/message.cc\r
 175 > +++ b/lib/message.cc\r
 176 > @@ -505,89 +505,27 @@ _notmuch_message_add_filename (notmuch_message_t *message,\r
 177 >      _notmuch_message_add_term (message, "file-direntry", direntry);\r
 178 >  \r
 179 >      /* New terms allow user to search with folder: specification. */\r
 180 > -    _notmuch_message_gen_terms (message, "folder", directory);\r
 181 > +    _notmuch_message_add_term (message, "folder", directory);\r
 182 >  \r
 183 >      talloc_free (local);\r
 184 >  \r
 185 >      return NOTMUCH_STATUS_SUCCESS;\r
 186 >  }\r
 187 >  \r
 188 > -/* Remove a particular 'filename' from 'message'.\r
 189 > - *\r
 190 > - * This change will not be reflected in the database until the next\r
 191 > - * call to _notmuch_message_sync.\r
 192 > - *\r
 193 > - * If this message still has other filenames, returns\r
 194 > - * NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID.\r
 195 > - *\r
 196 > - * Note: This function does not remove a document from the database,\r
 197 > - * even if the specified filename is the only filename for this\r
 198 > - * message. For that functionality, see\r
 199 > - * _notmuch_database_remove_message. */\r
 200 > -notmuch_status_t\r
 201 > -_notmuch_message_remove_filename (notmuch_message_t *message,\r
 202 > -                               const char *filename)\r
 203 > +static void\r
 204 > +_notmuch_message_remove_terms (notmuch_message_t *message, const char *prefix)\r
 205 >  {\r
 206 > -    const char *direntry_prefix = _find_prefix ("file-direntry");\r
 207 > -    int direntry_prefix_len = strlen (direntry_prefix);\r
 208 > -    const char *folder_prefix = _find_prefix ("folder");\r
 209 > -    int folder_prefix_len = strlen (folder_prefix);\r
 210 > -    void *local = talloc_new (message);\r
 211 > -    char *zfolder_prefix = talloc_asprintf(local, "Z%s", folder_prefix);\r
 212 > -    int zfolder_prefix_len = strlen (zfolder_prefix);\r
 213 > -    char *direntry;\r
 214 > -    notmuch_private_status_t private_status;\r
 215 > -    notmuch_status_t status;\r
 216 > -    Xapian::TermIterator i, last;\r
 217 > -\r
 218 > -    status = _notmuch_database_filename_to_direntry (\r
 219 > -     local, message->notmuch, filename, NOTMUCH_FIND_LOOKUP, &direntry);\r
 220 > -    if (status || !direntry)\r
 221 > -     return status;\r
 222 > +    Xapian::TermIterator i;\r
 223 > +    size_t prefix_len = strlen (prefix);\r
 224 >  \r
 225 > -    /* Unlink this file from its parent directory. */\r
 226 > -    private_status = _notmuch_message_remove_term (message,\r
 227 > -                                                "file-direntry", direntry);\r
 228 > -    status = COERCE_STATUS (private_status,\r
 229 > -                         "Unexpected error from _notmuch_message_remove_term");\r
 230 > -    if (status)\r
 231 > -     return status;\r
 232 > -\r
 233 > -    /* Re-synchronize "folder:" terms for this message. This requires:\r
 234 > -     *  1. removing all "folder:" terms\r
 235 > -     *  2. removing all "folder:" stemmed terms\r
 236 > -     *  3. adding back terms for all remaining filenames of the message. */\r
 237 > -\r
 238 > -    /* 1. removing all "folder:" terms */\r
 239 >      while (1) {\r
 240 >       i = message->doc.termlist_begin ();\r
 241 > -     i.skip_to (folder_prefix);\r
 242 > +     i.skip_to (prefix);\r
 243 >  \r
 244 >       /* Terminate loop when no terms remain with desired prefix. */\r
 245 >       if (i == message->doc.termlist_end () ||\r
 246 > -         strncmp ((*i).c_str (), folder_prefix, folder_prefix_len))\r
 247 > -     {\r
 248 > +         strncmp ((*i).c_str (), prefix, prefix_len))\r
 249 >           break;\r
 250 > -     }\r
 251 > -\r
 252 > -     try {\r
 253 > -         message->doc.remove_term ((*i));\r
 254 > -     } catch (const Xapian::InvalidArgumentError) {\r
 255 > -         /* Ignore failure to remove non-existent term. */\r
 256 > -     }\r
 257 > -    }\r
 258 > -\r
 259 > -    /* 2. removing all "folder:" stemmed terms */\r
 260 > -    while (1) {\r
 261 > -     i = message->doc.termlist_begin ();\r
 262 > -     i.skip_to (zfolder_prefix);\r
 263 > -\r
 264 > -     /* Terminate loop when no terms remain with desired prefix. */\r
 265 > -     if (i == message->doc.termlist_end () ||\r
 266 > -         strncmp ((*i).c_str (), zfolder_prefix, zfolder_prefix_len))\r
 267 > -     {\r
 268 > -         break;\r
 269 > -     }\r
 270 >  \r
 271 >       try {\r
 272 >           message->doc.remove_term ((*i));\r
 273 > @@ -595,12 +533,18 @@ _notmuch_message_remove_filename (notmuch_message_t *message,\r
 274 >           /* Ignore failure to remove non-existent term. */\r
 275 >       }\r
 276 >      }\r
 277 > +}\r
 278 >  \r
 279 > -    /* 3. adding back terms for all remaining filenames of the message. */\r
 280 > -    i = message->doc.termlist_begin ();\r
 281 > -    i.skip_to (direntry_prefix);\r
 282 > +/* Add "folder:" terms for all filenames of the message. */\r
 283 > +static notmuch_status_t\r
 284 > +_notmuch_message_add_folder_terms (void *ctx, notmuch_message_t *message)\r
 285 > +{\r
 286 > +    const char *direntry_prefix = _find_prefix ("file-direntry");\r
 287 > +    int direntry_prefix_len = strlen (direntry_prefix);\r
 288 > +    Xapian::TermIterator i = message->doc.termlist_begin ();\r
 289 > +    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;\r
 290 >  \r
 291 > -    for (; i != message->doc.termlist_end (); i++) {\r
 292 > +    for (i.skip_to (direntry_prefix); i != message->doc.termlist_end (); i++) {\r
 293 >       unsigned int directory_id;\r
 294 >       const char *direntry, *directory;\r
 295 >       char *colon;\r
 296 > @@ -620,18 +564,76 @@ _notmuch_message_remove_filename (notmuch_message_t *message,\r
 297 >       if (colon == NULL || *colon != ':')\r
 298 >           INTERNAL_ERROR ("malformed direntry");\r
 299 >  \r
 300 > -     directory = _notmuch_database_get_directory_path (local,\r
 301 > +     directory = _notmuch_database_get_directory_path (ctx,\r
 302 >                                                         message->notmuch,\r
 303 >                                                         directory_id);\r
 304 > -     if (strlen (directory))\r
 305 > -         _notmuch_message_gen_terms (message, "folder", directory);\r
 306 > +     _notmuch_message_add_term (message, "folder", directory);\r
 307 >      }\r
 308 >  \r
 309 > +    return status;\r
 310 > +}\r
 311 > +\r
 312 > +/* Remove a particular 'filename' from 'message'.\r
 313 > + *\r
 314 > + * This change will not be reflected in the database until the next\r
 315 > + * call to _notmuch_message_sync.\r
 316 > + *\r
 317 > + * If this message still has other filenames, returns\r
 318 > + * NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID.\r
 319 > + *\r
 320 > + * Note: This function does not remove a document from the database,\r
 321 > + * even if the specified filename is the only filename for this\r
 322 > + * message. For that functionality, see\r
 323 > + * _notmuch_database_remove_message. */\r
 324 > +notmuch_status_t\r
 325 > +_notmuch_message_remove_filename (notmuch_message_t *message,\r
 326 > +                               const char *filename)\r
 327 > +{\r
 328 > +    void *local = talloc_new (message);\r
 329 > +    char *direntry;\r
 330 > +    notmuch_private_status_t private_status;\r
 331 > +    notmuch_status_t status;\r
 332 > +\r
 333 > +    status = _notmuch_database_filename_to_direntry (\r
 334 > +     local, message->notmuch, filename, NOTMUCH_FIND_LOOKUP, &direntry);\r
 335 > +    if (status || !direntry)\r
 336 > +     return status;\r
 337 > +\r
 338 > +    /* Unlink this file from its parent directory. */\r
 339 > +    private_status = _notmuch_message_remove_term (message,\r
 340 > +                                                "file-direntry", direntry);\r
 341 > +    status = COERCE_STATUS (private_status,\r
 342 > +                         "Unexpected error from _notmuch_message_remove_term");\r
 343 > +    if (status)\r
 344 > +     return status;\r
 345 > +\r
 346 > +    /* Remove all "folder:" terms from the message. */\r
 347 > +    _notmuch_message_remove_terms (message, _find_prefix ("folder"));\r
 348 > +\r
 349 > +    /* Add back "folder:" terms for all remaining filenames of the message. */\r
 350 > +    status = _notmuch_message_add_folder_terms (local, message);\r
 351 > +\r
 352 >      talloc_free (local);\r
 353 >  \r
 354 >      return status;\r
 355 >  }\r
 356 >  \r
 357 > +/* Upgrade the "folder:" prefix from V1 to V2. */\r
 358 > +#define FOLDER_PREFIX_V1     "XFOLDER"\r
 359 > +#define ZFOLDER_PREFIX_V1    "Z" FOLDER_PREFIX_V1\r
 360 > +void\r
 361 > +_notmuch_message_upgrade_folder (notmuch_message_t *message)\r
 362 > +{\r
 363 > +    /* Remove all old "folder:" terms. */\r
 364 > +    _notmuch_message_remove_terms (message, FOLDER_PREFIX_V1);\r
 365 > +\r
 366 > +    /* Remove all old "folder:" stemmed terms. */\r
 367 > +    _notmuch_message_remove_terms (message, ZFOLDER_PREFIX_V1);\r
 368 > +\r
 369 > +    /* Add new boolean "folder:" terms. */\r
 370 > +    _notmuch_message_add_folder_terms (message, message);\r
 371 > +}\r
 372 > +\r
 373 >  char *\r
 374 >  _notmuch_message_talloc_copy_data (notmuch_message_t *message)\r
 375 >  {\r
 376 > diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h\r
 377 > index af185c7..59eb2bc 100644\r
 378 > --- a/lib/notmuch-private.h\r
 379 > +++ b/lib/notmuch-private.h\r
 380 > @@ -263,6 +263,9 @@ _notmuch_message_gen_terms (notmuch_message_t *message,\r
 381 >  void\r
 382 >  _notmuch_message_upgrade_filename_storage (notmuch_message_t *message);\r
 383 >  \r
 384 > +void\r
 385 > +_notmuch_message_upgrade_folder (notmuch_message_t *message);\r
 386 > +\r
 387 >  notmuch_status_t\r
 388 >  _notmuch_message_add_filename (notmuch_message_t *message,\r
 389 >                              const char *filename);\r
 390 > -- \r
 391 > 1.8.5.2\r
 392 >\r
 393 > _______________________________________________\r
 394 > notmuch mailing list\r
 395 > notmuch@notmuchmail.org\r
 396 > http://notmuchmail.org/mailman/listinfo/notmuch\r