91/f87fd49a2f1c0c4102d9cc516cc59bcce7f29c

   1 Return-Path: <jani@nikula.org>\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 5E878431FC2\r
   6         for <notmuch@notmuchmail.org>; Wed,  1 Jan 2014 04:05:14 -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 Ig-jlEt3qmPX for <notmuch@notmuchmail.org>;\r
  16         Wed,  1 Jan 2014 04:05:04 -0800 (PST)\r
  17 Received: from mail-ea0-f176.google.com (mail-ea0-f176.google.com\r
  18         [209.85.215.176]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
  19         (No client certificate requested)\r
  20         by olra.theworths.org (Postfix) with ESMTPS id 88C6F431FC0\r
  21         for <notmuch@notmuchmail.org>; Wed,  1 Jan 2014 04:05:04 -0800 (PST)\r
  22 Received: by mail-ea0-f176.google.com with SMTP id h14so5812179eaj.35\r
  23         for <notmuch@notmuchmail.org>; Wed, 01 Jan 2014 04:05:03 -0800 (PST)\r
  24 X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;\r
  25         d=1e100.net; s=20130820;\r
  26         h=x-gm-message-state:from:to:cc:subject:in-reply-to:references\r
  27         :user-agent:date:message-id:mime-version:content-type\r
  28         :content-transfer-encoding;\r
  29         bh=yfCvxMG62xd9D+0aOngvooYflgtc9V7expqjW04XtAM=;\r
  30         b=lJwLjMZlEUgVIMmUmYca9nz2y/hu8IlcHL0md+cawt99xQdUSFnpgaUzjuSmcWS9Th\r
  31         cNajHWdwuD2bPqg0IWn/9mpA5LfgWNRiYCQSMjPH5/UUY8K7WUsAoMNG1cBuxVwRcSwF\r
  32         k7nNgCx4+v8D0VtTp9Ep5n3KVLPdRCPj9td+9arqtYuce3AXhAIft3g6qpScm+Ea0bCV\r
  33         jTBSzqhU8ctFDPHS7YpKi1xmQ9KU63ze+1QORgM3HyuTEU761nKUj9Pn4pyQ1mtxsbs2\r
  34         hQ+4+lNJKIVghmbnbVqF4rJowBSrJOEnxXi7P2rW/oh1nne+MmNxGI8uYsYTMyKbEaO7\r
  35         vfUg==\r
  36 X-Gm-Message-State:\r
  37  ALoCoQnS9e9hvjCtjKKOGiLxrL1Pc9fdvaOc/0pVvg2haWgkI0QC8bFDGdw8M6s+bLbkPmKUJ0Lq\r
  38 X-Received: by 10.14.69.200 with SMTP id n48mr10888063eed.54.1388577901865;\r
  39         Wed, 01 Jan 2014 04:05:01 -0800 (PST)\r
  40 Received: from localhost (dsl-hkibrasgw2-58c36f-91.dhcp.inet.fi.\r
  41         [88.195.111.91])\r
  42         by mx.google.com with ESMTPSA id m1sm126542611eeg.0.2014.01.01.04.04.59\r
  43         for <multiple recipients>\r
  44         (version=TLSv1.2 cipher=RC4-SHA bits=128/128);\r
  45         Wed, 01 Jan 2014 04:05:00 -0800 (PST)\r
  46 From: Jani Nikula <jani@nikula.org>\r
  47 To: "Kirill A. Shutemov" <kirill@shutemov.name>\r
  48 Subject: Re: [PATCH] lib: Add a new prefix "list" to the search-terms syntax\r
  49 In-Reply-To: <20131217180322.GA9272@node.dhcp.inet.fi>\r
  50 References: <20130409083010.GA27675@raorn.name>\r
  51         <1365549369-12776-1-git-send-email-raorn@raorn.name>\r
  52         <87bo2ougmb.fsf@nikula.org>\r
  53         <20131217180322.GA9272@node.dhcp.inet.fi>\r
  54 User-Agent: Notmuch/0.17~rc2+18~g39a67a6 (http://notmuchmail.org) Emacs/24.3.1\r
  55         (x86_64-pc-linux-gnu)\r
  56 Date: Wed, 01 Jan 2014 14:04:58 +0200\r
  57 Message-ID: <87y52z29hx.fsf@nikula.org>\r
  58 MIME-Version: 1.0\r
  59 Content-Type: text/plain; charset=utf-8\r
  60 Content-Transfer-Encoding: quoted-printable\r
  61 Cc: notmuch@notmuchmail.org, "Alexey I. Froloff" <raorn@raorn.name>\r
  62 X-BeenThere: notmuch@notmuchmail.org\r
  63 X-Mailman-Version: 2.1.13\r
  64 Precedence: list\r
  65 List-Id: "Use and development of the notmuch mail system."\r
  66         <notmuch.notmuchmail.org>\r
  67 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  68         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  69 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  70 List-Post: <mailto:notmuch@notmuchmail.org>\r
  71 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  72 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  73         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  74 X-List-Received-Date: Wed, 01 Jan 2014 12:05:14 -0000\r
  75 \r
  76 On Tue, 17 Dec 2013, "Kirill A. Shutemov" <kirill@shutemov.name> wrote:\r
  77 > On Thu, Oct 17, 2013 at 05:17:00PM +0300, Jani Nikula wrote:\r
  78 >> On Wed, 10 Apr 2013, "Alexey I. Froloff" <raorn@raorn.name> wrote:\r
  79 >> > From: "Alexey I. Froloff" <raorn@raorn.name>\r
  80 >> >\r
  81 >> > Add support for indexing and searching the message's List-Id header.\r
  82 >> > This is useful when matching all the messages belonging to a particular\r
  83 >> > mailing list.\r
  84 >>=20\r
  85 >> There's an issue with our duplicate message-id handling that is likely\r
  86 >> to cause confusion with List-Id: searches. If you receive several\r
  87 >> duplicates of the same message (judged by the message-id), only the\r
  88 >> first one of them gets indexed, and the rest are ignored. This means\r
  89 >> that for messages you receive both directly and through a list, it will\r
  90 >> be arbitrary whether the List-Id: gets indexed or not. Therefore a list:\r
  91 >> search might not return all the messages you'd expect.\r
  92 >\r
  93 > I've tried to address this. The patch also adds few tests for the feature.\r
  94 >\r
  95 > There's still missing functionality: re-indexing existing messages for\r
  96 > list-id, handling message removal, etc.\r
  97 >\r
  98 > Any comments?\r
  99 \r
 100 Hi Kirill, sorry it took me so long to get to this!\r
 101 \r
 102 I've looked into our duplicate message-id handling and indexing before,\r
 103 and it's not very good.\r
 104 \r
 105 First, we should pay more attention to checking whether the messages\r
 106 really are duplicates or not. This is not trivial, but we should go a\r
 107 bit further than just comparing the message-ids. Sadly, handling the\r
 108 case of colliding message-ids on clearly different messages is not\r
 109 trivial either, as we rely on the message-ids being unique all around.\r
 110 \r
 111 Second, we should be more clever about indexing duplicates that we think\r
 112 are the same message. This is orthogonal to the first point. Currently,\r
 113 only the first duplicate gets indexed, and will remain indexed even if\r
 114 it's deleted and other copies remain. A message that matches a search\r
 115 might end up not having the matching search terms, for example. A\r
 116 rebuild of the database might index a different duplicate from the last\r
 117 time.\r
 118 \r
 119 Having said that (partially just to write the thoughts down somewhere!),\r
 120 I think your basic approach of indexing the list-id for duplicates is\r
 121 sane, and we can grow more smarts to _notmuch_message_index_file() for\r
 122 duplicate =3D=3D true in the future, checking more headers etc. One thing I\r
 123 wonder about though: what if more than one duplicate has list-id, and\r
 124 _index_list_id() gets called multiple times on a message? (CC Austin, he\r
 125 probably has more clues on this than me.)\r
 126 \r
 127 For merging, you should also address the previous comments to the\r
 128 original patch. There's been plenty of dropping the ball here it\r
 129 seems... I think we've also agreed (perhaps only on IRC, I forget) that\r
 130 we should use "listid" as the prefix, not "list" (sadly hyphens are not\r
 131 allowed). Splitting the patch to code, test, and man parts might be a\r
 132 good idea too.\r
 133 \r
 134 BR,\r
 135 Jani.\r
 136 \r
 137 \r
 138 >\r
 139 > diff --git a/lib/database.cc b/lib/database.cc\r
 140 > index f395061e3a73..196243e15d1a 100644\r
 141 > --- a/lib/database.cc\r
 142 > +++ b/lib/database.cc\r
 143 > @@ -205,6 +205,7 @@ static prefix_t BOOLEAN_PREFIX_INTERNAL[] =3D {\r
 144 >  };\r
 145 >=20=20\r
 146 >  static prefix_t BOOLEAN_PREFIX_EXTERNAL[] =3D {\r
 147 > +    { "list",                        "XLIST"},\r
 148 >      { "thread",                      "G" },\r
 149 >      { "tag",                 "K" },\r
 150 >      { "is",                  "K" },\r
 151 > @@ -2025,10 +2026,13 @@ notmuch_database_add_message (notmuch_database_t =\r
 152 *notmuch,\r
 153 >           date =3D notmuch_message_file_get_header (message_file, "date");\r
 154 >           _notmuch_message_set_header_values (message, date, from, subject);\r
 155 >=20=20\r
 156 > -         ret =3D _notmuch_message_index_file (message, filename);\r
 157 > +         ret =3D _notmuch_message_index_file (message, filename, false);\r
 158 >           if (ret)\r
 159 >               goto DONE;\r
 160 >       } else {\r
 161 > +         ret =3D _notmuch_message_index_file (message, filename, true);\r
 162 > +         if (ret)\r
 163 > +             goto DONE;\r
 164 >           ret =3D NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;\r
 165 >       }\r
 166 >=20=20\r
 167 > diff --git a/lib/index.cc b/lib/index.cc\r
 168 > index 78c18cf36d10..9fe1ad6502ed 100644\r
 169 > --- a/lib/index.cc\r
 170 > +++ b/lib/index.cc\r
 171 > @@ -304,6 +304,47 @@ _index_address_list (notmuch_message_t *message,\r
 172 >      }\r
 173 >  }\r
 174 >=20=20\r
 175 > +static void\r
 176 > +_index_list_id (notmuch_message_t *message,\r
 177 > +               const char *list_id_header)\r
 178 > +{\r
 179 > +    const char *begin_list_id, *end_list_id, *list_id;\r
 180 > +    void *local;\r
 181 > +\r
 182 > +    if (list_id_header =3D=3D NULL)\r
 183 > +     return;\r
 184 > +\r
 185 > +    /* RFC2919 says that the list-id is found at the end of the header\r
 186 > +     * and enclosed between angle brackets. If we cannot find a\r
 187 > +     * matching pair of brackets containing at least one character,\r
 188 > +     * we ignore the list id header. */\r
 189 > +    begin_list_id =3D strrchr (list_id_header, '<');\r
 190 > +    if (!begin_list_id) {\r
 191 > +     fprintf (stderr, "Warning: Not indexing mailformed List-Id tag.\n");\r
 192 > +     return;\r
 193 > +    }\r
 194 > +\r
 195 > +    end_list_id =3D strrchr(begin_list_id, '>');\r
 196 > +    if (!end_list_id || (end_list_id - begin_list_id < 2)) {\r
 197 > +     fprintf (stderr, "Warning: Not indexing mailformed List-Id tag.\n");\r
 198 > +     return;\r
 199 > +    }\r
 200 > +\r
 201 > +    local =3D talloc_new (message);\r
 202 > +\r
 203 > +    /* We extract the list id between the angle brackets */\r
 204 > +    list_id =3D talloc_strndup (local, begin_list_id + 1,\r
 205 > +                           end_list_id - begin_list_id - 1);\r
 206 > +\r
 207 > +    /* _notmuch_message_add_term() may return\r
 208 > +     * NOTMUCH_PRIVATE_STATUS_TERM_TOO_LONG here.  We can't fix it, but\r
 209 > +     * this is not a reason to exit with error... */\r
 210 > +    if (_notmuch_message_add_term (message, "list", list_id))\r
 211 > +     fprintf (stderr, "Warning: Not indexing List-Id: <%s>\n", list_id);\r
 212 > +\r
 213 > +    talloc_free (local);\r
 214 > +}\r
 215 > +\r
 216 >  /* Callback to generate terms for each mime part of a message. */\r
 217 >  static void\r
 218 >  _index_mime_part (notmuch_message_t *message,\r
 219 > @@ -425,14 +466,15 @@ _index_mime_part (notmuch_message_t *message,\r
 220 >=20=20\r
 221 >  notmuch_status_t\r
 222 >  _notmuch_message_index_file (notmuch_message_t *message,\r
 223 > -                          const char *filename)\r
 224 > +                          const char *filename,\r
 225 > +                          notmuch_bool_t duplicate)\r
 226 >  {\r
 227 >      GMimeStream *stream =3D NULL;\r
 228 >      GMimeParser *parser =3D NULL;\r
 229 >      GMimeMessage *mime_message =3D NULL;\r
 230 >      InternetAddressList *addresses;\r
 231 >      FILE *file =3D NULL;\r
 232 > -    const char *from, *subject;\r
 233 > +    const char *from, *subject, *list_id;\r
 234 >      notmuch_status_t ret =3D NOTMUCH_STATUS_SUCCESS;\r
 235 >      static int initialized =3D 0;\r
 236 >      char from_buf[5];\r
 237 > @@ -485,6 +527,9 @@ mboxes is deprecated and may be removed in the future=\r
 238 .\n", filename);\r
 239 >=20=20\r
 240 >      from =3D g_mime_message_get_sender (mime_message);\r
 241 >=20=20\r
 242 > +    if (duplicate)\r
 243 > +     goto DUP;\r
 244 > +\r
 245 >      addresses =3D internet_address_list_parse_string (from);\r
 246 >      if (addresses) {\r
 247 >       _index_address_list (message, "from", addresses);\r
 248 > @@ -502,6 +547,10 @@ mboxes is deprecated and may be removed in the futur=\r
 249 e.\n", filename);\r
 250 >=20=20\r
 251 >      _index_mime_part (message, g_mime_message_get_mime_part (mime_messag=\r
 252 e));\r
 253 >=20=20\r
 254 > +  DUP:\r
 255 > +    list_id =3D g_mime_object_get_header (GMIME_OBJECT (mime_message), "=\r
 256 List-Id");\r
 257 > +    _index_list_id (message, list_id);\r
 258 > +\r
 259 >    DONE:\r
 260 >      if (mime_message)\r
 261 >       g_object_unref (mime_message);\r
 262 > diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h\r
 263 > index af185c7c5ba8..138dfa58efc8 100644\r
 264 > --- a/lib/notmuch-private.h\r
 265 > +++ b/lib/notmuch-private.h\r
 266 > @@ -322,7 +322,8 @@ notmuch_message_get_author (notmuch_message_t *messag=\r
 267 e);\r
 268 >=20=20\r
 269 >  notmuch_status_t\r
 270 >  _notmuch_message_index_file (notmuch_message_t *message,\r
 271 > -                          const char *filename);\r
 272 > +                          const char *filename,\r
 273 > +                          notmuch_bool_t duplicate);\r
 274 >=20=20\r
 275 >  /* message-file.c */\r
 276 >=20=20\r
 277 > diff --git a/man/man7/notmuch-search-terms.7 b/man/man7/notmuch-search-te=\r
 278 rms.7\r
 279 > index f1627b3488f8..29b30b7b0b00 100644\r
 280 > --- a/man/man7/notmuch-search-terms.7\r
 281 > +++ b/man/man7/notmuch-search-terms.7\r
 282 > @@ -52,6 +52,8 @@ terms to match against specific portions of an email, (=\r
 283 where\r
 284 >=20=20\r
 285 >       thread:<thread-id>\r
 286 >=20=20\r
 287 > +     list:<list-id>\r
 288 > +\r
 289 >       folder:<directory-path>\r
 290 >=20=20\r
 291 >       date:<since>..<until>\r
 292 > @@ -109,6 +111,12 @@ within a matching directory. Only the directory comp=\r
 293 onents below the\r
 294 >  top-level mail database path are available to be searched.\r
 295 >=20=20\r
 296 >  The\r
 297 > +.BR list: ,\r
 298 > +is used to match mailing list ID of an email message \- contents of the\r
 299 > +List\-Id: header without the '<', '>' delimiters or decoded list\r
 300 > +description.\r
 301 > +\r
 302 > +The\r
 303 >  .B date:\r
 304 >  prefix can be used to restrict the results to only messages within a\r
 305 >  particular time range (based on the Date: header) with a range syntax\r
 306 > diff --git a/test/corpus/cur/18:2, b/test/corpus/cur/18:2,\r
 307 > index f522f69eb933..2b54925bd5d1 100644\r
 308 > --- a/test/corpus/cur/18:2,\r
 309 > +++ b/test/corpus/cur/18:2,\r
 310 > @@ -3,6 +3,7 @@ To: notmuch@notmuchmail.org\r
 311 >  Date: Tue, 17 Nov 2009 18:21:38 -0500\r
 312 >  Subject: [notmuch] archive\r
 313 >  Message-ID: <20091117232137.GA7669@griffis1.net>\r
 314 > +List-Id: <test1.example.com>\r
 315 >=20=20\r
 316 >  Just subscribed, I'd like to catch up on the previous postings,\r
 317 >  but the archive link seems to be bogus?\r
 318 > diff --git a/test/corpus/cur/51:2, b/test/corpus/cur/51:2,\r
 319 > index f522f69eb933..b155e6ee64a5 100644\r
 320 > --- a/test/corpus/cur/51:2,\r
 321 > +++ b/test/corpus/cur/51:2,\r
 322 > @@ -3,6 +3,7 @@ To: notmuch@notmuchmail.org\r
 323 >  Date: Tue, 17 Nov 2009 18:21:38 -0500\r
 324 >  Subject: [notmuch] archive\r
 325 >  Message-ID: <20091117232137.GA7669@griffis1.net>\r
 326 > +List-Id: <test2.example.com>\r
 327 >=20=20\r
 328 >  Just subscribed, I'd like to catch up on the previous postings,\r
 329 >  but the archive link seems to be bogus?\r
 330 > diff --git a/test/search b/test/search\r
 331 > index a7a0b18d2e48..bef42971226c 100755\r
 332 > --- a/test/search\r
 333 > +++ b/test/search\r
 334 > @@ -129,4 +129,28 @@ add_message '[subject]=3D"utf8-message-body-subject"=\r
 335 ' '[date]=3D"Sat, 01 Jan 2000 12\r
 336 >  output=3D$(notmuch search "b=C3=B6d=C3=BD" | notmuch_search_sanitize)\r
 337 >  test_expect_equal "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test =\r
 338 Suite; utf8-message-body-subject (inbox unread)"\r
 339 >=20=20\r
 340 > +test_begin_subtest "Search by List-Id"\r
 341 > +notmuch search list:notmuch.notmuchmail.org | notmuch_search_sanitize > =\r
 342 OUTPUT\r
 343 > +cat <<EOF >EXPECTED\r
 344 > +thread:XXX   2009-11-18 [2/2] Lars Kellogg-Stedman; [notmuch] "notmuch h=\r
 345 elp" outputs to stderr? (attachment inbox signed unread)\r
 346 > +thread:XXX   2009-11-18 [4/7] Lars Kellogg-Stedman, Mikhail Gusarov| Kei=\r
 347 th Packard, Carl Worth; [notmuch] Working with Maildir storage? (inbox sign=\r
 348 ed unread)\r
 349 > +thread:XXX   2009-11-18 [1/2] Alex Botero-Lowry| Carl Worth; [notmuch] [=\r
 350 PATCH] Error out if no query is supplied to search instead of going into an=\r
 351  infinite loop (attachment inbox unread)\r
 352 > +thread:XXX   2009-11-17 [1/3] Adrian Perez de Castro| Keith Packard, Car=\r
 353 l Worth; [notmuch] Introducing myself (inbox signed unread)\r
 354 > +thread:XXX   2009-11-17 [1/2] Alex Botero-Lowry| Carl Worth; [notmuch] p=\r
 355 reliminary FreeBSD support (attachment inbox unread)\r
 356 > +EOF\r
 357 > +test_expect_equal_file OUTPUT EXPECTED\r
 358 > +\r
 359 > +test_begin_subtest "Search by List-Id, duplicated messages, step 1"\r
 360 > +notmuch search list:test1.example.com | notmuch_search_sanitize > OUTPUT\r
 361 > +cat <<EOF >EXPECTED\r
 362 > +thread:XXX   2009-11-17 [1/3] Aron Griffis| Keith Packard, Carl Worth; [=\r
 363 notmuch] archive (inbox unread)\r
 364 > +EOF\r
 365 > +test_expect_equal_file OUTPUT EXPECTED\r
 366 > +\r
 367 > +test_begin_subtest "Search by List-Id, duplicated messages, step 2"\r
 368 > +notmuch search list:test2.example.com | notmuch_search_sanitize > OUTPUT\r
 369 > +cat <<EOF >EXPECTED\r
 370 > +thread:XXX   2009-11-17 [1/3] Aron Griffis| Keith Packard, Carl Worth; [=\r
 371 notmuch] archive (inbox unread)\r
 372 > +EOF\r
 373 > +test_expect_equal_file OUTPUT EXPECTED\r
 374 >  test_done\r
 375 > diff --git a/test/test-lib.sh b/test/test-lib.sh\r
 376 > index d8e0d9115a69..981bde4a4004 100644\r
 377 > --- a/test/test-lib.sh\r
 378 > +++ b/test/test-lib.sh\r
 379 > @@ -576,9 +576,9 @@ test_expect_equal_json () {\r
 380 >      # The test suite forces LC_ALL=3DC, but this causes Python 3 to\r
 381 >      # decode stdin as ASCII.  We need to read JSON in UTF-8, so\r
 382 >      # override Python's stdio encoding defaults.\r
 383 > -    output=3D$(echo "$1" | PYTHONIOENCODING=3Dutf-8 python -mjson.tool \\r
 384 > +    output=3D$(echo "$1" | PYTHONIOENCODING=3Dutf-8 python2 -mjson.tool \\r
 385 >          || echo "$1")\r
 386 > -    expected=3D$(echo "$2" | PYTHONIOENCODING=3Dutf-8 python -mjson.tool=\r
 387  \\r
 388 > +    expected=3D$(echo "$2" | PYTHONIOENCODING=3Dutf-8 python2 -mjson.too=\r
 389 l \\r
 390 >          || echo "$2")\r
 391 >      shift 2\r
 392 >      test_expect_equal "$output" "$expected" "$@"\r
 393 > --=20\r
 394 >  Kirill A. Shutemov\r