[PATCH] remove debugging spew from T590
[notmuch-archives.git] / 4a / a9cacf37b18b765420f44e34fba0e32b9ebd2b
1 Return-Path: <amdragon@mit.edu>\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 AFBB1431FBF\r
6         for <notmuch@notmuchmail.org>; Fri,  4 Apr 2014 14:56:35 -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.7\r
10 X-Spam-Level: \r
11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
12         tests=[RCVD_IN_DNSWL_LOW=-0.7] 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 LapDbwHj2+Ig for <notmuch@notmuchmail.org>;\r
16         Fri,  4 Apr 2014 14:56:30 -0700 (PDT)\r
17 Received: from dmz-mailsec-scanner-4.mit.edu (dmz-mailsec-scanner-4.mit.edu\r
18         [18.9.25.15])\r
19         (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
20         (No client certificate requested)\r
21         by olra.theworths.org (Postfix) with ESMTPS id 29E2B431FAF\r
22         for <notmuch@notmuchmail.org>; Fri,  4 Apr 2014 14:56:30 -0700 (PDT)\r
23 X-AuditID: 1209190f-f790b6d000000c3a-62-533f2a8d4372\r
24 Received: from mailhub-auth-2.mit.edu ( [18.7.62.36])\r
25         (using TLS with cipher AES256-SHA (256/256 bits))\r
26         (Client did not present a certificate)\r
27         by dmz-mailsec-scanner-4.mit.edu (Symantec Messaging Gateway) with SMTP\r
28         id 47.49.03130.D8A2F335; Fri,  4 Apr 2014 17:56:29 -0400 (EDT)\r
29 Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11])\r
30         by mailhub-auth-2.mit.edu (8.13.8/8.9.2) with ESMTP id s34LuRDG010027; \r
31         Fri, 4 Apr 2014 17:56:28 -0400\r
32 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
33         (authenticated bits=0)\r
34         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
35         by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id s34LuP2r013654\r
36         (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);\r
37         Fri, 4 Apr 2014 17:56:27 -0400\r
38 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80)\r
39         (envelope-from <amdragon@mit.edu>)\r
40         id 1WWC65-0004DI-N6; Fri, 04 Apr 2014 17:56:25 -0400\r
41 Date: Fri, 4 Apr 2014 17:56:25 -0400\r
42 From: Austin Clements <amdragon@MIT.EDU>\r
43 To: David Bremner <david@tethera.net>\r
44 Subject: Re: [Patch v6 4/6] restore: transparently support gzipped input\r
45 Message-ID: <20140404215625.GA15472@mit.edu>\r
46 References: <1396554083-3892-1-git-send-email-david@tethera.net>\r
47         <1396554083-3892-5-git-send-email-david@tethera.net>\r
48 MIME-Version: 1.0\r
49 Content-Type: text/plain; charset=us-ascii\r
50 Content-Disposition: inline\r
51 In-Reply-To: <1396554083-3892-5-git-send-email-david@tethera.net>\r
52 User-Agent: Mutt/1.5.21 (2010-09-15)\r
53 X-Brightmail-Tracker:\r
54  H4sIAAAAAAAAA+NgFmpkleLIzCtJLcpLzFFi42IRYrdT0e3Vsg826DrNY3GjtZvR4vrNmcwO\r
55         TB7PVt1i9thy6D1zAFMUl01Kak5mWWqRvl0CV0bP7gtsBVN0Kw5d3MPcwPhNqYuRk0NCwETi\r
56         /MTrTBC2mMSFe+vZQGwhgdlMEj96i7oYuYDsDYwST69+ZYdInGKSOLlBDCKxhFHi0NqprCAJ\r
57         FgEViUNznzGD2GwCGhLb9i9nBLFFBFQlrm6bDDaVWUBa4tvvZrBtwgIeErvfrAWr5xXQkdgw\r
58         aztQDQfQ0HKJp8tzIcKCEidnPmGBaNWSuPHvJRNICciY5f84QMKcAo4Sz658AJsiCnTBlJPb\r
59         2CYwCs1C0j0LSfcshO4FjMyrGGVTcqt0cxMzc4pTk3WLkxPz8lKLdE30cjNL9FJTSjcxgkKa\r
60         U5J/B+O3g0qHGAU4GJV4eDt22AULsSaWFVfmHmKU5GBSEuXtUrAPFuJLyk+pzEgszogvKs1J\r
61         LT7EKMHBrCTCK6MKlONNSaysSi3Kh0lJc7AoifO+tbYKFhJITyxJzU5NLUgtgsnKcHAoSfBO\r
62         0wRqFCxKTU+tSMvMKUFIM3FwggznARp+DqSGt7ggMbc4Mx0if4pRUUqcNwAkIQCSyCjNg+uF\r
63         pZxXjOJArwjzrgKp4gGmK7juV0CDmYAGN4TZgQwuSURISTUwZpysXdPuv0xss49+p9yZ52sy\r
64         mrO4/GIOs23pdze49Oj1wanNUSI+XsnZLXd/mDOeFdh+eGsiz2KRvdzCntp895mjV/bpfG6P\r
65         fMC9tV87+UGjisihX7pnVnxcMI/7ih73CrHCd/ObzNc0XA2znrN92ezfumdyrrxe1vBqq7yf\r
66         uNaJZUHlcdETlViKMxINtZiLihMBOQFFqxQDAAA=\r
67 Cc: notmuch@notmuchmail.org\r
68 X-BeenThere: notmuch@notmuchmail.org\r
69 X-Mailman-Version: 2.1.13\r
70 Precedence: list\r
71 List-Id: "Use and development of the notmuch mail system."\r
72         <notmuch.notmuchmail.org>\r
73 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
74         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
75 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
76 List-Post: <mailto:notmuch@notmuchmail.org>\r
77 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
78 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
79         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
80 X-List-Received-Date: Fri, 04 Apr 2014 21:56:35 -0000\r
81 \r
82 Quoth David Bremner on Apr 03 at  4:41 pm:\r
83 > We rely completely on zlib to do the right thing in detecting gzipped\r
84 > input. Since our dump format is chosen to be 7 bit ascii, this should\r
85 > be fine.\r
86 > ---\r
87 >  doc/man1/notmuch-restore.rst |  8 ++++++++\r
88 >  notmuch-restore.c            | 41 ++++++++++++++++++++++++++---------------\r
89 >  test/T240-dump-restore.sh    | 14 ++++++++++++++\r
90 >  3 files changed, 48 insertions(+), 15 deletions(-)\r
91\r
92 > diff --git a/doc/man1/notmuch-restore.rst b/doc/man1/notmuch-restore.rst\r
93 > index d6cf19a..936b138 100644\r
94 > --- a/doc/man1/notmuch-restore.rst\r
95 > +++ b/doc/man1/notmuch-restore.rst\r
96 > @@ -50,6 +50,14 @@ Supported options for **restore** include\r
97 >              format, this heuristic, based the fact that batch-tag format\r
98 >              contains no parentheses, should be accurate.\r
99 >  \r
100 > +GZIPPED INPUT\r
101 > +=============\r
102 > +\r
103 > +\ **notmuch restore** will detect if the input is compressed in\r
104 > +**gzip(1)** format and automatically decompress it while reading. This\r
105 > +detection does not depend on file naming and in particular works for\r
106 > +standard input.\r
107 > +\r
108 >  SEE ALSO\r
109 >  ========\r
110 >  \r
111 > diff --git a/notmuch-restore.c b/notmuch-restore.c\r
112 > index c54d513..eb5b7b2 100644\r
113 > --- a/notmuch-restore.c\r
114 > +++ b/notmuch-restore.c\r
115 > @@ -22,6 +22,7 @@\r
116 >  #include "hex-escape.h"\r
117 >  #include "tag-util.h"\r
118 >  #include "string-util.h"\r
119 > +#include "zlib-extra.h"\r
120 >  \r
121 >  static regex_t regex;\r
122 >  \r
123 > @@ -128,10 +129,9 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])\r
124 >      tag_op_list_t *tag_ops;\r
125 >  \r
126 >      char *input_file_name = NULL;\r
127 > -    FILE *input = stdin;\r
128 > +    gzFile input;\r
129 >      char *line = NULL;\r
130 >      void *line_ctx = NULL;\r
131 > -    size_t line_size;\r
132 >      ssize_t line_len;\r
133 >  \r
134 >      int ret = 0;\r
135 > @@ -163,13 +163,23 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])\r
136 >      if (! accumulate)\r
137 >       flags |= TAG_FLAG_REMOVE_ALL;\r
138 >  \r
139 > -    if (input_file_name) {\r
140 > -     input = fopen (input_file_name, "r");\r
141 > -     if (input == NULL) {\r
142 > -         fprintf (stderr, "Error opening %s for reading: %s\n",\r
143 > -                  input_file_name, strerror (errno));\r
144 > +    if (input_file_name)\r
145 > +     input = gzopen (input_file_name, "r");\r
146 > +    else {\r
147 > +     int infd = dup (STDIN_FILENO);\r
148 > +     if (infd < 0) {\r
149 > +         fprintf (stderr, "Error duping stdin\n");\r
150 >           return EXIT_FAILURE;\r
151 >       }\r
152 > +     input = gzdopen (infd, "r");\r
153 > +     if (! input)\r
154 > +         close (infd);\r
155 > +    }\r
156 > +\r
157 > +    if (input == NULL) {\r
158 > +     fprintf (stderr, "Error opening %s for (gzip) reading: %s\n",\r
159 > +              input_file_name ? input_file_name : "stdin", strerror (errno));\r
160 \r
161 There's a sketchy line about errno in the gz(d)open docs: "On error,\r
162 gzopen() *may* set the global variable errno to indicate the error."\r
163 (emphasis mine).  This suggests we should set errno to 0 before the\r
164 calls to gz(d)open above.\r
165 \r
166 > +     return EXIT_FAILURE;\r
167 >      }\r
168 >  \r
169 >      if (opt_index < argc) {\r
170 > @@ -184,12 +194,17 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])\r
171 >      }\r
172 >  \r
173 >      do {\r
174 > -     line_len = getline (&line, &line_size, input);\r
175 > +     util_status_t status;\r
176 > +\r
177 > +     status = gz_getline (line_ctx, &line, &line_len, input);\r
178 >  \r
179 >       /* empty input file not considered an error */\r
180 > -     if (line_len < 0)\r
181 > +     if (status == UTIL_EOF)\r
182 >           return EXIT_SUCCESS;\r
183 >  \r
184 > +     if (status)\r
185 \r
186 Will this lead to a silent exit failure if there's a problem with\r
187 decompression?  This suggests we should have a UTIL_GZERROR that tells\r
188 the caller to consult gzerror for the error message.  (Though this is\r
189 still an improvement over the original code, which would silently\r
190 succeed when getline failed!)\r
191 \r
192 > +         return EXIT_FAILURE;\r
193 > +\r
194 >      } while ((line_len == 0) ||\r
195 >            (line[0] == '#') ||\r
196 >            /* the cast is safe because we checked about for line_len < 0 */\r
197 > @@ -254,7 +269,7 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])\r
198 >       if (ret)\r
199 >           break;\r
200 >  \r
201 > -    }  while ((line_len = getline (&line, &line_size, input)) != -1);\r
202 > +    }  while (gz_getline (line_ctx, &line, &line_len, input) == UTIL_SUCCESS);\r
203 \r
204 It looks like a gz_getline error here will cause restore to stop and\r
205 claim that the restore was successful.  (The original code has this\r
206 problem, too.)\r
207 \r
208 >  \r
209 >      if (line_ctx != NULL)\r
210 >       talloc_free (line_ctx);\r
211 > @@ -262,13 +277,9 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])\r
212 >      if (input_format == DUMP_FORMAT_SUP)\r
213 >       regfree (&regex);\r
214 >  \r
215 > -    if (line)\r
216 > -     free (line);\r
217 > -\r
218 >      notmuch_database_destroy (notmuch);\r
219 >  \r
220 > -    if (input != stdin)\r
221 > -     fclose (input);\r
222 > +    gzclose_r (input);\r
223 >  \r
224 >      return ret ? EXIT_FAILURE : EXIT_SUCCESS;\r
225 >  }\r
226 > diff --git a/test/T240-dump-restore.sh b/test/T240-dump-restore.sh\r
227 > index b6d8602..efe463e 100755\r
228 > --- a/test/T240-dump-restore.sh\r
229 > +++ b/test/T240-dump-restore.sh\r
230 > @@ -80,6 +80,20 @@ notmuch dump --gzip --output=dump-gzip-outfile.gz\r
231 >  gunzip dump-gzip-outfile.gz\r
232 >  test_expect_equal_file dump.expected dump-gzip-outfile\r
233 >  \r
234 > +test_begin_subtest "restoring gzipped stdin"\r
235 > +notmuch dump --gzip --output=backup.gz\r
236 > +notmuch tag +new_tag '*'\r
237 > +notmuch restore < backup.gz\r
238 > +notmuch dump --output=dump.actual\r
239 > +test_expect_equal_file dump.expected dump.actual\r
240 > +\r
241 > +test_begin_subtest "restoring gzipped file"\r
242 > +notmuch dump --gzip --output=backup.gz\r
243 > +notmuch tag +new_tag '*'\r
244 > +notmuch restore --input=backup.gz\r
245 > +notmuch dump --output=dump.actual\r
246 > +test_expect_equal_file dump.expected dump.actual\r
247 > +\r
248 >  # Note, we assume all messages from cworth have a message-id\r
249 >  # containing cworth.org\r
250 >  \r