threading replies fixes v3

* threading replies fixes v3
@ 2018-08-30 11:29 David Bremner
  2018-08-30 11:29 ` [PATCH 01/15] util: add DEBUG_PRINTF, rename error_util.h -> debug_print.h David Bremner
                   ` (14 more replies)
  0 siblings, 15 replies; 23+ messages in thread
From: David Bremner @ 2018-08-30 11:29 UTC (permalink / raw)
  To: notmuch

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; i<count; i++){
+    for (size_t i = 0; i < count; i++) {
 	_notmuch_message_list_add_message (new_list, message_array[i]);
     }
-    talloc_free (message_array);
+
+    talloc_free (local);
+    talloc_free (list);
     return new_list;
 }
 
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 64f4e982..5bbaa292 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -560,7 +560,7 @@ _notmuch_message_label_depths (notmuch_message_t *message,
 			       size_t depth);
 
 notmuch_message_list_t *
-_notmuch_message_sort_subtrees (notmuch_message_list_t *list);
+_notmuch_message_sort_subtrees (void *ctx, notmuch_message_list_t *list);
 
 /* sha1.c */

######################################################################
# update comments

diff --git a/lib/thread.cc b/lib/thread.cc
index e9e15a5c..e4ab4319 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -498,15 +498,17 @@ _resolve_thread_relationships (notmuch_thread_t *thread)
 	 notmuch_messages_valid (roots);
 	 notmuch_messages_move_to_next (roots)) {
 	notmuch_message_t *message = notmuch_messages_get (roots);
-	/* XXX add _notmuch_messages_count */
+	/* XXX TODO: cleaner way to test iterator for last, list for emptiness  */
 	if (roots->iterator->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 <<EOF > 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 <<EOF > 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 <talloc.h>
 #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 <cworth@cworth.org>
+ */
+
 #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

^ permalink raw reply related	[flat|nested] 23+ messages in thread