01/2c8be81335e284d0a86e40f25745227ddd200a

   1 Return-Path: <tomi.ollila@iki.fi>\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 4483D431FB6\r
   6         for <notmuch@notmuchmail.org>; Tue, 21 Aug 2012 00:35:08 -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\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]\r
  12         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 wQu4IE9wGVsi for <notmuch@notmuchmail.org>;\r
  16         Tue, 21 Aug 2012 00:35:07 -0700 (PDT)\r
  17 Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34])\r
  18         by olra.theworths.org (Postfix) with ESMTP id 0C613431FAE\r
  19         for <notmuch@notmuchmail.org>; Tue, 21 Aug 2012 00:35:07 -0700 (PDT)\r
  20 Received: by guru.guru-group.fi (Postfix, from userid 501)\r
  21         id 9CE1C100386; Tue, 21 Aug 2012 10:35:14 +0300 (EEST)\r
  22 From: Tomi Ollila <tomi.ollila@iki.fi>\r
  23 To: Ben Gamari <bgamari.foss@gmail.com>, notmuch@notmuchmail.org\r
  24 Subject: Re: [PATCH 1/3] Add notmuch_database_close_compact\r
  25 In-Reply-To: <1345476704-17091-2-git-send-email-bgamari.foss@gmail.com>\r
  26 References: <1345476704-17091-1-git-send-email-bgamari.foss@gmail.com>\r
  27         <1345476704-17091-2-git-send-email-bgamari.foss@gmail.com>\r
  28 User-Agent: Notmuch/0.13.2+128~g2eb637a (http://notmuchmail.org) Emacs/23.1.1\r
  29         (x86_64-redhat-linux-gnu)\r
  30 X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL\r
  31         $[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F\r
  32         !)g;OY^,BjTbr)Np:%c_o'jj,Z\r
  33 Date: Tue, 21 Aug 2012 10:35:14 +0300\r
  34 Message-ID: <m27gssitot.fsf@guru.guru-group.fi>\r
  35 MIME-Version: 1.0\r
  36 Content-Type: text/plain; charset=us-ascii\r
  37 X-BeenThere: notmuch@notmuchmail.org\r
  38 X-Mailman-Version: 2.1.13\r
  39 Precedence: list\r
  40 List-Id: "Use and development of the notmuch mail system."\r
  41         <notmuch.notmuchmail.org>\r
  42 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  43         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  44 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  45 List-Post: <mailto:notmuch@notmuchmail.org>\r
  46 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  47 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  48         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  49 X-List-Received-Date: Tue, 21 Aug 2012 07:35:08 -0000\r
  50 \r
  51 On Mon, Aug 20 2012, Ben Gamari <bgamari.foss@gmail.com> wrote:\r
  52 \r
  53 > ---\r
  54 >  configure       |   25 ++++++++++++++++++++++++-\r
  55 >  lib/database.cc |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\r
  56 >  lib/notmuch.h   |   14 ++++++++++++++\r
  57 >  3 files changed, 92 insertions(+), 1 deletion(-)\r
  58 >\r
  59 > diff --git a/configure b/configure\r
  60 > index dc0dba4..370fedd 100755\r
  61 > --- a/configure\r
  62 > +++ b/configure\r
  63 > @@ -270,7 +270,8 @@ printf "Checking for Xapian development files... "\r
  64 >  have_xapian=0\r
  65 >  for xapian_config in ${XAPIAN_CONFIG}; do\r
  66 >      if ${xapian_config} --version > /dev/null 2>&1; then\r
  67 > -     printf "Yes (%s).\n" $(${xapian_config} --version | sed -e 's/.* //')\r
  68 > +     xapian_version=$(${xapian_config} --version | sed -e 's/.* //')\r
  69 > +     printf "Yes (%s).\n" ${xapian_version}\r
  70 >       have_xapian=1\r
  71 >       xapian_cxxflags=$(${xapian_config} --cxxflags)\r
  72 >       xapian_ldflags=$(${xapian_config} --libs)\r
  73 > @@ -282,6 +283,24 @@ if [ ${have_xapian} = "0" ]; then\r
  74 >      errors=$((errors + 1))\r
  75 >  fi\r
  76 >  \r
  77 > +have_xapian_compact=0\r
  78 > +if [ ${have_xapian} = "1" ]; then\r
  79 > +    printf "Checking for Xapian compact support... "\r
  80 > +    IFS='.'\r
  81 > +    old_args=$@\r
  82 > +    set -- ${xapian_version}\r
  83 > +    xapian_major_version=$1\r
  84 > +    xapian_minor_version=$2\r
  85 > +    xapian_subminor_version=$3\r
  86 > +    set -- ${old_args}\r
  87 \r
  88 The part above breaks the argument vector in case there are spaces in \r
  89 args (I though $IFS chars, but try the script), execute:\r
  90 \r
  91 $ echo '#!/bin/bash\r
  92 IFS=.\r
  93 for x in "$@"; do echo $x; done; echo\r
  94 foo=$@\r
  95 for x in $foo; do echo $x; done; echo\r
  96 set -- $foo\r
  97 for x in "$@"; do echo $x; done; echo\r
  98 ' > foo.bash\r
  99 \r
 100 $ bash foo.bash a "b c" d\r
 101 \r
 102 Also, after processing, IFS is not restored (to $DEFAULT_IFS)\r
 103 \r
 104 an alternative is to put the code in function like the following\r
 105 way:\r
 106 \r
 107 set_xapian_version ()\r
 108 {\r
 109         xapian_major_version=$1\r
 110         xapian_minor_version=$2\r
 111         xapian_subminor_version=$3\r
 112 }\r
 113 \r
 114 have_xapian_compact=0\r
 115 if [ ${have_xapian} = "1" ]; then\r
 116     printf "Checking for Xapian compact support... "\r
 117     IFS=.\r
 118     set_xapian_version ${xapian_version}\r
 119     IFS=$DEFAULT_IFS\r
 120     if [ "${xapian_major_version}" -gt 1 ] || [ ${xapian_minor_version} -gt 2 ] || [ ${xapian_subminor_version} -ge 6 ]; then\r
 121         printf "Yes.\n"\r
 122         have_xapian_compact=1\r
 123     else\r
 124         printf "No.\n"\r
 125     fi\r
 126 fi\r
 127 \r
 128 Hmm, I guess the check above is to determine whether xapian version is\r
 129 1.2.6 or newer, but is there xapian version 1.1.6 or 1.0.6 or 0.3.0 or so ?\r
 130 \r
 131 I am not qualified to comment about compaction itself :)\r
 132 \r
 133 In the last patch you give copyright to Carl (which is OK). However I'd\r
 134 debate whether it is good practise to declare Carl as author; I'd say that\r
 135 is OK if he agrees to claim authorship to your potentially shi^H^H^Hperfect\r
 136 code ;)\r
 137 \r
 138 There are at least a few style issues below: e.g. no space between function\r
 139 name and opening parenthesis.\r
 140 \r
 141 Tomi\r
 142 \r
 143 \r
 144 > +    if [ "${xapian_major_version}" -gt 1 ] || [ ${xapian_minor_version} -gt 2 ] || [ ${xapian_subminor_version} -ge 6 ]; then\r
 145 > +     printf "Yes.\n"\r
 146 > +     have_xapian_compact=1\r
 147 > +    else\r
 148 > +     printf "No.\n"\r
 149 > +    fi\r
 150 > +fi\r
 151 > +\r
 152 >  printf "Checking for GMime development files... "\r
 153 >  have_gmime=0\r
 154 >  IFS=';'\r
 155 > @@ -675,6 +694,9 @@ LINKER_RESOLVES_LIBRARY_DEPENDENCIES = ${linker_resolves_library_dependencies}\r
 156 >  XAPIAN_CXXFLAGS = ${xapian_cxxflags}\r
 157 >  XAPIAN_LDFLAGS = ${xapian_ldflags}\r
 158 >  \r
 159 > +# Whether compact is supported by this version of Xapian\r
 160 > +HAVE_XAPIAN_COMPACT = ${have_xapian_compact}\r
 161 > +\r
 162 >  # Flags needed to compile and link against GMime-2.4\r
 163 >  GMIME_CFLAGS = ${gmime_cflags}\r
 164 >  GMIME_LDFLAGS = ${gmime_ldflags}\r
 165 > @@ -711,6 +733,7 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)      \\\r
 166 >  CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\\r
 167 >                    \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\\r
 168 >                    \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS)             \\\r
 169 > +                     -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)       \\\r
 170 >                       -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR)\r
 171 >  CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS)\r
 172 >  EOF\r
 173 > diff --git a/lib/database.cc b/lib/database.cc\r
 174 > index 761dc1a..6e83a61 100644\r
 175 > --- a/lib/database.cc\r
 176 > +++ b/lib/database.cc\r
 177 > @@ -781,6 +781,60 @@ notmuch_database_close (notmuch_database_t *notmuch)\r
 178 >  }\r
 179 >  \r
 180 >  void\r
 181 > +notmuch_database_close_compact (notmuch_database_t *notmuch)\r
 182 > +{\r
 183 > +    void *local = talloc_new (NULL);\r
 184 > +    Xapian::Compactor compactor;\r
 185 > +    char *notmuch_path, *xapian_path, *compact_xapian_path, *old_xapian_path;\r
 186 > +\r
 187 > +#if HAVE_XAPIAN_COMPACT\r
 188 > +    if (! (notmuch_path = talloc_asprintf (local, "%s/%s", notmuch->path, ".notmuch"))) {\r
 189 > +     fprintf (stderr, "Out of memory\n");\r
 190 > +     goto DONE;\r
 191 > +    }\r
 192 > +\r
 193 > +    if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) {\r
 194 > +     fprintf (stderr, "Out of memory\n");\r
 195 > +     goto DONE;\r
 196 > +    }\r
 197 > +\r
 198 > +    if (! (compact_xapian_path = talloc_asprintf (local, "%s.compact", xapian_path))) {\r
 199 > +     fprintf (stderr, "Out of memory\n");\r
 200 > +     goto DONE;\r
 201 > +    }\r
 202 > +\r
 203 > +    if (! (old_xapian_path = talloc_asprintf (local, "%s.old", xapian_path))) {\r
 204 > +     fprintf (stderr, "Out of memory\n");\r
 205 > +     goto DONE;\r
 206 > +    }\r
 207 > +\r
 208 > +    try {\r
 209 > +     compactor.set_renumber(false);\r
 210 > +     compactor.add_source(xapian_path);\r
 211 > +     compactor.set_destdir(compact_xapian_path);\r
 212 > +     compactor.compact();\r
 213 > +\r
 214 > +     if (rename(xapian_path, old_xapian_path)) {\r
 215 > +         fprintf (stderr, "Error moving old database out of the way\n");\r
 216 > +         goto DONE;\r
 217 > +     }\r
 218 > +\r
 219 > +     if (rename(compact_xapian_path, xapian_path)) {\r
 220 > +         fprintf (stderr, "Error moving compacted database\n");\r
 221 > +         goto DONE;\r
 222 > +     }\r
 223 > +    } catch (Xapian::InvalidArgumentError e) {\r
 224 > +     fprintf (stderr, "Error while compacting: %s", e.get_msg().c_str());\r
 225 > +    }\r
 226 > +    \r
 227 > +#endif\r
 228 > +\r
 229 > +    notmuch_database_close(notmuch);\r
 230 > +DONE:\r
 231 > +    talloc_free(local);\r
 232 > +}\r
 233 > +\r
 234 > +void\r
 235 >  notmuch_database_destroy (notmuch_database_t *notmuch)\r
 236 >  {\r
 237 >      notmuch_database_close (notmuch);\r
 238 > diff --git a/lib/notmuch.h b/lib/notmuch.h\r
 239 > index 3633bed..50babfb 100644\r
 240 > --- a/lib/notmuch.h\r
 241 > +++ b/lib/notmuch.h\r
 242 > @@ -215,6 +215,20 @@ notmuch_database_open (const char *path,\r
 243 >  void\r
 244 >  notmuch_database_close (notmuch_database_t *database);\r
 245 >  \r
 246 > +/* Close the given notmuch database and then compact it.\r
 247 > + *\r
 248 > + * After notmuch_database_close_compact has been called, calls to\r
 249 > + * other functions on objects derived from this database may either\r
 250 > + * behave as if the database had not been closed (e.g., if the\r
 251 > + * required data has been cached) or may fail with a\r
 252 > + * NOTMUCH_STATUS_XAPIAN_EXCEPTION.\r
 253 > + *\r
 254 > + * notmuch_database_close_compact can be called multiple times.  Later\r
 255 > + * calls have no effect.\r
 256 > + */\r
 257 > +void\r
 258 > +notmuch_database_close_compact (notmuch_database_t *notmuch);\r
 259 > +\r
 260 >  /* Destroy the notmuch database, closing it if necessary and freeing\r
 261 >  * all associated resources. */\r
 262 >  void\r
 263 > -- \r
 264 > 1.7.9.5\r
 265 >\r
 266 > _______________________________________________\r
 267 > notmuch mailing list\r
 268 > notmuch@notmuchmail.org\r
 269 > http://notmuchmail.org/mailman/listinfo/notmuch\r