[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#990445: marked as done (unblock: notmuch/0.31.4-2)



Your message dated Tue, 29 Jun 2021 19:00:45 +0000
with message-id <E1lyIyT-0000hf-6t@respighi.debian.org>
and subject line unblock notmuch
has caused the Debian Bug report #990445,
regarding unblock: notmuch/0.31.4-2
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
990445: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=990445
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Please unblock package notmuch

[ Reason ]

The new version contains a backported (well, cherry-picked) fix for a
database corruption bug fixed in 0.32.2.  

[ Impact ]

The corruption is relatively mild, if that is possible.  Roughly
speaking, when multiple mail files have the same message-id, the
message document with that message id gets added to multiple
threads. I introduced this bug in 2017, but there has only been
occasional mystified users complaining about misthreading of
messages. Still, it's not very nice to have a subtle failure in a core
feature.

[ Tests ]

I have backported the new integration test, and it (and the rest of
the test suite) passes). I ran for about 24 hours with the proposed
version as my MUA, didn't notice anything untoward.

[ Risks ]

The changed code is confined to about 6 lines of code in
add-message.c. Most of the diff is adjusting the test suite (which had
unknowingly encoded the bug in several tests). On the other hand, this
is a change in the core of notmuch, so potentially affects several
rdeps, most notably (from a popcon point of view) neomutt.

[ Checklist ]
  [x] all changes are documented in the d/changelog
  [x] I reviewed all changes and I approve them
  [x] attach debdiff against the package in testing

[ Other info ]

I also attach the output of git diff --diff-filter=MRC, to hide the
contents of debian/patches

unblock notmuch/0.31.4-2


-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEkiyHYXwaY0SiY6fqA0U5G1WqFSEFAmDa/4gACgkQA0U5G1Wq
FSELyA//ZHb3pqhnMRXvxlM6yHUYrinl2yvIl0ZVizKTOTVMvU3TpijbJ5ozUPe5
7HD6vgOz0YovosCr/FvBleCSp6n8z5tspsiIoTnaDFZfaQ8QslYRCTpAaFZ2u939
L4nE13J8aWP58Az8Pq6zKf+P88xFZkd2kzKBtgiBwl2mB6guIc9XDtlz4LvFAUZP
XuqGD6wIO+hX72HIDTzeN/NsC43fEkEtmIOpbDnlX0qLOm1pEf8AqwzCvkkFwwgs
Ml2Bq6jZJwSfzCAKXQohl7fu+SAgvd5ifBngCQq/RDcj1T+u4EiOfAVlQC2cylwL
vIjAb/Kg0/QpjA1glj3zgv7eX18iVWbQ6MKZDnlGzeqzAUfRSd8BvN8SY9uBhTqX
S12pXsD0ZYW2Qr2bRVIChFHkoAxppU38shpQ81gAXyP1W0kicfmMMLIidr8R22Xg
PB4SBhtbfsorc7KiJ2Y3lCs/pcTaq3zupm2UA3xAdFSDj9ugXJg2iye7wqNnmmV9
uchNeLoY9LD6dnJN1FP3rH5HPL4DMejFoKzhjBKwlZwsLjCyNdu/pQPczSopWDcN
pgI969jNUzfEV95VpexTpRbFJtNwNUuwF1g5Yy1p36ZKnDDOph99TOvmJks0V9+I
/RXMLJdA4oohmWu069q6VmdR1nC+mnMLwsbRJq1ld4FfLowW0ZU=
=Pb2W
-----END PGP SIGNATURE-----
diff -Nru notmuch-0.31.4/debian/changelog notmuch-0.31.4/debian/changelog
--- notmuch-0.31.4/debian/changelog	2021-02-18 07:23:00.000000000 -0400
+++ notmuch-0.31.4/debian/changelog	2021-06-28 22:09:09.000000000 -0300
@@ -1,3 +1,11 @@
+notmuch (0.31.4-2) unstable; urgency=medium
+
+  * Cherry pick upstream commit 3f4de98e7c8, which fixes a bug where
+    duplicate message-ids can cause multiple thread-ids for some message
+    documents.
+
+ -- David Bremner <bremner@debian.org>  Mon, 28 Jun 2021 22:09:09 -0300
+
 notmuch (0.31.4-1) unstable; urgency=medium
 
   * New upstream bugfix release
diff -Nru notmuch-0.31.4/debian/control notmuch-0.31.4/debian/control
--- notmuch-0.31.4/debian/control	2021-02-18 07:23:00.000000000 -0400
+++ notmuch-0.31.4/debian/control	2021-06-28 22:09:09.000000000 -0300
@@ -37,6 +37,7 @@
  ruby,
  ruby-dev (>>1:1.9.3~),
  texinfo,
+ xapian-tools <!nocheck>,
 Standards-Version: 4.4.1
 Homepage: https://notmuchmail.org/
 Vcs-Git: https://git.notmuchmail.org/git/notmuch -b release
diff -Nru notmuch-0.31.4/debian/patches/debian-changes notmuch-0.31.4/debian/patches/debian-changes
--- notmuch-0.31.4/debian/patches/debian-changes	1969-12-31 20:00:00.000000000 -0400
+++ notmuch-0.31.4/debian/patches/debian-changes	2021-06-28 22:09:09.000000000 -0300
@@ -0,0 +1,136 @@
+This is an autogenerated patch header for a single-debian-patch file. The
+delta against upstream is either kept as a single patch, or maintained
+in some VCS, and exported as a single patch instead of more manageable
+atomic patches.
+
+--- notmuch-0.31.4.orig/lib/add-message.cc
++++ notmuch-0.31.4/lib/add-message.cc
+@@ -407,14 +407,17 @@ static notmuch_status_t
+ _notmuch_database_link_message (notmuch_database_t *notmuch,
+ 				notmuch_message_t *message,
+ 				notmuch_message_file_t *message_file,
+-				bool is_ghost)
++				bool is_ghost,
++				bool is_new)
+ {
+     void *local = talloc_new (NULL);
+     notmuch_status_t status;
+     const char *thread_id = NULL;
+ 
+     /* Check if the message already had a thread ID */
+-    if (notmuch->features & NOTMUCH_FEATURE_GHOSTS) {
++    if (! is_new) {
++	thread_id = notmuch_message_get_thread_id (message);
++    } else if (notmuch->features & NOTMUCH_FEATURE_GHOSTS) {
+ 	if (is_ghost)
+ 	    thread_id = notmuch_message_get_thread_id (message);
+     } else {
+@@ -539,7 +542,7 @@ notmuch_database_index_file (notmuch_dat
+ 	}
+ 
+ 	ret = _notmuch_database_link_message (notmuch, message,
+-					      message_file, is_ghost);
++					      message_file, is_ghost, is_new);
+ 	if (ret)
+ 	    goto DONE;
+ 
+--- notmuch-0.31.4.orig/test/T357-index-decryption.sh
++++ notmuch-0.31.4/test/T357-index-decryption.sh
+@@ -112,12 +112,10 @@ test_expect_equal \
+     "$expected"
+ 
+ # try inserting it with decryption, should appear as a single copy
+-# (note: i think thread id skips 4 because of duplicate message-id
+-# insertion, above)
+ test_begin_subtest "message cleartext is present with insert --decrypt=true"
+ notmuch insert --folder=sent --decrypt=true <<<"$contents"
+-output=$(notmuch search wumpus)
+-expected='thread:0000000000000005   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (encrypted inbox unread)'
++output=$(notmuch search wumpus | notmuch_search_sanitize)
++expected='thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (encrypted inbox unread)'
+ test_expect_equal \
+     "$output" \
+     "$expected"
+@@ -127,9 +125,9 @@ test_expect_equal \
+ test_begin_subtest 'tagging all messages'
+ test_expect_success 'notmuch tag +blarney "encrypted message"'
+ test_begin_subtest "verify that tags have not changed"
+-output=$(notmuch search tag:blarney)
+-expected='thread:0000000000000001   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 001 (blarney encrypted inbox)
+-thread:0000000000000005   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (blarney encrypted inbox unread)'
++output=$(notmuch search tag:blarney | notmuch_search_sanitize)
++expected='thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 001 (blarney encrypted inbox)
++thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (blarney encrypted inbox unread)'
+ test_expect_equal \
+     "$output" \
+     "$expected"
+@@ -138,14 +136,14 @@ test_expect_equal \
+ test_begin_subtest 'reindex old messages'
+ test_expect_success 'notmuch reindex --decrypt=true tag:encrypted and not property:index.decryption=success'
+ test_begin_subtest "reindexed encrypted message, including cleartext"
+-output=$(notmuch search wumpus)
++output=$(notmuch search wumpus | notmuch_search_sanitize)
+ test_expect_equal \
+     "$output" \
+     "$expected"
+ 
+ # and the same search, but by property ($expected is untouched):
+ test_begin_subtest "emacs search by property for both messages"
+-output=$(notmuch search property:index.decryption=success)
++output=$(notmuch search property:index.decryption=success | notmuch_search_sanitize)
+ test_expect_equal \
+     "$output" \
+     "$expected"
+@@ -154,7 +152,7 @@ test_expect_equal \
+ test_begin_subtest 'reindex in auto mode'
+ test_expect_success 'notmuch reindex tag:encrypted and property:index.decryption=success'
+ test_begin_subtest "reindexed encrypted messages, should not have changed"
+-output=$(notmuch search wumpus)
++output=$(notmuch search wumpus | notmuch_search_sanitize)
+ test_expect_equal \
+     "$output" \
+     "$expected"
+@@ -188,9 +186,9 @@ test_expect_equal \
+ 
+ # ensure that the tags remain even when we are dropping the cleartext.
+ test_begin_subtest "verify that tags remain without cleartext"
+-output=$(notmuch search tag:blarney)
+-expected='thread:0000000000000001   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 001 (blarney encrypted inbox)
+-thread:0000000000000005   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (blarney encrypted inbox unread)'
++output=$(notmuch search tag:blarney | notmuch_search_sanitize)
++expected='thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 001 (blarney encrypted inbox)
++thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (blarney encrypted inbox unread)'
+ test_expect_equal \
+     "$output" \
+     "$expected"
+@@ -199,7 +197,7 @@ test_begin_subtest "index cleartext with
+ test_expect_success "notmuch reindex --decrypt=nostash tag:blarney"
+ 
+ test_begin_subtest "Ensure that the indexed terms are present"
+-output=$(notmuch search wumpus)
++output=$(notmuch search wumpus | notmuch_search_sanitize)
+ test_expect_equal \
+     "$output" \
+     "$expected"
+--- notmuch-0.31.4.orig/test/T670-duplicate-mid.sh
++++ notmuch-0.31.4/test/T670-duplicate-mid.sh
+@@ -6,6 +6,19 @@ add_message '[id]="duplicate"' '[subject
+ add_message '[id]="duplicate"' '[subject]="message 2" [filename]=copy2'
+ 
+ add_message '[id]="duplicate"' '[subject]="message 0" [filename]=copy0'
++
++test_begin_subtest 'at most 1 thread-id per xapian document'
++db=${MAIL_DIR}/.notmuch/xapian
++for doc in $(xapian-delve -1 -t '' "$db" | grep '^[1-9]'); do
++    xapian-delve -1 -r "$doc" "$db" | grep -c '^G'
++done > OUTPUT.raw
++sort -u < OUTPUT.raw > OUTPUT
++cat <<EOF > EXPECTED
++0
++1
++EOF
++test_expect_equal_file EXPECTED OUTPUT
++
+ test_begin_subtest 'search: first indexed subject preserved'
+ cat <<EOF > EXPECTED
+ thread:XXX   2001-01-05 [1/1(3)] Notmuch Test Suite; message 1 (inbox unread)
diff -Nru notmuch-0.31.4/debian/patches/series notmuch-0.31.4/debian/patches/series
--- notmuch-0.31.4/debian/patches/series	1969-12-31 20:00:00.000000000 -0400
+++ notmuch-0.31.4/debian/patches/series	2021-06-28 22:09:09.000000000 -0300
@@ -0,0 +1 @@
+debian-changes
diff --git a/debian/changelog b/debian/changelog
index ec3de63d..ebfd9209 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+notmuch (0.31.4-2) unstable; urgency=medium
+
+  * Cherry pick upstream commit 3f4de98e7c8, which fixes a bug where
+    duplicate message-ids can cause multiple thread-ids for some message
+    documents.
+  * Add build-dependency on xapian-tools, for new test
+
+ -- David Bremner <bremner@debian.org>  Mon, 28 Jun 2021 22:48:02 -0300
+
 notmuch (0.31.4-1) unstable; urgency=medium
 
   * New upstream bugfix release
diff --git a/debian/control b/debian/control
index 8b34d9ed..633d0be8 100644
--- a/debian/control
+++ b/debian/control
@@ -37,6 +37,7 @@ Build-Depends:
  ruby,
  ruby-dev (>>1:1.9.3~),
  texinfo,
+ xapian-tools <!nocheck>,
 Standards-Version: 4.4.1
 Homepage: https://notmuchmail.org/
 Vcs-Git: https://git.notmuchmail.org/git/notmuch -b release
diff --git a/lib/add-message.cc b/lib/add-message.cc
index 485debad..b47aa501 100644
--- a/lib/add-message.cc
+++ b/lib/add-message.cc
@@ -407,14 +407,17 @@ static notmuch_status_t
 _notmuch_database_link_message (notmuch_database_t *notmuch,
 				notmuch_message_t *message,
 				notmuch_message_file_t *message_file,
-				bool is_ghost)
+				bool is_ghost,
+				bool is_new)
 {
     void *local = talloc_new (NULL);
     notmuch_status_t status;
     const char *thread_id = NULL;
 
     /* Check if the message already had a thread ID */
-    if (notmuch->features & NOTMUCH_FEATURE_GHOSTS) {
+    if (! is_new) {
+	thread_id = notmuch_message_get_thread_id (message);
+    } else if (notmuch->features & NOTMUCH_FEATURE_GHOSTS) {
 	if (is_ghost)
 	    thread_id = notmuch_message_get_thread_id (message);
     } else {
@@ -539,7 +542,7 @@ notmuch_database_index_file (notmuch_database_t *notmuch,
 	}
 
 	ret = _notmuch_database_link_message (notmuch, message,
-					      message_file, is_ghost);
+					      message_file, is_ghost, is_new);
 	if (ret)
 	    goto DONE;
 
diff --git a/test/T357-index-decryption.sh b/test/T357-index-decryption.sh
index 1ed5f28c..1951ef78 100755
--- a/test/T357-index-decryption.sh
+++ b/test/T357-index-decryption.sh
@@ -112,12 +112,10 @@ test_expect_equal \
     "$expected"
 
 # try inserting it with decryption, should appear as a single copy
-# (note: i think thread id skips 4 because of duplicate message-id
-# insertion, above)
 test_begin_subtest "message cleartext is present with insert --decrypt=true"
 notmuch insert --folder=sent --decrypt=true <<<"$contents"
-output=$(notmuch search wumpus)
-expected='thread:0000000000000005   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (encrypted inbox unread)'
+output=$(notmuch search wumpus | notmuch_search_sanitize)
+expected='thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (encrypted inbox unread)'
 test_expect_equal \
     "$output" \
     "$expected"
@@ -127,9 +125,9 @@ test_expect_equal \
 test_begin_subtest 'tagging all messages'
 test_expect_success 'notmuch tag +blarney "encrypted message"'
 test_begin_subtest "verify that tags have not changed"
-output=$(notmuch search tag:blarney)
-expected='thread:0000000000000001   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 001 (blarney encrypted inbox)
-thread:0000000000000005   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (blarney encrypted inbox unread)'
+output=$(notmuch search tag:blarney | notmuch_search_sanitize)
+expected='thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 001 (blarney encrypted inbox)
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (blarney encrypted inbox unread)'
 test_expect_equal \
     "$output" \
     "$expected"
@@ -138,14 +136,14 @@ test_expect_equal \
 test_begin_subtest 'reindex old messages'
 test_expect_success 'notmuch reindex --decrypt=true tag:encrypted and not property:index.decryption=success'
 test_begin_subtest "reindexed encrypted message, including cleartext"
-output=$(notmuch search wumpus)
+output=$(notmuch search wumpus | notmuch_search_sanitize)
 test_expect_equal \
     "$output" \
     "$expected"
 
 # and the same search, but by property ($expected is untouched):
 test_begin_subtest "emacs search by property for both messages"
-output=$(notmuch search property:index.decryption=success)
+output=$(notmuch search property:index.decryption=success | notmuch_search_sanitize)
 test_expect_equal \
     "$output" \
     "$expected"
@@ -154,7 +152,7 @@ test_expect_equal \
 test_begin_subtest 'reindex in auto mode'
 test_expect_success 'notmuch reindex tag:encrypted and property:index.decryption=success'
 test_begin_subtest "reindexed encrypted messages, should not have changed"
-output=$(notmuch search wumpus)
+output=$(notmuch search wumpus | notmuch_search_sanitize)
 test_expect_equal \
     "$output" \
     "$expected"
@@ -188,9 +186,9 @@ test_expect_equal \
 
 # ensure that the tags remain even when we are dropping the cleartext.
 test_begin_subtest "verify that tags remain without cleartext"
-output=$(notmuch search tag:blarney)
-expected='thread:0000000000000001   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 001 (blarney encrypted inbox)
-thread:0000000000000005   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (blarney encrypted inbox unread)'
+output=$(notmuch search tag:blarney | notmuch_search_sanitize)
+expected='thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 001 (blarney encrypted inbox)
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (blarney encrypted inbox unread)'
 test_expect_equal \
     "$output" \
     "$expected"
@@ -199,7 +197,7 @@ test_begin_subtest "index cleartext without keeping session keys"
 test_expect_success "notmuch reindex --decrypt=nostash tag:blarney"
 
 test_begin_subtest "Ensure that the indexed terms are present"
-output=$(notmuch search wumpus)
+output=$(notmuch search wumpus | notmuch_search_sanitize)
 test_expect_equal \
     "$output" \
     "$expected"
diff --git a/test/T670-duplicate-mid.sh b/test/T670-duplicate-mid.sh
index 4e5672ab..7ae60595 100755
--- a/test/T670-duplicate-mid.sh
+++ b/test/T670-duplicate-mid.sh
@@ -6,6 +6,19 @@ add_message '[id]="duplicate"' '[subject]="message 1" [filename]=copy1'
 add_message '[id]="duplicate"' '[subject]="message 2" [filename]=copy2'
 
 add_message '[id]="duplicate"' '[subject]="message 0" [filename]=copy0'
+
+test_begin_subtest 'at most 1 thread-id per xapian document'
+db=${MAIL_DIR}/.notmuch/xapian
+for doc in $(xapian-delve -1 -t '' "$db" | grep '^[1-9]'); do
+    xapian-delve -1 -r "$doc" "$db" | grep -c '^G'
+done > OUTPUT.raw
+sort -u < OUTPUT.raw > OUTPUT
+cat <<EOF > EXPECTED
+0
+1
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest 'search: first indexed subject preserved'
 cat <<EOF > EXPECTED
 thread:XXX   2001-01-05 [1/1(3)] Notmuch Test Suite; message 1 (inbox unread)

--- End Message ---
--- Begin Message ---
Unblocked.

--- End Message ---

Reply to: