Re: [PATCH v2 3/7] fix thread breakage via ghost-on-removal

diff --git a/ad/b5e66b0cc9756646b90d4e3eaf120a5de5f15b b/ad/b5e66b0cc9756646b90d4e3eaf120a5de5f15b
new file mode 100644 (file)
index 0000000..c37a11a
--- /dev/null
+++ b/ad/b5e66b0cc9756646b90d4e3eaf120a5de5f15b
@@ -0,0 +1,137 @@
+Return-Path: <dkg@fifthhorseman.net>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by arlo.cworth.org (Postfix) with ESMTP id 86AFB6DE0243\r
+ for <notmuch@notmuchmail.org>; Tue,  5 Apr 2016 13:05:31 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at cworth.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.029\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.029 tagged_above=-999 required=5\r
+ tests=[AWL=-0.029] autolearn=disabled\r
+Received: from arlo.cworth.org ([127.0.0.1])\r
+ by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id b0atHe6C1KdY for <notmuch@notmuchmail.org>;\r
+ Tue,  5 Apr 2016 13:05:22 -0700 (PDT)\r
+Received: from che.mayfirst.org (che.mayfirst.org [209.234.253.108])\r
+ by arlo.cworth.org (Postfix) with ESMTP id 6DDBE6DE01F7\r
+ for <notmuch@notmuchmail.org>; Tue,  5 Apr 2016 13:05:22 -0700 (PDT)\r
+Received: from fifthhorseman.net (dhcp-a215.meeting.ietf.org [31.133.162.21])\r
+ by che.mayfirst.org (Postfix) with ESMTPSA id D31E8F991;\r
+ Tue,  5 Apr 2016 16:05:17 -0400 (EDT)\r
+Received: by fifthhorseman.net (Postfix, from userid 1000)\r
+ id 75104200DD; Tue,  5 Apr 2016 17:05:09 -0300 (BRT)\r
+From: Daniel Kahn Gillmor <dkg@fifthhorseman.net>\r
+To: Tomi Ollila <tomi.ollila@iki.fi>, Notmuch Mail <notmuch@notmuchmail.org>\r
+Subject: Re: [PATCH v2 3/7] fix thread breakage via ghost-on-removal\r
+In-Reply-To: <m2lh4sczrl.fsf@guru.guru-group.fi>\r
+References: <1459445693-3900-1-git-send-email-dkg@fifthhorseman.net>\r
+ <1459606541-23889-1-git-send-email-dkg@fifthhorseman.net>\r
+ <1459606541-23889-3-git-send-email-dkg@fifthhorseman.net>\r
+ <m2lh4sczrl.fsf@guru.guru-group.fi>\r
+User-Agent: Notmuch/0.21+124~gbf604e9 (http://notmuchmail.org) Emacs/24.5.1\r
+ (x86_64-pc-linux-gnu)\r
+Date: Tue, 05 Apr 2016 17:05:06 -0300\r
+Message-ID: <874mbfvn2l.fsf@alice.fifthhorseman.net>\r
+MIME-Version: 1.0\r
+Content-Type: multipart/signed; boundary="=-=-=";\r
+ micalg=pgp-sha512; protocol="application/pgp-signature"\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.20\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <https://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch/>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <https://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Tue, 05 Apr 2016 20:05:31 -0000\r
+\r
+--=-=-=\r
+Content-Type: text/plain\r
+\r
+Hi Tomi--\r
+\r
+On Tue 2016-04-05 03:53:34 -0300, Tomi Ollila wrote:\r
+> I fetched your changes from lair.fifthhorseman.net yesterday and diffed\r
+> against master; looks pretty good, some quick comments (this email has\r
+> most relevant changes related to those changes):\r
+\r
+thanks for the review!\r
+\r
+>> -/* Delete a message document from the database. */\r
+>> +/* Delete a message document from the database, leaving a ghost\r
+>> + * message in its place */\r
+>\r
+> This comment could tell the condition when ghost message is left --\r
+> versus the case all ghost messages are dropped (thread becomes empty of\r
+> mail messages).\r
+\r
+right, i can improve these comments.\r
+\r
+> In next lines the expected case NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND\r
+> could be first. Although the performance difference is negligible to me\r
+> it looks silly do this first check and basically always fail there and\r
+> then do 'else if' which is likely to succeeed...\r
+> (your latest version in lair does not return in this first case but sets\r
+> status to NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID. Perhaps later messages in this\r
+> thread does the same but those are somewhat out-of-context for this reply).\r
+\r
+sure, i can do the positive check first :)\r
+\r
+>> +    if (private_status == NOTMUCH_PRIVATE_STATUS_SUCCESS) {\r
+>> +   /* this is deeply weird, and we should not have gotten into\r
+>> +      this state.  is there a better error message to return\r
+>> +      here? */\r
+>> +   return NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;\r
+>> +    } else if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {\r
+>> +   private_status = _notmuch_message_initialize_ghost (ghost, tid);\r
+>> +   if (! private_status)\r
+>> +       _notmuch_message_sync (ghost);\r
+>> +    }\r
+>> +    notmuch_message_destroy (ghost);\r
+>> +    return COERCE_STATUS (private_status, "Error converting to ghost message");\r
+>>  }\r
+>\r
+> Outside of this patch, but in some of the next messages, adds functions\r
+> _notmuch_message_has_term() and _notmuch_message_has_term_st(). Perhaps\r
+> the _notmuch_message_has_term() could be left unimplemented?\r
+\r
+yeah, i can do that, though i have to say it's programmatically\r
+convenient to have a simple boolean test that defaults to some value if\r
+there was an error.\r
+\r
+I'll post one more round of patches to the mailing list to address these\r
+cleanup bits in the next day or two.\r
+\r
+I'm happy to read more reviews if anyone has them,\r
+\r
+      --dkg\r
+\r
+--=-=-=\r
+Content-Type: application/pgp-signature; name="signature.asc"\r
+\r
+-----BEGIN PGP SIGNATURE-----\r
+Version: GnuPG v2\r
+\r
+iQJ8BAEBCgBmBQJXBBpyXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w\r
+ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRFREIyRTc0RjU2RkNGMkI2NzI5N0I3MzUy\r
+NEVDRkY1QUZGNjgzNzBBAAoJECTs/1r/aDcKna8QAJP3qDgHG/NXoLnrPZfnTTII\r
+pPvNxW+EfeV5QT8z2LOzOCV3qAhty7cg/ga0AyJsB2RDcSa4biWiGzh3MbvJz/ZJ\r
+uML2vuW/qOKENkTbfOYNgDUVK9+7tOEjLw85Py1U6EsHGeUAT0aazA9trdR6naC/\r
+I0udxxaNMKqE8JLC1aWNw629AcwkKklvqP1bX4/q2DASdVMnOJwAPZTwkOxDRkpP\r
+gMkOrBZA+MSkzd55OWFbDCstlIF27PmUx/6dqBaYHp4l+Qk3ikbw1hAquyPNgK5v\r
+68XtR8yabu8ULktTbYR0xin3NuPv/osYEH01PHWzFn/6B7BlimlFhd4JqmYR5ZGT\r
+QHTLLFI8tb8wPK7VTj0gt7WnnpARDTBpGOFbV7UW6asi18IiEdfUrdVe58W9wpCG\r
+72o6aa3aYPEB13Jbz2MHMo6UrdVvB6aaYIaPaK1ngAbnXeatGko/v2zQq9Gi4142\r
+4yHqcDYZmh1xE3yl6mI/zb38l1ywYfh7k4lBjbtggoIzcXk0FuBDqZjVltvNlKjC\r
+J33E7QAxEhP9tB9boVjUbhHrfqNY4nzMpF6FESkjQoFbZqpucBVDexqwkjWJkxBu\r
+ovsmsH4hfdfdAVc0pbKF8SlCiF3ptG7vqsTuJz1fRRaNEQ4tNB3tdXPD+FeTZAzL\r
+9gTwM9LAtdqO1YtRDW4R\r
+=nBqb\r
+-----END PGP SIGNATURE-----\r
+--=-=-=--\r