From: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
To: David Bremner <david@tethera.net>, Notmuch Mail <notmuch@notmuchmail.org>
Subject: Re: [PATCH v4 7/7] complete
 ghost-on-removal-when-shared-thread-exists
In-Reply-To: <87r3ed6l35.fsf@zancas.localnet>
References: <1459445693-3900-1-git-send-email-dkg@fifthhorseman.net>
 <1460166892-29721-1-git-send-email-dkg@fifthhorseman.net>
 <1460166892-29721-7-git-send-email-dkg@fifthhorseman.net>
 <87r3ed6l35.fsf@zancas.localnet>
User-Agent: Notmuch/0.21+124~gbf604e9 (http://notmuchmail.org) Emacs/24.5.1
 (x86_64-pc-linux-gnu)
Date: Mon, 11 Apr 2016 15:18:44 -0400
Message-ID: <87zit0t0mj.fsf@alice.fifthhorseman.net>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
 micalg=pgp-sha512; protocol="application/pgp-signature"
Precedence: list

--=-=-=
Content-Type: text/plain

On Sun 2016-04-10 20:33:02 -0400, David Bremner <david@tethera.net> wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> To fully complete the ghost-on-removal-when-shared-thread-exists
>> proposal, we need to clear all ghost messages when the last active
>> message is removed from a thread.
>
> For me this patch breaks T530 as follows
>
> T530-upgrade: Testing database upgrade
>  FAIL   ghost message retains thread ID
> 	--- T530-upgrade.13.expected	2016-04-11 00:25:19.482196274 +0000
> 	+++ T530-upgrade.13.output	2016-04-11 00:25:19.482196274 +0000
> 	@@ -1 +1 @@
> 	-thread:000000000000001d
> 	+thread:0000000000000011
> No new mail.
> No new mail. Removed 1 message.
>
> I pushed my rebased version of the patches to
>
>   https://git.notmuchmail.org/git?p=notmuch;a=shortlog;h=refs/heads/thread-fix
>
> in case the problem is some mistake on my part.

After having done "make download-test-databases", I can confirm that
this is happening for me as well: it's not an issue with bremner's
config.

however, i think this particular test is wrong, and should probably be
removed.

For review, here's the final test:

----------
# Ghost messages are difficult to test since they're nearly invisible.
# However, if the upgrade works correctly, the ghost message should
# retain the right thread ID even if all of the original messages in
# the thread are deleted.  That's what we test.  This won't detect if
# the upgrade just plain didn't happen, but it should detect if
# something went wrong.
test_begin_subtest "ghost message retains thread ID"
# Upgrade database
notmuch new
# Get thread ID of real message
thread=$(notmuch search --output=threads id:4EFC743A.3060609@april.org)
# Delete all real messages in that thread
rm $(notmuch search --output=files $thread)
notmuch new
# "Deliver" ghost message
add_message '[subject]=Ghost' '[id]=4EFC3931.6030007@april.org'
# If the ghost upgrade worked, the new message should be attached to
# the existing thread ID.
nthread=$(notmuch search --output=threads id:4EFC3931.6030007@april.org)
test_expect_equal "$thread" "$nthread"
-------------

I don't think this reasoning is sensible.  If the entire thread is
deleted, and a new message comes in, it should *not* get the same mesage
ID.  ghosts should only exist in the database when other messages point
to them.

So i'd be fine with killing this entire last test, unless someone can
propose a good reason to keep it.

        --dkg

--=-=-=
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQJ8BAEBCgBmBQJXC/iVXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w
ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRFREIyRTc0RjU2RkNGMkI2NzI5N0I3MzUy
NEVDRkY1QUZGNjgzNzBBAAoJECTs/1r/aDcK0mMP/2uFFlhIVN90Elg2y0qb7ahb
iA+i9iI9tARYaKyo26zxlOVCMfrzYPfgyQC9F1T4VZzTj9mtigOLzR1bEtoYTvoG
XuZK78Ytkjx2INeWzLEw/QxNFcJahDMEkC80UiGNMYwYnPUYjNPLprIrWQ9SAM7T
vTBVYjjkkNJpcYMIRd7uTz63sC/kuk8MWK9CmnuUdpMYnnJayOAUFAeJNjR0b/ZG
hGNWY9vbJrehKKYxozayn500NfDuASdpdtdGXLXjFxzMwQEVDSl99men6nxaTijT
gYKTotB6A3/YF1XIptMmo7IHu6DnhthlfEXXfLZAbaTIff95eQLq5J196Om8fNNB
jY9AL46w4JLtSNH95bH8NGuegQ4SMhskXBZb2D4LTpxmAktgglWlhcFemlp7omsR
lwJufU4B7EcLL1OxlUb9CEvHUsG96zyVT65b1f5xVtf5tTwJctT9KfFThy1KDBcc
TVuJC86j+Z/pEf/3s1tes6iYFjXz5UVV1Pdlp6SDVP5xUi6IaKQ5Pp+H36jKaz1S
F+s2nhc34P/IZB+nx/qLt1EgAsN+zT7NmyQCuN4lum2ntGX0NwP7p1OKlxnFNGWi
YDVaAsAjQOY4w4fVB0yWAg5MslvVtCb+pgCKiZX1rHb6P5fBxjWSU08jRFerSmaa
Dgh4S1MMFHK5T1qcKJiM
=f7Sb
-----END PGP SIGNATURE-----
--=-=-=--