9f/6b4502423abe2c7a616aa275f749361606a098

   1 Return-Path: <jani@nikula.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 09437431FD2\r
   6         for <notmuch@notmuchmail.org>; Wed,  4 Dec 2013 10:40:47 -0800 (PST)\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 MgAPnpA6ZwOR for <notmuch@notmuchmail.org>;\r
  16         Wed,  4 Dec 2013 10:40:39 -0800 (PST)\r
  17 Received: from mail-qe0-f53.google.com (mail-qe0-f53.google.com\r
  18         [209.85.128.53]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
  19         (No client certificate requested)\r
  20         by olra.theworths.org (Postfix) with ESMTPS id 81F7B431FD0\r
  21         for <notmuch@notmuchmail.org>; Wed,  4 Dec 2013 10:40:39 -0800 (PST)\r
  22 Received: by mail-qe0-f53.google.com with SMTP id nc12so14632638qeb.40\r
  23         for <notmuch@notmuchmail.org>; Wed, 04 Dec 2013 10:40:39 -0800 (PST)\r
  24 X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;\r
  25         d=1e100.net; s=20130820;\r
  26         h=x-gm-message-state:from:to:cc:subject:in-reply-to:references\r
  27         :user-agent:date:message-id:mime-version:content-type;\r
  28         bh=H3AnR/WHVt9NqSYD2Xsm24B7qu1fT96IxNbbgD2Iuvs=;\r
  29         b=J7Pg+drn/5r3oRflblzLM0w4eS5Bw37kSDhG782GLoSJeogBUMPF3JDhD6yW4eHe7Q\r
  30         +v2KBnQNWIdEo1RBUwjgxGz1QKXkSJbSIT1xl7WmiqckasMvFUi8OE+6myXnzMmJ9Jo6\r
  31         VZhXgYQRGp6oGaFwKIk3DHlSDsZT/kagZM+oMVcQTShwOYef4VppdyUNOyv4K2Mkua+p\r
  32         +R+7HY1dmeOUxjO0SWSJCfCJkuvnP7AG7YZsdqbhzNl0j6fBDxfwiUfVQS1om7FeaSL9\r
  33         CUTtVlkn7d/JjHE4uXlQ9GInd6uBQUKQ5WEs9jNvK7rrXfwecxnCAubZEA9i3YJMqlNc\r
  34         bFMQ==\r
  35 X-Gm-Message-State:\r
  36  ALoCoQlByi/hNkAMF3+cPcbjZrKoNff0LtMMLMrkONRF+2OhYFADob6sisC5t6nbWk/QRoDyMU6W\r
  37 X-Received: by 10.224.75.200 with SMTP id z8mr138784229qaj.71.1386182438919;\r
  38         Wed, 04 Dec 2013 10:40:38 -0800 (PST)\r
  39 Received: from localhost ([2001:4b98:dc0:43:216:3eff:fe1b:25f3])\r
  40         by mx.google.com with ESMTPSA id hb2sm20160543qeb.6.2013.12.04.10.40.38\r
  41         for <multiple recipients>\r
  42         (version=TLSv1.1 cipher=RC4-SHA bits=128/128);\r
  43         Wed, 04 Dec 2013 10:40:38 -0800 (PST)\r
  44 From: Jani Nikula <jani@nikula.org>\r
  45 To: Austin Clements <amdragon@MIT.EDU>\r
  46 Subject: Re: [PATCH 1/2] lib: add return status to database close and destroy\r
  47 In-Reply-To: <20131204163132.GC8854@mit.edu>\r
  48 References: <cover.1385903109.git.jani@nikula.org>\r
  49         <29b808bb6bf051fe21b6a72f12bb4ad1badfbf97.1385903109.git.jani@nikula.org>\r
  50         <20131204163132.GC8854@mit.edu>\r
  51 User-Agent: Notmuch/0.17~rc2+4~gd7b0a0a (http://notmuchmail.org) Emacs/23.2.1\r
  52         (x86_64-pc-linux-gnu)\r
  53 Date: Wed, 04 Dec 2013 19:40:19 +0100\r
  54 Message-ID: <87ob4wsbn0.fsf@nikula.org>\r
  55 MIME-Version: 1.0\r
  56 Content-Type: text/plain; charset=us-ascii\r
  57 Cc: notmuch@notmuchmail.org\r
  58 X-BeenThere: notmuch@notmuchmail.org\r
  59 X-Mailman-Version: 2.1.13\r
  60 Precedence: list\r
  61 List-Id: "Use and development of the notmuch mail system."\r
  62         <notmuch.notmuchmail.org>\r
  63 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  64         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  65 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  66 List-Post: <mailto:notmuch@notmuchmail.org>\r
  67 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  68 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  69         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  70 X-List-Received-Date: Wed, 04 Dec 2013 18:40:47 -0000\r
  71 \r
  72 On Wed, 04 Dec 2013, Austin Clements <amdragon@MIT.EDU> wrote:\r
  73 > Quoth Jani Nikula on Dec 01 at  3:13 pm:\r
  74 >> notmuch_database_close may fail in Xapian ->flush() or ->close(), so\r
  75 >> report the status. Similarly for notmuch_database_destroy which calls\r
  76 >> close.\r
  77 >> \r
  78 >> This is required for notmuch insert to report error status if message\r
  79 >> indexing failed.\r
  80 >> \r
  81 >> Bump the notmuch version to allow clients to conditional build against\r
  82 >> both the current release and the next release (current git master).\r
  83 >> ---\r
  84 >>  lib/database.cc | 18 ++++++++++++++----\r
  85 >>  lib/notmuch.h   | 17 ++++++++++++++---\r
  86 >>  2 files changed, 28 insertions(+), 7 deletions(-)\r
  87 >> \r
  88 >> diff --git a/lib/database.cc b/lib/database.cc\r
  89 >> index f395061..98e2c31 100644\r
  90 >> --- a/lib/database.cc\r
  91 >> +++ b/lib/database.cc\r
  92 >> @@ -767,14 +767,17 @@ notmuch_database_open (const char *path,\r
  93 >>      return status;\r
  94 >>  }\r
  95 >>  \r
  96 >> -void\r
  97 >> +notmuch_status_t\r
  98 >>  notmuch_database_close (notmuch_database_t *notmuch)\r
  99 >>  {\r
 100 >> +    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;\r
 101 >> +\r
 102 >>      try {\r
 103 >>      if (notmuch->xapian_db != NULL &&\r
 104 >>          notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE)\r
 105 >>          (static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))->flush ();\r
 106 >>      } catch (const Xapian::Error &error) {\r
 107 >> +    status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;\r
 108 >>      if (! notmuch->exception_reported) {\r
 109 >>          fprintf (stderr, "Error: A Xapian exception occurred flushing database: %s\n",\r
 110 >>                   error.get_msg().c_str());\r
 111 >> @@ -789,6 +792,7 @@ notmuch_database_close (notmuch_database_t *notmuch)\r
 112 >>          notmuch->xapian_db->close();\r
 113 >>      } catch (const Xapian::Error &error) {\r
 114 >>          /* do nothing */\r
 115 >> +        status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;\r
 116 >>      }\r
 117 >>      }\r
 118 >>  \r
 119 >> @@ -802,6 +806,8 @@ notmuch_database_close (notmuch_database_t *notmuch)\r
 120 >>      notmuch->value_range_processor = NULL;\r
 121 >>      delete notmuch->date_range_processor;\r
 122 >>      notmuch->date_range_processor = NULL;\r
 123 >> +\r
 124 >> +    return status;\r
 125 >>  }\r
 126 >>  \r
 127 >>  #if HAVE_XAPIAN_COMPACT\r
 128 >> @@ -966,7 +972,7 @@ notmuch_database_compact (const char *path,\r
 129 >>  \r
 130 >>    DONE:\r
 131 >>      if (notmuch)\r
 132 >> -    notmuch_database_destroy (notmuch);\r
 133 >> +    ret = notmuch_database_destroy (notmuch);\r
 134 >\r
 135 > This will clobber the error code on most of the error paths.  Maybe\r
 136 > you want to only set ret if it's currently NOTMUCH_STATUS_SUCCESS?\r
 137 \r
 138 Good point, will fix.\r
 139 \r
 140 BR,\r
 141 Jani.\r
 142 \r
 143 \r
 144 >\r
 145 >>  \r
 146 >>      talloc_free (local);\r
 147 >>  \r
 148 >> @@ -984,11 +990,15 @@ notmuch_database_compact (unused (const char *path),\r
 149 >>  }\r
 150 >>  #endif\r
 151 >>  \r
 152 >> -void\r
 153 >> +notmuch_status_t\r
 154 >>  notmuch_database_destroy (notmuch_database_t *notmuch)\r
 155 >>  {\r
 156 >> -    notmuch_database_close (notmuch);\r
 157 >> +    notmuch_status_t status;\r
 158 >> +\r
 159 >> +    status = notmuch_database_close (notmuch);\r
 160 >>      talloc_free (notmuch);\r
 161 >> +\r
 162 >> +    return status;\r
 163 >>  }\r
 164 >>  \r
 165 >>  const char *\r
 166 >> diff --git a/lib/notmuch.h b/lib/notmuch.h\r
 167 >> index 7c3a30c..dbdce86 100644\r
 168 >> --- a/lib/notmuch.h\r
 169 >> +++ b/lib/notmuch.h\r
 170 >> @@ -42,7 +42,7 @@ NOTMUCH_BEGIN_DECLS\r
 171 >>  #endif\r
 172 >>  \r
 173 >>  #define NOTMUCH_MAJOR_VERSION       0\r
 174 >> -#define NOTMUCH_MINOR_VERSION       17\r
 175 >> +#define NOTMUCH_MINOR_VERSION       18\r
 176 >>  #define NOTMUCH_MICRO_VERSION       0\r
 177 >\r
 178 > This is actually what got me thinking about\r
 179 > id:1386173986-9624-1-git-send-email-amdragon@mit.edu (which obviously\r
 180 > conflicts).  In particular, the Python bindings don't have access to\r
 181 > these macros, and can only use the soname version.  (I think the Go\r
 182 > ones can use the macros; the Ruby ones certainly can.)\r
 183 >\r
 184 >>  \r
 185 >>  /*\r
 186 >> @@ -236,8 +236,16 @@ notmuch_database_open (const char *path,\r
 187 >>   *\r
 188 >>   * notmuch_database_close can be called multiple times.  Later calls\r
 189 >>   * have no effect.\r
 190 >> + *\r
 191 >> + * Return value:\r
 192 >> + *\r
 193 >> + * NOTMUCH_STATUS_SUCCESS: Successfully closed the database.\r
 194 >> + *\r
 195 >> + * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred; the\r
 196 >> + *  database has been closed but there are no guarantees the\r
 197 >> + *  changes to the database, if any, have been flushed to disk.\r
 198 >>   */\r
 199 >> -void\r
 200 >> +notmuch_status_t\r
 201 >>  notmuch_database_close (notmuch_database_t *database);\r
 202 >>  \r
 203 >>  /* A callback invoked by notmuch_database_compact to notify the user\r
 204 >> @@ -263,8 +271,11 @@ notmuch_database_compact (const char* path,\r
 205 >>  \r
 206 >>  /* Destroy the notmuch database, closing it if necessary and freeing\r
 207 >>   * all associated resources.\r
 208 >> + *\r
 209 >> + * Return value as in notmuch_database_close if the database was open;\r
 210 >> + * notmuch_database_destroy itself has no failure modes.\r
 211 >>   */\r
 212 >> -void\r
 213 >> +notmuch_status_t\r
 214 >>  notmuch_database_destroy (notmuch_database_t *database);\r
 215 >>  \r
 216 >>  /* Return the database path of the given database.\r
 217 >\r
 218 > Regarding bindings (since you asked me to think about them), these\r
 219 > should be a safe changes from an ABI perspective (though obviously the\r
 220 > bindings will need changes to take advantage of the new return code).\r