release-candidate/0.6

release-candidate/0.6

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 1746 bytes --]

Hi, folks.  As some of you already know, I am trying to put together a
release candidate for a 0.6 release that we can present to cworth for
approval.  This branch can be found in my public notmuch repository in
the "release-candidate/0.6" branch:

git remote add jrollins git://finestructure.net/notmuch
git remote update
git checkout -t -b release-candidate/0.6 jrollins/release-candidate/0.6

So far, this release candidate includes a couple of patch series that
are not currently on cworth's master branch:

json structure now replicates mime structure
* dme's json-fully-reflects-MIME-structure improvements
* dkg/jrollins's PGP/MIME cryptographic support
* amdragon's atomicity improvements
* a couple other small improvements

I might try to add a couple of more things before declaring the
candidate release-ready, but this is more-or-less it.  Please start
using this branch "in production" as much as possible, so that we can
build up a chorus of support for pushing this release out.  Once you
have build, tested, and started using the branch, please reply to this
email thread to express your support for it's release.

jamie.

PS: note to Debian users:

The release-candidate/0.6 branch also includes debian package updates,
so you should be able to easily build debian packages from the branch:

git buildpackage -uc -us --git-ignore-branch

However, the current version of libgmime-2.4 in testing and unstable
(2.4.23-1) unfortunately breaks signature verification, which means that
many of the crypto tests will fail.  The issue has been fixed upstream,
and we've been pushing on the libgmime maintainer to push a new release.
Hopefully a release of notmuch 0.6 into debian will put extra pressure
on getting this issue resolved.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: release-candidate/0.6

[Sebastian Spaeth]

[-- Attachment #1: Type: text/plain, Size: 610 bytes --]

On Fri, 06 May 2011 12:46:34 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> I might try to add a couple of more things before declaring the
> candidate release-ready, but this is more-or-less it.  Please start
> using this branch "in production" as much as possible, so that we can
> build up a chorus of support for pushing this release out.  Once you
> have build, tested, and started using the branch, please reply to this
> email thread to express your support for it's release.

+1, thanks for doing this Jameson

Used-and-approved-by-a-happy: Sebastian Spaeth <Sebastian@SSpaeth.de>


[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: release-candidate/0.6

[Florian Friesdorf]

[-- Attachment #1: Type: text/plain, Size: 3592 bytes --]

On Fri, 06 May 2011 12:46:34 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Hi, folks.  As some of you already know, I am trying to put together a
> release candidate for a 0.6 release that we can present to cworth for
> approval.  This branch can be found in my public notmuch repository in
> the "release-candidate/0.6" branch:
> 
> git remote add jrollins git://finestructure.net/notmuch
> git remote update
> git checkout -t -b release-candidate/0.6 jrollins/release-candidate/0.6
> 
> So far, this release candidate includes a couple of patch series that
> are not currently on cworth's master branch:
> 
> json structure now replicates mime structure
> * dme's json-fully-reflects-MIME-structure improvements
> * dkg/jrollins's PGP/MIME cryptographic support
> * amdragon's atomicity improvements
> * a couple other small improvements

I'm delighted! 

> I might try to add a couple of more things before declaring the
> candidate release-ready, but this is more-or-less it.  Please start
> using this branch "in production" as much as possible, so that we can
> build up a chorus of support for pushing this release out.  Once you
> have build, tested, and started using the branch, please reply to this
> email thread to express your support for it's release.

I did already run this combination for quite a while with general
satisfaction.

Problems in emacs (i.e. the json format) with displaying certain mime
messages are solved by using gmime 2.4.24 (thx dkg for the debugging
support). Problems were: notmuch show consuming 100% CPU without a result
and "EOF while reading message".

An open issue (also with gmime 2.4.24) is the extraction of a PDF from
an encrypted message via emacs (discussed on irc before, mentioned here
for completeness of the archive).

> PS: note to Debian users:
> 
> The release-candidate/0.6 branch also includes debian package updates,
> so you should be able to easily build debian packages from the branch:
> 
> git buildpackage -uc -us --git-ignore-branch
> 
> However, the current version of libgmime-2.4 in testing and unstable
> (2.4.23-1) unfortunately breaks signature verification, which means that
> many of the crypto tests will fail.  The issue has been fixed upstream,
> and we've been pushing on the libgmime maintainer to push a new release.
> Hopefully a release of notmuch 0.6 into debian will put extra pressure
> on getting this issue resolved.

An option for these systems:

- Linux (particularly on x86, x86_64, and PowerPC).
- Mac OS X, both on Intel and PowerPC.
- FreeBSD (only tested on Intel).
- Windows through Cygwin.

is using nix to install emacs and notmuch from nixpkgs. It does not
interfere with other software installed on the system but installs into
/nix and creates links of installed software into a profile which can be
included in the PATH.

Current release candidate and gmime 2.4.24 are packaged in nixpkgs.

Installation instructions at [1]. For simplicity I recommend the single
user install.

In case of question, I and #nixos are happy to help.

Nix/Nixpkgs is not an alternative to debian, but helps in having some
newer things independent of the system. However, there is no security
team yet ensuring timely updates.

regards
florian

[1] http://hydra.nixos.org/build/1069287/download/1/manual/#chap-installation
-- 
Florian Friesdorf <flo@chaoflow.net>
  GPG FPR: 7A13 5EEE 1421 9FC2 108D  BAAF 38F8 99A3 0C45 F083
Jabber/XMPP: flo@chaoflow.net
IRC: chaoflow on freenode,ircnet,blafasel,OFTC

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: release-candidate/0.6

[Florian Friesdorf]

[-- Attachment #1: Type: text/plain, Size: 514 bytes --]

On Sat, 07 May 2011 01:51:25 +0200, Florian Friesdorf <flo@chaoflow.net> wrote:
> (..)
> An open issue (also with gmime 2.4.24) is the extraction of a PDF from
> an encrypted message via emacs (discussed on irc before, mentioned here
> for completeness of the 

In the current release candidate this got fixed by Jamie.

-- 
Florian Friesdorf <flo@chaoflow.net>
  GPG FPR: 7A13 5EEE 1421 9FC2 108D  BAAF 38F8 99A3 0C45 F083
Jabber/XMPP: flo@chaoflow.net
IRC: chaoflow on freenode,ircnet,blafasel,OFTC

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: release-candidate/0.6

[James Vasile]

Thanks for pulling this together.

I sent two patches to the list on March 16.  They were a bug fix to
allow notmuch to correctly handle some poorly formatted email.  Nobody
ever replied, but I'd like to see the bug fixed nonetheless, as it
results in unreadable mail.  Since I've actually seen such mail in the
wild, I'd like to see Notmuch handle it.

Those messages were:

Message-ID: <87ipvifrlv.fsf@softwarefreedom.org>
Message-ID: <87fwqmfr2x.fsf@softwarefreedom.org>

Thanks,
James

Re: release-candidate/0.6

[Florian Friesdorf]

[-- Attachment #1: Type: text/plain, Size: 1185 bytes --]

On Fri, 06 May 2011 19:56:30 -0400, James Vasile <james@hackervisions.org> wrote:
> Thanks for pulling this together.
> 
> I sent two patches to the list on March 16.  They were a bug fix to
> allow notmuch to correctly handle some poorly formatted email.  Nobody
> ever replied, but I'd like to see the bug fixed nonetheless, as it
> results in unreadable mail.  Since I've actually seen such mail in the
> wild, I'd like to see Notmuch handle it.
> 
> Those messages were:
> 
> Message-ID: <87ipvifrlv.fsf@softwarefreedom.org>
> Message-ID: <87fwqmfr2x.fsf@softwarefreedom.org>

The first seems to be damaged, the second I did not try:

~/dev/notmuch]$ cat $(notmuch search --output=files id:87ipvifrlv.fsf@softwarefreedom.org) |patch -p1
patching file lib/thread.cc
Hunk #1 FAILED at 266.
patch: **** malformed patch at line 153: hread,

But it sounds like a different approach to:
id:8739ljwy7z.fsf@msstf091.ucc.ie,
id:1304723846-29134-1-git-send-email-flo@chaoflow.net

-- 
Florian Friesdorf <flo@chaoflow.net>
  GPG FPR: 7A13 5EEE 1421 9FC2 108D  BAAF 38F8 99A3 0C45 F083
Jabber/XMPP: flo@chaoflow.net
IRC: chaoflow on freenode,ircnet,blafasel,OFTC

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: release-candidate/0.6

[Tim Gray]

On May 06, 2011 at 07:56 PM -0400, James Vasile wrote:
>Thanks for pulling this together.
>
>I sent two patches to the list on March 16.  They were a bug fix to
> 
>allow notmuch to correctly handle some poorly formatted email.  Nobody
>ever replied, but I'd like to see the bug fixed nonetheless, as it
>results in unreadable mail.  Since I've actually seen such mail in the
>wild, I'd like to see Notmuch handle it.

Ditto with the very very simple patch I sent about OS X compiling.  I 
take it there are few users of OS X on this list, so every little bit 
could help.

I'm running the release candidate.  My simple patch still applied 
cleanly and worked.

The message-id was:
<20110403070333.GA71620@selenium.125px.com>

Re: release-candidate/0.6

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 701 bytes --]

On Fri, 6 May 2011 22:31:09 -0400, Tim Gray <tgray@protozoic.com> wrote:
> Ditto with the very very simple patch I sent about OS X compiling.  I 
> take it there are few users of OS X on this list, so every little bit 
> could help.

Hi, Tim.  Have you tried compiling the release-candidate/0.6 on OS X
without this patch?  I fixed an issue with the library linking that
could have also been the source of the issue you're seeing.

If release-candidate/0.6 still requires your patch to build, if you
could rebase your patch against the head of release-candidate/0.6 I'll
look at applying it again (it wasn't applying cleanly against the
current head of that branch).

Thanks.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: release-candidate/0.6

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 794 bytes --]

On Fri, 06 May 2011 19:56:30 -0400, James Vasile <james@hackervisions.org> wrote:
> I sent two patches to the list on March 16.  They were a bug fix to
> allow notmuch to correctly handle some poorly formatted email.  Nobody
> ever replied, but I'd like to see the bug fixed nonetheless, as it
> results in unreadable mail.  Since I've actually seen such mail in the
> wild, I'd like to see Notmuch handle it.

Hey, James.  Florian Friesdorf sent in a patch to sanitize the output of
notmuch search, which I think is the issue that you're patches were also
dealing with.  Florian's patch has be pushed to my
release-candidate/0.6.  Can you see if your issue is fixed there?  If
not maybe you could try submitting a new patch that would apply to the
release-candidate/0.6 head.

Thanks.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: release-candidate/0.6

[James Vasile]

On Sun, 08 May 2011 17:57:48 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Fri, 06 May 2011 19:56:30 -0400, James Vasile <james@hackervisions.org> wrote:
> > I sent two patches to the list on March 16.  They were a bug fix to
> > allow notmuch to correctly handle some poorly formatted email.  Nobody
> > ever replied, but I'd like to see the bug fixed nonetheless, as it
> > results in unreadable mail.  Since I've actually seen such mail in the
> > wild, I'd like to see Notmuch handle it.
> 
> Hey, James.  Florian Friesdorf sent in a patch to sanitize the output of
> notmuch search, which I think is the issue that you're patches were also
> dealing with.  Florian's patch has be pushed to my
> release-candidate/0.6.  Can you see if your issue is fixed there?  If
> not maybe you could try submitting a new patch that would apply to the
> release-candidate/0.6 head.
> 
> Thanks.

I'll take a look at it this week.  Thanks much.

Re: release-candidate/0.6

[Darren McGuicken]

[-- Attachment #1: Type: text/plain, Size: 772 bytes --]

On Fri, 06 May 2011 12:46:34 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> However, the current version of libgmime-2.4 in testing and unstable
> (2.4.23-1) unfortunately breaks signature verification, which means
> that many of the crypto tests will fail.  The issue has been fixed
> upstream, and we've been pushing on the libgmime maintainer to push a
> new release.  Hopefully a release of notmuch 0.6 into debian will put
> extra pressure on getting this issue resolved.

Another +1.  RC continues to work well (I've been running the crypto
branch since its creation), and I'm now using it with gmime-2.4.24.  No
issues with signature verification that I can see, including viewing
threads previously borked under gmime-2.4.14.

Thanks for this!

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: release-candidate/0.6

[Dmitry Kurochkin]

Hi Jameson.

First of all, thank you for your effort on notmuch.  It is a great
project and I am happy to see it going forward (again)!

Can we include FCC fix in the 0.6 please?  It was broken in 0.5 (IIRC)
because of old configuration check.  There are two patches on the ML to
address it.  The first one is from Carl
id:"1290632444-10046-1-git-send-email-cworth@cworth.org" where the check
is just commented out.  Another one is a proper fix from David
id:"1290682750-30283-2-git-send-email-dme@dme.org".  It is part of a
patch series that adds some ERT (Emacs Lisp Regression Testing) support.
I see few options: apply a "fix" from Carl, apply ERT patch series from
David, or factor out the proper fix from David's series and apply it
separately.  Not sure what is the best.  Perhaps David can comment about
the ERT stuff.  I am fine with any option, just do not leave it as is,
i.e. broken.  I am ready to help with the factoring out the proper fix,
if all agree it is the best solution and no one (David?) volunteers for
it.

Thank you,
  Dmitry

Re: release-candidate/0.6

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 987 bytes --]

Hi, folks.  I've pushed some more patches to the release-candidate/0.6
branch [0] (which should now be at commit id
89ca01b6104dd732903104e50777a7b4a211e1f4):

* support for decryption of parts with notmuch show --format=part
* emacs support for retrieving parts (like attachments) from encrypted
  messages
* Dmitry Kurochkin's patch to include filenames for inline attachments
* Andreas Amann and Florian's patch for sanitizing search text output
* various other cleanups

Dmitry Kurochkin have also asked for some patches to be applied dealing
with fcc paths.  I haven't dealt with that yet, since it doesn't apply
cleanly against the branch head and the patch is too large for me to
resolve easily.  If someone ones to take a crack at fixing those issues
up for the next release that would be great.

Please keep using the head of this branch, and report any issues, and
hopefully we'll be able to push it out soon.

jamie.

[0] git://finestructure.net/notmuch release-candidate/0.6

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: release-candidate/0.6

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 1067 bytes --]

On 05/08/2011 09:03 PM, Jameson Graef Rollins wrote:
> Hi, folks.  I've pushed some more patches to the release-candidate/0.6
> branch [0] (which should now be at commit id
> 89ca01b6104dd732903104e50777a7b4a211e1f4):
> 
> * support for decryption of parts with notmuch show --format=part
> * emacs support for retrieving parts (like attachments) from encrypted
>   messages
> * Dmitry Kurochkin's patch to include filenames for inline attachments
> * Andreas Amann and Florian's patch for sanitizing search text output
> * various other cleanups

I'm running this now.  It's great.  Thank you, Jamie!

One new feature that you didn't mention is that --decrypt is passed
through to notmuch reply based on the state of the current buffer.

That is: it used to be that you had to remember whether you'd viewed a
thread with M-RET or RET (to trigger decryption+verification) and adjust
your reply invocation to match if you wanted proper quoting and
attribution when replying to encrypted mail.

Now it Just Works™.  Excellent Work.

	--dkg


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 1030 bytes --]

Re: release-candidate/0.6

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 662 bytes --]

On Sun, 08 May 2011 21:24:52 -0400, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> One new feature that you didn't mention is that --decrypt is passed
> through to notmuch reply based on the state of the current buffer.
> 
> That is: it used to be that you had to remember whether you'd viewed a
> thread with M-RET or RET (to trigger decryption+verification) and adjust
> your reply invocation to match if you wanted proper quoting and
> attribution when replying to encrypted mail.

Thanks for point that out, dkg.  I did forget to mention that.

Now we just need to figure out how to auto-encrypt replies to encrypted
messages...

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: release-candidate/0.6

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 719 bytes --]

Hi, folks.  I have pushed a couple of more patches to
release-candidate/0.6 [0]:

* Dmitry's fix for emacs fcc
* Anton Khirnov's memleak fixes

I think that everything else can wait for later releases.

***I hereby declare that release-candidate/0.6 is ready for release.***

I think the only remaining release tasks would be to update the NEWS
file, and then possibly debian/changelog if we want to expand the 0.6
entries a bit.

Carl: what do you think?  Are you willing to push this out as a new
release?  Please let me know if there's anything else I/we can do to
facilitate that.  Thanks!

jamie.

[0] git://finestructure.net/notmuch release-candidate/0.6
    (commit id: 31b65daf40dbb5fd98a875452674fe6d1e5368db)

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: release-candidate/0.6

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 1831 bytes --]

On Mon, 09 May 2011 10:20:18 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> ***I hereby declare that release-candidate/0.6 is ready for release.***

After all of that pomp, I take it all back!

Fully fearful of further delaying release of 0.6, I decided I wanted to
slip in a couple last-minute patches, most of which are really bug
fixes:

I told Felipe I would include his vim patches, so I wanted to follow
through on that.

Dmitry submitted a patch to fix sporadic seg faults in one of the emacs
tests.

I built release-candidate/0.6 against the Ubuntu package of
libgmime-2.4-dev, version 2.4.24-0ubuntu1 [0].  This fixed all but one
of the failing crypto tests, and the delinquent test only needed to be
updated to expect libgmime's improved error reporting.  This means that
all 184 tests now consistently cleanly pass (after 100 runs).  This
persuaded me to decide that 0.6 really should depend on this newly
released 2.4.24 version of libgmime-2.4-2, and I updated the debian
packaging accordingly.

If people think this is not the way to go, I'm open to arguments.  I
think this /will/ create a bit of a snag for debian users.  libgmime's
dependency is not currently available in debian, so declaring notmuch
0.6 dependent on libgmime-2.4-2 2.4.24 will render the debian package of
0.6 uninstallable (and maybe even unbuildable).  But I'm hoping we can
use this to convince the debian maintainer of libgmime to release 2.4.24
concurrently with notmuch 0.6.  If this sounds ridiculous, please let me
know and we can look at a different solutions.

Again, sorry for the last minute churn, but I think it's worth it [1].

jamie.

[0] https://launchpad.net/ubuntu/+source/gmime2.4/2.4.24-0ubuntu1/
[1] git://finestructure.net/notmuch
    release-candidate/0.6
    f357eafd3c315c7b59abe3088479a3b4e68a4d11

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: release-candidate/0.6

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 626 bytes --]

Arg.  One last bit of churn.

dkg found a bug in the new sanitize_string function that was causing
segfaults on messages with empty headers.  This is obviously an imprtant
thing to fix.

After chatting with some folks on #notmuch, we decided that the debian
build dependency on libgmime 2.4.24 is a bit too extreme, particularly
since it will actually prevent the package from being uploaded to
debian.  I therefore rolled-back the stricter build dependency.

A new version of release-candidate/0.6 is pushed.

jamie.

[0] git://finestructure.net/notmuch
    release-candidate/0.6
    aac1936bc9f878bd9ad2e097c5a686e146b73143

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: release-candidate/0.6

[Pieter Praet]

On Tue, 10 May 2011 09:42:39 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Arg.  One last bit of churn.
> 
> dkg found a bug in the new sanitize_string function that was causing
> segfaults on messages with empty headers.  This is obviously an imprtant
> thing to fix.
> 
> After chatting with some folks on #notmuch, we decided that the debian
> build dependency on libgmime 2.4.24 is a bit too extreme, particularly
> since it will actually prevent the package from being uploaded to
> debian.  I therefore rolled-back the stricter build dependency.
> 
> A new version of release-candidate/0.6 is pushed.

The atomicity tests were failing here because I didn't have GDB
installed, so I've added it as a prereq.

While I was at it, I "fixed" the crypto and emacs tests as well.

> jamie.
> 
> [0] git://finestructure.net/notmuch
>     release-candidate/0.6
>     aac1936bc9f878bd9ad2e097c5a686e146b73143
Non-text part: application/pgp-signature
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Peace


[0] git://github.com/praet/notmuch.git
    for-review/test-prereqs
    a845ca793873b6ab637a

-- 
Pieter

Re: release-candidate/0.6

[Austin Clements]

On Thu, May 12, 2011 at 8:22 AM, Pieter Praet <pieter@praet.org> wrote:
> The atomicity tests were failing here because I didn't have GDB
> installed, so I've added it as a prereq.

Sorry, I've had a patch to address that sitting around, but hadn't
sent it out (and I only fixed that one test).  I would suggest a
somewhat gentler approach than "error", though:

if test_expect_success "prereq: gdb is present" "which gdb"; then
    test_set_prereq GDB
fi

(Plus the two test-lib patches I just sent:
id:1305206080-17461-1-git-send-email-amdragon@mit.edu and
id:1305206110-17511-1-git-send-email-amdragon@mit.edu).

"error" has the disadvantage that it doesn't get counted as a failed
test in the final tally (because, indeed, it's not a failed test) and
also that it immediately terminates the test script so it's not
actually using the prereq system (which is fine for the atomicity test
since all of the test cases depend on GDB, but the pattern I'm
proposing works for finer-grained prerequisites).  Plus, with the
above approach, if you don't have a prerequisite, the final tally
shows one failed test plus some number of skipped tests (and the total
number of tests never changes), which I would argue is cleaner.

Re: release-candidate/0.6

[Pieter Praet]

On Thu, 12 May 2011 09:18:48 -0400, Austin Clements <amdragon@mit.edu> wrote:
> On Thu, May 12, 2011 at 8:22 AM, Pieter Praet <pieter@praet.org> wrote:
> > The atomicity tests were failing here because I didn't have GDB
> > installed, so I've added it as a prereq.
> 
> Sorry, I've had a patch to address that sitting around, but hadn't
> sent it out (and I only fixed that one test).  I would suggest a
> somewhat gentler approach than "error", though:
> 
> if test_expect_success "prereq: gdb is present" "which gdb"; then
>     test_set_prereq GDB
> fi
> 
> (Plus the two test-lib patches I just sent:
> id:1305206080-17461-1-git-send-email-amdragon@mit.edu and
> id:1305206110-17511-1-git-send-email-amdragon@mit.edu).
> 
> "error" has the disadvantage that it doesn't get counted as a failed
> test in the final tally (because, indeed, it's not a failed test) and
> also that it immediately terminates the test script so it's not
> actually using the prereq system (which is fine for the atomicity test
> since all of the test cases depend on GDB, but the pattern I'm
> proposing works for finer-grained prerequisites).  Plus, with the
> above approach, if you don't have a prerequisite, the final tally
> shows one failed test plus some number of skipped tests (and the total
> number of tests never changes), which I would argue is cleaner.

Much obliged for the correction!


Peace


[0] git://github.com/praet/notmuch.git
    for-review/test-prereqs-v2
    c9a785fc5c48db13

-- 
Pieter

Re: release-candidate/0.6

[Austin Clements]

Nifty.  I was afraid to go romping through all of the other test
dependencies; I'm glad somebody wasn't.  ]:--8)

It should be noted that these patches depend on
id:1305206110-17511-1-git-send-email-amdragon@mit.edu for correctness
and id:1305206080-17461-1-git-send-email-amdragon@mit.edu for sanity.

Given that these patches aren't specifically 0.6-related and that it's
good practice to send patches to the list, perhaps you could git
send-email these patches as a new thread to the list?

On Thu, May 12, 2011 at 10:09 AM, Pieter Praet <pieter@praet.org> wrote:
> On Thu, 12 May 2011 09:18:48 -0400, Austin Clements <amdragon@mit.edu> wrote:
>> On Thu, May 12, 2011 at 8:22 AM, Pieter Praet <pieter@praet.org> wrote:
>> > The atomicity tests were failing here because I didn't have GDB
>> > installed, so I've added it as a prereq.
>>
>> Sorry, I've had a patch to address that sitting around, but hadn't
>> sent it out (and I only fixed that one test).  I would suggest a
>> somewhat gentler approach than "error", though:
>>
>> if test_expect_success "prereq: gdb is present" "which gdb"; then
>>     test_set_prereq GDB
>> fi
>>
>> (Plus the two test-lib patches I just sent:
>> id:1305206080-17461-1-git-send-email-amdragon@mit.edu and
>> id:1305206110-17511-1-git-send-email-amdragon@mit.edu).
>>
>> "error" has the disadvantage that it doesn't get counted as a failed
>> test in the final tally (because, indeed, it's not a failed test) and
>> also that it immediately terminates the test script so it's not
>> actually using the prereq system (which is fine for the atomicity test
>> since all of the test cases depend on GDB, but the pattern I'm
>> proposing works for finer-grained prerequisites).  Plus, with the
>> above approach, if you don't have a prerequisite, the final tally
>> shows one failed test plus some number of skipped tests (and the total
>> number of tests never changes), which I would argue is cleaner.
>
> Much obliged for the correction!
>
>
> Peace
>
>
> [0] git://github.com/praet/notmuch.git
>    for-review/test-prereqs-v2
>    c9a785fc5c48db13
>
> --
> Pieter
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
>

[PATCH 0/4] set test prereqs (Emacs, GDB, GPG)

[Pieter Praet]

The atomicity tests were failing here because I didn't have GDB
installed, so I've added it as a prereq.

While I was at it, I "fixed" the crypto and emacs tests as well.


Thank Austin for providing me with a hammer (I was using a brick) [1].

(!) Depends:
- "[PATCH] test: Fix message when skipping test_expect_equal* tests" [2]
- "[PATCH] test: Report test failures from test_expect_*" [3]


Pieter Praet (4):
  test: add 'GDB' prereq to 'atomicity' tests
  test: add 'GPG' prereq to 'crypto' tests
  test: add 'Emacs' prereq to 'emacs' tests
  test: add 'Emacs' prereq to 'emacs-large-search-buffer' tests

 test/atomicity                 |   10 ++++++++--
 test/crypto                    |   30 ++++++++++++++++++------------
 test/emacs                     |   40 +++++++++++++++++++++++-----------------
 test/emacs-large-search-buffer |    9 +++++++--
 4 files changed, 56 insertions(+), 33 deletions(-)

-- 
1.7.4.1


[1] id:"BANLkTi=iz6F+1-WQ5KDno6N-MF=dQeTbEA@mail.gmail.com"
[2] id:"1305206080-17461-1-git-send-email-amdragon@mit.edu"
[3] id:"1305206110-17511-1-git-send-email-amdragon@mit.edu"

[PATCH 1/4] test: add 'GDB' prereq to 'atomicity' tests

[Pieter Praet]

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 test/atomicity |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/test/atomicity b/test/atomicity
index cd58c4c..78280b2 100755
--- a/test/atomicity
+++ b/test/atomicity
@@ -7,6 +7,12 @@ test_description='atomicity'
 # final database contents should be the same regardless of when (or
 # if) it is killed and restarted.
 
+# GDB is a prereq.
+if test_expect_success "prereq: GDB is present" "which gdb"; then
+    test_set_prereq GDB
+fi
+
+
 # Create a maildir structure to also stress flag synchronization
 mkdir $MAIL_DIR/cur
 mkdir $MAIL_DIR/new
@@ -89,8 +95,8 @@ for ((i = 0; i < $outcount; i++)); do
         i=$(expr $end - 1)
     fi
 done
-test_expect_equal "$(cat searchall)" "$(cat expectall)"
+test_expect_equal GDB "$(cat searchall)" "$(cat expectall)"
 
-test_expect_success "detected $outcount>10 abort points" "test $outcount -gt 10"
+test_expect_success GDB "detected $outcount>10 abort points" "test $outcount -gt 10"
 
 test_done
-- 
1.7.4.1

[PATCH 2/4] test: add 'GPG' prereq to 'crypto' tests

[Pieter Praet]

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 test/crypto |   30 ++++++++++++++++++------------
 1 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/test/crypto b/test/crypto
index 961d035..c2f381c 100755
--- a/test/crypto
+++ b/test/crypto
@@ -7,6 +7,12 @@
 test_description='PGP/MIME signature verification and decryption'
 . ./test-lib.sh
 
+# GnuPG is a prereq.
+if test_expect_success "prereq: GnuPG is present" "which gpg"; then
+    test_set_prereq GPG
+fi
+
+
 add_gnupg_home ()
 {
     local output
@@ -31,7 +37,7 @@ FINGERPRINT=$(gpg --no-tty --list-secret-keys --with-colons --fingerprint | grep
 # although I can't figure out why
 add_email_corpus
 
-test_expect_success 'emacs delivery of signed message' \
+test_expect_success GPG 'emacs delivery of signed message' \
 'emacs_deliver_message \
     "test signed message 001" \
     "This is a test signed message." \
@@ -66,7 +72,7 @@ expected='[[[{"id": "XXXXX",
  "content-type": "application/pgp-signature"}]}
 ]},
  []]]]'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 
@@ -103,7 +109,7 @@ expected='[[[{"id": "XXXXX",
  "content-type": "application/pgp-signature"}]}
 ]},
  []]]]'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 
@@ -139,7 +145,7 @@ expected='[[[{"id": "XXXXX",
  "content-type": "application/pgp-signature"}]}
 ]},
  []]]]'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 mv "${GNUPGHOME}"{.bak,}
@@ -148,7 +154,7 @@ mv "${GNUPGHOME}"{.bak,}
 cat <<EOF >TESTATTACHMENT
 This is a test file.
 EOF
-test_expect_success 'emacs delivery of encrypted message with attachment' \
+test_expect_success GPG 'emacs delivery of encrypted message with attachment' \
 'emacs_deliver_message \
     "test encrypted message 001" \
     "This is a test encrypted message." \
@@ -186,7 +192,7 @@ expected='[[[{"id": "XXXXX",
  "filename": "TESTATTACHMENT"}]}
 ]}]},
  []]]]'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 
@@ -195,7 +201,7 @@ notmuch show \
     --format=part --part=4 \
     --decrypt \
     subject:"test encrypted message 001" >OUTPUT
-test_expect_equal_file OUTPUT TESTATTACHMENT
+test_expect_equal_file GPG OUTPUT TESTATTACHMENT
 
 test_begin_subtest "decryption failure with missing key"
 mv "${GNUPGHOME}"{,.bak}
@@ -224,12 +230,12 @@ expected='[[[{"id": "XXXXX",
  "content-type": "application/octet-stream"}]}
 ]},
  []]]]'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 mv "${GNUPGHOME}"{.bak,}
 
-test_expect_success 'emacs delivery of encrypted/signed message' \
+test_expect_success GPG 'emacs delivery of encrypted/signed message' \
 'emacs_deliver_message \
     "test encrypted message 002" \
     "This is a test encrypted message." \
@@ -263,7 +269,7 @@ expected='[[[{"id": "XXXXX",
  "content-type": "text/plain",
  "content": "This is a test encrypted message.\n"}]}]},
  []]]]'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 
@@ -275,7 +281,7 @@ Subject: Re: test encrypted message 002
 
 On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> wrote:
 > This is a test encrypted message.'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 
@@ -319,7 +325,7 @@ expected='[[[{"id": "XXXXX",
  "content-type": "application/pgp-signature"}]}
 ]},
  []]]]'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 
-- 
1.7.4.1

[PATCH 3/4] test: add 'Emacs' prereq to 'emacs' tests

[Pieter Praet]

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 test/emacs |   40 +++++++++++++++++++++++-----------------
 1 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/test/emacs b/test/emacs
index 3264bf2..011deac 100755
--- a/test/emacs
+++ b/test/emacs
@@ -2,6 +2,12 @@
 test_description="emacs interface"
 . test-lib.sh
 
+# Emacs is a prereq.
+if test_expect_success "prereq: Emacs is present" "which emacs"; then
+    test_set_prereq EMACS
+fi
+
+
 EXPECTED=../emacs.expected-output
 
 add_email_corpus
@@ -9,64 +15,64 @@ add_email_corpus
 test_begin_subtest "Basic notmuch-hello view in emacs"
 output=$(test_emacs '(notmuch-hello) (princ (buffer-string))')
 expected=$(cat $EXPECTED/notmuch-hello)
-test_expect_equal "$output" "$expected"
+test_expect_equal EMACS "$output" "$expected"
 
 test_begin_subtest "Saved search with 0 results"
 output=$(test_emacs '(setq notmuch-show-empty-saved-searches t) (setq notmuch-saved-searches '\''(("inbox" . "tag:inbox") ("unread" . "tag:unread") ("empty" . "tag:doesnotexist"))) (notmuch-hello) (princ (buffer-string))')
 expected=$(cat $EXPECTED/notmuch-hello-with-empty)
-test_expect_equal "$output" "$expected"
+test_expect_equal EMACS "$output" "$expected"
 
 test_begin_subtest "No saved searches displayed (all with 0 results)"
 output=$(test_emacs '(setq notmuch-saved-searches '\''(("empty" . "tag:doesnotexist"))) (notmuch-hello) (princ (buffer-string))')
 expected=$(cat $EXPECTED/notmuch-hello-no-saved-searches)
-test_expect_equal "$output" "$expected"
+test_expect_equal EMACS "$output" "$expected"
 
 test_begin_subtest "Basic notmuch-search view in emacs"
 output=$(test_emacs '(notmuch-search "tag:inbox") (notmuch-test-wait) (princ (buffer-string))')
 expected=$(cat $EXPECTED/notmuch-search-tag-inbox)
-test_expect_equal "$output" "$expected"
+test_expect_equal EMACS "$output" "$expected"
 
 test_begin_subtest "Navigation of notmuch-hello to search results"
 output=$(test_emacs '(notmuch-hello) (goto-char (point-min)) (re-search-forward "inbox") (widget-button-press (point)) (notmuch-test-wait) (princ (buffer-string))')
 expected=$(cat $EXPECTED/notmuch-hello-view-inbox)
-test_expect_equal "$output" "$expected"
+test_expect_equal EMACS "$output" "$expected"
 
 test_begin_subtest "Basic notmuch-show view in emacs"
 maildir_storage_thread=$(notmuch search --output=threads id:20091117190054.GU3165@dottiness.seas.harvard.edu)
 output=$(test_emacs "(notmuch-show \"$maildir_storage_thread\") (princ (buffer-string))")
 expected=$(cat $EXPECTED/notmuch-show-thread-maildir-storage)
-test_expect_equal "$output" "$expected"
+test_expect_equal EMACS "$output" "$expected"
 
 test_begin_subtest "Navigation of notmuch-search to thread view"
 output=$(test_emacs '(notmuch-search "tag:inbox") (notmuch-test-wait) (goto-char (point-min)) (re-search-forward "Working with Maildir") (notmuch-search-show-thread) (notmuch-test-wait) (princ (buffer-string))')
-test_expect_equal "$output" "$expected"
+test_expect_equal EMACS "$output" "$expected"
 
 test_begin_subtest "Add tag from search view"
 os_x_darwin_thread=$(notmuch search --output=threads id:ddd65cda0911171950o4eea4389v86de9525e46052d3@mail.gmail.com)
 test_emacs "(notmuch-search \"$os_x_darwin_thread\") (notmuch-test-wait) (notmuch-search-add-tag \"tag-from-search-view\")"
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox tag-from-search-view unread)"
+test_expect_equal EMACS "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox tag-from-search-view unread)"
 
 test_begin_subtest "Remove tag from search view"
 test_emacs "(notmuch-search \"$os_x_darwin_thread\") (notmuch-test-wait) (notmuch-search-remove-tag \"tag-from-search-view\")"
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
+test_expect_equal EMACS "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
 
 test_begin_subtest "Add tag from notmuch-show view"
 test_emacs "(notmuch-show \"$os_x_darwin_thread\") (notmuch-show-add-tag \"tag-from-show-view\")"
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox tag-from-show-view unread)"
+test_expect_equal EMACS "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox tag-from-show-view unread)"
 
 test_begin_subtest "Remove tag from notmuch-show view"
 test_emacs "(notmuch-show \"$os_x_darwin_thread\") (notmuch-show-remove-tag \"tag-from-show-view\")"
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
+test_expect_equal EMACS "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
 
 test_begin_subtest "Message with .. in Message-Id:"
 add_message [id]=123..456@example '[subject]="Message with .. in Message-Id"'
 test_emacs '(notmuch-search "id:\"123..456@example\"") (notmuch-test-wait) (notmuch-search-add-tag "search-add") (notmuch-search-add-tag "search-remove") (notmuch-search-remove-tag "search-remove") (notmuch-show "id:\"123..456@example\"") (notmuch-test-wait) (notmuch-show-add-tag "show-add") (notmuch-show-add-tag "show-remove") (notmuch-show-remove-tag "show-remove")'
 output=$(notmuch search 'id:"123..456@example"' | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Message with .. in Message-Id (inbox search-add show-add)"
+test_expect_equal EMACS "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Message with .. in Message-Id (inbox search-add show-add)"
 
 test_begin_subtest "Sending a message via (fake) SMTP"
 
@@ -83,7 +89,7 @@ wait ${smtp_dummy_pid}
 output=$(sed \
     -e s',^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX Emacs/XXX,' \
     -e s',^Message-ID: <.*>$,Message-ID: <XXX>,' < sent_message)
-test_expect_equal "$output" "From: Notmuch Test Suite <test_suite@notmuchmail.org>
+test_expect_equal EMACS "$output" "From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: user@example.com
 Subject: Testing message sent via SMTP
 Date: Fri, 29 Mar 1974 10:00:00 -0000
@@ -97,13 +103,13 @@ This is a test that messages are sent via SMTP"
 test_begin_subtest "Verify that sent messages are saved/searchable (via FCC)"
 notmuch new > /dev/null
 output=$(notmuch search 'subject:"testing message sent via SMTP"' | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   1974-03-29 [1/1] Notmuch Test Suite; Testing message sent via SMTP (inbox)"
+test_expect_equal EMACS "$output" "thread:XXX   1974-03-29 [1/1] Notmuch Test Suite; Testing message sent via SMTP (inbox)"
 
 test_begin_subtest "Reply within emacs"
 # We sed away everything before the ^From in the output to avoid getting
 # confused by messages such as "Parsing /home/cworth/.mailrc... done"
 output=$(test_emacs '(notmuch-search "subject:\"testing message sent via SMTP\"") (notmuch-test-wait) (notmuch-search-reply-to-thread) (princ (buffer-string))' | sed -ne '/^From/,$ p' | sed -e 's/^In-Reply-To: <.*>$/In-Reply-To: <XXX>/')
-test_expect_equal "$output" "From: Notmuch Test Suite <test_suite@notmuchmail.org>
+test_expect_equal EMACS "$output" "From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: user@example.com
 Subject: Re: Testing message sent via SMTP
 In-Reply-To: <XXX>
@@ -116,12 +122,12 @@ test_begin_subtest "Save attachment from within emacs"
 echo "./attachment" | test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com") (notmuch-show-save-attachments)' > /dev/null 2>&1
 output=$(cat attachment)
 expected=$(cat $EXPECTED/attachment)
-test_expect_equal "$output" "$expected"
+test_expect_equal EMACS "$output" "$expected"
 
 test_begin_subtest "View raw message within emacs"
 expected=$(cat $EXPECTED/raw-message-cf0c4d-52ad0a)
 first_line=$(echo "$expected" | head -n1)
 output=$(test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com") (notmuch-show-view-raw-message) (princ (buffer-string))' | sed -ne "/$first_line/,\$ p")
-test_expect_equal "$output" "$expected"
+test_expect_equal EMACS "$output" "$expected"
 
 test_done
-- 
1.7.4.1

[PATCH 4/4] test: add 'Emacs' prereq to 'emacs-large-search-buffer' tests

[Pieter Praet]

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 test/emacs-large-search-buffer |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/test/emacs-large-search-buffer b/test/emacs-large-search-buffer
index 2751a32..54b1827 100755
--- a/test/emacs-large-search-buffer
+++ b/test/emacs-large-search-buffer
@@ -2,6 +2,11 @@
 test_description="Emacs with large search results buffer"
 . test-lib.sh
 
+# Emacs is a prereq.
+if test_expect_success "prereq: Emacs is present" "which emacs"; then
+    test_set_prereq EMACS
+fi
+
 x=xxxxxxxxxx # 10
 x=$x$x$x$x$x$x$x$x$x$x # 100
 x=$x$x$x$x$x$x$x$x$x # 900
@@ -23,11 +28,11 @@ expected="$(notmuch search '*' | sed -e 's/^thread:[0-9a-f]*  //' -e 's/;//' -e
 End of search results."
 
 output=$(test_emacs '(notmuch-search "*") (notmuch-test-wait) (princ (buffer-string))' | sed -e s',  *, ,g' -e 's/xxx*/[BLOB]/g')
-test_expect_equal "$output" "$expected"
+test_expect_equal EMACS "$output" "$expected"
 
 test_begin_subtest "Ensure that emacs doesn't drop error messages"
 output=$(test_emacs '(notmuch-search "--this-option-does-not-exist") (notmuch-test-wait) (princ (buffer-string))')
-test_expect_equal "$output" "Error: Unexpected output from notmuch search:
+test_expect_equal EMACS "$output" "Error: Unexpected output from notmuch search:
 Unrecognized option: --this-option-does-not-exist
 End of search results. (process returned 1)"
 
-- 
1.7.4.1

Re: release-candidate/0.6

[micah anderson]

[-- Attachment #1: Type: text/plain, Size: 1002 bytes --]

On Fri, 06 May 2011 12:46:34 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
>
> I might try to add a couple of more things before declaring the
> candidate release-ready, but this is more-or-less it.  Please start
> using this branch "in production" as much as possible, so that we can
> build up a chorus of support for pushing this release out.  Once you
> have build, tested, and started using the branch, please reply to this
> email thread to express your support for it's release.

I've been using this branch now for a couple of days and its solid,
thanks for pulling this together Mr. Rollins.

> The release-candidate/0.6 branch also includes debian package updates,
> so you should be able to easily build debian packages from the branch:
> 
> git buildpackage -uc -us --git-ignore-branch

I used this mechanism to build a package and install it, it worked
well. I also ran lintian on it just to see if there were any issues, but
you get an A+.

micah

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: release-candidate/0.6

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 2896 bytes --]

On Fri, 06 May 2011 12:46:34 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Hi, folks.  As some of you already know, I am trying to put together a
> release candidate for a 0.6 release that we can present to cworth for
> approval.

Hi Jameson,

This is really great! I appreciate you collecting useful patches for
review like this. I hope this helps us get things moving once again.

> So far, this release candidate includes a couple of patch series that
> are not currently on cworth's master branch:
> 
> json structure now replicates mime structure
> * dme's json-fully-reflects-MIME-structure improvements

I got stalled on the first commit on this branch:

	commit 5a5aae66bf41d3c621c412da711fec9b33f37dcc
	Author: David Edmondson <dme@dme.org>
	Date:   Mon May 10 10:25:15 2010 +0100

	    Improved MIME support.
    
	Signed-off-by: Jameson Rollins <jrollins@finestructure.net>

It's probably the same issue that stalled me when David first sent this
patch series for review over a year ago[!]. I'm guessing I didn't do a
good job of stating the issue then, so I'll try to do a better job here.

This first patch appears to be doing multiple things:

   * It changes how the emacs code inserts MIME parts when constructing
     a mail view.

   * It changes something in the json formatting. (Just extra newlines?)

   * It changes something about the MIME part counting.

   * It appears to be doing new emission of multipart part when doing
     json output, (and for this has some hard-coded printf("]}\n") stuff
     rather than using the formatter).

   * It has new explicit support for an embedded Message as a MIME part,
     (inserting the headers of the enclosed message).

So that's five seemingly independent changes. I'd really like to see
those split up into separate commits as much as possible. Or, excepting
that, there should be some explanation/justification in the commit
message for why some must be combined.

The worst part is that not a single change is actually described in the
commit message. All we have is "Improved MIME support". Improved how?
What changed? Why? What impact does it have? Does it fix bugs? Lay
groundwork for future changes?

What's really changing here and why?

If the patch series were sufficiently-well described in the commit
message, then my review process could be more or less:

  * Read the commit message. Does it describe a desirable change?

  * If so, read the patch content. Does it do what the commit message
    describes? Accurately? Without doing unrelated things?

  * If so, accept the patch.

Does anyone want to attempt to fix up this first patch? (It doesn't
necessarily have to be David).

From a quick skim, many later patches in the series appear to be better
in this regard.

-Carl

[!] Has it really been that long?

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: release-candidate/0.6

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 775 bytes --]

On Thu, 12 May 2011 15:36:43 -0700, Carl Worth <cworth@cworth.org> wrote:
> Does anyone want to attempt to fix up this first patch? (It doesn't
> necessarily have to be David).

Hi, Carl.  I went through dme's multipart patch series and cleaned
things up.  I split up that first commit into a couple of separate ones,
and then combined a couple of patches together and cherry-picked through
the rest of the series to make the overall series a little cleaner.  I
also let out some stuff that was in the originally series
(ie. extraneous json newlines).  I then cherry-picked the entire rest of
the release-candidate/0.6 branch on top of that.  The result is the new

release-candidate/0.6+mpmfix

branch, which I have pushed to my repo.

jamie.[!]

[!] yes, it's been a year!

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: release-candidate/0.6

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 2691 bytes --]

On Fri, 13 May 2011 01:07:08 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Hi, Carl.  I went through dme's multipart patch series and cleaned
> things up.
...
> The result is the new
> 
> release-candidate/0.6+mpmfix

Thanks so much! This looks much better than before.

I'm still hitting some snags quite early in the review process, (I'm
hoping that real soon we'll be able to just start merging like crazy).

Here at the things I've seen so far:

d3e7415d "add argument to part format function to indicate initial part"

	This one fails to build due to a simple missing argument, (easy
	mistake with the recent splitting of patches).

373f352b "Have output structure fully reflect message MIME structure"

	I'm not comfortable with this commit. Previously there was
	recursion in one function, (show_message_part), and afterwards
	there is duplicated code performing recursion in both
	format_part_text and format_part_json.

	Similarly, the code adds header formatting code that duplicates
	functionality in the existing format_headers_text and
	format_headers_json functions.

	Meanwhile, I still can't tell exactly what the behavioral change
	intended is. The commit message talks about "fully recursing"
	and "match[ing] the MIME structure of the message". Was it not
	fully recursing before? In what way did the output not match the
	MIME structure before?

	It would probably be easier to tell what was going on if the
	test suite were updated at the same time (or in a subsequent
	commit at least). As is, this commit introduces test suite
	failures, (re-ordering of MIME part ID numbers and then
	emacs-client confusion), which remain until commit 7ee6aaa1

	But what does the rest of the series really need from this
	commit? Is there some structural change to the json output aside
	from the part ID? If so, we're definitely missing a test for
	that directly, (other than the indirect testing we get with the
	emacs tests).

28ab74e0 "clean up expected emacs output to reflect cleaner from lines introduced in 78d6c196d90"

	This commit message refers to a stale (now non-existent) commit
	ID. I presume it's attempting to reference the commit with a
	message of "emacs: Show cleaner `From:' addresses in the summary
	line.".

	Granted, it's hard to keep commit IDs valid in a branch that's
	getting continually rebuilt. One fix is to not use the commit
	identifiers. Another help would be to have the test-suite
	cleanups occurring more closely after the commits that changed
	things.

I'm happy to help work on cleaning up some of these issues if that would
be useful. Let me know.

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

MIME restructuring [was: Re: release-candidate/0.6]

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]

On 05/16/2011 04:42 PM, Carl Worth wrote:
> 	Meanwhile, I still can't tell exactly what the behavioral change
> 	intended is. The commit message talks about "fully recursing"
> 	and "match[ing] the MIME structure of the message". Was it not
> 	fully recursing before? In what way did the output not match the
> 	MIME structure before?

before, the output was a linearized version of the mime tree, in
particular removing the multipart pieces and only enumerating the leaves
in a depth-first walk of the tree.

So a message like this:

A└┬╴multipart/signed 355339 bytes
B ├┬╴multipart/mixed 353462 bytes
C │├╴text/plain 235 bytes
D │└╴image/jpeg attachment [foo.jpg] 352752 bytes
E └╴application/pgp-signature attachment [signature.asc] 1030 bytes

would come out with three parts:

 1) C
 2) D
 3) E

the new code assigns this message to 5 parts:

 1) A
 2) B
 3) C
 4) D
 5) E

This change is critical to be able to properly delineate which parts of
a message were signed and/or encrypted.

hth,

	--dkg


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 1030 bytes --]

Re: MIME restructuring [was: Re: release-candidate/0.6]

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 733 bytes --]

On Mon, 16 May 2011 16:50:06 -0400, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> before, the output was a linearized version of the mime tree, in
> particular removing the multipart pieces and only enumerating the leaves
> in a depth-first walk of the tree.
> 
> So a message like this:

[snip example of change]

Thanks, Daniel! That does make things much more clear.

So what I'd love to see from here is a commit with a description like
the above, and then a test case looking like your example.

From there, I'd next like a new version of the commit that gets the
intended behavior with less code duplication.

I'll work on each of the above unless someone beats me to any of it. Let
me know.

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: MIME restructuring [was: Re: release-candidate/0.6]

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]

On Mon, 16 May 2011 13:59:51 -0700, Carl Worth <cworth@cworth.org> wrote:
> So what I'd love to see from here is a commit with a description like
> the above, and then a test case looking like your example.
> 
> From there, I'd next like a new version of the commit that gets the
> intended behavior with less code duplication.
> 
> I'll work on each of the above unless someone beats me to any of it. Let
> me know.

I've now pushed out my own version of the MIME restructuring feature.

It differs from what was presented here in avoiding the code
duplication.

It also provides properly nested mutlipart/* output for the
--format=text case. That's not that I expect anybody to *do* anything
with that nested output, just that it was cleaner and easier to fix both
text/json at the same time, (and avoiding doing that is perhaps what led
to the original code duplication).

I also cherry-picked in a piece of later patch from Jamie so that the
existing emacs tests still pass. And I updated the documentation for
this new feature.

So I'm happy with this new feature now, (which I know provides an
essential part of the basis for the rest of the crypto branch).

From here, I'm hoping that my review of the rest of Jamie's
release-candidate branch goes faster. The general shape of the commits
and commit messages looks pretty good to me, so I think it will.

I think there are still features added here and there without
corresponding test cases (multipart/alternative is one that comes to
mind) and perhaps without updated documentation (--decrypt is at least
documented---but I do think it's strange that it's documented to only
work for json output).

More from me tomorrow.

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: MIME restructuring [was: Re: release-candidate/0.6]

[Daniel Kahn Gillmor]

[-- Attachment #1.1: Type: text/plain, Size: 907 bytes --]

On 05/16/2011 04:50 PM, Daniel Kahn Gillmor wrote:
> So a message like this:
> 
> A└┬╴multipart/signed 355339 bytes
> B ├┬╴multipart/mixed 353462 bytes
> C │├╴text/plain 235 bytes
> D │└╴image/jpeg attachment [foo.jpg] 352752 bytes
> E └╴application/pgp-signature attachment [signature.asc] 1030 bytes
> 
> would come out with three parts:
> 
>  1) C
>  2) D
>  3) E
> 
> the new code assigns this message to 5 parts:
> 
>  1) A
>  2) B
>  3) C
>  4) D
>  5) E

This message should itself be a comparably complex message, with a tiny
attached image of a geek with a bike.  Feel free to use this in the test
suite (the picture and this message are hereby released under the same
license as notmuch itself).

Note that if the mailing list attaches a footer, the MIME tree will be
even more delightfully complicated, for added fun and games.

	--dkg

[-- Attachment #1.2: dkg-tiny.png --]
[-- Type: image/png, Size: 15893 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 1030 bytes --]

Re: MIME restructuring [was: Re: release-candidate/0.6]

[Simon Hürlimann]

lol, made my day!

Simon
On 05/16/2011 11:05 PM, Daniel Kahn Gillmor wrote:
> On 05/16/2011 04:50 PM, Daniel Kahn Gillmor wrote:
>> So a message like this:
>>
>> A└┬╴multipart/signed 355339 bytes
>> B ├┬╴multipart/mixed 353462 bytes
>> C │├╴text/plain 235 bytes
>> D │└╴image/jpeg attachment [foo.jpg] 352752 bytes
>> E └╴application/pgp-signature attachment [signature.asc] 1030 bytes
>>
>> would come out with three parts:
>>
>>   1) C
>>   2) D
>>   3) E
>>
>> the new code assigns this message to 5 parts:
>>
>>   1) A
>>   2) B
>>   3) C
>>   4) D
>>   5) E
>
> This message should itself be a comparably complex message, with a tiny
> attached image of a geek with a bike.  Feel free to use this in the test
> suite (the picture and this message are hereby released under the same
> license as notmuch itself).
>
> Note that if the mailing list attaches a footer, the MIME tree will be
> even more delightfully complicated, for added fun and games.
>
> 	--dkg
>
>
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: MIME restructuring [was: Re: release-candidate/0.6]

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1998 bytes --]

On Mon, 16 May 2011 16:50:06 -0400, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> So a message like this:
> 
> A└┬╴multipart/signed 355339 bytes
> B ├┬╴multipart/mixed 353462 bytes
> C │├╴text/plain 235 bytes
> D │└╴image/jpeg attachment [foo.jpg] 352752 bytes
> E └╴application/pgp-signature attachment [signature.asc] 1030 bytes

I tried creating a message like that but mine came out slightly
differently:

> A└┬╴multipart/mixed
> B ├┬╴multipart/signed
> C │├╴text/plain
> D │└╴application/pgp-signature
> E └╴application/octet-stream

I'll have to learn better how to control the emacs mail composer in
order to understand how to get signatures to cover attachments if I want
to do that kind of thing.

> would come out with three parts:
> 
>  1) C
>  2) D
>  3) E

That is indeed the behavior I see with master (for both text and json
output).

> the new code assigns this message to 5 parts:
> 
>  1) A
>  2) B
>  3) C
>  4) D
>  5) E

Interestingly, this is not quite the behavior I get (with commit
373f352). With --format=text I'm now seeing:

2) C
3) D
4) E

and with --format=json I'm seeing (I think this structure is right):

1) A
  3) B
    5) C
    7) D
  9) E

So that explains some of my confusion. The behavioral change of this
commit is really only impacting the json format, and not the text. That
wasn't clear from the commit message (and I had only been doing my
testing with the text backend).

This seems to be justifying my fears about the code duplication---the
two code paths are already divergent, (which means that things like
notmuch part-number identifiers cannot be used between the different
formats). I'd like to fix that by preventing the code duplication.

Also, both paths seem to be suffering from some excess part-number
incrementing somewhere.

All of this should be easy to get right with a careful test case or two.

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: MIME restructuring [was: Re: release-candidate/0.6]

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 1565 bytes --]

On 05/16/2011 05:20 PM, Carl Worth wrote:
> Interestingly, this is not quite the behavior I get (with commit
> 373f352). With --format=text I'm now seeing:
> 
> 2) C
> 3) D
> 4) E

--format=text should only show the parts that are readable in text.

the ultimate goal is to get the part numbers aligned across all --format
choices, regardless of the ability of the format to show the actual nesting.

> So that explains some of my confusion. The behavioral change of this
> commit is really only impacting the json format, and not the text. That
> wasn't clear from the commit message (and I had only been doing my
> testing with the text backend).

It should ultimately affect the numbering in all parts.  There's no way
for the text part to do anything like the nesting that we're doing in
json, though, due to the output format.

> This seems to be justifying my fears about the code duplication---the
> two code paths are already divergent, (which means that things like
> notmuch part-number identifiers cannot be used between the different
> formats). I'd like to fix that by preventing the code duplication.
> 
> Also, both paths seem to be suffering from some excess part-number
> incrementing somewhere.

The confusions you outline were fixed (by me, iirc) somewhere later in
the crypto tree.  I did not want to tamper with dme's crypto branch
directly (though i now suspect that's what i should have done).

i'd give you a commit ID, but i suspect you're sufficiently rebased that
this wouldn't be helpful.

	--dkg


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 1030 bytes --]

Re: MIME restructuring [was: Re: release-candidate/0.6]

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 872 bytes --]

On Mon, 16 May 2011 14:20:07 -0700, Carl Worth <cworth@cworth.org> wrote:
> I'll have to learn better how to control the emacs mail composer in
> order to understand how to get signatures to cover attachments if I want
> to do that kind of thing.

See mml-secure-message-sign-pgpmime to sign an entire message, as
opposed to just a single part.

> This seems to be justifying my fears about the code duplication---the
> two code paths are already divergent, (which means that things like
> notmuch part-number identifiers cannot be used between the different
> formats). I'd like to fix that by preventing the code duplication.
> 
> Also, both paths seem to be suffering from some excess part-number
> incrementing somewhere.

I think the two paths reconverge later in the series.  Can you look
ahead a bit to see if that concern is addressed?

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: MIME restructuring [was: Re: release-candidate/0.6]

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 680 bytes --]

On Mon, 16 May 2011 15:37:49 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> See mml-secure-message-sign-pgpmime to sign an entire message, as
> opposed to just a single part.

Thanks! That's good to know. (Trying here.)

> I think the two paths reconverge later in the series.  Can you look
> ahead a bit to see if that concern is addressed?

Daniel did point me to later in the series where the numbers are at
least the same in text and json output. I've grabbed that state for
generating a test case, (also with Daniels test message). And I'm now
cooking up a new commit that will pass that test but with less code
duplication than the current patch.

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]