Bug#990445: unblock: notmuch/0.31.4-2
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)
Reply to: