1 Return-Path: <aclements@csail.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 arlo.cworth.org (Postfix) with ESMTP id B39476DE0188
\r
6 for <notmuch@notmuchmail.org>; Tue, 19 Apr 2016 21:03:56 -0700 (PDT)
\r
7 X-Virus-Scanned: Debian amavisd-new at cworth.org
\r
11 X-Spam-Status: No, score=-1.259 tagged_above=-999 required=5 tests=[AWL=1.052,
\r
12 RCVD_IN_DNSWL_MED=-2.3, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01]
\r
14 Received: from arlo.cworth.org ([127.0.0.1])
\r
15 by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024)
\r
16 with ESMTP id 8O28GugfpjdO for <notmuch@notmuchmail.org>;
\r
17 Tue, 19 Apr 2016 21:03:48 -0700 (PDT)
\r
18 X-Greylist: delayed 1614 seconds by postgrey-1.35 at arlo;
\r
19 Tue, 19 Apr 2016 21:03:48 PDT
\r
20 Received: from outgoing.csail.mit.edu (outgoing.csail.mit.edu [128.30.2.149])
\r
21 by arlo.cworth.org (Postfix) with ESMTP id 043DD6DE00D3
\r
22 for <notmuch@notmuchmail.org>; Tue, 19 Apr 2016 21:03:47 -0700 (PDT)
\r
23 Received: from awakening.a20.io ([104.131.20.129] helo=awakening)
\r
24 by outgoing.csail.mit.edu with esmtpsa (TLS1.0:RSA_AES_128_CBC_SHA1:16)
\r
25 (Exim 4.72) (envelope-from <aclements@csail.mit.edu>)
\r
26 id 1asiwb-0004cv-IA; Tue, 19 Apr 2016 23:36:49 -0400
\r
27 Received: from amthrax by awakening with local (Exim 4.86)
\r
28 (envelope-from <aclements@csail.mit.edu>)
\r
29 id 1asiwa-0004PY-Hk; Tue, 19 Apr 2016 23:36:48 -0400
\r
30 From: Austin Clements <aclements@csail.mit.edu>
\r
31 To: Daniel Kahn Gillmor <dkg@fifthhorseman.net>,
\r
32 David Bremner <david@tethera.net>, Notmuch Mail <notmuch@notmuchmail.org>
\r
33 Subject: Re: [PATCH v4 7/7] complete
\r
34 ghost-on-removal-when-shared-thread-exists
\r
35 In-Reply-To: <87zit0t0mj.fsf@alice.fifthhorseman.net>
\r
36 References: <1459445693-3900-1-git-send-email-dkg@fifthhorseman.net>
\r
37 <1460166892-29721-1-git-send-email-dkg@fifthhorseman.net>
\r
38 <1460166892-29721-7-git-send-email-dkg@fifthhorseman.net>
\r
39 <87r3ed6l35.fsf@zancas.localnet> <87zit0t0mj.fsf@alice.fifthhorseman.net>
\r
40 User-Agent: Notmuch/0.21+62~g7edba1d (http://notmuchmail.org) Emacs/24.5.1
\r
41 (x86_64-pc-linux-gnu)
\r
42 Date: Tue, 19 Apr 2016 23:36:48 -0400
\r
43 Message-ID: <87zispne7j.fsf@csail.mit.edu>
\r
45 Content-Type: text/plain
\r
46 X-BeenThere: notmuch@notmuchmail.org
\r
47 X-Mailman-Version: 2.1.20
\r
49 List-Id: "Use and development of the notmuch mail system."
\r
50 <notmuch.notmuchmail.org>
\r
51 List-Unsubscribe: <https://notmuchmail.org/mailman/options/notmuch>,
\r
52 <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>
\r
53 List-Archive: <http://notmuchmail.org/pipermail/notmuch/>
\r
54 List-Post: <mailto:notmuch@notmuchmail.org>
\r
55 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>
\r
56 List-Subscribe: <https://notmuchmail.org/mailman/listinfo/notmuch>,
\r
57 <mailto:notmuch-request@notmuchmail.org?subject=subscribe>
\r
58 X-List-Received-Date: Wed, 20 Apr 2016 04:03:56 -0000
\r
60 On Mon, 11 Apr 2016, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
\r
61 > On Sun 2016-04-10 20:33:02 -0400, David Bremner <david@tethera.net> wrote:
\r
62 >> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
\r
64 >>> To fully complete the ghost-on-removal-when-shared-thread-exists
\r
65 >>> proposal, we need to clear all ghost messages when the last active
\r
66 >>> message is removed from a thread.
\r
68 >> For me this patch breaks T530 as follows
\r
70 >> T530-upgrade: Testing database upgrade
\r
71 >> FAIL ghost message retains thread ID
\r
72 >> --- T530-upgrade.13.expected 2016-04-11 00:25:19.482196274 +0000
\r
73 >> +++ T530-upgrade.13.output 2016-04-11 00:25:19.482196274 +0000
\r
75 >> -thread:000000000000001d
\r
76 >> +thread:0000000000000011
\r
78 >> No new mail. Removed 1 message.
\r
80 >> I pushed my rebased version of the patches to
\r
82 >> https://git.notmuchmail.org/git?p=notmuch;a=shortlog;h=refs/heads/thread-fix
\r
84 >> in case the problem is some mistake on my part.
\r
86 > After having done "make download-test-databases", I can confirm that
\r
87 > this is happening for me as well: it's not an issue with bremner's
\r
90 > however, i think this particular test is wrong, and should probably be
\r
93 > For review, here's the final test:
\r
96 > # Ghost messages are difficult to test since they're nearly invisible.
\r
97 > # However, if the upgrade works correctly, the ghost message should
\r
98 > # retain the right thread ID even if all of the original messages in
\r
99 > # the thread are deleted. That's what we test. This won't detect if
\r
100 > # the upgrade just plain didn't happen, but it should detect if
\r
101 > # something went wrong.
\r
102 > test_begin_subtest "ghost message retains thread ID"
\r
103 > # Upgrade database
\r
105 > # Get thread ID of real message
\r
106 > thread=$(notmuch search --output=threads id:4EFC743A.3060609@april.org)
\r
107 > # Delete all real messages in that thread
\r
108 > rm $(notmuch search --output=files $thread)
\r
110 > # "Deliver" ghost message
\r
111 > add_message '[subject]=Ghost' '[id]=4EFC3931.6030007@april.org'
\r
112 > # If the ghost upgrade worked, the new message should be attached to
\r
113 > # the existing thread ID.
\r
114 > nthread=$(notmuch search --output=threads id:4EFC3931.6030007@april.org)
\r
115 > test_expect_equal "$thread" "$nthread"
\r
118 > I don't think this reasoning is sensible. If the entire thread is
\r
119 > deleted, and a new message comes in, it should *not* get the same mesage
\r
120 > ID. ghosts should only exist in the database when other messages point
\r
123 > So i'd be fine with killing this entire last test, unless someone can
\r
124 > propose a good reason to keep it.
\r
126 I agree that it's fine to remove this test. From what I recall, it
\r
127 wasn't intended to test that ghost messages retained their thread IDs;
\r
128 this was just the implementation property it exploited to test that
\r
129 ghosts were working.
\r
131 I haven't looked through this series, but it would be nice if there was
\r
132 still some tests that ghosts were working across upgrade.
\r