6e/5b28060da10a8cd6dee3b9e31e437e7cd0710c

   1 Return-Path: <cworth@cworth.org>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 973E5431FD0\r
   6         for <notmuch@notmuchmail.org>; Fri,  3 Jun 2011 15:38:42 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: 0.01\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=0.01 tagged_above=-999 required=5\r
  12         tests=[T_MIME_NO_TEXT=0.01] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id TZkBs+xkvFAH for <notmuch@notmuchmail.org>;\r
  16         Fri,  3 Jun 2011 15:38:41 -0700 (PDT)\r
  17 Received: from arlo.cworth.org (arlo.cworth.org [50.43.72.2])\r
  18         by olra.theworths.org (Postfix) with ESMTP id 9F8A2431FB6\r
  19         for <notmuch@notmuchmail.org>; Fri,  3 Jun 2011 15:38:41 -0700 (PDT)\r
  20 Received: from yoom.home.cworth.org (localhost [127.0.0.1])\r
  21         by arlo.cworth.org (Postfix) with ESMTP id 9C17229A5CC;\r
  22         Fri,  3 Jun 2011 15:38:40 -0700 (PDT)\r
  23 Received: by yoom.home.cworth.org (Postfix, from userid 1000)\r
  24         id 8EA0C54C02C; Fri,  3 Jun 2011 15:38:40 -0700 (PDT)\r
  25 From: Carl Worth <cworth@cworth.org>\r
  26 To: Jameson Graef Rollins <jrollins@finestructure.net>,\r
  27         Notmuch Mail <notmuch@notmuchmail.org>\r
  28 Subject: Re: [PATCH 25/25] Fix stdout stream grabbing in\r
  29         format_part_content_text\r
  30 In-Reply-To: <87lixihahz.fsf@servo.factory.finestructure.net>\r
  31 References: <1306619520-25730-1-git-send-email-jrollins@finestructure.net>\r
  32         <1306619520-25730-2-git-send-email-jrollins@finestructure.net>\r
  33         <1306619520-25730-3-git-send-email-jrollins@finestructure.net>\r
  34         <1306619520-25730-4-git-send-email-jrollins@finestructure.net>\r
  35         <1306619520-25730-5-git-send-email-jrollins@finestructure.net>\r
  36         <1306619520-25730-6-git-send-email-jrollins@finestructure.net>\r
  37         <1306619520-25730-7-git-send-email-jrollins@finestructure.net>\r
  38         <1306619520-25730-8-git-send-email-jrollins@finestructure.net>\r
  39         <1306619520-25730-9-git-send-email-jrollins@finestructure.net>\r
  40         <1306619520-25730-10-git-send-email-jrollins@finestructure.net>\r
  41         <1306619520-25730-11-git-send-email-jrollins@finestructure.net>\r
  42         <1306619520-25730-12-git-send-email-jrollins@finestructure.net>\r
  43         <1306619520-25730-13-git-send-email-jrollins@finestructure.net>\r
  44         <1306619520-25730-14-git-send-email-jrollins@finestructure.net>\r
  45         <1306619520-25730-15-git-send-email-jrollins@finestructure.net>\r
  46         <1306619520-25730-16-git-se nd-email-jrollins@finestructure.net>\r
  47         <1306619520-25730-17-git-send-email-jrollins@finestructure.net>\r
  48         <1306619520-25730-18-git-send-email-jrollins@finestructure.net>\r
  49         <1306619520-25730-19-git-send-email-jrollins@finestructure.net>\r
  50         <1306619520-25730-20-git-send-email-jrollins@finestructure.net>\r
  51         <1306619520-25730-21-git-send-email-jrollins@finestructure.net>\r
  52         <1306619520-25730-22-git-send-email-jrollins@finestructure.net>\r
  53         <1306619520-25730-23-git-send-email-jrollins@finestructure.net>\r
  54         <1306619520-25730-24-git-send-email-jrollins@finestructure.net>\r
  55         <1306619520-25730-25-git-send-email-jrollins@finestructure.net>\r
  56         <1306619520-25730-26-git-send-email-jrollins@finestructure.net>\r
  57         <87r57almd9.fsf@yoom.home.cworth.org>\r
  58         <87lixihahz.fsf@servo.factory.finestructure.net>\r
  59 User-Agent: Notmuch/0.5 (http://notmuchmail.org) Emacs/23.3.1\r
  60         (i486-pc-linux-gnu)\r
  61 Date: Fri, 03 Jun 2011 15:38:29 -0700\r
  62 Message-ID: <87d3iulevu.fsf@yoom.home.cworth.org>\r
  63 MIME-Version: 1.0\r
  64 Content-Type: multipart/signed; boundary="=-=-=";\r
  65         micalg=pgp-sha1; protocol="application/pgp-signature"\r
  66 X-BeenThere: notmuch@notmuchmail.org\r
  67 X-Mailman-Version: 2.1.13\r
  68 Precedence: list\r
  69 List-Id: "Use and development of the notmuch mail system."\r
  70         <notmuch.notmuchmail.org>\r
  71 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  72         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  73 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  74 List-Post: <mailto:notmuch@notmuchmail.org>\r
  75 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  76 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  77         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  78 X-List-Received-Date: Fri, 03 Jun 2011 22:38:42 -0000\r
  79 \r
  80 --=-=-=\r
  81 Content-Transfer-Encoding: quoted-printable\r
  82 \r
  83 On Fri, 03 Jun 2011 14:26:48 -0700, Jameson Graef Rollins <jrollins@finestr=\r
  84 ucture.net> wrote:\r
  85 > Well actually it's only meant to sound like the committer doesn't\r
  86 > understand the problem!\r
  87 \r
  88 Heh, OK.\r
  89 \r
  90 > > I'd like to investigate this more. Perhaps with a test case?\r
  91 >=20\r
  92 > The current tests are how I found the problem!  Without this patch at\r
  93 > least the multipart tests will fail.  I don't see how another test will\r
  94 > add anything.\r
  95 \r
  96 Ah, in my review I'd managed to get this commit detached from the\r
  97 original previous commit that introduced the test failures. It's funny\r
  98 that while you were replying I was reviewing *that* commit and thinking,\r
  99 "why are all these tests failing now just because they changed to\r
 100 test_expect_equal_file"?\r
 101 \r
 102 > Carl, if you (or anyone else) understands what the issue is, then please\r
 103 > go ahead and modify the commit message.\r
 104 \r
 105 Done.\r
 106 \r
 107 >                                         I don't understand things\r
 108 > enough myself to do any better.  Clearly there is some strange\r
 109 > interaction with things that try to use stdout after\r
 110 > g_mime_stream_file_new() has already grabbed it.\r
 111 \r
 112 g_mime_stream_file_new is a bad citizen, API-wise. It closes files that\r
 113 it didn't open, (by default).\r
 114 \r
 115 > I really wouldn't block on this, though, since the patch does fix an\r
 116 > actual bug.\r
 117 \r
 118 Not blocked. All pushed. New commit message below for reference.\r
 119 \r
 120 =2DCarl\r
 121 \r
 122 commit d5b4d950245605b84c56ce991fa3c59a073a70e5\r
 123 Author: Jameson Graef Rollins <jrollins@finestructure.net>\r
 124 Date:   Sat May 28 14:52:00 2011 -0700\r
 125 \r
 126     show: Avoid inadvertently closing stdout\r
 127 =20=20=20=20\r
 128     GMime has a nasty habit of taking ownership by default of any FILE*\r
 129     handed to it va g_mime_stream_file_new. Specifically it will close the\r
 130     FILE* when the stream is destroyed---even though GMime didn't open the\r
 131     file itself.\r
 132 =20=20=20=20\r
 133     To avoid this bad behavior, we have to carefully set_owner(FALSE)\r
 134     after calling g_mime_stream_file_new. In the format_part_content_text\r
 135     function, since commit d92146d3a6809f8ad940302af49cd99a0820665e we've\r
 136     been calling g_mime_stream_file_new unconditionally, but only calling\r
 137     g_mime_stream_file_set_owner(FALSE) conditionally.\r
 138 =20=20=20=20\r
 139     This led to the FILE* being closed early when notmuch show output was\r
 140     redirected to a file.\r
 141 =20=20=20=20\r
 142     Fixing this fixes the test-suite cases that broke with the previous\r
 143     commit, (which added redirected "notmuch show" calls to the test suite\r
 144     to expose this bug).\r
 145 =20=20=20=20\r
 146     Edited-by: Carl Worth <cworth@cworth.org> with a new commit message to\r
 147     explain the bug and fix.\r
 148 \r
 149 --=-=-=\r
 150 Content-Type: application/pgp-signature\r
 151 \r
 152 -----BEGIN PGP SIGNATURE-----\r
 153 Version: GnuPG v1.4.11 (GNU/Linux)\r
 154 \r
 155 iEYEARECAAYFAk3pYmUACgkQ6JDdNq8qSWjz3QCfZ3bmMGBdHscBBSOKiUSRaxHo\r
 156 7hUAnRFkfAXcy9bK6c/+KTcsj+SOsL8o\r
 157 =r/3M\r
 158 -----END PGP SIGNATURE-----\r
 159 --=-=-=--\r