Re: [PATCH v4 7/7] complete ghost-on-removal-when-shared-thread-exists

[Daniel Kahn Gillmor <dkg@fifthhorseman.net>]

[-- Attachment #1: Type: text/plain, Size: 2454 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 948 bytes --]