From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id 025FE6DE0236 for ; Thu, 30 Aug 2018 04:29:38 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: 0.251 X-Spam-Level: X-Spam-Status: No, score=0.251 tagged_above=-999 required=5 tests=[AWL=-0.238, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01, URI_NOVOWEL=0.5] autolearn=disabled Received: from arlo.cworth.org ([127.0.0.1]) by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id nSLHbwkUaAqj for ; Thu, 30 Aug 2018 04:29:36 -0700 (PDT) Received: from fethera.tethera.net (fethera.tethera.net [198.245.60.197]) by arlo.cworth.org (Postfix) with ESMTPS id BCDC16DE0233 for ; Thu, 30 Aug 2018 04:29:36 -0700 (PDT) Received: from remotemail by fethera.tethera.net with local (Exim 4.89) (envelope-from ) id 1fvL8n-0007EB-Dd for notmuch@notmuchmail.org; Thu, 30 Aug 2018 07:29:33 -0400 Received: (nullmailer pid 12109 invoked by uid 1000); Thu, 30 Aug 2018 11:29:32 -0000 From: David Bremner To: notmuch@notmuchmail.org Subject: threading replies fixes v3 Date: Thu, 30 Aug 2018 08:29:00 -0300 Message-Id: <20180830112915.11761-1-david@tethera.net> X-Mailer: git-send-email 2.18.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Aug 2018 11:29:38 -0000 This obsoletes the series at id:20180730224555.26047-1-david@tethera.net An annotated interdiff is at the end. At high level this represents a cleanup and light re-organization of the previous series. I also dropped the test for multiple thread terms, as it is not relevant to these changes. There are two main changes: patches 1 to 10 introduce the use of references to parent messages. Patches 11 to 15 conditionally revert to trusting In-Reply-To (when it looks like a single message-id). [PATCH 04/15] lib/thread: sort sibling messages by date This has the WIP patch of a few days ago squashed into it, along with memory allocation fixes. [PATCH 06/15] lib/thread: refactor in_reply_to test [PATCH 07/15] lib/thread: initial use of references as for fallback Patch 07 is split out of patch 06 in the previous series. There are style fixes throughout from uncrustify. ###################################################################### # Bug fix: we need to make sure the lazy reading is done. diff --git a/lib/message.cc b/lib/message.cc index f329be20..5e17029a 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -625,6 +625,7 @@ _notmuch_message_label_depths (notmuch_message_t *message, const notmuch_string_list_t * _notmuch_message_get_references (notmuch_message_t *message) { + _notmuch_message_ensure_metadata (message, message->reference_list); return message->reference_list; } @@ -640,15 +641,18 @@ _cmpmsg (const void *pa, const void *pb) } ###################################################################### # Memory management fixes for sort_subtrees notmuch_message_list_t * -_notmuch_message_sort_subtrees (notmuch_message_list_t *list) { +_notmuch_message_sort_subtrees (void *ctx, notmuch_message_list_t *list) +{ size_t count = 0; size_t capacity = 16; - if (!list) + if (! list) return list; - notmuch_message_t **message_array = talloc_zero_array (list, notmuch_message_t *, capacity); + void *local = talloc_new (NULL); + notmuch_message_list_t *new_list = _notmuch_message_list_create (ctx); + notmuch_message_t **message_array = talloc_zero_array (local, notmuch_message_t *, capacity); for (notmuch_messages_t *messages = _notmuch_messages_create (list); notmuch_messages_valid (messages); @@ -656,19 +660,19 @@ _notmuch_message_sort_subtrees (notmuch_message_list_t *list) { notmuch_message_t *root = notmuch_messages_get (messages); if (count >= capacity) { capacity *= 2; - message_array = talloc_realloc (root, message_array, notmuch_message_t *, capacity); + message_array = talloc_realloc (local, message_array, notmuch_message_t *, capacity); } message_array[count++] = root; - root->replies = _notmuch_message_sort_subtrees (root->replies); + root->replies = _notmuch_message_sort_subtrees (root, root->replies); } - notmuch_message_list_t *new_list = _notmuch_message_list_create (list); - qsort (message_array, count, sizeof (notmuch_message_t *), _cmpmsg); - for (size_t i=0; iiterator->next || thread->toplevel_list->head) _parent_or_toplevel (thread, message); else _notmuch_message_list_add_message (thread->toplevel_list, message); } - /* XXX this creates garbage */ - thread->toplevel_list = _notmuch_message_sort_subtrees (thread->toplevel_list); + /* XXX this could be made conditional on messages being inserted + * (out of order) in later passes + */ + thread->toplevel_list = _notmuch_message_sort_subtrees (thread, thread->toplevel_list); talloc_free (local); } ###################################################################### # drop the last patch in the series diff --git a/test/.gitignore b/test/.gitignore index 71bbd7ed..73fe7e24 100644 --- a/test/.gitignore +++ b/test/.gitignore @@ -8,5 +8,4 @@ /make-db-version /test-results /ghost-report -/term-report /tmp.* diff --git a/test/Makefile.local b/test/Makefile.local index c39feace..1cf09778 100644 --- a/test/Makefile.local +++ b/test/Makefile.local @@ -44,9 +44,6 @@ $(dir)/make-db-version: $(dir)/make-db-version.o $(dir)/ghost-report: $(dir)/ghost-report.o $(call quiet,CXX) $^ -o $@ $(LDFLAGS) $(XAPIAN_LDFLAGS) -$(dir)/term-report: $(dir)/term-report.o - $(call quiet,CXX) $^ -o $@ $(LDFLAGS) $(XAPIAN_LDFLAGS) - .PHONY: test check test_main_srcs=$(dir)/arg-test.c \ @@ -57,9 +54,7 @@ test_main_srcs=$(dir)/arg-test.c \ $(dir)/symbol-test.cc \ $(dir)/make-db-version.cc \ $(dir)/ghost-report.cc \ - $(dir)/message-id-parse.c \ - $(dir)/term-report.cc - + $(dir)/message-id-parse.c test_srcs=$(test_main_srcs) $(dir)/database-test.c diff --git a/test/T510-thread-replies.sh b/test/T510-thread-replies.sh index f5ee81fe..4688c0ab 100755 --- a/test/T510-thread-replies.sh +++ b/test/T510-thread-replies.sh @@ -189,6 +189,26 @@ End of search results. EOF test_expect_equal_file EXPECTED OUTPUT ###################################################################### # New test cases, based on Gregor's anonymized data +test_begin_subtest "reply to ghost (RT)" +notmuch show --entire-thread=true id:87bmc6lp3h.fsf@len.workgroup | grep ^Subject: | head -1 > OUTPUT +cat < EXPECTED +Subject: FYI: xxxx xxxxxxx xxxxxxxxxxxx xxx +EOF +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest "reply to ghost (RT/tree view)" +test_emacs '(notmuch-tree "id:87bmc6lp3h.fsf@len.workgroup") + (notmuch-test-wait) + (test-output) + (delete-other-windows)' +cat < EXPECTED + 2016-06-19 Gregor Zattler ┬┬►FYI: xxxx xxxxxxx xxxxxxxxxxxx xxx (inbox unread) + 2016-06-19 via RT │╰─►[support.xxxxxxxxxxx-xxxxxxxxx-xxxxxxxxx.de #33575] AutoReply: FYI: xxxx xxxxxxx xxxxxxxxxxxx xxx (inbox unread) + 2016-06-26 via RT ╰─►[support.xxxxxxxxxxx-xxxxxxxxx-xxxxxxxxx.de #33575] Resolved: FYI: xxxx xxxxxxx xxxxxxxxxxxx xxx (inbox unread) +End of search results. +EOF +test_expect_equal_file EXPECTED OUTPUT + test_begin_subtest "trusting reply-to (tree view)" test_emacs '(notmuch-tree "id:B00-root@example.org") (notmuch-test-wait) ###################################################################### # drop last patch diff --git a/test/T720-database-schema.sh b/test/T720-database-schema.sh deleted file mode 100755 index a6a99239..00000000 ###################################################################### # new test data, see patches for content diff --git a/test/corpora/threading/ghost-root/1529425589.M615261P21663.len:2,S b/test/corpora/threading/ghost-root/1529425589.M615261P21663.len:2,S new file mode 100644 diff --git a/test/corpora/threading/ghost-root/1532672447.R3166642290392477575.len:2,S b/test/corpora/threading/ghost-root/1532672447.R3166642290392477575.len:2,S new file mode 100644 diff --git a/test/corpora/threading/ghost-root/1532672447.R6968667928580738175.len:2,S b/test/corpora/threading/ghost-root/1532672447.R6968667928580738175.len:2,S new file mode 100644 ###################################################################### # drop last patch diff --git a/test/corpora/threading/mutant-ref/file1 b/test/corpora/threading/mutant-ref/file1 deleted file mode 100644 index 97f8db58..00000000 diff --git a/test/corpora/threading/mutant-ref/file2 b/test/corpora/threading/mutant-ref/file2 deleted file mode 100644 index 2b2ccd1d..00000000 diff --git a/test/corpora/threading/mutant-ref/file3 b/test/corpora/threading/mutant-ref/file3 deleted file mode 100644 index a8e705bc..00000000 diff --git a/test/corpora/threading/mutant-ref/file4 b/test/corpora/threading/mutant-ref/file4 deleted file mode 100644 index 3a0a5a13..00000000 diff --git a/test/corpora/threading/mutant-ref/file5 b/test/corpora/threading/mutant-ref/file5 deleted file mode 100644 index 8f525d63..00000000 ###################################################################### # uncrustify diff --git a/test/message-id-parse.c b/test/message-id-parse.c index 25decc9b..752eb1fd 100644 --- a/test/message-id-parse.c +++ b/test/message-id-parse.c @@ -2,22 +2,24 @@ #include #include "notmuch-private.h" -int main(unused (int argc), unused (char **argv)){ +int +main (unused (int argc), unused (char **argv)) +{ char *line = NULL; size_t len = 0; ssize_t nread; void *local = talloc_new (NULL); - while ((nread = getline(&line, &len, stdin)) != -1) { - int last = strlen(line) - 1; + while ((nread = getline (&line, &len, stdin)) != -1) { + int last = strlen (line) - 1; if (line[last] == '\n') line[last] = '\0'; char *mid = _notmuch_message_id_parse_strict (local, line); if (mid) - printf("GOOD: %s\n", mid); + printf ("GOOD: %s\n", mid); else - printf("BAD: %s\n", line); + printf ("BAD: %s\n", line); } talloc_free (local); diff --git a/test/term-report.cc b/test/term-report.cc deleted file mode 100644 index 88cd1bf5..00000000 ###################################################################### # put back copyright header diff --git a/util/debug_print.c b/util/debug_print.c index 9f00f69b..8b4bc73d 100644 --- a/util/debug_print.c +++ b/util/debug_print.c @@ -1,3 +1,24 @@ +/* error_util.c - internal error utilities for notmuch. + * + * Copyright © 2009 Carl Worth + * Copyright © 2018 David Bremner [snip] + * + * Author: Carl Worth + */ + #include "debug_print.h" ###################################################################### # whitespace fix void @@ -8,7 +29,6 @@ _debug_printf (const char *format, ...) va_start (va_args, format); vfprintf (stderr, format, va_args); va_end (va_args); - } void