unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* reference loop fix and tests, v2
@ 2018-04-14  1:46 David Bremner
  2018-04-14  1:46 ` [PATCH 1/5] test: two new messages for the 'broken' corpus David Bremner
                   ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: David Bremner @ 2018-04-14  1:46 UTC (permalink / raw)
  To: notmuch

Here's a tidied version of the reference loop fix. There's actually
only one non-comment line of code changed. As you can guess from the
NEWS patch, I'm planning a point release to include this bugfix. I'd
also like to include the mset fix [1], so it would be great if someone
could test that.

[1]: id:20180406114307.7067-1-david@tethera.net

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

* [PATCH 1/5] test: two new messages for the 'broken' corpus
  2018-04-14  1:46 reference loop fix and tests, v2 David Bremner
@ 2018-04-14  1:46 ` David Bremner
  2018-04-14 22:24   ` Tomi Ollila
  2018-04-14  1:46 ` [PATCH 2/5] test: add known broken test for indexing an In-Reply-To loop David Bremner
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: David Bremner @ 2018-04-14  1:46 UTC (permalink / raw)
  To: notmuch

These have an 'In-Reply-To' loop, which currently confuses "notmuch
new".

Courtesy of anarcat.
---
 ...521463752.R13151765805797588408.curie:2,FS | 173 +++++++++++++++++
 ...1521463753.R9368947314807690338.curie:2,FS | 180 ++++++++++++++++++
 2 files changed, 353 insertions(+)
 create mode 100644 test/corpora/broken/gitlab/cur/1521463752.R13151765805797588408.curie:2,FS
 create mode 100644 test/corpora/broken/gitlab/cur/1521463753.R9368947314807690338.curie:2,FS

diff --git a/test/corpora/broken/gitlab/cur/1521463752.R13151765805797588408.curie:2,FS b/test/corpora/broken/gitlab/cur/1521463752.R13151765805797588408.curie:2,FS
new file mode 100644
index 00000000..0dc4ecf8
--- /dev/null
+++ b/test/corpora/broken/gitlab/cur/1521463752.R13151765805797588408.curie:2,FS
@@ -0,0 +1,173 @@
+Return-Path: <support@gitlab.com>
+X-Original-To: anarcat+gitlab@anarc.at
+Delivered-To: anarcat+gitlab@anarc.at
+Received: from marcos.anarc.at (localhost [127.0.0.1])
+	by delivery.anarc.at (Postfix) with ESMTP id 99EA510E050
+	for <anarcat+gitlab@anarc.at>; Tue, 13 Mar 2018 20:30:59 -0400 (EDT)
+X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on marcos.anarc.at
+X-Spam-Level: 
+X-Spam-Status: No, score=-2.8 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,
+	DKIM_VALID,HTML_FONT_LOW_CONTRAST,HTML_MESSAGE,RCVD_IN_DNSWL_MED
+	autolearn=ham autolearn_force=no version=3.4.1
+Received: from deferred1.pod6.iad1.zdsys.com (deferred1.pod6.iad1.zdsys.com [192.161.153.116])
+	by mx.anarc.at (Postfix) with ESMTPS id 83B8F10E04F
+	for <anarcat+gitlab@anarc.at>; Tue, 13 Mar 2018 20:30:59 -0400 (EDT)
+Received: from out4.pod6.iad1.zdsys.com (out4.pod6.iad1.zdsys.com [192.161.153.103])
+	(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
+	(No client certificate requested)
+	by deferred1.pod6.iad1.zdsys.com (Postfix) with ESMTPS id D31684091244
+	for <anarcat+gitlab@anarc.at>; Wed, 14 Mar 2018 00:21:40 +0000 (UTC)
+Received: from out4.pod6.iad1.zdsys.com (localhost.localdomain [127.0.0.1])
+	by out4.pod6.iad1.zdsys.com (Postfix) with ESMTP id CEBCE21CBE26
+	for <anarcat+gitlab@anarc.at>; Wed, 14 Mar 2018 00:21:39 +0000 (UTC)
+Received: from zendesk.com (work11.pod6.iad1.zdsys.com [10.112.38.50])
+	by out4.pod6.iad1.zdsys.com (Postfix) with ESMTP id AFEB621CBE2F
+	for <anarcat+gitlab@anarc.at>; Wed, 14 Mar 2018 00:21:39 +0000 (UTC)
+Date: Wed, 14 Mar 2018 00:21:39 +0000
+From: GitLab Support <support@gitlab.com>
+Reply-To: GitLab Support <support@gitlab.com>
+To: Anarcat+gitlab <anarcat+gitlab@anarc.at>
+Message-ID: <9379QM5Z39_5aa86b1350504_174eb3fc97a2cb98c71674_sprut@zendesk.com>
+In-Reply-To: <9379QM5Z39@zendesk.com>
+ <9379QM5Z39_5aa86b134fcfb_174033fc97a2cb98c7198d_sprut@zendesk.com>
+Subject: comments not showing up?
+Mime-Version: 1.0
+Content-Type: multipart/alternative;
+ boundary="--==_mimepart_5aa86b13a739d_174eb3fc97a2cb98c71811";
+ charset=utf-8
+Content-Transfer-Encoding: 7bit
+X-Auto-Response-Suppress: All
+Auto-Submitted: auto-generated
+X-Mailer: Zendesk Mailer
+X-Delivery-Context: event-id-485367902188
+X-Zendesk-From-Account-Id: 2cbf2e0
+DKIM-Signature:  v=1; a=rsa-sha256; c=relaxed/relaxed; d=zendesk.com;
+ q=dns/txt; s=zendesk1; t=1520986899;
+ bh=ef6zoCM91gZVUja0Us+jy2UVcPK+sNhZocvn333kfzE=;
+ h=date:from:reply-to:to:message-id:in-reply-to:subject:mime-version:content-type:content-transfer-encoding;
+ b=tvPyIz3Cw61n5u2siOiyRiEMAKeDmEu2DMg1Ss534+0PPvbTgruWrWbZklJzy56RDIPi4hoK+Ui6gz0/ih6TyQXG6tpFMeZ4xI49Gqypu1Q2Xo1Uvu6WPYDe8n7D2BJ/8wP6+uqZ+DpAa7ldNi2opHVvmd6GKCuL0fN8lWvdDm4=
+DKIM-Signature:  v=1; a=rsa-sha256; c=relaxed/relaxed;
+ d=gmailmarkup.zendesk.com; q=dns/txt; s=zendesk1; t=1520986899;
+ bh=ef6zoCM91gZVUja0Us+jy2UVcPK+sNhZocvn333kfzE=;
+ h=date:from:reply-to:to:message-id:in-reply-to:subject:mime-version:content-type:content-transfer-encoding;
+ b=sqLp5VpKfrylgT2N7zbweDs3dccEXM44wokM/rxnZ49p9/wYDJNMbffB8yXXZa1BJ0KRfl/UFqoP8YZPYr72a+E291Ug+zq12UJi5MW2VnwMPJxAp+X9hQe90AzNecBDjOUn95qiCKnvVjhtT/LVePm9BbNh8UwC5W3qh/qFjVk=
+X-CMAE-Score: 0
+X-CMAE-Analysis: v=2.3 cv=RMOd4bq+ c=1 sm=1 tr=0
+	a=PnllObM1nxnwd3s59rm/oQ==:117 a=KiCxJD0x+Pe5VASQKmYoJrcyuOo=:19
+	a=IkcTkHD0fZMA:10 a=ZZnuYtJkoWoA:10 a=p0WdMEafAAAA:8
+	a=mvNKaxnlhnHt-7JDRIsA:9 a=QEXdDO2ut3YA:10 a=iCiO2nKxGhgA:10
+	a=AxTCswu-AAAA:8 a=A-Ay9Xv3AAAA:8 a=TeuQqM9sAAAA:8 a=SSmOFEACAAAA:8
+	a=9TiCOqs2P4ysbtUUcEZcEYoys8A=:19 a=DNiwoKe6oyfyDjaX:21 a=_W_S_7VecoQA:10
+	a=frz4AuCg-hUA:10 a=grImUnVaQDeKGm2TToIA:22
+X-AV-Checked: Zendesk using ClamAV - CLEAN
+X-CMAE-Score: 0
+X-CMAE-Analysis: v=2.3 cv=RMOd4bq+ c=1 sm=1 tr=0
+	a=WkljmVdYkabdwxfqvArNOQ==:117 a=KiCxJD0x+Pe5VASQKmYoJrcyuOo=:19
+	a=IkcTkHD0fZMA:10 a=v2DPQv5-lfwA:10 a=ZZnuYtJkoWoA:10 a=p0WdMEafAAAA:8
+	a=mvNKaxnlhnHt-7JDRIsA:9 a=QEXdDO2ut3YA:10 a=iCiO2nKxGhgA:10
+	a=AxTCswu-AAAA:8 a=A-Ay9Xv3AAAA:8 a=TeuQqM9sAAAA:8 a=SSmOFEACAAAA:8
+	a=9TiCOqs2P4ysbtUUcEZcEYoys8A=:19 a=DNiwoKe6oyfyDjaX:21 a=_W_S_7VecoQA:10
+	a=frz4AuCg-hUA:10 a=grImUnVaQDeKGm2TToIA:22
+Content-Length: 4613
+
+
+----==_mimepart_5aa86b13a739d_174eb3fc97a2cb98c71811
+Content-Type: text/plain;
+ charset=utf-8
+Content-Transfer-Encoding: quoted-printable
+
+##- Please type your reply above this line -##
+
+----------------------------------------------
+
+Anarcat+gitlab, Mar 13, 20:21 EDT
+
+in [this issue](https://gitlab.com/anarcat/wallabako/issues/15), there do=
+esn't seem to be any comments. yet you can see there is more than one par=
+ticipant on the sidebar, and, in the [issue listing](https://gitlab.com/)=
+ it actually says the issue has 6 comments.
+
+where did those comments go?
+
+--------------------------------
+This email is a service from GitLab, Inc..
+
+
+
+
+
+
+
+
+
+[9379QM-5Z39]=
+
+----==_mimepart_5aa86b13a739d_174eb3fc97a2cb98c71811
+Content-Type: text/html;
+ charset=utf-8
+Content-Transfer-Encoding: quoted-printable
+
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://ww=
+w.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+<html>
+<head>
+  <meta http-equiv=3D"Content-Type" content=3D"text/html; charset=3Dutf-8=
+" />
+  <style type=3D"text/css">
+    table td {
+      border-collapse: collapse;
+    }
+  </style>
+</head>
+<body style=3D"width: 100%!important; margin: 0; padding: 0;">
+  <div style=3D"padding: 10px ; line-height: 18px; font-family: 'Lucida G=
+rande',Verdana,Arial,sans-serif; font-size: 12px; color:#444444;">
+    <div style=3D"color: #b5b5b5;">##- Please type your reply above this =
+line -##</div>
+    <div style=3D"margin-top: 25px" data-version=3D"2"><table width=3D"10=
+0%" cellpadding=3D"0" cellspacing=3D"0" border=3D"0">  <tr>    <td width=3D=
+"100%" style=3D"padding: 15px 0; border-top: 1px dotted #c5c5c5;">      <=
+table width=3D"100%" cellpadding=3D"0" cellspacing=3D"0" border=3D"0" sty=
+le=3D"table-layout:fixed;">        <tr>                      <td valign=3D=
+"top" style=3D"padding: 0 15px 0 15px; width: 40px;">              <img a=
+lt=3D"Anarcat+gi" height=3D"40" src=3D"https://secure.gravatar.com/avatar=
+/35b1eb000c098a531e6f3cc6886fe2d2?size=3D40&amp;default=3Dhttps%3A%2F%2Fa=
+ssets.zendesk.com%2Fimages%2F2016%2Fdefault-avatar-80.png&amp;r=3Dg" styl=
+e=3D"height: auto; line-height: 100%; outline: none; text-decoration: non=
+e; -webkit-border-radius: 5px; -moz-border-radius: 5px; border-radius: 5p=
+x;" width=3D"40" />            </td>                    <td width=3D"100%=
+" style=3D"padding: 0; margin: 0;" valign=3D"top">            <p style=3D=
+"font-family: 'Lucida Grande','Lucida Sans Unicode','Lucida Sans',Verdana=
+,Tahoma,sans-serif; font-size: 15px; line-height: 18px; margin-bottom: 0;=
+ margin-top: 0; padding: 0; color:#1b1d1e;">                             =
+ <strong>Anarcat+gitlab</strong>                          </p>           =
+ <p style=3D"font-family: 'Lucida Grande','Lucida Sans Unicode','Lucida S=
+ans',Verdana,Tahoma,sans-serif; font-size: 13px; line-height: 25px; margi=
+n-bottom: 15px; margin-top: 0; padding: 0; color:#bbbbbb;">              =
+Mar 13, 20:21 EDT            </p>                                    <div=
+ class=3D"zd-comment" style=3D"color: #2b2e2f; font-family: 'Lucida Sans =
+Unicode', 'Lucida Grande', 'Tahoma', Verdana, sans-serif; font-size: 14px=
+; line-height: 22px; margin: 15px 0"><p style=3D"color: #2b2e2f; font-fam=
+ily: 'Lucida Sans Unicode', 'Lucida Grande', 'Tahoma', Verdana, sans-seri=
+f; font-size: 14px; line-height: 22px; margin: 15px 0" dir=3D"auto">in [t=
+his issue](<a href=3D"https://gitlab.com/anarcat/wallabako/issues/15" rel=
+=3D"nofollow noreferrer" target=3D"_blank">https://gitlab.com/anarcat/wal=
+labako/issues/15</a>), there doesn't seem to be any comments. yet you can=
+ see there is more than one participant on the sidebar, and, in the [issu=
+e listing](<a href=3D"https://gitlab.com/" rel=3D"nofollow noreferrer" ta=
+rget=3D"_blank">https://gitlab.com/</a>) it actually says the issue has 6=
+ comments.</p><p style=3D"color: #2b2e2f; font-family: 'Lucida Sans Unico=
+de', 'Lucida Grande', 'Tahoma', Verdana, sans-serif; font-size: 14px; lin=
+e-height: 22px; margin: 15px 0" dir=3D"auto">where did those comments go?=
+</p></div>                                  </td>        </tr>      </tab=
+le>    </td>  </tr></table></div>
+  </div>
+<span style=3D'color:#FFFFFF'>[9379QM-5Z39]</span></body>
+</html>
+<div itemscope itemtype=3D"http://schema.org/EmailMessage" style=3D"displ=
+ay:none">  <div itemprop=3D"action" itemscope itemtype=3D"http://schema.o=
+rg/ViewAction">    <link itemprop=3D"url" href=3D"https://support.gitlab.=
+com/hc/requests/92542" />    <meta itemprop=3D"name" content=3D"View tick=
+et"/>  </div></div>=
+
+----==_mimepart_5aa86b13a739d_174eb3fc97a2cb98c71811--
diff --git a/test/corpora/broken/gitlab/cur/1521463753.R9368947314807690338.curie:2,FS b/test/corpora/broken/gitlab/cur/1521463753.R9368947314807690338.curie:2,FS
new file mode 100644
index 00000000..4e50a7a0
--- /dev/null
+++ b/test/corpora/broken/gitlab/cur/1521463753.R9368947314807690338.curie:2,FS
@@ -0,0 +1,180 @@
+Return-Path: <support@gitlab.com>
+X-Original-To: anarcat+gitlab@anarc.at
+Delivered-To: anarcat+gitlab@anarc.at
+Received: from marcos.anarc.at (localhost [127.0.0.1])
+	by delivery.anarc.at (Postfix) with ESMTP id 14A5310E050
+	for <anarcat+gitlab@anarc.at>; Tue, 13 Mar 2018 20:27:37 -0400 (EDT)
+X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on marcos.anarc.at
+X-Spam-Level: 
+X-Spam-Status: No, score=-1.5 required=5.0 tests=BAYES_50,DKIM_SIGNED,
+	DKIM_VALID,HTML_FONT_LOW_CONTRAST,HTML_MESSAGE,RCVD_IN_DNSWL_MED
+	autolearn=ham autolearn_force=no version=3.4.1
+X-Greylist: delayed 356 seconds by postgrey-1.36 at marcos; Tue, 13 Mar 2018 20:27:36 EDT
+Received: from deferred4.pod6.iad1.zdsys.com (deferred4.pod6.iad1.zdsys.com [192.161.153.119])
+	by mx.anarc.at (Postfix) with ESMTPS id F22C310E04F
+	for <anarcat+gitlab@anarc.at>; Tue, 13 Mar 2018 20:27:36 -0400 (EDT)
+Received: from out1.pod6.iad1.zdsys.com (out1.pod6.iad1.zdsys.com [192.161.153.100])
+	(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
+	(No client certificate requested)
+	by deferred4.pod6.iad1.zdsys.com (Postfix) with ESMTPS id C448461A2A01
+	for <anarcat+gitlab@anarc.at>; Wed, 14 Mar 2018 00:21:40 +0000 (UTC)
+Received: from out1.pod6.iad1.zdsys.com (localhost.localdomain [127.0.0.1])
+	by out1.pod6.iad1.zdsys.com (Postfix) with ESMTP id BACEC2027796
+	for <anarcat+gitlab@anarc.at>; Wed, 14 Mar 2018 00:21:39 +0000 (UTC)
+Received: from zendesk.com (work11.pod6.iad1.zdsys.com [10.112.38.50])
+	by out1.pod6.iad1.zdsys.com (Postfix) with ESMTP id 9BB1720002E5
+	for <anarcat+gitlab@anarc.at>; Wed, 14 Mar 2018 00:21:39 +0000 (UTC)
+Date: Wed, 14 Mar 2018 00:21:39 +0000
+From: GitLab Support <support@gitlab.com>
+Reply-To: GitLab Support <support@gitlab.com>
+To: Anarcat+gitlab <anarcat+gitlab@anarc.at>
+Message-ID: <9379QM5Z39_5aa86b134fcfb_174033fc97a2cb98c7198d_sprut@zendesk.com>
+In-Reply-To: <9379QM5Z39@zendesk.com>
+ <9379QM5Z39_5aa86b1350504_174eb3fc97a2cb98c71674_sprut@zendesk.com>
+Subject: Your GitLab support request has been received
+Mime-Version: 1.0
+Content-Type: multipart/alternative;
+ boundary="--==_mimepart_5aa86b1391a39_174033fc97a2cb98c72164";
+ charset=utf-8
+Content-Transfer-Encoding: 7bit
+X-Auto-Response-Suppress: All
+Auto-Submitted: auto-generated
+X-Mailer: Zendesk Mailer
+X-Delivery-Context: event-id-485367902228
+X-Zendesk-From-Account-Id: 2cbf2e0
+DKIM-Signature:  v=1; a=rsa-sha256; c=relaxed/relaxed; d=zendesk.com;
+ q=dns/txt; s=zendesk1; t=1520986899;
+ bh=3+2uGgphLJeV0hFB5SjWTxUaMpTILdwsqdcMyqiNU5g=;
+ h=date:from:reply-to:to:message-id:in-reply-to:subject:mime-version:content-type:content-transfer-encoding;
+ b=LcMwrW9N89j0zoa6+NevVVKPVyx5k5o4jvJlenwPQKPDF7i8M8Jpf+Olx+VFna5eEkV0xlFtLFdgYrGdZ6kVOSjBOjW58a1rxs3Xdgn300VG0dVx9dH//CdAg7sb3f7EMIPF4nBE7ororf+yvceDIY2XIdDHTyJiRk629RX5Q+Q=
+DKIM-Signature:  v=1; a=rsa-sha256; c=relaxed/relaxed;
+ d=gmailmarkup.zendesk.com; q=dns/txt; s=zendesk1; t=1520986899;
+ bh=3+2uGgphLJeV0hFB5SjWTxUaMpTILdwsqdcMyqiNU5g=;
+ h=date:from:reply-to:to:message-id:in-reply-to:subject:mime-version:content-type:content-transfer-encoding;
+ b=KA/qHvi70gJ/IjsrgG+NWB4BJ9i+QUTYCk8hSPFfG/AHb1dXldrezjyGgy13g85VGrtQmzLdj2bpFpH3gHDuKY9nvbNMepj8WhogeapUsuaqYlQxHtX2HnvBbsbOv5xTYz+uVlQBkFuvRUim8P64eFvpXdsk6eqXZCWPUoDmRBI=
+X-CMAE-Score: 0
+X-CMAE-Analysis: v=2.3 cv=DJShHRFb c=1 sm=1 tr=0
+	a=PnllObM1nxnwd3s59rm/oQ==:117 a=KiCxJD0x+Pe5VASQKmYoJrcyuOo=:19
+	a=IkcTkHD0fZMA:10 a=ZZnuYtJkoWoA:10 a=p0WdMEafAAAA:8 a=A-Ay9Xv3AAAA:8
+	a=zAaIuZ-tlQE1c5dlMioA:9 a=QEXdDO2ut3YA:10 a=TeuQqM9sAAAA:8
+	a=SSmOFEACAAAA:8 a=9TiCOqs2P4ysbtUUcEZcEYoys8A=:19 a=5vMuza9J3cUQAzhx:21
+	a=_W_S_7VecoQA:10 a=frz4AuCg-hUA:10
+X-AV-Checked: Zendesk using ClamAV - CLEAN
+X-CMAE-Score: 0
+X-CMAE-Analysis: v=2.3 cv=DJShHRFb c=1 sm=1 tr=0
+	a=WkljmVdYkabdwxfqvArNOQ==:117 a=KiCxJD0x+Pe5VASQKmYoJrcyuOo=:19
+	a=IkcTkHD0fZMA:10 a=v2DPQv5-lfwA:10 a=ZZnuYtJkoWoA:10 a=p0WdMEafAAAA:8
+	a=A-Ay9Xv3AAAA:8 a=zAaIuZ-tlQE1c5dlMioA:9 a=QEXdDO2ut3YA:10
+	a=TeuQqM9sAAAA:8 a=SSmOFEACAAAA:8 a=9TiCOqs2P4ysbtUUcEZcEYoys8A=:19
+	a=5vMuza9J3cUQAzhx:21 a=_W_S_7VecoQA:10 a=frz4AuCg-hUA:10
+Content-Length: 4768
+
+
+----==_mimepart_5aa86b1391a39_174033fc97a2cb98c72164
+Content-Type: text/plain;
+ charset=utf-8
+Content-Transfer-Encoding: quoted-printable
+
+##- Please type your reply above this line -##
+
+Hi Anarcat+gitlab,
+
+Thank you for contacting GitLab Support. You can reply to this email at a=
+ny time to add new information to the ticket. While you wait, you may fin=
+d helpful information in our documentation at https://docs.gitlab.com. =
+
+
+You opened this ticket by sending us an email. You can also create ticket=
+s via our web form at https://support.gitlab.com/hc/en-us/requests/new. T=
+he form is tailored to the product you're using and helps ensure our supp=
+ort team has all the details necessary to understand your problem. =
+
+
+We look forward to helping you resolve your request shortly!
+
+Best regards,
+The GitLab team
+
+Did you know? You can keep track of all of your tickets and their current=
+ status using our support web interface! Visit https://support.gitlab.com=
+ and sign in. You can also go directly to details about this ticket at ht=
+tps://support.gitlab.com/hc/requests/92542. We recommend using the suppor=
+t web interface for a superior experience managing your tickets. Email co=
+mments can be difficult to follow.
+
+Don't know your support account password? By emailing us, an account was =
+pre-created for you but you will need to reset your password first. Reque=
+st a new password at https://gitlab.zendesk.com/auth/v2/login/password_re=
+set. Follow the instructions in the password reset email to gain access t=
+o your support account. Now you will be able to see all of your tickets!
+
+--------------------------------
+This email is a service from GitLab, Inc..
+
+
+
+
+
+
+
+
+
+[9379QM-5Z39]=
+
+----==_mimepart_5aa86b1391a39_174033fc97a2cb98c72164
+Content-Type: text/html;
+ charset=utf-8
+Content-Transfer-Encoding: quoted-printable
+
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://ww=
+w.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+<html>
+<head>
+  <meta http-equiv=3D"Content-Type" content=3D"text/html; charset=3Dutf-8=
+" />
+  <style type=3D"text/css">
+    table td {
+      border-collapse: collapse;
+    }
+  </style>
+</head>
+<body style=3D"width: 100%!important; margin: 0; padding: 0;">
+  <div style=3D"padding: 10px ; line-height: 18px; font-family: 'Lucida G=
+rande',Verdana,Arial,sans-serif; font-size: 12px; color:#444444;">
+    <div style=3D"color: #b5b5b5;">##- Please type your reply above this =
+line -##</div>
+    <p>Hi Anarcat+gitlab,</p><p>Thank you for contacting GitLab Support. =
+You can reply to this email at any time to add new information to the tic=
+ket. While you wait, you may find helpful information in our documentatio=
+n at <a href=3D"https://docs.gitlab.com" rel=3D"noreferrer">https://docs.=
+gitlab.com</a>. </p><p>You opened this ticket by sending us an email. You=
+ can also create tickets via our web form at <a href=3D"https://support.g=
+itlab.com/hc/en-us/requests/new" rel=3D"noreferrer">https://support.gitla=
+b.com/hc/en-us/requests/new</a>. The form is tailored to the product you'=
+re using and helps ensure our support team has all the details necessary =
+to understand your problem. </p><p>We look forward to helping you resolve=
+ your request shortly!</p><p>Best regards,<br />The GitLab team</p><p>Did=
+ you know? You can keep track of all of your tickets and their current st=
+atus using our support web interface! Visit <a href=3D"https://support.gi=
+tlab.com" rel=3D"noreferrer">https://support.gitlab.com</a> and sign in. =
+You can also go directly to details about this ticket at <a href=3D"https=
+://support.gitlab.com/hc/requests/92542" rel=3D"noreferrer">https://suppo=
+rt.gitlab.com/hc/requests/92542</a>. We recommend using the support web i=
+nterface for a superior experience managing your tickets. Email comments =
+can be difficult to follow.</p><p>Don't know your support account passwor=
+d? By emailing us, an account was pre-created for you but you will need t=
+o reset your password first. Request a new password at <a href=3D"https:/=
+/gitlab.zendesk.com/auth/v2/login/password_reset" rel=3D"noreferrer">http=
+s://gitlab.zendesk.com/auth/v2/login/password_reset</a>. Follow the instr=
+uctions in the password reset email to gain access to your support accoun=
+t. Now you will be able to see all of your tickets!</p>
+  </div>
+<span style=3D'color:#FFFFFF'>[9379QM-5Z39]</span></body>
+</html>
+<div itemscope itemtype=3D"http://schema.org/EmailMessage" style=3D"displ=
+ay:none">  <div itemprop=3D"action" itemscope itemtype=3D"http://schema.o=
+rg/ViewAction">    <link itemprop=3D"url" href=3D"https://support.gitlab.=
+com/hc/requests/92542" />    <meta itemprop=3D"name" content=3D"View tick=
+et"/>  </div></div>=
+
+----==_mimepart_5aa86b1391a39_174033fc97a2cb98c72164--
-- 
2.17.0

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

* [PATCH 2/5] test: add known broken test for indexing an In-Reply-To loop.
  2018-04-14  1:46 reference loop fix and tests, v2 David Bremner
  2018-04-14  1:46 ` [PATCH 1/5] test: two new messages for the 'broken' corpus David Bremner
@ 2018-04-14  1:46 ` David Bremner
  2018-04-14  1:46 ` [PATCH 3/5] lib: break reference loop by choosing arbitrary top level msg David Bremner
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: David Bremner @ 2018-04-14  1:46 UTC (permalink / raw)
  To: notmuch

This documents the bug discussed in

     id:87d10042pu.fsf@curie.anarc.at
---
 test/T050-new.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/test/T050-new.sh b/test/T050-new.sh
index cd522364..c55a2d97 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -354,4 +354,9 @@ exit status: 75
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+add_email_corpus broken
+test_begin_subtest "reference loop"
+test_subtest_known_broken
+test_expect_code 0 "notmuch show --format=json id:9379QM5Z39_5aa86b134fcfb_174033fc97a2cb98c7198d_sprut@zendesk.com > OUTPUT"
+
 test_done
-- 
2.17.0

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

* [PATCH 3/5] lib: break reference loop by choosing arbitrary top level msg
  2018-04-14  1:46 reference loop fix and tests, v2 David Bremner
  2018-04-14  1:46 ` [PATCH 1/5] test: two new messages for the 'broken' corpus David Bremner
  2018-04-14  1:46 ` [PATCH 2/5] test: add known broken test for indexing an In-Reply-To loop David Bremner
@ 2018-04-14  1:46 ` David Bremner
  2018-04-15  7:38   ` Daniel Kahn Gillmor
  2018-04-14  1:46 ` [PATCH 4/5] test: add second reference loop test David Bremner
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: David Bremner @ 2018-04-14  1:46 UTC (permalink / raw)
  To: notmuch

Other parts of notmuch (e.g. notmuch show) expect each thread to
contain at least one top level message, and crash if this expectation
is not met.
---
 lib/thread.cc    | 8 +++++++-
 test/T050-new.sh | 1 -
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/thread.cc b/lib/thread.cc
index 3561b27f..dbac002f 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -397,7 +397,13 @@ _resolve_thread_relationships (notmuch_thread_t *thread)
     for (node = thread->message_list->head; node; node = node->next) {
 	message = node->message;
 	in_reply_to = _notmuch_message_get_in_reply_to (message);
-	if (in_reply_to && strlen (in_reply_to) &&
+	/*
+	 * if we reach the end of the list without finding a top-level
+	 * message, that means the thread is a cycle (or set of
+	 * cycles) and any message can be considered top-level
+	 */
+	if ((thread->toplevel_list->head || node->next) &&
+	     in_reply_to && strlen (in_reply_to) &&
 	    g_hash_table_lookup_extended (thread->message_hash,
 					  in_reply_to, NULL,
 					  (void **) &parent))
diff --git a/test/T050-new.sh b/test/T050-new.sh
index c55a2d97..222c341e 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -356,7 +356,6 @@ test_expect_equal_file EXPECTED OUTPUT
 
 add_email_corpus broken
 test_begin_subtest "reference loop"
-test_subtest_known_broken
 test_expect_code 0 "notmuch show --format=json id:9379QM5Z39_5aa86b134fcfb_174033fc97a2cb98c7198d_sprut@zendesk.com > OUTPUT"
 
 test_done
-- 
2.17.0

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

* [PATCH 4/5] test: add second reference loop test
  2018-04-14  1:46 reference loop fix and tests, v2 David Bremner
                   ` (2 preceding siblings ...)
  2018-04-14  1:46 ` [PATCH 3/5] lib: break reference loop by choosing arbitrary top level msg David Bremner
@ 2018-04-14  1:46 ` David Bremner
  2018-04-14  1:46 ` [PATCH 5/5] NEWS: add item for reference loop fix David Bremner
  2018-04-25 20:41 ` reference loop fix and tests, v2 David Bremner
  5 siblings, 0 replies; 20+ messages in thread
From: David Bremner @ 2018-04-14  1:46 UTC (permalink / raw)
  To: notmuch

Guard against regressions where there is no crash, but output is
wrong.
---
 test/T050-new.sh | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/test/T050-new.sh b/test/T050-new.sh
index 222c341e..91722e24 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -355,7 +355,16 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 add_email_corpus broken
-test_begin_subtest "reference loop"
+test_begin_subtest "reference loop does not crash"
 test_expect_code 0 "notmuch show --format=json id:9379QM5Z39_5aa86b134fcfb_174033fc97a2cb98c7198d_sprut@zendesk.com > OUTPUT"
 
+test_begin_subtest "reference loop outputs both messages"
+threadid=$(notmuch search --output=threads  id:9379QM5Z39_5aa86b134fcfb_174033fc97a2cb98c7198d_sprut@zendesk.com)
+notmuch show --format=mbox $threadid | grep '^Message-ID' | sort > OUTPUT
+cat <<EOF
+Message-ID: <9379QM5Z39_5aa86b134fcfb_174033fc97a2cb98c7198d_sprut@zendesk.com>
+Message-ID: <9379QM5Z39_5aa86b1350504_174eb3fc97a2cb98c71674_sprut@zendesk.com>
+EOF
+test_expect_equal_file /dev/null OUTPUT
+
 test_done
-- 
2.17.0

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

* [PATCH 5/5] NEWS: add item for reference loop fix.
  2018-04-14  1:46 reference loop fix and tests, v2 David Bremner
                   ` (3 preceding siblings ...)
  2018-04-14  1:46 ` [PATCH 4/5] test: add second reference loop test David Bremner
@ 2018-04-14  1:46 ` David Bremner
  2018-04-20 18:31   ` v3 " David Bremner
  2018-04-25 20:41 ` reference loop fix and tests, v2 David Bremner
  5 siblings, 1 reply; 20+ messages in thread
From: David Bremner @ 2018-04-14  1:46 UTC (permalink / raw)
  To: notmuch

---
 NEWS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/NEWS b/NEWS
index 39ce7707..e9b1dcd9 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,12 @@
+Notmuch 0.26.2 (UNRELEASED)
+===========================
+
+Library Changes
+---------------
+
+Make thread indexing more robust against reference loops. Fix a
+related abort in "notmuch show".
+
 Notmuch 0.26.1 (2018-04-02)
 ===========================
 
-- 
2.17.0

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

* Re: [PATCH 1/5] test: two new messages for the 'broken' corpus
  2018-04-14  1:46 ` [PATCH 1/5] test: two new messages for the 'broken' corpus David Bremner
@ 2018-04-14 22:24   ` Tomi Ollila
  2018-04-15  7:36     ` Daniel Kahn Gillmor
  0 siblings, 1 reply; 20+ messages in thread
From: Tomi Ollila @ 2018-04-14 22:24 UTC (permalink / raw)
  To: David Bremner, notmuch

On Fri, Apr 13 2018, David Bremner wrote:

> These have an 'In-Reply-To' loop, which currently confuses "notmuch
> new".
>
> Courtesy of anarcat.

Instead the huge amount of noise in these messages, I suggest:

(copied test/corpora/broken/broken-cc as base)

It is so late today but I might test these tomorrow.

--8<----8<----8<----8<----8<----8<----8<----8<----8<--
From: Alice <alice@example.org>
To: Daniel <daniel@example.org>
Subject: referencing in-reply-to-loop-21
Message-ID: <mid-loop-12@example.org>
In-Reply-To: <mid-loop-21@example.org>
Date: Thu, 16 Jun 2016 22:14:41 -0400

Note Message-ID and In-Reply-To: in file in-reply-to-loop-21
--8<----8<----8<----8<----8<----8<----8<----8<----8<--

--8<----8<----8<----8<----8<----8<----8<----8<----8<--
From: Alice <alice@example.org>
To: Daniel <daniel@example.org>
Subject: referencing in-reply-to-loop-12
Message-ID: <mid-loop-21@example.org>
In-Reply-To: <mid-loop-12@example.org>
Date: Thu, 16 Jun 2016 22:14:41 -0400

Note Message-ID and In-Reply-To: in file in-reply-to-loop-12
--8<----8<----8<----8<----8<----8<----8<----8<----8<--


Not because the files are too large for the computer, but the
all the rest make it harder for human observer to compare...


Tomi

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

* Re: [PATCH 1/5] test: two new messages for the 'broken' corpus
  2018-04-14 22:24   ` Tomi Ollila
@ 2018-04-15  7:36     ` Daniel Kahn Gillmor
  0 siblings, 0 replies; 20+ messages in thread
From: Daniel Kahn Gillmor @ 2018-04-15  7:36 UTC (permalink / raw)
  To: Tomi Ollila, David Bremner, notmuch

On Sun 2018-04-15 01:24:03 +0300, Tomi Ollila wrote:
> Instead the huge amount of noise in these messages, I suggest:

I agree with Tomi's suggestion that the simplest possible messages are
the best for testing (i haven't tried either of them yet myself though).

    --dkg

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

* Re: [PATCH 3/5] lib: break reference loop by choosing arbitrary top level msg
  2018-04-14  1:46 ` [PATCH 3/5] lib: break reference loop by choosing arbitrary top level msg David Bremner
@ 2018-04-15  7:38   ` Daniel Kahn Gillmor
  0 siblings, 0 replies; 20+ messages in thread
From: Daniel Kahn Gillmor @ 2018-04-15  7:38 UTC (permalink / raw)
  To: David Bremner, notmuch

On Fri 2018-04-13 22:46:08 -0300, David Bremner wrote:
> Other parts of notmuch (e.g. notmuch show) expect each thread to
> contain at least one top level message, and crash if this expectation
> is not met.
> ---
>  lib/thread.cc    | 8 +++++++-
>  test/T050-new.sh | 1 -
>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/lib/thread.cc b/lib/thread.cc
> index 3561b27f..dbac002f 100644
> --- a/lib/thread.cc
> +++ b/lib/thread.cc
> @@ -397,7 +397,13 @@ _resolve_thread_relationships (notmuch_thread_t *thread)
>      for (node = thread->message_list->head; node; node = node->next) {
>  	message = node->message;
>  	in_reply_to = _notmuch_message_get_in_reply_to (message);
> -	if (in_reply_to && strlen (in_reply_to) &&
> +	/*
> +	 * if we reach the end of the list without finding a top-level
> +	 * message, that means the thread is a cycle (or set of
> +	 * cycles) and any message can be considered top-level
> +	 */

Just how arbitrary should we be?  Do we want it to be non-deterministic?

If we care about determinism, i'd recommend selecting the message with
the earliest date as the top-level message, and if multiple messages
have the same date, we should sort by Message-ID.

     --dkg

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

* v3 reference loop fix
  2018-04-14  1:46 ` [PATCH 5/5] NEWS: add item for reference loop fix David Bremner
@ 2018-04-20 18:31   ` David Bremner
  2018-04-20 18:31     ` [PATCH 1/6] test: two new messages for the 'broken' corpus David Bremner
                       ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: David Bremner @ 2018-04-20 18:31 UTC (permalink / raw)
  To: notmuch, David Bremner

this version obsoletes

     id:20180414014610.15438-1-david@tethera.net

changes:

        - simplified test messages from Tomi
        - deterministic thread order suggested by dkg

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

* [PATCH 1/6] test: two new messages for the 'broken' corpus
  2018-04-20 18:31   ` v3 " David Bremner
@ 2018-04-20 18:31     ` David Bremner
  2018-04-20 18:31     ` [PATCH 2/6] test: add known broken test for indexing an In-Reply-To loop David Bremner
                       ` (4 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: David Bremner @ 2018-04-20 18:31 UTC (permalink / raw)
  To: notmuch, David Bremner

These have an 'In-Reply-To' loop, which currently confuses "notmuch
new".
---
 test/corpora/broken/loop/loop-12 | 8 ++++++++
 test/corpora/broken/loop/loop-21 | 8 ++++++++
 2 files changed, 16 insertions(+)
 create mode 100644 test/corpora/broken/loop/loop-12
 create mode 100644 test/corpora/broken/loop/loop-21

diff --git a/test/corpora/broken/loop/loop-12 b/test/corpora/broken/loop/loop-12
new file mode 100644
index 00000000..b5c3af7e
--- /dev/null
+++ b/test/corpora/broken/loop/loop-12
@@ -0,0 +1,8 @@
+From: Alice <alice@example.org>
+To: Daniel <daniel@example.org>
+Subject: referencing in-reply-to-loop-21
+Message-ID: <mid-loop-12@example.org>
+In-Reply-To: <mid-loop-21@example.org>
+Date: Thu, 16 Jun 2016 22:14:41 -0400
+
+Note Message-ID and In-Reply-To: in file in-reply-to-loop-21
diff --git a/test/corpora/broken/loop/loop-21 b/test/corpora/broken/loop/loop-21
new file mode 100644
index 00000000..234f0323
--- /dev/null
+++ b/test/corpora/broken/loop/loop-21
@@ -0,0 +1,8 @@
+From: Alice <alice@example.org>
+To: Daniel <daniel@example.org>
+Subject: referencing in-reply-to-loop-12
+Message-ID: <mid-loop-21@example.org>
+In-Reply-To: <mid-loop-12@example.org>
+Date: Fri, 17 Jun 2016 22:14:41 -0400
+
+Note Message-ID and In-Reply-To: in file in-reply-to-loop-12
-- 
2.17.0

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

* [PATCH 2/6] test: add known broken test for indexing an In-Reply-To loop.
  2018-04-20 18:31   ` v3 " David Bremner
  2018-04-20 18:31     ` [PATCH 1/6] test: two new messages for the 'broken' corpus David Bremner
@ 2018-04-20 18:31     ` David Bremner
  2018-04-20 18:31     ` [PATCH 3/6] lib: break reference loop by choosing arbitrary top level msg David Bremner
                       ` (3 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: David Bremner @ 2018-04-20 18:31 UTC (permalink / raw)
  To: notmuch, David Bremner

This documents the bug discussed in

     id:87d10042pu.fsf@curie.anarc.at
---
 test/T050-new.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/test/T050-new.sh b/test/T050-new.sh
index cd522364..b9854978 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -354,4 +354,9 @@ exit status: 75
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+add_email_corpus broken
+test_begin_subtest "reference loop does not crash"
+test_subtest_known_broken
+test_expect_code 0 "notmuch show --format=json id:mid-loop-12@example.org id:mid-loop-21@example.org > OUTPUT"
+
 test_done
-- 
2.17.0

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

* [PATCH 3/6] lib: break reference loop by choosing arbitrary top level msg
  2018-04-20 18:31   ` v3 " David Bremner
  2018-04-20 18:31     ` [PATCH 1/6] test: two new messages for the 'broken' corpus David Bremner
  2018-04-20 18:31     ` [PATCH 2/6] test: add known broken test for indexing an In-Reply-To loop David Bremner
@ 2018-04-20 18:31     ` David Bremner
  2018-04-20 18:31     ` [PATCH 4/6] NEWS: add item for reference loop fix David Bremner
                       ` (2 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: David Bremner @ 2018-04-20 18:31 UTC (permalink / raw)
  To: notmuch, David Bremner

Other parts of notmuch (e.g. notmuch show) expect each thread to
contain at least one top level message, and crash if this expectation
is not met.
---
 lib/thread.cc    | 8 +++++++-
 test/T050-new.sh | 1 -
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/thread.cc b/lib/thread.cc
index 3561b27f..dbac002f 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -397,7 +397,13 @@ _resolve_thread_relationships (notmuch_thread_t *thread)
     for (node = thread->message_list->head; node; node = node->next) {
 	message = node->message;
 	in_reply_to = _notmuch_message_get_in_reply_to (message);
-	if (in_reply_to && strlen (in_reply_to) &&
+	/*
+	 * if we reach the end of the list without finding a top-level
+	 * message, that means the thread is a cycle (or set of
+	 * cycles) and any message can be considered top-level
+	 */
+	if ((thread->toplevel_list->head || node->next) &&
+	     in_reply_to && strlen (in_reply_to) &&
 	    g_hash_table_lookup_extended (thread->message_hash,
 					  in_reply_to, NULL,
 					  (void **) &parent))
diff --git a/test/T050-new.sh b/test/T050-new.sh
index b9854978..f3bfe7b1 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -356,7 +356,6 @@ test_expect_equal_file EXPECTED OUTPUT
 
 add_email_corpus broken
 test_begin_subtest "reference loop does not crash"
-test_subtest_known_broken
 test_expect_code 0 "notmuch show --format=json id:mid-loop-12@example.org id:mid-loop-21@example.org > OUTPUT"
 
 test_done
-- 
2.17.0

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

* [PATCH 4/6] NEWS: add item for reference loop fix.
  2018-04-20 18:31   ` v3 " David Bremner
                       ` (2 preceding siblings ...)
  2018-04-20 18:31     ` [PATCH 3/6] lib: break reference loop by choosing arbitrary top level msg David Bremner
@ 2018-04-20 18:31     ` David Bremner
  2018-04-20 18:31     ` [PATCH 5/6] test: add known broken test for thread ordering from a loop David Bremner
  2018-04-20 18:31     ` [PATCH 6/6] lib: choose oldest message when breaking reference loops David Bremner
  5 siblings, 0 replies; 20+ messages in thread
From: David Bremner @ 2018-04-20 18:31 UTC (permalink / raw)
  To: notmuch, David Bremner

---
 NEWS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/NEWS b/NEWS
index 39ce7707..e9b1dcd9 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,12 @@
+Notmuch 0.26.2 (UNRELEASED)
+===========================
+
+Library Changes
+---------------
+
+Make thread indexing more robust against reference loops. Fix a
+related abort in "notmuch show".
+
 Notmuch 0.26.1 (2018-04-02)
 ===========================
 
-- 
2.17.0

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

* [PATCH 5/6] test: add known broken test for thread ordering from a loop
  2018-04-20 18:31   ` v3 " David Bremner
                       ` (3 preceding siblings ...)
  2018-04-20 18:31     ` [PATCH 4/6] NEWS: add item for reference loop fix David Bremner
@ 2018-04-20 18:31     ` David Bremner
  2018-04-20 18:31     ` [PATCH 6/6] lib: choose oldest message when breaking reference loops David Bremner
  5 siblings, 0 replies; 20+ messages in thread
From: David Bremner @ 2018-04-20 18:31 UTC (permalink / raw)
  To: notmuch, David Bremner

The previous loop handling code chooses the last message in the
message list, which turns out to be the last in date order.
See the comment in _notmuch_thread_create.
---
 test/T050-new.sh | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/test/T050-new.sh b/test/T050-new.sh
index f3bfe7b1..320a7646 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -358,4 +358,14 @@ add_email_corpus broken
 test_begin_subtest "reference loop does not crash"
 test_expect_code 0 "notmuch show --format=json id:mid-loop-12@example.org id:mid-loop-21@example.org > OUTPUT"
 
+test_begin_subtest "reference loop ordered by date"
+test_subtest_known_broken
+threadid=$(notmuch search --output=threads  id:mid-loop-12@example.org)
+notmuch show --format=mbox $threadid | grep '^Date'  > OUTPUT
+cat <<EOF > EXPECTED
+Date: Thu, 16 Jun 2016 22:14:41 -0400
+Date: Fri, 17 Jun 2016 22:14:41 -0400
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.17.0

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

* [PATCH 6/6] lib: choose oldest message when breaking reference loops
  2018-04-20 18:31   ` v3 " David Bremner
                       ` (4 preceding siblings ...)
  2018-04-20 18:31     ` [PATCH 5/6] test: add known broken test for thread ordering from a loop David Bremner
@ 2018-04-20 18:31     ` David Bremner
  2018-04-21 17:53       ` Tomi Ollila
  5 siblings, 1 reply; 20+ messages in thread
From: David Bremner @ 2018-04-20 18:31 UTC (permalink / raw)
  To: notmuch, David Bremner

This preserves a sensible thread order
---
 lib/thread.cc    | 33 ++++++++++++++++++++++++---------
 test/T050-new.sh |  1 -
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/lib/thread.cc b/lib/thread.cc
index dbac002f..a6dc4e5a 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -390,20 +390,18 @@ _thread_add_matched_message (notmuch_thread_t *thread,
 static void
 _resolve_thread_relationships (notmuch_thread_t *thread)
 {
-    notmuch_message_node_t *node;
+    notmuch_message_node_t *node, *first_node;
     notmuch_message_t *message, *parent;
     const char *in_reply_to;
 
-    for (node = thread->message_list->head; node; node = node->next) {
+    first_node = thread->message_list->head;
+    if (!first_node)
+	return;
+
+    for (node = first_node->next; node; node = node->next) {
 	message = node->message;
 	in_reply_to = _notmuch_message_get_in_reply_to (message);
-	/*
-	 * if we reach the end of the list without finding a top-level
-	 * message, that means the thread is a cycle (or set of
-	 * cycles) and any message can be considered top-level
-	 */
-	if ((thread->toplevel_list->head || node->next) &&
-	     in_reply_to && strlen (in_reply_to) &&
+	if (in_reply_to && strlen (in_reply_to) &&
 	    g_hash_table_lookup_extended (thread->message_hash,
 					  in_reply_to, NULL,
 					  (void **) &parent))
@@ -412,6 +410,23 @@ _resolve_thread_relationships (notmuch_thread_t *thread)
 	    _notmuch_message_list_add_message (thread->toplevel_list, message);
     }
 
+    /*
+     * if we reach the end of the list without finding a top-level
+     * message, that means the thread is a cycle (or set of cycles)
+     * and any message can be considered top-level.  Choose the oldest
+     * message, which happens to be first in our list.
+     */
+    message=first_node->message;
+    in_reply_to = _notmuch_message_get_in_reply_to (message);
+    if (thread->toplevel_list->head &&
+	in_reply_to && strlen (in_reply_to) &&
+	g_hash_table_lookup_extended (thread->message_hash,
+				      in_reply_to, NULL,
+				      (void **) &parent))
+	_notmuch_message_add_reply (parent, message);
+    else
+	_notmuch_message_list_add_message (thread->toplevel_list, message);
+
     /* XXX: After scanning through the entire list looking for parents
      * via "In-Reply-To", we should do a second pass that looks at the
      * list of messages IDs in the "References" header instead. (And
diff --git a/test/T050-new.sh b/test/T050-new.sh
index 320a7646..9025fa7a 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -359,7 +359,6 @@ test_begin_subtest "reference loop does not crash"
 test_expect_code 0 "notmuch show --format=json id:mid-loop-12@example.org id:mid-loop-21@example.org > OUTPUT"
 
 test_begin_subtest "reference loop ordered by date"
-test_subtest_known_broken
 threadid=$(notmuch search --output=threads  id:mid-loop-12@example.org)
 notmuch show --format=mbox $threadid | grep '^Date'  > OUTPUT
 cat <<EOF > EXPECTED
-- 
2.17.0

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

* Re: [PATCH 6/6] lib: choose oldest message when breaking reference loops
  2018-04-20 18:31     ` [PATCH 6/6] lib: choose oldest message when breaking reference loops David Bremner
@ 2018-04-21 17:53       ` Tomi Ollila
  2018-04-25  1:52         ` David Bremner
  0 siblings, 1 reply; 20+ messages in thread
From: Tomi Ollila @ 2018-04-21 17:53 UTC (permalink / raw)
  To: David Bremner, notmuch, David Bremner

On Fri, Apr 20 2018, David Bremner wrote:

> This preserves a sensible thread order
> ---
>  lib/thread.cc    | 33 ++++++++++++++++++++++++---------
>  test/T050-new.sh |  1 -
>  2 files changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/lib/thread.cc b/lib/thread.cc
> index dbac002f..a6dc4e5a 100644
> --- a/lib/thread.cc
> +++ b/lib/thread.cc
> @@ -390,20 +390,18 @@ _thread_add_matched_message (notmuch_thread_t *thread,
>  static void
>  _resolve_thread_relationships (notmuch_thread_t *thread)
>  {
> -    notmuch_message_node_t *node;
> +    notmuch_message_node_t *node, *first_node;
>      notmuch_message_t *message, *parent;
>      const char *in_reply_to;
>  
> -    for (node = thread->message_list->head; node; node = node->next) {
> +    first_node = thread->message_list->head;
> +    if (!first_node)

Our style *dictates* space after '!'.

> +	return;
> +
> +    for (node = first_node->next; node; node = node->next) {
>  	message = node->message;
>  	in_reply_to = _notmuch_message_get_in_reply_to (message);
> -	/*
> -	 * if we reach the end of the list without finding a top-level
> -	 * message, that means the thread is a cycle (or set of
> -	 * cycles) and any message can be considered top-level
> -	 */
> -	if ((thread->toplevel_list->head || node->next) &&
> -	     in_reply_to && strlen (in_reply_to) &&
> +	if (in_reply_to && strlen (in_reply_to) &&
>  	    g_hash_table_lookup_extended (thread->message_hash,
>  					  in_reply_to, NULL,
>  					  (void **) &parent))
> @@ -412,6 +410,23 @@ _resolve_thread_relationships (notmuch_thread_t *thread)
>  	    _notmuch_message_list_add_message (thread->toplevel_list, message);
>      }
>  
> +    /*
> +     * if we reach the end of the list without finding a top-level
> +     * message, that means the thread is a cycle (or set of cycles)
> +     * and any message can be considered top-level.  Choose the oldest
> +     * message, which happens to be first in our list.
> +     */
> +    message=first_node->message;
> +    in_reply_to = _notmuch_message_get_in_reply_to (message);
> +    if (thread->toplevel_list->head &&
> +	in_reply_to && strlen (in_reply_to) &&

Otherwise the series _looks_ good do me. The thing that disturbs me are
these `strlen (in_reply_to)` contents. Perhaps SomeOne(TM) changes these
to e.g. in_reply_to[0] in the future...

Tomi


> +	g_hash_table_lookup_extended (thread->message_hash,
> +				      in_reply_to, NULL,
> +				      (void **) &parent))
> +	_notmuch_message_add_reply (parent, message);
> +    else
> +	_notmuch_message_list_add_message (thread->toplevel_list, message);
> +
>      /* XXX: After scanning through the entire list looking for parents
>       * via "In-Reply-To", we should do a second pass that looks at the
>       * list of messages IDs in the "References" header instead. (And
> diff --git a/test/T050-new.sh b/test/T050-new.sh
> index 320a7646..9025fa7a 100755
> --- a/test/T050-new.sh
> +++ b/test/T050-new.sh
> @@ -359,7 +359,6 @@ test_begin_subtest "reference loop does not crash"
>  test_expect_code 0 "notmuch show --format=json id:mid-loop-12@example.org id:mid-loop-21@example.org > OUTPUT"
>  
>  test_begin_subtest "reference loop ordered by date"
> -test_subtest_known_broken
>  threadid=$(notmuch search --output=threads  id:mid-loop-12@example.org)
>  notmuch show --format=mbox $threadid | grep '^Date'  > OUTPUT
>  cat <<EOF > EXPECTED
> -- 
> 2.17.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

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

* Re: [PATCH 6/6] lib: choose oldest message when breaking reference loops
  2018-04-21 17:53       ` Tomi Ollila
@ 2018-04-25  1:52         ` David Bremner
  2018-04-25 11:48           ` Tomi Ollila
  0 siblings, 1 reply; 20+ messages in thread
From: David Bremner @ 2018-04-25  1:52 UTC (permalink / raw)
  To: Tomi Ollila, notmuch

Tomi Ollila <tomi.ollila@iki.fi> writes:

> Otherwise the series _looks_ good do me. The thing that disturbs me are
> these `strlen (in_reply_to)` contents. Perhaps SomeOne(TM) changes these
> to e.g. in_reply_to[0] in the future...
>

That same file defines and uses an EMPTY_STRING macro. We should
probably be consistent, either use that macro everywehre or
nowhere. Perhaps move it to lib/notmuch-private.h?

d

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

* Re: [PATCH 6/6] lib: choose oldest message when breaking reference loops
  2018-04-25  1:52         ` David Bremner
@ 2018-04-25 11:48           ` Tomi Ollila
  0 siblings, 0 replies; 20+ messages in thread
From: Tomi Ollila @ 2018-04-25 11:48 UTC (permalink / raw)
  To: David Bremner, notmuch

On Tue, Apr 24 2018, David Bremner wrote:

> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>> Otherwise the series _looks_ good do me. The thing that disturbs me are
>> these `strlen (in_reply_to)` contents. Perhaps SomeOne(TM) changes these
>> to e.g. in_reply_to[0] in the future...
>>
>
> That same file defines and uses an EMPTY_STRING macro. We should
> probably be consistent, either use that macro everywehre or
> nowhere. Perhaps move it to lib/notmuch-private.h?

I looked macro replacement for in_reply_to[0] through internet, but 
weren't smart enough to git grep notmuch source tree. 

I'd say use it everwhere now that it is there (and move it...)

SMOP (or not so) for someone(tm)

>
> d

Tomi

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

* Re: reference loop fix and tests, v2
  2018-04-14  1:46 reference loop fix and tests, v2 David Bremner
                   ` (4 preceding siblings ...)
  2018-04-14  1:46 ` [PATCH 5/5] NEWS: add item for reference loop fix David Bremner
@ 2018-04-25 20:41 ` David Bremner
  5 siblings, 0 replies; 20+ messages in thread
From: David Bremner @ 2018-04-25 20:41 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

> Here's a tidied version of the reference loop fix. There's actually
> only one non-comment line of code changed. As you can guess from the
> NEWS patch, I'm planning a point release to include this bugfix. I'd
> also like to include the mset fix [1], so it would be great if someone
> could test that.

Pushed this series, with the all import 's/!/! /' fix, and a reformatted
news entry, to release and master

d

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

end of thread, other threads:[~2018-04-25 20:41 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-14  1:46 reference loop fix and tests, v2 David Bremner
2018-04-14  1:46 ` [PATCH 1/5] test: two new messages for the 'broken' corpus David Bremner
2018-04-14 22:24   ` Tomi Ollila
2018-04-15  7:36     ` Daniel Kahn Gillmor
2018-04-14  1:46 ` [PATCH 2/5] test: add known broken test for indexing an In-Reply-To loop David Bremner
2018-04-14  1:46 ` [PATCH 3/5] lib: break reference loop by choosing arbitrary top level msg David Bremner
2018-04-15  7:38   ` Daniel Kahn Gillmor
2018-04-14  1:46 ` [PATCH 4/5] test: add second reference loop test David Bremner
2018-04-14  1:46 ` [PATCH 5/5] NEWS: add item for reference loop fix David Bremner
2018-04-20 18:31   ` v3 " David Bremner
2018-04-20 18:31     ` [PATCH 1/6] test: two new messages for the 'broken' corpus David Bremner
2018-04-20 18:31     ` [PATCH 2/6] test: add known broken test for indexing an In-Reply-To loop David Bremner
2018-04-20 18:31     ` [PATCH 3/6] lib: break reference loop by choosing arbitrary top level msg David Bremner
2018-04-20 18:31     ` [PATCH 4/6] NEWS: add item for reference loop fix David Bremner
2018-04-20 18:31     ` [PATCH 5/6] test: add known broken test for thread ordering from a loop David Bremner
2018-04-20 18:31     ` [PATCH 6/6] lib: choose oldest message when breaking reference loops David Bremner
2018-04-21 17:53       ` Tomi Ollila
2018-04-25  1:52         ` David Bremner
2018-04-25 11:48           ` Tomi Ollila
2018-04-25 20:41 ` reference loop fix and tests, v2 David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).