[PATCH 1/3] Add notmuch_database_close_compact
On Wed, Oct 17 2012, Ben Gamari wrote: > --- > configure | 21 - > lib/database.cc | 54 ++ > lib/notmuch.h | 14 ++ > 3 files changed, 88 insertions(+), 1 deletion(-) > > diff --git a/configure b/configure > index acb90a8..6551b13 100755 > --- a/configure > +++ b/configure > @@ -270,7 +270,8 @@ printf "Checking for Xapian development files... " > have_xapian=0 > for xapian_config in ${XAPIAN_CONFIG}; do > if ${xapian_config} --version > /dev/null 2>&1; then > - printf "Yes (%s).\n" $(${xapian_config} --version | sed -e 's/.* //') > + xapian_version=$(${xapian_config} --version | sed -e 's/.* //') > + printf "Yes (%s).\n" ${xapian_version} > have_xapian=1 > xapian_cxxflags=$(${xapian_config} --cxxflags) > xapian_ldflags=$(${xapian_config} --libs) > @@ -282,6 +283,20 @@ if [ ${have_xapian} = "0" ]; then > errors=$((errors + 1)) > fi > > +have_xapian_compact=0 > +if [ ${have_xapian} = "1" ]; then > +printf "Checking for Xapian compact support... " > +case "${xapian_version}" in > +0.*|1.[01].*|1.2.[0-5]) > +printf "No.\n" ;; > +[1-9]*.[0-9]*.[0-9]*) > +have_xapian_compact=1 > +printf "Yes.\n" ;; > +*) > +printf "Unknown version.\n" ;; > +esac > +fi This is pretty good approximation(*) for the version check, but some comments are in place, like mentioning that compaction is supported in Xapian versions 1.2.6 and newer (as the code is not obvious to casual reader). (*) The match for yes part gives some room for non-digit version parts (if that would ever be needed) and also it rules out (most) cases where garbage were assigned to xapian_version. Tomi > + > printf "Checking for GMime development files... " > have_gmime=0 > IFS=';' > @@ -679,6 +694,9 @@ LINKER_RESOLVES_LIBRARY_DEPENDENCIES = > ${linker_resolves_library_dependencies} > XAPIAN_CXXFLAGS = ${xapian_cxxflags} > XAPIAN_LDFLAGS = ${xapian_ldflags} > > +# Whether compact is supported by this version of Xapian > +HAVE_XAPIAN_COMPACT = ${have_xapian_compact} > + > # Flags needed to compile and link against GMime-2.4 > GMIME_CFLAGS = ${gmime_cflags} > GMIME_LDFLAGS = ${gmime_ldflags} > @@ -715,6 +733,7 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) > \$(GMIME_CFLAGS) \\ > CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)\\ >\$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\ >\$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS) \\ > + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) \\ > -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR) > CONFIGURE_LDFLAGS = \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS) > EOF > diff --git a/lib/database.cc b/lib/database.cc > index 761dc1a..6e83a61 100644 > --- a/lib/database.cc > +++ b/lib/database.cc > @@ -781,6 +781,60 @@ notmuch_database_close (notmuch_database_t *notmuch) > } > > void > +notmuch_database_close_compact (notmuch_database_t *notmuch) > +{ > +void *local = talloc_new (NULL); > +Xapian::Compactor compactor; > +char *notmuch_path, *xapian_path, *compact_xapian_path, *old_xapian_path; > + > +#if HAVE_XAPIAN_COMPACT > +if (! (notmuch_path = talloc_asprintf (local, "%s/%s", notmuch->path, > ".notmuch"))) { > + fprintf (stderr, "Out of memory\n"); > + goto DONE; > +} > + > +if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, > "xapian"))) { > + fprintf (stderr, "Out of memory\n"); > + goto DONE; > +} > + > +if (! (compact_xapian_path = talloc_asprintf (local, "%s.compact", > xapian_path))) { > + fprintf (stderr, "Out of memory\n"); > + goto DONE; > +} > + > +if (! (old_xapian_path = talloc_asprintf (local, "%s.old", > xapian_path))) { > + fprintf (stderr, "Out of memory\n"); > + goto DONE; > +} > + > +try { > + compactor.set_renumber(false); > + compactor.add_source(xapian_path); > + compactor.set_destdir(compact_xapian_path); > + compactor.compact(); > + > + if (rename(xapian_path, old_xapian_path)) { > + fprintf (stderr, "Error moving old database out of the way\n"); > + goto DONE; > + } > + > + if (rename(compact_xapian_path, xapian_path)) { > + fprintf (stderr, "Error moving compacted database\n"); > + goto DONE; > + } > +} catch (Xapian::InvalidArgumentError e) { > + fprintf (stderr, "Error while compacting: %s", e.get_msg().c_str()); > +} > + > +#endif > + > +notmuch_database_close(notmuch); > +DONE: > +talloc_free(local); > +} > + > +void > notmuch_database_destroy (notmuch_database_t *notmuch) > { > notmuch_database_close (notmuch); > diff --git a/lib/notmuch.h b/lib/notmuch.h > index 3633bed..50babfb 100644 > --- a/lib/notmuch.h > +++
Re: [PATCH 1/3] Add notmuch_database_close_compact
On Wed, Oct 17 2012, Ben Gamari bgamari.f...@gmail.com wrote: --- configure | 21 - lib/database.cc | 54 ++ lib/notmuch.h | 14 ++ 3 files changed, 88 insertions(+), 1 deletion(-) diff --git a/configure b/configure index acb90a8..6551b13 100755 --- a/configure +++ b/configure @@ -270,7 +270,8 @@ printf Checking for Xapian development files... have_xapian=0 for xapian_config in ${XAPIAN_CONFIG}; do if ${xapian_config} --version /dev/null 21; then - printf Yes (%s).\n $(${xapian_config} --version | sed -e 's/.* //') + xapian_version=$(${xapian_config} --version | sed -e 's/.* //') + printf Yes (%s).\n ${xapian_version} have_xapian=1 xapian_cxxflags=$(${xapian_config} --cxxflags) xapian_ldflags=$(${xapian_config} --libs) @@ -282,6 +283,20 @@ if [ ${have_xapian} = 0 ]; then errors=$((errors + 1)) fi +have_xapian_compact=0 +if [ ${have_xapian} = 1 ]; then +printf Checking for Xapian compact support... +case ${xapian_version} in +0.*|1.[01].*|1.2.[0-5]) +printf No.\n ;; +[1-9]*.[0-9]*.[0-9]*) +have_xapian_compact=1 +printf Yes.\n ;; +*) +printf Unknown version.\n ;; +esac +fi This is pretty good approximation(*) for the version check, but some comments are in place, like mentioning that compaction is supported in Xapian versions 1.2.6 and newer (as the code is not obvious to casual reader). (*) The match for yes part gives some room for non-digit version parts (if that would ever be needed) and also it rules out (most) cases where garbage were assigned to xapian_version. Tomi + printf Checking for GMime development files... have_gmime=0 IFS=';' @@ -679,6 +694,9 @@ LINKER_RESOLVES_LIBRARY_DEPENDENCIES = ${linker_resolves_library_dependencies} XAPIAN_CXXFLAGS = ${xapian_cxxflags} XAPIAN_LDFLAGS = ${xapian_ldflags} +# Whether compact is supported by this version of Xapian +HAVE_XAPIAN_COMPACT = ${have_xapian_compact} + # Flags needed to compile and link against GMime-2.4 GMIME_CFLAGS = ${gmime_cflags} GMIME_LDFLAGS = ${gmime_ldflags} @@ -715,6 +733,7 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)\\ \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\ \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS) \\ + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) \\ -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR) CONFIGURE_LDFLAGS = \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS) EOF diff --git a/lib/database.cc b/lib/database.cc index 761dc1a..6e83a61 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -781,6 +781,60 @@ notmuch_database_close (notmuch_database_t *notmuch) } void +notmuch_database_close_compact (notmuch_database_t *notmuch) +{ +void *local = talloc_new (NULL); +Xapian::Compactor compactor; +char *notmuch_path, *xapian_path, *compact_xapian_path, *old_xapian_path; + +#if HAVE_XAPIAN_COMPACT +if (! (notmuch_path = talloc_asprintf (local, %s/%s, notmuch-path, .notmuch))) { + fprintf (stderr, Out of memory\n); + goto DONE; +} + +if (! (xapian_path = talloc_asprintf (local, %s/%s, notmuch_path, xapian))) { + fprintf (stderr, Out of memory\n); + goto DONE; +} + +if (! (compact_xapian_path = talloc_asprintf (local, %s.compact, xapian_path))) { + fprintf (stderr, Out of memory\n); + goto DONE; +} + +if (! (old_xapian_path = talloc_asprintf (local, %s.old, xapian_path))) { + fprintf (stderr, Out of memory\n); + goto DONE; +} + +try { + compactor.set_renumber(false); + compactor.add_source(xapian_path); + compactor.set_destdir(compact_xapian_path); + compactor.compact(); + + if (rename(xapian_path, old_xapian_path)) { + fprintf (stderr, Error moving old database out of the way\n); + goto DONE; + } + + if (rename(compact_xapian_path, xapian_path)) { + fprintf (stderr, Error moving compacted database\n); + goto DONE; + } +} catch (Xapian::InvalidArgumentError e) { + fprintf (stderr, Error while compacting: %s, e.get_msg().c_str()); +} + +#endif + +notmuch_database_close(notmuch); +DONE: +talloc_free(local); +} + +void notmuch_database_destroy (notmuch_database_t *notmuch) { notmuch_database_close (notmuch); diff --git a/lib/notmuch.h b/lib/notmuch.h index 3633bed..50babfb 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -215,6 +215,20 @@ notmuch_database_open (const char *path, void notmuch_database_close (notmuch_database_t *database); +/* Close the given
[PATCH 1/3] Add notmuch_database_close_compact
Hi Ben - I'd like some meaningful commit messages here. Please find other comments inline. BR, Jani. On Wed, 17 Oct 2012, Ben Gamari wrote: > --- > configure | 21 - > lib/database.cc | 54 ++ > lib/notmuch.h | 14 ++ > 3 files changed, 88 insertions(+), 1 deletion(-) > > diff --git a/configure b/configure > index acb90a8..6551b13 100755 > --- a/configure > +++ b/configure > @@ -270,7 +270,8 @@ printf "Checking for Xapian development files... " > have_xapian=0 > for xapian_config in ${XAPIAN_CONFIG}; do > if ${xapian_config} --version > /dev/null 2>&1; then > - printf "Yes (%s).\n" $(${xapian_config} --version | sed -e 's/.* //') > + xapian_version=$(${xapian_config} --version | sed -e 's/.* //') > + printf "Yes (%s).\n" ${xapian_version} > have_xapian=1 > xapian_cxxflags=$(${xapian_config} --cxxflags) > xapian_ldflags=$(${xapian_config} --libs) > @@ -282,6 +283,20 @@ if [ ${have_xapian} = "0" ]; then > errors=$((errors + 1)) > fi > > +have_xapian_compact=0 > +if [ ${have_xapian} = "1" ]; then > +printf "Checking for Xapian compact support... " > +case "${xapian_version}" in > +0.*|1.[01].*|1.2.[0-5]) > +printf "No.\n" ;; > +[1-9]*.[0-9]*.[0-9]*) > +have_xapian_compact=1 > +printf "Yes.\n" ;; > +*) > +printf "Unknown version.\n" ;; > +esac > +fi > + > printf "Checking for GMime development files... " > have_gmime=0 > IFS=';' > @@ -679,6 +694,9 @@ LINKER_RESOLVES_LIBRARY_DEPENDENCIES = > ${linker_resolves_library_dependencies} > XAPIAN_CXXFLAGS = ${xapian_cxxflags} > XAPIAN_LDFLAGS = ${xapian_ldflags} > > +# Whether compact is supported by this version of Xapian > +HAVE_XAPIAN_COMPACT = ${have_xapian_compact} > + > # Flags needed to compile and link against GMime-2.4 > GMIME_CFLAGS = ${gmime_cflags} > GMIME_LDFLAGS = ${gmime_ldflags} > @@ -715,6 +733,7 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) > \$(GMIME_CFLAGS) \\ > CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)\\ >\$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\ >\$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS) \\ > + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) \\ > -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR) > CONFIGURE_LDFLAGS = \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS) > EOF > diff --git a/lib/database.cc b/lib/database.cc > index 761dc1a..6e83a61 100644 > --- a/lib/database.cc > +++ b/lib/database.cc > @@ -781,6 +781,60 @@ notmuch_database_close (notmuch_database_t *notmuch) > } > > void > +notmuch_database_close_compact (notmuch_database_t *notmuch) It is clear that there are error conditions, so this should be of type notmuch_status_t. Even if you don't know what to do with the errors now, you'll find that changing the API is a PITA later. > +{ > +void *local = talloc_new (NULL); > +Xapian::Compactor compactor; > +char *notmuch_path, *xapian_path, *compact_xapian_path, *old_xapian_path; > + > +#if HAVE_XAPIAN_COMPACT > +if (! (notmuch_path = talloc_asprintf (local, "%s/%s", notmuch->path, > ".notmuch"))) { > + fprintf (stderr, "Out of memory\n"); > + goto DONE; > +} You could include notmuch->path and .notmuch above into the asprintf below. A personal preference, I think this style would be much more readable: xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"); if (!xapian_path) { ... } > + > +if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, > "xapian"))) { > + fprintf (stderr, "Out of memory\n"); > + goto DONE; > +} > + > +if (! (compact_xapian_path = talloc_asprintf (local, "%s.compact", > xapian_path))) { > + fprintf (stderr, "Out of memory\n"); > + goto DONE; > +} > + > +if (! (old_xapian_path = talloc_asprintf (local, "%s.old", > xapian_path))) { > + fprintf (stderr, "Out of memory\n"); > + goto DONE; > +} > + > +try { > + compactor.set_renumber(false); > + compactor.add_source(xapian_path); > + compactor.set_destdir(compact_xapian_path); > + compactor.compact(); > + > + if (rename(xapian_path, old_xapian_path)) { > + fprintf (stderr, "Error moving old database out of the way\n"); > + goto DONE; > + } > + > + if (rename(compact_xapian_path, xapian_path)) { > + fprintf (stderr, "Error moving compacted database\n"); > + goto DONE; > + } Please add strerror(errno) into the two error prints above. The user would probably like to know why the errors occurred. > +} catch (Xapian::InvalidArgumentError e) { Should this be catch (const Xapian::Error ) ? > + fprintf (stderr, "Error while compacting: %s", e.get_msg().c_str()); > +}
[PATCH 1/3] Add notmuch_database_close_compact
--- configure | 21 - lib/database.cc | 54 ++ lib/notmuch.h | 14 ++ 3 files changed, 88 insertions(+), 1 deletion(-) diff --git a/configure b/configure index acb90a8..6551b13 100755 --- a/configure +++ b/configure @@ -270,7 +270,8 @@ printf "Checking for Xapian development files... " have_xapian=0 for xapian_config in ${XAPIAN_CONFIG}; do if ${xapian_config} --version > /dev/null 2>&1; then - printf "Yes (%s).\n" $(${xapian_config} --version | sed -e 's/.* //') + xapian_version=$(${xapian_config} --version | sed -e 's/.* //') + printf "Yes (%s).\n" ${xapian_version} have_xapian=1 xapian_cxxflags=$(${xapian_config} --cxxflags) xapian_ldflags=$(${xapian_config} --libs) @@ -282,6 +283,20 @@ if [ ${have_xapian} = "0" ]; then errors=$((errors + 1)) fi +have_xapian_compact=0 +if [ ${have_xapian} = "1" ]; then +printf "Checking for Xapian compact support... " +case "${xapian_version}" in +0.*|1.[01].*|1.2.[0-5]) +printf "No.\n" ;; +[1-9]*.[0-9]*.[0-9]*) +have_xapian_compact=1 +printf "Yes.\n" ;; +*) +printf "Unknown version.\n" ;; +esac +fi + printf "Checking for GMime development files... " have_gmime=0 IFS=';' @@ -679,6 +694,9 @@ LINKER_RESOLVES_LIBRARY_DEPENDENCIES = ${linker_resolves_library_dependencies} XAPIAN_CXXFLAGS = ${xapian_cxxflags} XAPIAN_LDFLAGS = ${xapian_ldflags} +# Whether compact is supported by this version of Xapian +HAVE_XAPIAN_COMPACT = ${have_xapian_compact} + # Flags needed to compile and link against GMime-2.4 GMIME_CFLAGS = ${gmime_cflags} GMIME_LDFLAGS = ${gmime_ldflags} @@ -715,6 +733,7 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)\\ \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\ \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS) \\ + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) \\ -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR) CONFIGURE_LDFLAGS = \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS) EOF diff --git a/lib/database.cc b/lib/database.cc index 761dc1a..6e83a61 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -781,6 +781,60 @@ notmuch_database_close (notmuch_database_t *notmuch) } void +notmuch_database_close_compact (notmuch_database_t *notmuch) +{ +void *local = talloc_new (NULL); +Xapian::Compactor compactor; +char *notmuch_path, *xapian_path, *compact_xapian_path, *old_xapian_path; + +#if HAVE_XAPIAN_COMPACT +if (! (notmuch_path = talloc_asprintf (local, "%s/%s", notmuch->path, ".notmuch"))) { + fprintf (stderr, "Out of memory\n"); + goto DONE; +} + +if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) { + fprintf (stderr, "Out of memory\n"); + goto DONE; +} + +if (! (compact_xapian_path = talloc_asprintf (local, "%s.compact", xapian_path))) { + fprintf (stderr, "Out of memory\n"); + goto DONE; +} + +if (! (old_xapian_path = talloc_asprintf (local, "%s.old", xapian_path))) { + fprintf (stderr, "Out of memory\n"); + goto DONE; +} + +try { + compactor.set_renumber(false); + compactor.add_source(xapian_path); + compactor.set_destdir(compact_xapian_path); + compactor.compact(); + + if (rename(xapian_path, old_xapian_path)) { + fprintf (stderr, "Error moving old database out of the way\n"); + goto DONE; + } + + if (rename(compact_xapian_path, xapian_path)) { + fprintf (stderr, "Error moving compacted database\n"); + goto DONE; + } +} catch (Xapian::InvalidArgumentError e) { + fprintf (stderr, "Error while compacting: %s", e.get_msg().c_str()); +} + +#endif + +notmuch_database_close(notmuch); +DONE: +talloc_free(local); +} + +void notmuch_database_destroy (notmuch_database_t *notmuch) { notmuch_database_close (notmuch); diff --git a/lib/notmuch.h b/lib/notmuch.h index 3633bed..50babfb 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -215,6 +215,20 @@ notmuch_database_open (const char *path, void notmuch_database_close (notmuch_database_t *database); +/* Close the given notmuch database and then compact it. + * + * After notmuch_database_close_compact has been called, calls to + * other functions on objects derived from this database may either + * behave as if the database had not been closed (e.g., if the + * required data has been cached) or may fail with a + * NOTMUCH_STATUS_XAPIAN_EXCEPTION. + * + * notmuch_database_close_compact can be called multiple times. Later + * calls have no effect. + */ +void +notmuch_database_close_compact (notmuch_database_t *notmuch); +
Re: [PATCH 1/3] Add notmuch_database_close_compact
Hi Ben - I'd like some meaningful commit messages here. Please find other comments inline. BR, Jani. On Wed, 17 Oct 2012, Ben Gamari bgamari.f...@gmail.com wrote: --- configure | 21 - lib/database.cc | 54 ++ lib/notmuch.h | 14 ++ 3 files changed, 88 insertions(+), 1 deletion(-) diff --git a/configure b/configure index acb90a8..6551b13 100755 --- a/configure +++ b/configure @@ -270,7 +270,8 @@ printf Checking for Xapian development files... have_xapian=0 for xapian_config in ${XAPIAN_CONFIG}; do if ${xapian_config} --version /dev/null 21; then - printf Yes (%s).\n $(${xapian_config} --version | sed -e 's/.* //') + xapian_version=$(${xapian_config} --version | sed -e 's/.* //') + printf Yes (%s).\n ${xapian_version} have_xapian=1 xapian_cxxflags=$(${xapian_config} --cxxflags) xapian_ldflags=$(${xapian_config} --libs) @@ -282,6 +283,20 @@ if [ ${have_xapian} = 0 ]; then errors=$((errors + 1)) fi +have_xapian_compact=0 +if [ ${have_xapian} = 1 ]; then +printf Checking for Xapian compact support... +case ${xapian_version} in +0.*|1.[01].*|1.2.[0-5]) +printf No.\n ;; +[1-9]*.[0-9]*.[0-9]*) +have_xapian_compact=1 +printf Yes.\n ;; +*) +printf Unknown version.\n ;; +esac +fi + printf Checking for GMime development files... have_gmime=0 IFS=';' @@ -679,6 +694,9 @@ LINKER_RESOLVES_LIBRARY_DEPENDENCIES = ${linker_resolves_library_dependencies} XAPIAN_CXXFLAGS = ${xapian_cxxflags} XAPIAN_LDFLAGS = ${xapian_ldflags} +# Whether compact is supported by this version of Xapian +HAVE_XAPIAN_COMPACT = ${have_xapian_compact} + # Flags needed to compile and link against GMime-2.4 GMIME_CFLAGS = ${gmime_cflags} GMIME_LDFLAGS = ${gmime_ldflags} @@ -715,6 +733,7 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)\\ \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\ \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS) \\ + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) \\ -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR) CONFIGURE_LDFLAGS = \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS) EOF diff --git a/lib/database.cc b/lib/database.cc index 761dc1a..6e83a61 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -781,6 +781,60 @@ notmuch_database_close (notmuch_database_t *notmuch) } void +notmuch_database_close_compact (notmuch_database_t *notmuch) It is clear that there are error conditions, so this should be of type notmuch_status_t. Even if you don't know what to do with the errors now, you'll find that changing the API is a PITA later. +{ +void *local = talloc_new (NULL); +Xapian::Compactor compactor; +char *notmuch_path, *xapian_path, *compact_xapian_path, *old_xapian_path; + +#if HAVE_XAPIAN_COMPACT +if (! (notmuch_path = talloc_asprintf (local, %s/%s, notmuch-path, .notmuch))) { + fprintf (stderr, Out of memory\n); + goto DONE; +} You could include notmuch-path and .notmuch above into the asprintf below. A personal preference, I think this style would be much more readable: xapian_path = talloc_asprintf (local, %s/%s, notmuch_path, xapian); if (!xapian_path) { ... } + +if (! (xapian_path = talloc_asprintf (local, %s/%s, notmuch_path, xapian))) { + fprintf (stderr, Out of memory\n); + goto DONE; +} + +if (! (compact_xapian_path = talloc_asprintf (local, %s.compact, xapian_path))) { + fprintf (stderr, Out of memory\n); + goto DONE; +} + +if (! (old_xapian_path = talloc_asprintf (local, %s.old, xapian_path))) { + fprintf (stderr, Out of memory\n); + goto DONE; +} + +try { + compactor.set_renumber(false); + compactor.add_source(xapian_path); + compactor.set_destdir(compact_xapian_path); + compactor.compact(); + + if (rename(xapian_path, old_xapian_path)) { + fprintf (stderr, Error moving old database out of the way\n); + goto DONE; + } + + if (rename(compact_xapian_path, xapian_path)) { + fprintf (stderr, Error moving compacted database\n); + goto DONE; + } Please add strerror(errno) into the two error prints above. The user would probably like to know why the errors occurred. +} catch (Xapian::InvalidArgumentError e) { Should this be catch (const Xapian::Error error) ? + fprintf (stderr, Error while compacting: %s, e.get_msg().c_str()); +} + +#endif + +notmuch_database_close(notmuch); The database gets closed on Xapian errors, but not on talloc or rename errors, and the
[PATCH 1/3] Add notmuch_database_close_compact
On Thu, Aug 23 2012, Kim Minh Kaplanwrote: > Tomi Ollila?: > >> On Tue, Aug 21 2012, Ben Gamari wrote: >> >>> Eh? 1.2.6 is the first Xapian release to have Compactor exposed in the >>> public API. >> >> Presuming that those variables are always numeric the comparison could be: >> >> if [ ${xapian_major_version} -gt 1 ] || >>[ ${xapian_major_version} -eq 1 -a ${xapian_minor_version} -gt 2 ] || >>[ ${xapian_major_version} -eq 1 -a ${xapian_minor_version} -eq 2 -a \ >> ${xapian_subminor_version} -ge 6 ]; then >> >> (I could not figure out anything shorter and/or cleaner) > > Try: > > case "${xapian_version}" in > 0.*|1.[01].*|1.2.[0-5]) ;; > *) ... ;; > esac That sure is shorter -- and splitting xapian_version is not required... .. also that would take care the (improbable?) case that `${xapian_config} -- version` outputs something else than #.#.# in the future. On the other hand, the above doesn't catch junk, so maybe: case ${xapian_version} in 0.*|1.[01].*|1.2.[0-5]) handle no case ;; [1-9]*.[0-9]*.[0-9]*) handle yes case -- approximated test ;; *) failure ;; esac (and we (approximately) expect #.#.#) In any case, excellent idea ! > Kim Minh. Thanks, Tomi
[PATCH 1/3] Add notmuch_database_close_compact
Tomi Ollila?: > On Tue, Aug 21 2012, Ben Gamari wrote: > >> Eh? 1.2.6 is the first Xapian release to have Compactor exposed in the >> public API. > > Presuming that those variables are always numeric the comparison could be: > > if [ ${xapian_major_version} -gt 1 ] || >[ ${xapian_major_version} -eq 1 -a ${xapian_minor_version} -gt 2 ] || >[ ${xapian_major_version} -eq 1 -a ${xapian_minor_version} -eq 2 -a \ > ${xapian_subminor_version} -ge 6 ]; then > > (I could not figure out anything shorter and/or cleaner) Try: case "${xapian_version}" in 0.*|1.[01].*|1.2.[0-5]) ;; *) ... ;; esac Kim Minh.
Re: [PATCH 1/3] Add notmuch_database_close_compact
On Thu, Aug 23 2012, Kim Minh Kaplan kimminh.kaplan+nom...@afnic.fr wrote: Tomi Ollila : On Tue, Aug 21 2012, Ben Gamari wrote: Eh? 1.2.6 is the first Xapian release to have Compactor exposed in the public API. Presuming that those variables are always numeric the comparison could be: if [ ${xapian_major_version} -gt 1 ] || [ ${xapian_major_version} -eq 1 -a ${xapian_minor_version} -gt 2 ] || [ ${xapian_major_version} -eq 1 -a ${xapian_minor_version} -eq 2 -a \ ${xapian_subminor_version} -ge 6 ]; then (I could not figure out anything shorter and/or cleaner) Try: case ${xapian_version} in 0.*|1.[01].*|1.2.[0-5]) ;; *) ... ;; esac That sure is shorter -- and splitting xapian_version is not required... .. also that would take care the (improbable?) case that `${xapian_config} -- version` outputs something else than #.#.# in the future. On the other hand, the above doesn't catch junk, so maybe: case ${xapian_version} in 0.*|1.[01].*|1.2.[0-5]) handle no case ;; [1-9]*.[0-9]*.[0-9]*) handle yes case -- approximated test ;; *) failure ;; esac (and we (approximately) expect #.#.#) In any case, excellent idea ! Kim Minh. Thanks, Tomi ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/3] Add notmuch_database_close_compact
On Tue, Aug 21 2012, Ben Gamari wrote: > Tomi Ollila writes: > >> On Mon, Aug 20 2012, Ben Gamari wrote: >> > >> Hmm, I guess the check above is to determine whether xapian version is >> 1.2.6 or newer, but is there xapian version 1.1.6 or 1.0.6 or 0.3.0 or so ? >> > Eh? 1.2.6 is the first Xapian release to have Compactor exposed in the > public API. The comparison code >>> if [ "${xapian_major_version}" -gt 1 ] || >>>[ ${xapian_minor_version} -gt 2 ] || >>>[ ${xapian_subminor_version} -ge 6 ]; then would match e.g. 1.1.6, 1.0.6 and 0.3.0 Presuming that those variables are always numeric the comparison could be: if [ ${xapian_major_version} -gt 1 ] || [ ${xapian_major_version} -eq 1 -a ${xapian_minor_version} -gt 2 ] || [ ${xapian_major_version} -eq 1 -a ${xapian_minor_version} -eq 2 -a \ ${xapian_subminor_version} -ge 6 ]; then (I could not figure out anything shorter and/or cleaner) > > Thanks! > > Cheers, > > - Ben Tomi
Re: [PATCH 1/3] Add notmuch_database_close_compact
On Tue, Aug 21 2012, Ben Gamari wrote: Tomi Ollila tomi.oll...@iki.fi writes: On Mon, Aug 20 2012, Ben Gamari bgamari.f...@gmail.com wrote: Hmm, I guess the check above is to determine whether xapian version is 1.2.6 or newer, but is there xapian version 1.1.6 or 1.0.6 or 0.3.0 or so ? Eh? 1.2.6 is the first Xapian release to have Compactor exposed in the public API. The comparison code if [ ${xapian_major_version} -gt 1 ] || [ ${xapian_minor_version} -gt 2 ] || [ ${xapian_subminor_version} -ge 6 ]; then would match e.g. 1.1.6, 1.0.6 and 0.3.0 Presuming that those variables are always numeric the comparison could be: if [ ${xapian_major_version} -gt 1 ] || [ ${xapian_major_version} -eq 1 -a ${xapian_minor_version} -gt 2 ] || [ ${xapian_major_version} -eq 1 -a ${xapian_minor_version} -eq 2 -a \ ${xapian_subminor_version} -ge 6 ]; then (I could not figure out anything shorter and/or cleaner) Thanks! Cheers, - Ben Tomi ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/3] Add notmuch_database_close_compact
Tomi Ollila : On Tue, Aug 21 2012, Ben Gamari wrote: Eh? 1.2.6 is the first Xapian release to have Compactor exposed in the public API. Presuming that those variables are always numeric the comparison could be: if [ ${xapian_major_version} -gt 1 ] || [ ${xapian_major_version} -eq 1 -a ${xapian_minor_version} -gt 2 ] || [ ${xapian_major_version} -eq 1 -a ${xapian_minor_version} -eq 2 -a \ ${xapian_subminor_version} -ge 6 ]; then (I could not figure out anything shorter and/or cleaner) Try: case ${xapian_version} in 0.*|1.[01].*|1.2.[0-5]) ;; *) ... ;; esac Kim Minh. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/3] Add notmuch_database_close_compact
Tomi Ollila writes: > On Mon, Aug 20 2012, Ben Gamari wrote: > >> --- >> configure | 25 - >> lib/database.cc | 54 >> ++ >> lib/notmuch.h | 14 ++ >> 3 files changed, 92 insertions(+), 1 deletion(-) >> >> diff --git a/configure b/configure >> index dc0dba4..370fedd 100755 >> --- a/configure >> +++ b/configure >> @@ -270,7 +270,8 @@ printf "Checking for Xapian development files... " >> have_xapian=0 >> for xapian_config in ${XAPIAN_CONFIG}; do >> if ${xapian_config} --version > /dev/null 2>&1; then >> -printf "Yes (%s).\n" $(${xapian_config} --version | sed -e 's/.* //') >> +xapian_version=$(${xapian_config} --version | sed -e 's/.* //') >> +printf "Yes (%s).\n" ${xapian_version} >> have_xapian=1 >> xapian_cxxflags=$(${xapian_config} --cxxflags) >> xapian_ldflags=$(${xapian_config} --libs) >> @@ -282,6 +283,24 @@ if [ ${have_xapian} = "0" ]; then >> errors=$((errors + 1)) >> fi >> >> +have_xapian_compact=0 >> +if [ ${have_xapian} = "1" ]; then >> +printf "Checking for Xapian compact support... " >> +IFS='.' >> +old_args=$@ >> +set -- ${xapian_version} >> +xapian_major_version=$1 >> +xapian_minor_version=$2 >> +xapian_subminor_version=$3 >> +set -- ${old_args} > > The part above breaks the argument vector in case there are spaces in > args (I though $IFS chars, but try the script), execute: > Hmmm, I suppose so. > $ echo '#!/bin/bash > IFS=. > for x in "$@"; do echo $x; done; echo > foo=$@ > for x in $foo; do echo $x; done; echo > set -- $foo > for x in "$@"; do echo $x; done; echo > ' > foo.bash > > $ bash foo.bash a "b c" d > > Also, after processing, IFS is not restored (to $DEFAULT_IFS) > I assumed this would be alright since IFS is set in the next > an alternative is to put the code in function like the following > way: > Sounds good to me. > Hmm, I guess the check above is to determine whether xapian version is > 1.2.6 or newer, but is there xapian version 1.1.6 or 1.0.6 or 0.3.0 or so ? > Eh? 1.2.6 is the first Xapian release to have Compactor exposed in the public API. > I am not qualified to comment about compaction itself :) > Nor am I really. I just noticed that this functionality was blocking on library support which is now in place. It seemed that a pretty straightforward thing to implement and it hasn't broken my index yet. > In the last patch you give copyright to Carl (which is OK). However I'd > debate whether it is good practise to declare Carl as author; I'd say that > is OK if he agrees to claim authorship to your potentially shi^H^H^Hperfect > code ;) > Yikes. That certainly wasn't intentional. I'll fix this in the next iteration. > There are at least a few style issues below: e.g. no space between function > name and opening parenthesis. > Duly noted. Thanks! Cheers, - Ben
[PATCH 1/3] Add notmuch_database_close_compact
On Mon, Aug 20 2012, Ben Gamari wrote: > --- > configure | 25 - > lib/database.cc | 54 ++ > lib/notmuch.h | 14 ++ > 3 files changed, 92 insertions(+), 1 deletion(-) > > diff --git a/configure b/configure > index dc0dba4..370fedd 100755 > --- a/configure > +++ b/configure > @@ -270,7 +270,8 @@ printf "Checking for Xapian development files... " > have_xapian=0 > for xapian_config in ${XAPIAN_CONFIG}; do > if ${xapian_config} --version > /dev/null 2>&1; then > - printf "Yes (%s).\n" $(${xapian_config} --version | sed -e 's/.* //') > + xapian_version=$(${xapian_config} --version | sed -e 's/.* //') > + printf "Yes (%s).\n" ${xapian_version} > have_xapian=1 > xapian_cxxflags=$(${xapian_config} --cxxflags) > xapian_ldflags=$(${xapian_config} --libs) > @@ -282,6 +283,24 @@ if [ ${have_xapian} = "0" ]; then > errors=$((errors + 1)) > fi > > +have_xapian_compact=0 > +if [ ${have_xapian} = "1" ]; then > +printf "Checking for Xapian compact support... " > +IFS='.' > +old_args=$@ > +set -- ${xapian_version} > +xapian_major_version=$1 > +xapian_minor_version=$2 > +xapian_subminor_version=$3 > +set -- ${old_args} The part above breaks the argument vector in case there are spaces in args (I though $IFS chars, but try the script), execute: $ echo '#!/bin/bash IFS=. for x in "$@"; do echo $x; done; echo foo=$@ for x in $foo; do echo $x; done; echo set -- $foo for x in "$@"; do echo $x; done; echo ' > foo.bash $ bash foo.bash a "b c" d Also, after processing, IFS is not restored (to $DEFAULT_IFS) an alternative is to put the code in function like the following way: set_xapian_version () { xapian_major_version=$1 xapian_minor_version=$2 xapian_subminor_version=$3 } have_xapian_compact=0 if [ ${have_xapian} = "1" ]; then printf "Checking for Xapian compact support... " IFS=. set_xapian_version ${xapian_version} IFS=$DEFAULT_IFS if [ "${xapian_major_version}" -gt 1 ] || [ ${xapian_minor_version} -gt 2 ] || [ ${xapian_subminor_version} -ge 6 ]; then printf "Yes.\n" have_xapian_compact=1 else printf "No.\n" fi fi Hmm, I guess the check above is to determine whether xapian version is 1.2.6 or newer, but is there xapian version 1.1.6 or 1.0.6 or 0.3.0 or so ? I am not qualified to comment about compaction itself :) In the last patch you give copyright to Carl (which is OK). However I'd debate whether it is good practise to declare Carl as author; I'd say that is OK if he agrees to claim authorship to your potentially shi^H^H^Hperfect code ;) There are at least a few style issues below: e.g. no space between function name and opening parenthesis. Tomi > +if [ "${xapian_major_version}" -gt 1 ] || [ ${xapian_minor_version} -gt > 2 ] || [ ${xapian_subminor_version} -ge 6 ]; then > + printf "Yes.\n" > + have_xapian_compact=1 > +else > + printf "No.\n" > +fi > +fi > + > printf "Checking for GMime development files... " > have_gmime=0 > IFS=';' > @@ -675,6 +694,9 @@ LINKER_RESOLVES_LIBRARY_DEPENDENCIES = > ${linker_resolves_library_dependencies} > XAPIAN_CXXFLAGS = ${xapian_cxxflags} > XAPIAN_LDFLAGS = ${xapian_ldflags} > > +# Whether compact is supported by this version of Xapian > +HAVE_XAPIAN_COMPACT = ${have_xapian_compact} > + > # Flags needed to compile and link against GMime-2.4 > GMIME_CFLAGS = ${gmime_cflags} > GMIME_LDFLAGS = ${gmime_ldflags} > @@ -711,6 +733,7 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) > \$(GMIME_CFLAGS) \\ > CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)\\ >\$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\ >\$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS) \\ > + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) \\ > -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR) > CONFIGURE_LDFLAGS = \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS) > EOF > diff --git a/lib/database.cc b/lib/database.cc > index 761dc1a..6e83a61 100644 > --- a/lib/database.cc > +++ b/lib/database.cc > @@ -781,6 +781,60 @@ notmuch_database_close (notmuch_database_t *notmuch) > } > > void > +notmuch_database_close_compact (notmuch_database_t *notmuch) > +{ > +void *local = talloc_new (NULL); > +Xapian::Compactor compactor; > +char *notmuch_path, *xapian_path, *compact_xapian_path, *old_xapian_path; > + > +#if HAVE_XAPIAN_COMPACT > +if (! (notmuch_path = talloc_asprintf (local, "%s/%s", notmuch->path, > ".notmuch"))) { > + fprintf (stderr, "Out of memory\n"); > + goto DONE; > +} > + > +if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, > "xapian"))) { > + fprintf (stderr, "Out of memory\n"); > + goto DONE; > +} > + > +if (!
Re: [PATCH 1/3] Add notmuch_database_close_compact
On Mon, Aug 20 2012, Ben Gamari bgamari.f...@gmail.com wrote: --- configure | 25 - lib/database.cc | 54 ++ lib/notmuch.h | 14 ++ 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/configure b/configure index dc0dba4..370fedd 100755 --- a/configure +++ b/configure @@ -270,7 +270,8 @@ printf Checking for Xapian development files... have_xapian=0 for xapian_config in ${XAPIAN_CONFIG}; do if ${xapian_config} --version /dev/null 21; then - printf Yes (%s).\n $(${xapian_config} --version | sed -e 's/.* //') + xapian_version=$(${xapian_config} --version | sed -e 's/.* //') + printf Yes (%s).\n ${xapian_version} have_xapian=1 xapian_cxxflags=$(${xapian_config} --cxxflags) xapian_ldflags=$(${xapian_config} --libs) @@ -282,6 +283,24 @@ if [ ${have_xapian} = 0 ]; then errors=$((errors + 1)) fi +have_xapian_compact=0 +if [ ${have_xapian} = 1 ]; then +printf Checking for Xapian compact support... +IFS='.' +old_args=$@ +set -- ${xapian_version} +xapian_major_version=$1 +xapian_minor_version=$2 +xapian_subminor_version=$3 +set -- ${old_args} The part above breaks the argument vector in case there are spaces in args (I though $IFS chars, but try the script), execute: $ echo '#!/bin/bash IFS=. for x in $@; do echo $x; done; echo foo=$@ for x in $foo; do echo $x; done; echo set -- $foo for x in $@; do echo $x; done; echo ' foo.bash $ bash foo.bash a b c d Also, after processing, IFS is not restored (to $DEFAULT_IFS) an alternative is to put the code in function like the following way: set_xapian_version () { xapian_major_version=$1 xapian_minor_version=$2 xapian_subminor_version=$3 } have_xapian_compact=0 if [ ${have_xapian} = 1 ]; then printf Checking for Xapian compact support... IFS=. set_xapian_version ${xapian_version} IFS=$DEFAULT_IFS if [ ${xapian_major_version} -gt 1 ] || [ ${xapian_minor_version} -gt 2 ] || [ ${xapian_subminor_version} -ge 6 ]; then printf Yes.\n have_xapian_compact=1 else printf No.\n fi fi Hmm, I guess the check above is to determine whether xapian version is 1.2.6 or newer, but is there xapian version 1.1.6 or 1.0.6 or 0.3.0 or so ? I am not qualified to comment about compaction itself :) In the last patch you give copyright to Carl (which is OK). However I'd debate whether it is good practise to declare Carl as author; I'd say that is OK if he agrees to claim authorship to your potentially shi^H^H^Hperfect code ;) There are at least a few style issues below: e.g. no space between function name and opening parenthesis. Tomi +if [ ${xapian_major_version} -gt 1 ] || [ ${xapian_minor_version} -gt 2 ] || [ ${xapian_subminor_version} -ge 6 ]; then + printf Yes.\n + have_xapian_compact=1 +else + printf No.\n +fi +fi + printf Checking for GMime development files... have_gmime=0 IFS=';' @@ -675,6 +694,9 @@ LINKER_RESOLVES_LIBRARY_DEPENDENCIES = ${linker_resolves_library_dependencies} XAPIAN_CXXFLAGS = ${xapian_cxxflags} XAPIAN_LDFLAGS = ${xapian_ldflags} +# Whether compact is supported by this version of Xapian +HAVE_XAPIAN_COMPACT = ${have_xapian_compact} + # Flags needed to compile and link against GMime-2.4 GMIME_CFLAGS = ${gmime_cflags} GMIME_LDFLAGS = ${gmime_ldflags} @@ -711,6 +733,7 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)\\ \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\ \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS) \\ + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) \\ -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR) CONFIGURE_LDFLAGS = \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS) EOF diff --git a/lib/database.cc b/lib/database.cc index 761dc1a..6e83a61 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -781,6 +781,60 @@ notmuch_database_close (notmuch_database_t *notmuch) } void +notmuch_database_close_compact (notmuch_database_t *notmuch) +{ +void *local = talloc_new (NULL); +Xapian::Compactor compactor; +char *notmuch_path, *xapian_path, *compact_xapian_path, *old_xapian_path; + +#if HAVE_XAPIAN_COMPACT +if (! (notmuch_path = talloc_asprintf (local, %s/%s, notmuch-path, .notmuch))) { + fprintf (stderr, Out of memory\n); + goto DONE; +} + +if (! (xapian_path = talloc_asprintf (local, %s/%s, notmuch_path, xapian))) { + fprintf (stderr, Out of memory\n); + goto DONE; +} + +if (! (compact_xapian_path = talloc_asprintf (local, %s.compact, xapian_path))) { + fprintf (stderr, Out of memory\n); + goto
[PATCH 1/3] Add notmuch_database_close_compact
--- configure | 25 - lib/database.cc | 54 ++ lib/notmuch.h | 14 ++ 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/configure b/configure index dc0dba4..370fedd 100755 --- a/configure +++ b/configure @@ -270,7 +270,8 @@ printf "Checking for Xapian development files... " have_xapian=0 for xapian_config in ${XAPIAN_CONFIG}; do if ${xapian_config} --version > /dev/null 2>&1; then - printf "Yes (%s).\n" $(${xapian_config} --version | sed -e 's/.* //') + xapian_version=$(${xapian_config} --version | sed -e 's/.* //') + printf "Yes (%s).\n" ${xapian_version} have_xapian=1 xapian_cxxflags=$(${xapian_config} --cxxflags) xapian_ldflags=$(${xapian_config} --libs) @@ -282,6 +283,24 @@ if [ ${have_xapian} = "0" ]; then errors=$((errors + 1)) fi +have_xapian_compact=0 +if [ ${have_xapian} = "1" ]; then +printf "Checking for Xapian compact support... " +IFS='.' +old_args=$@ +set -- ${xapian_version} +xapian_major_version=$1 +xapian_minor_version=$2 +xapian_subminor_version=$3 +set -- ${old_args} +if [ "${xapian_major_version}" -gt 1 ] || [ ${xapian_minor_version} -gt 2 ] || [ ${xapian_subminor_version} -ge 6 ]; then + printf "Yes.\n" + have_xapian_compact=1 +else + printf "No.\n" +fi +fi + printf "Checking for GMime development files... " have_gmime=0 IFS=';' @@ -675,6 +694,9 @@ LINKER_RESOLVES_LIBRARY_DEPENDENCIES = ${linker_resolves_library_dependencies} XAPIAN_CXXFLAGS = ${xapian_cxxflags} XAPIAN_LDFLAGS = ${xapian_ldflags} +# Whether compact is supported by this version of Xapian +HAVE_XAPIAN_COMPACT = ${have_xapian_compact} + # Flags needed to compile and link against GMime-2.4 GMIME_CFLAGS = ${gmime_cflags} GMIME_LDFLAGS = ${gmime_ldflags} @@ -711,6 +733,7 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)\\ \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\ \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS) \\ + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) \\ -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR) CONFIGURE_LDFLAGS = \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS) EOF diff --git a/lib/database.cc b/lib/database.cc index 761dc1a..6e83a61 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -781,6 +781,60 @@ notmuch_database_close (notmuch_database_t *notmuch) } void +notmuch_database_close_compact (notmuch_database_t *notmuch) +{ +void *local = talloc_new (NULL); +Xapian::Compactor compactor; +char *notmuch_path, *xapian_path, *compact_xapian_path, *old_xapian_path; + +#if HAVE_XAPIAN_COMPACT +if (! (notmuch_path = talloc_asprintf (local, "%s/%s", notmuch->path, ".notmuch"))) { + fprintf (stderr, "Out of memory\n"); + goto DONE; +} + +if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) { + fprintf (stderr, "Out of memory\n"); + goto DONE; +} + +if (! (compact_xapian_path = talloc_asprintf (local, "%s.compact", xapian_path))) { + fprintf (stderr, "Out of memory\n"); + goto DONE; +} + +if (! (old_xapian_path = talloc_asprintf (local, "%s.old", xapian_path))) { + fprintf (stderr, "Out of memory\n"); + goto DONE; +} + +try { + compactor.set_renumber(false); + compactor.add_source(xapian_path); + compactor.set_destdir(compact_xapian_path); + compactor.compact(); + + if (rename(xapian_path, old_xapian_path)) { + fprintf (stderr, "Error moving old database out of the way\n"); + goto DONE; + } + + if (rename(compact_xapian_path, xapian_path)) { + fprintf (stderr, "Error moving compacted database\n"); + goto DONE; + } +} catch (Xapian::InvalidArgumentError e) { + fprintf (stderr, "Error while compacting: %s", e.get_msg().c_str()); +} + +#endif + +notmuch_database_close(notmuch); +DONE: +talloc_free(local); +} + +void notmuch_database_destroy (notmuch_database_t *notmuch) { notmuch_database_close (notmuch); diff --git a/lib/notmuch.h b/lib/notmuch.h index 3633bed..50babfb 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -215,6 +215,20 @@ notmuch_database_open (const char *path, void notmuch_database_close (notmuch_database_t *database); +/* Close the given notmuch database and then compact it. + * + * After notmuch_database_close_compact has been called, calls to + * other functions on objects derived from this database may either + * behave as if the database had not been closed (e.g., if the + * required data has been cached) or may fail with a + * NOTMUCH_STATUS_XAPIAN_EXCEPTION. + * + * notmuch_database_close_compact
[PATCH 1/3] Add notmuch_database_close_compact
--- configure | 25 - lib/database.cc | 54 ++ lib/notmuch.h | 14 ++ 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/configure b/configure index dc0dba4..370fedd 100755 --- a/configure +++ b/configure @@ -270,7 +270,8 @@ printf Checking for Xapian development files... have_xapian=0 for xapian_config in ${XAPIAN_CONFIG}; do if ${xapian_config} --version /dev/null 21; then - printf Yes (%s).\n $(${xapian_config} --version | sed -e 's/.* //') + xapian_version=$(${xapian_config} --version | sed -e 's/.* //') + printf Yes (%s).\n ${xapian_version} have_xapian=1 xapian_cxxflags=$(${xapian_config} --cxxflags) xapian_ldflags=$(${xapian_config} --libs) @@ -282,6 +283,24 @@ if [ ${have_xapian} = 0 ]; then errors=$((errors + 1)) fi +have_xapian_compact=0 +if [ ${have_xapian} = 1 ]; then +printf Checking for Xapian compact support... +IFS='.' +old_args=$@ +set -- ${xapian_version} +xapian_major_version=$1 +xapian_minor_version=$2 +xapian_subminor_version=$3 +set -- ${old_args} +if [ ${xapian_major_version} -gt 1 ] || [ ${xapian_minor_version} -gt 2 ] || [ ${xapian_subminor_version} -ge 6 ]; then + printf Yes.\n + have_xapian_compact=1 +else + printf No.\n +fi +fi + printf Checking for GMime development files... have_gmime=0 IFS=';' @@ -675,6 +694,9 @@ LINKER_RESOLVES_LIBRARY_DEPENDENCIES = ${linker_resolves_library_dependencies} XAPIAN_CXXFLAGS = ${xapian_cxxflags} XAPIAN_LDFLAGS = ${xapian_ldflags} +# Whether compact is supported by this version of Xapian +HAVE_XAPIAN_COMPACT = ${have_xapian_compact} + # Flags needed to compile and link against GMime-2.4 GMIME_CFLAGS = ${gmime_cflags} GMIME_LDFLAGS = ${gmime_ldflags} @@ -711,6 +733,7 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)\\ \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\ \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS) \\ + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) \\ -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR) CONFIGURE_LDFLAGS = \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS) EOF diff --git a/lib/database.cc b/lib/database.cc index 761dc1a..6e83a61 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -781,6 +781,60 @@ notmuch_database_close (notmuch_database_t *notmuch) } void +notmuch_database_close_compact (notmuch_database_t *notmuch) +{ +void *local = talloc_new (NULL); +Xapian::Compactor compactor; +char *notmuch_path, *xapian_path, *compact_xapian_path, *old_xapian_path; + +#if HAVE_XAPIAN_COMPACT +if (! (notmuch_path = talloc_asprintf (local, %s/%s, notmuch-path, .notmuch))) { + fprintf (stderr, Out of memory\n); + goto DONE; +} + +if (! (xapian_path = talloc_asprintf (local, %s/%s, notmuch_path, xapian))) { + fprintf (stderr, Out of memory\n); + goto DONE; +} + +if (! (compact_xapian_path = talloc_asprintf (local, %s.compact, xapian_path))) { + fprintf (stderr, Out of memory\n); + goto DONE; +} + +if (! (old_xapian_path = talloc_asprintf (local, %s.old, xapian_path))) { + fprintf (stderr, Out of memory\n); + goto DONE; +} + +try { + compactor.set_renumber(false); + compactor.add_source(xapian_path); + compactor.set_destdir(compact_xapian_path); + compactor.compact(); + + if (rename(xapian_path, old_xapian_path)) { + fprintf (stderr, Error moving old database out of the way\n); + goto DONE; + } + + if (rename(compact_xapian_path, xapian_path)) { + fprintf (stderr, Error moving compacted database\n); + goto DONE; + } +} catch (Xapian::InvalidArgumentError e) { + fprintf (stderr, Error while compacting: %s, e.get_msg().c_str()); +} + +#endif + +notmuch_database_close(notmuch); +DONE: +talloc_free(local); +} + +void notmuch_database_destroy (notmuch_database_t *notmuch) { notmuch_database_close (notmuch); diff --git a/lib/notmuch.h b/lib/notmuch.h index 3633bed..50babfb 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -215,6 +215,20 @@ notmuch_database_open (const char *path, void notmuch_database_close (notmuch_database_t *database); +/* Close the given notmuch database and then compact it. + * + * After notmuch_database_close_compact has been called, calls to + * other functions on objects derived from this database may either + * behave as if the database had not been closed (e.g., if the + * required data has been cached) or may fail with a + * NOTMUCH_STATUS_XAPIAN_EXCEPTION. + * + * notmuch_database_close_compact can be called multiple times. Later + * calls