date:20150314

[Patch v4 6/9] lib: add a log function with output to a string in notmuch_database_t

2015-03-14 Thread David Bremner

This is a thin wrapper around the previously implemented string
logging infrastructure. In the future it could have more configurable
output options.
---
 lib/database.cc   | 14 ++
 lib/notmuch-private.h |  4 
 2 files changed, 18 insertions(+)

diff --git a/lib/database.cc b/lib/database.cc
index 48a830f..0409128 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -377,6 +377,20 @@ log_to_string (char **str,
 va_end (va_args);
 }
 
+void
+_notmuch_database_log (notmuch_database_t *notmuch,
+ const char *format,
+ ...)
+{
+va_list va_args;
+
+va_start (va_args, format);
+
+vlog_to_string (notmuch, notmuch-status_string, format, va_args);
+
+va_end (va_args);
+}
+
 static void
 find_doc_ids_for_term (notmuch_database_t *notmuch,
   const char *term,
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 8a1f2fa..7cb6fd4 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -190,6 +190,10 @@ _notmuch_message_id_compressed (void *ctx, const char 
*message_id);
 notmuch_status_t
 _notmuch_database_ensure_writable (notmuch_database_t *notmuch);
 
+void
+_notmuch_database_log (notmuch_database_t *notmuch,
+  const char *format, ...);
+
 const char *
 _notmuch_database_relative_path (notmuch_database_t *notmuch,
 const char *path);
-- 
2.1.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

libnotmuch logging overhaul v4

2015-03-14 Thread David Bremner

Obsoletes

id:1419788030-10567-1-git-send-email-da...@tethera.net

The bad news is it is 3 patches longer; the good news is that I think
those patches are just testing and make sense whether we do the rest
or not.  The first 4 patches together now have the have the effect
adding a quiet version of open and create, slightly perversely named
_verbose. As always, naming things is hard.

Patches 5 through 7 are just support for patch 8,
and are unchanged from the previous version.

[Patch v4 5/9] lib/database: add field for last error string
[Patch v4 6/9] lib: add a log function with output to a string in
[Patch v4 7/9] lib: add private function to extract the database for

I'm not sure about the way notmuch_database_compact is handled here;
maybe it should use an extra string argument like
notmuch_database_open.

[Patch v4 8/9] lib: replace almost all fprintfs in library with

Unmodified from the previous series.

[Patch v4 9/9] lib: eliminate fprintf from _notmuch_message_file_open
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[Patch v4 8/9] lib: replace almost all fprintfs in library with _n_d_log

2015-03-14 Thread David Bremner

This is not supposed to change any functionality from an end user
point of view. Note that it will eliminate some output to stderr. The
query debugging output is left as is; it doesn't really fit with the
current primitive logging model. The remaining bad fprintf will need
an internal API change.
---
 lib/database.cc| 38 +-
 lib/directory.cc   |  4 ++--
 lib/index.cc   | 11 +++
 lib/message.cc |  6 +++---
 lib/query.cc   |  8 
 test/T560-lib-error.sh |  6 +-
 6 files changed, 42 insertions(+), 31 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 0409128..6828654 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -499,7 +499,7 @@ notmuch_database_find_message (notmuch_database_t *notmuch,
 
return NOTMUCH_STATUS_SUCCESS;
 } catch (const Xapian::Error error) {
-   fprintf (stderr, A Xapian exception occurred finding message: %s.\n,
+   _notmuch_database_log (notmuch, A Xapian exception occurred finding 
message: %s.\n,
 error.get_msg().c_str());
notmuch-exception_reported = TRUE;
*message_ret = NULL;
@@ -732,7 +732,7 @@ notmuch_status_t
 _notmuch_database_ensure_writable (notmuch_database_t *notmuch)
 {
 if (notmuch-mode == NOTMUCH_DATABASE_MODE_READ_ONLY) {
-   fprintf (stderr, Cannot write to a read-only database.\n);
+   _notmuch_database_log (notmuch, Cannot write to a read-only 
database.\n);
return NOTMUCH_STATUS_READ_ONLY_DATABASE;
 }
 
@@ -1025,7 +1025,7 @@ notmuch_database_close (notmuch_database_t *notmuch)
} catch (const Xapian::Error error) {
status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
if (! notmuch-exception_reported) {
-   fprintf (stderr, Error: A Xapian exception occurred closing 
database: %s\n,
+   _notmuch_database_log (notmuch, Error: A Xapian exception 
occurred closing database: %s\n,
 error.get_msg().c_str());
}
}
@@ -1157,12 +1157,12 @@ notmuch_database_compact (const char *path,
 }
 
 if (stat (backup_path, statbuf) != -1) {
-   fprintf (stderr, Path already exists: %s\n, backup_path);
+   _notmuch_database_log (notmuch, Path already exists: %s\n, 
backup_path);
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
 }
 if (errno != ENOENT) {
-   fprintf (stderr, Unknown error while stat()ing path: %s\n,
+   _notmuch_database_log (notmuch, Unknown error while stat()ing path: 
%s\n,
 strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
@@ -1182,20 +1182,20 @@ notmuch_database_compact (const char *path,
compactor.set_destdir (compact_xapian_path);
compactor.compact ();
 } catch (const Xapian::Error error) {
-   fprintf (stderr, Error while compacting: %s\n, 
error.get_msg().c_str());
+   _notmuch_database_log (notmuch, Error while compacting: %s\n, 
error.get_msg().c_str());
ret = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
goto DONE;
 }
 
 if (rename (xapian_path, backup_path)) {
-   fprintf (stderr, Error moving %s to %s: %s\n,
+   _notmuch_database_log (notmuch, Error moving %s to %s: %s\n,
 xapian_path, backup_path, strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
 }
 
 if (rename (compact_xapian_path, xapian_path)) {
-   fprintf (stderr, Error moving %s to %s: %s\n,
+   _notmuch_database_log (notmuch, Error moving %s to %s: %s\n,
 compact_xapian_path, xapian_path, strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
@@ -1203,7 +1203,7 @@ notmuch_database_compact (const char *path,
 
 if (! keep_backup) {
if (rmtree (backup_path)) {
-   fprintf (stderr, Error removing old database %s: %s\n,
+   _notmuch_database_log (notmuch, Error removing old database %s: 
%s\n,
 backup_path, strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
@@ -1214,6 +1214,10 @@ notmuch_database_compact (const char *path,
 if (notmuch) {
notmuch_status_t ret2;
 
+   const char *str = notmuch_database_status_string (notmuch);
+   if (status_cb  str)
+   status_cb (str, closure);
+
ret2 = notmuch_database_destroy (notmuch);
 
/* don't clobber previous error status */
@@ -1232,7 +1236,7 @@ notmuch_database_compact (unused (const char *path),
  unused (notmuch_compact_status_cb_t status_cb),
  unused (void *closure))
 {
-fprintf (stderr, notmuch was compiled against a xapian version lacking 
compaction support.\n);
+_notmuch_database_log (notmuch, notmuch was compiled against a xapian 
version lacking compaction support.\n);
 return NOTMUCH_STATUS_UNSUPPORTED_OPERATION;
 }
 #endif
@@ -1510,7 +1514,7 @@

[Patch v4 5/9] lib/database: add field for last error string

2015-03-14 Thread David Bremner

The idea is to have a logging function setting this string instead of
printing to stderr.
---
 lib/database-private.h | 4 
 lib/database.cc| 7 +++
 lib/notmuch.h  | 7 +++
 3 files changed, 18 insertions(+)

diff --git a/lib/database-private.h b/lib/database-private.h
index 6d6fa2c..24243db 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -154,6 +154,10 @@ struct _notmuch_database {
 unsigned int last_doc_id;
 uint64_t last_thread_id;
 
+/* error reporting; this value persists only until the
+ * next library call. May be NULL */
+char *status_string;
+
 Xapian::QueryParser *query_parser;
 Xapian::TermGenerator *term_gen;
 Xapian::ValueRangeProcessor *value_range_processor;
diff --git a/lib/database.cc b/lib/database.cc
index b774edc..48a830f 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -873,6 +873,7 @@ notmuch_database_open_verbose (const char *path,
 
 notmuch = talloc_zero (NULL, notmuch_database_t);
 notmuch-exception_reported = FALSE;
+notmuch-status_string = NULL;
 notmuch-path = talloc_strdup (notmuch, path);
 
 if (notmuch-path[strlen (notmuch-path) - 1] == '/')
@@ -2558,3 +2559,9 @@ notmuch_database_get_all_tags (notmuch_database_t *db)
return NULL;
 }
 }
+
+const char *
+notmuch_database_status_string (notmuch_database_t *notmuch)
+{
+return notmuch-status_string;
+}
diff --git a/lib/notmuch.h b/lib/notmuch.h
index c671d82..20c4e01 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -302,6 +302,13 @@ notmuch_database_open_verbose (const char *path,
   char **error_message);
 
 /**
+ * Retrieve last status string for given database.
+ *
+ */
+const char *
+notmuch_database_status_string (notmuch_database_t *notmuch);
+
+/**
  * Commit changes and close the given notmuch database.
  *
  * After notmuch_database_close has been called, calls to other
-- 
2.1.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[Patch v4 3/9] test: add error reporting tests for lib/database.cc

2015-03-14 Thread David Bremner

Unfortunately quite a few of the error handling paths here require
more sophisticated tests using e.g. gdb.
---
 test/T560-lib-error.sh | 91 ++
 1 file changed, 91 insertions(+)
 create mode 100755 test/T560-lib-error.sh

diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
new file mode 100755
index 000..dc2022f
--- /dev/null
+++ b/test/T560-lib-error.sh
@@ -0,0 +1,91 @@
+#!/usr/bin/env bash
+test_description=error reporting for library
+
+. ./test-lib.sh
+
+add_email_corpus
+
+test_expect_success building database NOTMUCH_NEW
+
+test_begin_subtest Open null pointer
+test_C EOF
+#include stdio.h
+#include notmuch.h
+int main (int argc, char** argv)
+{
+notmuch_database_t *db;
+notmuch_status_t stat;
+stat = notmuch_database_open (NULL, 0, 0);
+}
+EOF
+cat EOF EXPECTED
+== stdout ==
+== stderr ==
+Error: Cannot open a database for a NULL path.
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest Open nonexistent database
+test_C EOF
+#include stdio.h
+#include notmuch.h
+int main (int argc, char** argv)
+{
+notmuch_database_t *db;
+notmuch_status_t stat;
+stat = notmuch_database_open (/nonexistent/foo, 0, 0);
+}
+EOF
+cat EOF EXPECTED
+== stdout ==
+== stderr ==
+Error opening database at /nonexistent/foo/.notmuch: No such file or directory
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest Write to read-only database
+test_C ${MAIL_DIR} EOF
+#include stdio.h
+#include notmuch.h
+int main (int argc, char** argv)
+{
+   notmuch_database_t *db;
+   notmuch_status_t stat;
+   stat = notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_ONLY, 
db);
+   if (stat != NOTMUCH_STATUS_SUCCESS) {
+ fprintf (stderr, error opening database: %d\n, stat);
+   }
+   stat = notmuch_database_add_message (db, /dev/null, NULL);
+}
+EOF
+cat EOF EXPECTED
+== stdout ==
+== stderr ==
+Cannot write to a read-only database.
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest compact, overwriting existing backup
+test_C ${MAIL_DIR} EOF
+#include stdio.h
+#include notmuch.h
+static void
+status_cb (const char *msg, void *closure)
+{
+printf (%s\n, msg);
+}
+int main (int argc, char** argv)
+{
+   notmuch_database_t *db;
+   notmuch_status_t stat;
+   stat = notmuch_database_compact (argv[1], argv[1], status_cb, NULL);
+}
+EOF
+cat EOF EXPECTED
+== stdout ==
+== stderr ==
+Path already exists: CWD/mail
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_done
-- 
2.1.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[Patch v4 1/9] test: Add two tests for error output from notmuch_database_open

2015-03-14 Thread David Bremner

This is arguably testing the same thing twice, but in the brave new
future where we don't use printf anymore, each subcommand will be
responsible for handling the output on it's own.
---
 test/T050-new.sh | 7 +++
 test/T150-tagging.sh | 6 ++
 2 files changed, 13 insertions(+)

diff --git a/test/T050-new.sh b/test/T050-new.sh
index 7119356..e6c3291 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -276,4 +276,11 @@ test_expect_code 1 Invalid tags set exit code \
 
 notmuch config set new.tags $OLDCONFIG
 
+
+test_begin_subtest Xapian exception: read only files
+chmod u-w  ${MAIL_DIR}/.notmuch/xapian/*.DB
+output=$(NOTMUCH_NEW 21 | sed 's/: .*$//' )
+chmod u+w  ${MAIL_DIR}/.notmuch/xapian/*.DB
+test_expect_equal $output A Xapian exception occurred opening database
+
 test_done
diff --git a/test/T150-tagging.sh b/test/T150-tagging.sh
index 45471ac..4a2673d 100755
--- a/test/T150-tagging.sh
+++ b/test/T150-tagging.sh
@@ -261,4 +261,10 @@ test_expect_code 1 Empty tag names 'notmuch tag + One'
 
 test_expect_code 1 Tag name beginning with - 'notmuch tag +- One'
 
+test_begin_subtest Xapian exception: read only files
+chmod u-w  ${MAIL_DIR}/.notmuch/xapian/*.DB
+output=$(notmuch tag +something '*' 21 | sed 's/: .*$//' )
+chmod u+w  ${MAIL_DIR}/.notmuch/xapian/*.DB
+test_expect_equal $output A Xapian exception occurred opening database
+
 test_done
-- 
2.1.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[Patch v4 7/9] lib: add private function to extract the database for a message.

2015-03-14 Thread David Bremner

This is needed by logging in functions outside message.cc that take
only a notmuch_message_t object.
---
 lib/message.cc| 6 ++
 lib/notmuch-private.h | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/lib/message.cc b/lib/message.cc
index 956a70a..b84e5e1 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1626,3 +1626,9 @@ notmuch_message_destroy (notmuch_message_t *message)
 {
 talloc_free (message);
 }
+
+notmuch_database_t *
+_notmuch_message_database (notmuch_message_t *message)
+{
+return message-notmuch;
+}
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 7cb6fd4..dc58a2f 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -472,6 +472,8 @@ _notmuch_doc_id_set_remove (notmuch_doc_id_set_t *doc_ids,
 void
 _notmuch_message_add_reply (notmuch_message_t *message,
notmuch_message_t *reply);
+notmuch_database_t *
+_notmuch_message_database (notmuch_message_t *message);
 
 /* sha1.c */
 
-- 
2.1.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[Patch v4 4/9] lib: add verbose versions of notmuch_database_{open,create}

2015-03-14 Thread David Bremner

The compatibility wrapper ensures that clients calling
notmuch_database_open will receive consistent output for now.

The stdargs based infrastucture will be used in following commits for
a more general logging mechanism.

The changes to notmuch-{new,search} and test/symbol-test are just to
make the test suite pass.
---
 lib/database.cc | 96 -
 lib/notmuch.h   | 21 
 notmuch-new.c   |  8 +++--
 notmuch-search.c|  8 +++--
 test/symbol-test.cc |  6 +++-
 5 files changed, 119 insertions(+), 20 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 3974e2e..b774edc 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -349,6 +349,35 @@ notmuch_status_to_string (notmuch_status_t status)
 }
 
 static void
+vlog_to_string (void *ctx,
+  char **status_string,
+  const char *format,
+  va_list va_args)
+{
+if (!status_string)
+   return;
+
+if (*status_string)
+   talloc_free (*status_string);
+
+*status_string = talloc_vasprintf (ctx, format, va_args);
+}
+
+static void
+log_to_string (char **str,
+  const char *format,
+  ...)
+{
+va_list va_args;
+
+va_start (va_args, format);
+
+vlog_to_string (NULL, str, format, va_args);
+
+va_end (va_args);
+}
+
+static void
 find_doc_ids_for_term (notmuch_database_t *notmuch,
   const char *term,
   Xapian::PostingIterator *begin,
@@ -608,28 +637,37 @@ parse_references (void *ctx,
 notmuch_status_t
 notmuch_database_create (const char *path, notmuch_database_t **database)
 {
+return notmuch_database_create_verbose (path, database, NULL);
+}
+
+notmuch_status_t
+notmuch_database_create_verbose (const char *path,
+notmuch_database_t **database,
+char **status_string)
+{
 notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
 notmuch_database_t *notmuch = NULL;
 char *notmuch_path = NULL;
+char *message = NULL;
 struct stat st;
 int err;
 
 if (path == NULL) {
-   fprintf (stderr, Error: Cannot create a database for a NULL path.\n);
+   log_to_string (message, Error: Cannot create a database for a NULL 
path.\n);
status = NOTMUCH_STATUS_NULL_POINTER;
goto DONE;
 }
 
 err = stat (path, st);
 if (err) {
-   fprintf (stderr, Error: Cannot create database at %s: %s.\n,
+   log_to_string (message, Error: Cannot create database at %s: %s.\n,
 path, strerror (errno));
status = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
 }
 
 if (! S_ISDIR (st.st_mode)) {
-   fprintf (stderr, Error: Cannot create database at %s: Not a 
directory.\n,
+   log_to_string (message, Error: Cannot create database at %s: Not a 
directory.\n,
 path);
status = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
@@ -640,15 +678,15 @@ notmuch_database_create (const char *path, 
notmuch_database_t **database)
 err = mkdir (notmuch_path, 0755);
 
 if (err) {
-   fprintf (stderr, Error: Cannot create directory %s: %s.\n,
+   log_to_string (message, Error: Cannot create directory %s: %s.\n,
 notmuch_path, strerror (errno));
status = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
 }
 
-status = notmuch_database_open (path,
-   NOTMUCH_DATABASE_MODE_READ_WRITE,
-   notmuch);
+status = notmuch_database_open_verbose (path,
+   NOTMUCH_DATABASE_MODE_READ_WRITE,
+   notmuch, message);
 if (status)
goto DONE;
 
@@ -667,6 +705,8 @@ notmuch_database_create (const char *path, 
notmuch_database_t **database)
 if (notmuch_path)
talloc_free (notmuch_path);
 
+if (message)
+   *status_string = strdup(message);
 if (database)
*database = notmuch;
 else
@@ -767,37 +807,55 @@ notmuch_database_open (const char *path,
   notmuch_database_mode_t mode,
   notmuch_database_t **database)
 {
+char *status_string = NULL;
+notmuch_status_t status;
+
+status = notmuch_database_open_verbose(path, mode, database,
+  status_string);
+
+if (status_string) fputs(status_string, stderr);
+
+return status;
+}
+
+notmuch_status_t
+notmuch_database_open_verbose (const char *path,
+  notmuch_database_mode_t mode,
+  notmuch_database_t **database,
+  char **status_string)
+{
 notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
 void *local = talloc_new (NULL);
 notmuch_database_t *notmuch = NULL;
 char *notmuch_path, *xapian_path, *incompat_features;
+char *message = NULL;
 struct stat st;

libnotmuch logging overhaul v4

2015-03-14 Thread David Bremner

Obsoletes

id:1419788030-10567-1-git-send-email-david at tethera.net

The bad news is it is 3 patches longer; the good news is that I think
those patches are just testing and make sense whether we do the rest
or not.  The first 4 patches together now have the have the effect
adding a "quiet" version of open and create, slightly perversely named
"_verbose". As always, naming things is hard.

Patches 5 through 7 are just support for patch 8,
and are unchanged from the previous version.

[Patch v4 5/9] lib/database: add field for last error string
[Patch v4 6/9] lib: add a log function with output to a string in
[Patch v4 7/9] lib: add private function to extract the database for

I'm not sure about the way notmuch_database_compact is handled here;
maybe it should use an extra string argument like
notmuch_database_open.

[Patch v4 8/9] lib: replace almost all fprintfs in library with

Unmodified from the previous series.

[Patch v4 9/9] lib: eliminate fprintf from _notmuch_message_file_open

[Patch v4 8/9] lib: replace almost all fprintfs in library with _n_d_log

2015-03-14 Thread David Bremner

This is not supposed to change any functionality from an end user
point of view. Note that it will eliminate some output to stderr. The
query debugging output is left as is; it doesn't really fit with the
current primitive logging model. The remaining "bad" fprintf will need
an internal API change.
---
 lib/database.cc| 38 +-
 lib/directory.cc   |  4 ++--
 lib/index.cc   | 11 +++
 lib/message.cc |  6 +++---
 lib/query.cc   |  8 
 test/T560-lib-error.sh |  6 +-
 6 files changed, 42 insertions(+), 31 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 0409128..6828654 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -499,7 +499,7 @@ notmuch_database_find_message (notmuch_database_t *notmuch,

return NOTMUCH_STATUS_SUCCESS;
 } catch (const Xapian::Error ) {
-   fprintf (stderr, "A Xapian exception occurred finding message: %s.\n",
+   _notmuch_database_log (notmuch, "A Xapian exception occurred finding 
message: %s.\n",
 error.get_msg().c_str());
notmuch->exception_reported = TRUE;
*message_ret = NULL;
@@ -732,7 +732,7 @@ notmuch_status_t
 _notmuch_database_ensure_writable (notmuch_database_t *notmuch)
 {
 if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY) {
-   fprintf (stderr, "Cannot write to a read-only database.\n");
+   _notmuch_database_log (notmuch, "Cannot write to a read-only 
database.\n");
return NOTMUCH_STATUS_READ_ONLY_DATABASE;
 }

@@ -1025,7 +1025,7 @@ notmuch_database_close (notmuch_database_t *notmuch)
} catch (const Xapian::Error ) {
status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
if (! notmuch->exception_reported) {
-   fprintf (stderr, "Error: A Xapian exception occurred closing 
database: %s\n",
+   _notmuch_database_log (notmuch, "Error: A Xapian exception 
occurred closing database: %s\n",
 error.get_msg().c_str());
}
}
@@ -1157,12 +1157,12 @@ notmuch_database_compact (const char *path,
 }

 if (stat (backup_path, ) != -1) {
-   fprintf (stderr, "Path already exists: %s\n", backup_path);
+   _notmuch_database_log (notmuch, "Path already exists: %s\n", 
backup_path);
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
 }
 if (errno != ENOENT) {
-   fprintf (stderr, "Unknown error while stat()ing path: %s\n",
+   _notmuch_database_log (notmuch, "Unknown error while stat()ing path: 
%s\n",
 strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
@@ -1182,20 +1182,20 @@ notmuch_database_compact (const char *path,
compactor.set_destdir (compact_xapian_path);
compactor.compact ();
 } catch (const Xapian::Error ) {
-   fprintf (stderr, "Error while compacting: %s\n", 
error.get_msg().c_str());
+   _notmuch_database_log (notmuch, "Error while compacting: %s\n", 
error.get_msg().c_str());
ret = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
goto DONE;
 }

 if (rename (xapian_path, backup_path)) {
-   fprintf (stderr, "Error moving %s to %s: %s\n",
+   _notmuch_database_log (notmuch, "Error moving %s to %s: %s\n",
 xapian_path, backup_path, strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
 }

 if (rename (compact_xapian_path, xapian_path)) {
-   fprintf (stderr, "Error moving %s to %s: %s\n",
+   _notmuch_database_log (notmuch, "Error moving %s to %s: %s\n",
 compact_xapian_path, xapian_path, strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
@@ -1203,7 +1203,7 @@ notmuch_database_compact (const char *path,

 if (! keep_backup) {
if (rmtree (backup_path)) {
-   fprintf (stderr, "Error removing old database %s: %s\n",
+   _notmuch_database_log (notmuch, "Error removing old database %s: 
%s\n",
 backup_path, strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
@@ -1214,6 +1214,10 @@ notmuch_database_compact (const char *path,
 if (notmuch) {
notmuch_status_t ret2;

+   const char *str = notmuch_database_status_string (notmuch);
+   if (status_cb && str)
+   status_cb (str, closure);
+
ret2 = notmuch_database_destroy (notmuch);

/* don't clobber previous error status */
@@ -1232,7 +1236,7 @@ notmuch_database_compact (unused (const char *path),
  unused (notmuch_compact_status_cb_t status_cb),
  unused (void *closure))
 {
-fprintf (stderr, "notmuch was compiled against a xapian version lacking 
compaction support.\n");
+_notmuch_database_log (notmuch, "notmuch was compiled against a xapian 
version lacking compaction support.\n");
 return NOTMUCH_STATUS_UNSUPPORTED_OPERATION;
 }
 #endif
@@ -1510,7 +1514,7 @@

[Patch v4 2/9] test: add support for compiling and running C snippets

2015-03-14 Thread David Bremner

This is to limit the copy-pasta involved in running C tests. I decided
to keep things simple and not try to provide an actual C skeleton.

The setting of LD_LIBRARY_PATH is to force using the built libnotmuch
rather than any potential system one.
---
 test/README  |  5 +
 test/test-lib.sh | 15 +++
 2 files changed, 20 insertions(+)

diff --git a/test/README b/test/README
index 81a1c82..5b40474 100644
--- a/test/README
+++ b/test/README
@@ -84,6 +84,11 @@ the tests in one of the following ways.
TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient ./emacs
make test TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient

+Some tests may require a c compiler. You can choose the name and flags 
similarly 
+to with emacs, e.g.
+
+ make test TEST_CC=gcc TEST_CFLAGS="-g -O2"
+ 
 Quiet Execution
 ---

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 133fbe4..c7af003 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -73,6 +73,8 @@ if [[ ( -n "$TEST_EMACS" && -z "$TEST_EMACSCLIENT" ) || \
 fi
 TEST_EMACS=${TEST_EMACS:-${EMACS:-emacs}}
 TEST_EMACSCLIENT=${TEST_EMACSCLIENT:-emacsclient}
+TEST_CC=${TEST_CC:-cc}
+TEST_CFLAGS=${TEST_CFLAGS:-"-g -O0"}

 # Protect ourselves from common misconfiguration to export
 # CDPATH into the environment
@@ -1161,6 +1163,19 @@ test_python() {
| $cmd -
 }

+test_C() {
+test_file="test${test_count}.c"
+exec_file=${test_file%%.c}
+cat > ${test_file}
+export LD_LIBRARY_PATH=${TEST_DIRECTORY}/../lib
+${TEST_CC} ${TEST_CFLAGS} -I${TEST_DIRECTORY}/../lib -o ${exec_file} 
${test_file} -L${TEST_DIRECTORY}/../lib/ -lnotmuch
+echo "== stdout ==" > OUTPUT.stdout
+echo "== stderr ==" > OUTPUT.stderr
+./${exec_file} "$@" 1>>OUTPUT.stdout 2>>OUTPUT.stderr
+cat OUTPUT.stdout OUTPUT.stderr | sed "s,$(pwd),CWD," > OUTPUT
+}
+
+
 # Creates a script that counts how much time it is executed and calls
 # notmuch.  $notmuch_counter_command is set to the path to the
 # generated script.  Use notmuch_counter_value() function to get the
-- 
2.1.4

[Patch v4 6/9] lib: add a log function with output to a string in notmuch_database_t

2015-03-14 Thread David Bremner

This is a thin wrapper around the previously implemented string
logging infrastructure. In the future it could have more configurable
output options.
---
 lib/database.cc   | 14 ++
 lib/notmuch-private.h |  4 
 2 files changed, 18 insertions(+)

diff --git a/lib/database.cc b/lib/database.cc
index 48a830f..0409128 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -377,6 +377,20 @@ log_to_string (char **str,
 va_end (va_args);
 }

+void
+_notmuch_database_log (notmuch_database_t *notmuch,
+ const char *format,
+ ...)
+{
+va_list va_args;
+
+va_start (va_args, format);
+
+vlog_to_string (notmuch, >status_string, format, va_args);
+
+va_end (va_args);
+}
+
 static void
 find_doc_ids_for_term (notmuch_database_t *notmuch,
   const char *term,
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 8a1f2fa..7cb6fd4 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -190,6 +190,10 @@ _notmuch_message_id_compressed (void *ctx, const char 
*message_id);
 notmuch_status_t
 _notmuch_database_ensure_writable (notmuch_database_t *notmuch);

+void
+_notmuch_database_log (notmuch_database_t *notmuch,
+  const char *format, ...);
+
 const char *
 _notmuch_database_relative_path (notmuch_database_t *notmuch,
 const char *path);
-- 
2.1.4

[Patch v4 3/9] test: add error reporting tests for lib/database.cc

2015-03-14 Thread David Bremner

Unfortunately quite a few of the error handling paths here require
more sophisticated tests using e.g. gdb.
---
 test/T560-lib-error.sh | 91 ++
 1 file changed, 91 insertions(+)
 create mode 100755 test/T560-lib-error.sh

diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
new file mode 100755
index 000..dc2022f
--- /dev/null
+++ b/test/T560-lib-error.sh
@@ -0,0 +1,91 @@
+#!/usr/bin/env bash
+test_description="error reporting for library"
+
+. ./test-lib.sh
+
+add_email_corpus
+
+test_expect_success "building database" "NOTMUCH_NEW"
+
+test_begin_subtest "Open null pointer"
+test_C <
+#include 
+int main (int argc, char** argv)
+{
+notmuch_database_t *db;
+notmuch_status_t stat;
+stat = notmuch_database_open (NULL, 0, 0);
+}
+EOF
+cat

[Patch v4 7/9] lib: add private function to extract the database for a message.

2015-03-14 Thread David Bremner

This is needed by logging in functions outside message.cc that take
only a notmuch_message_t object.
---
 lib/message.cc| 6 ++
 lib/notmuch-private.h | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/lib/message.cc b/lib/message.cc
index 956a70a..b84e5e1 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1626,3 +1626,9 @@ notmuch_message_destroy (notmuch_message_t *message)
 {
 talloc_free (message);
 }
+
+notmuch_database_t *
+_notmuch_message_database (notmuch_message_t *message)
+{
+return message->notmuch;
+}
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 7cb6fd4..dc58a2f 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -472,6 +472,8 @@ _notmuch_doc_id_set_remove (notmuch_doc_id_set_t *doc_ids,
 void
 _notmuch_message_add_reply (notmuch_message_t *message,
notmuch_message_t *reply);
+notmuch_database_t *
+_notmuch_message_database (notmuch_message_t *message);

 /* sha1.c */

-- 
2.1.4

[Patch v4 9/9] lib: eliminate fprintf from _notmuch_message_file_open

2015-03-14 Thread David Bremner

You may wonder why _notmuch_message_file_open_ctx has two parameters.
This is because we need sometime to use a ctx which is a
notmuch_message_t. While we could get the database from this, there is
no easy way in C to tell type we are getting. The remaining fprintf is
removed by Jani's series un-deprecating single message mboxes.
---
 lib/database.cc   |  2 +-
 lib/message-file.c| 11 +++
 lib/message.cc|  3 ++-
 lib/notmuch-private.h |  5 +++--
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 6828654..b5f3549 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -2317,7 +2317,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
 if (ret)
return ret;

-message_file = _notmuch_message_file_open (filename);
+message_file = _notmuch_message_file_open (notmuch, filename);
 if (message_file == NULL)
return NOTMUCH_STATUS_FILE_ERROR;

diff --git a/lib/message-file.c b/lib/message-file.c
index a41d9ad..8ac96e8 100644
--- a/lib/message-file.c
+++ b/lib/message-file.c
@@ -76,7 +76,8 @@ _notmuch_message_file_destructor (notmuch_message_file_t 
*message)
 /* Create a new notmuch_message_file_t for 'filename' with 'ctx' as
  * the talloc owner. */
 notmuch_message_file_t *
-_notmuch_message_file_open_ctx (void *ctx, const char *filename)
+_notmuch_message_file_open_ctx (notmuch_database_t *notmuch,
+   void *ctx, const char *filename)
 {
 notmuch_message_file_t *message;

@@ -98,16 +99,18 @@ _notmuch_message_file_open_ctx (void *ctx, const char 
*filename)
 return message;

   FAIL:
-fprintf (stderr, "Error opening %s: %s\n", filename, strerror (errno));
+_notmuch_database_log (notmuch, "Error opening %s: %s\n",
+ filename, strerror (errno));
 _notmuch_message_file_close (message);

 return NULL;
 }

 notmuch_message_file_t *
-_notmuch_message_file_open (const char *filename)
+_notmuch_message_file_open (notmuch_database_t *notmuch,
+   const char *filename)
 {
-return _notmuch_message_file_open_ctx (NULL, filename);
+return _notmuch_message_file_open_ctx (notmuch, NULL, filename);
 }

 void
diff --git a/lib/message.cc b/lib/message.cc
index a8ca988..5bc7aff 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -437,7 +437,8 @@ _notmuch_message_ensure_message_file (notmuch_message_t 
*message)
 if (unlikely (filename == NULL))
return;

-message->message_file = _notmuch_message_file_open_ctx (message, filename);
+message->message_file = _notmuch_message_file_open_ctx (
+   _notmuch_message_database (message), message, filename);
 }

 const char *
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index dc58a2f..cc9ce12 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -358,11 +358,12 @@ typedef struct _notmuch_message_file 
notmuch_message_file_t;
  * Returns NULL if any error occurs.
  */
 notmuch_message_file_t *
-_notmuch_message_file_open (const char *filename);
+_notmuch_message_file_open (notmuch_database_t *notmuch, const char *filename);

 /* Like notmuch_message_file_open but with 'ctx' as the talloc owner. */
 notmuch_message_file_t *
-_notmuch_message_file_open_ctx (void *ctx, const char *filename);
+_notmuch_message_file_open_ctx (notmuch_database_t *notmuch,
+   void *ctx, const char *filename);

 /* Close a notmuch message previously opened with notmuch_message_open. */
 void
-- 
2.1.4

[Patch v4 5/9] lib/database: add field for last error string

2015-03-14 Thread David Bremner

The idea is to have a logging function setting this string instead of
printing to stderr.
---
 lib/database-private.h | 4 
 lib/database.cc| 7 +++
 lib/notmuch.h  | 7 +++
 3 files changed, 18 insertions(+)

diff --git a/lib/database-private.h b/lib/database-private.h
index 6d6fa2c..24243db 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -154,6 +154,10 @@ struct _notmuch_database {
 unsigned int last_doc_id;
 uint64_t last_thread_id;

+/* error reporting; this value persists only until the
+ * next library call. May be NULL */
+char *status_string;
+
 Xapian::QueryParser *query_parser;
 Xapian::TermGenerator *term_gen;
 Xapian::ValueRangeProcessor *value_range_processor;
diff --git a/lib/database.cc b/lib/database.cc
index b774edc..48a830f 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -873,6 +873,7 @@ notmuch_database_open_verbose (const char *path,

 notmuch = talloc_zero (NULL, notmuch_database_t);
 notmuch->exception_reported = FALSE;
+notmuch->status_string = NULL;
 notmuch->path = talloc_strdup (notmuch, path);

 if (notmuch->path[strlen (notmuch->path) - 1] == '/')
@@ -2558,3 +2559,9 @@ notmuch_database_get_all_tags (notmuch_database_t *db)
return NULL;
 }
 }
+
+const char *
+notmuch_database_status_string (notmuch_database_t *notmuch)
+{
+return notmuch->status_string;
+}
diff --git a/lib/notmuch.h b/lib/notmuch.h
index c671d82..20c4e01 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -302,6 +302,13 @@ notmuch_database_open_verbose (const char *path,
   char **error_message);

 /**
+ * Retrieve last status string for given database.
+ *
+ */
+const char *
+notmuch_database_status_string (notmuch_database_t *notmuch);
+
+/**
  * Commit changes and close the given notmuch database.
  *
  * After notmuch_database_close has been called, calls to other
-- 
2.1.4

[Patch v4 4/9] lib: add "verbose" versions of notmuch_database_{open,create}

2015-03-14 Thread David Bremner

The compatibility wrapper ensures that clients calling
notmuch_database_open will receive consistent output for now.

The stdargs based infrastucture will be used in following commits for
a more general logging mechanism.

The changes to notmuch-{new,search} and test/symbol-test are just to
make the test suite pass.
---
 lib/database.cc | 96 -
 lib/notmuch.h   | 21 
 notmuch-new.c   |  8 +++--
 notmuch-search.c|  8 +++--
 test/symbol-test.cc |  6 +++-
 5 files changed, 119 insertions(+), 20 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 3974e2e..b774edc 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -349,6 +349,35 @@ notmuch_status_to_string (notmuch_status_t status)
 }

 static void
+vlog_to_string (void *ctx,
+  char **status_string,
+  const char *format,
+  va_list va_args)
+{
+if (!status_string)
+   return;
+
+if (*status_string)
+   talloc_free (*status_string);
+
+*status_string = talloc_vasprintf (ctx, format, va_args);
+}
+
+static void
+log_to_string (char **str,
+  const char *format,
+  ...)
+{
+va_list va_args;
+
+va_start (va_args, format);
+
+vlog_to_string (NULL, str, format, va_args);
+
+va_end (va_args);
+}
+
+static void
 find_doc_ids_for_term (notmuch_database_t *notmuch,
   const char *term,
   Xapian::PostingIterator *begin,
@@ -608,28 +637,37 @@ parse_references (void *ctx,
 notmuch_status_t
 notmuch_database_create (const char *path, notmuch_database_t **database)
 {
+return notmuch_database_create_verbose (path, database, NULL);
+}
+
+notmuch_status_t
+notmuch_database_create_verbose (const char *path,
+notmuch_database_t **database,
+char **status_string)
+{
 notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
 notmuch_database_t *notmuch = NULL;
 char *notmuch_path = NULL;
+char *message = NULL;
 struct stat st;
 int err;

 if (path == NULL) {
-   fprintf (stderr, "Error: Cannot create a database for a NULL path.\n");
+   log_to_string (, "Error: Cannot create a database for a NULL 
path.\n");
status = NOTMUCH_STATUS_NULL_POINTER;
goto DONE;
 }

 err = stat (path, );
 if (err) {
-   fprintf (stderr, "Error: Cannot create database at %s: %s.\n",
+   log_to_string (, "Error: Cannot create database at %s: %s.\n",
 path, strerror (errno));
status = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
 }

 if (! S_ISDIR (st.st_mode)) {
-   fprintf (stderr, "Error: Cannot create database at %s: Not a 
directory.\n",
+   log_to_string (, "Error: Cannot create database at %s: Not a 
directory.\n",
 path);
status = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
@@ -640,15 +678,15 @@ notmuch_database_create (const char *path, 
notmuch_database_t **database)
 err = mkdir (notmuch_path, 0755);

 if (err) {
-   fprintf (stderr, "Error: Cannot create directory %s: %s.\n",
+   log_to_string (, "Error: Cannot create directory %s: %s.\n",
 notmuch_path, strerror (errno));
status = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
 }

-status = notmuch_database_open (path,
-   NOTMUCH_DATABASE_MODE_READ_WRITE,
-   );
+status = notmuch_database_open_verbose (path,
+   NOTMUCH_DATABASE_MODE_READ_WRITE,
+   , );
 if (status)
goto DONE;

@@ -667,6 +705,8 @@ notmuch_database_create (const char *path, 
notmuch_database_t **database)
 if (notmuch_path)
talloc_free (notmuch_path);

+if (message)
+   *status_string = strdup(message);
 if (database)
*database = notmuch;
 else
@@ -767,37 +807,55 @@ notmuch_database_open (const char *path,
   notmuch_database_mode_t mode,
   notmuch_database_t **database)
 {
+char *status_string = NULL;
+notmuch_status_t status;
+
+status = notmuch_database_open_verbose(path, mode, database,
+  _string);
+
+if (status_string) fputs(status_string, stderr);
+
+return status;
+}
+
+notmuch_status_t
+notmuch_database_open_verbose (const char *path,
+  notmuch_database_mode_t mode,
+  notmuch_database_t **database,
+  char **status_string)
+{
 notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
 void *local = talloc_new (NULL);
 notmuch_database_t *notmuch = NULL;
 char *notmuch_path, *xapian_path, *incompat_features;
+char *message = NULL;
 struct stat st;
 int err;
 unsigned int i, version;
 static

[Patch v4 1/9] test: Add two tests for error output from notmuch_database_open

2015-03-14 Thread David Bremner

This is arguably testing the same thing twice, but in the brave new
future where we don't use printf anymore, each subcommand will be
responsible for handling the output on it's own.
---
 test/T050-new.sh | 7 +++
 test/T150-tagging.sh | 6 ++
 2 files changed, 13 insertions(+)

diff --git a/test/T050-new.sh b/test/T050-new.sh
index 7119356..e6c3291 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -276,4 +276,11 @@ test_expect_code 1 "Invalid tags set exit code" \

 notmuch config set new.tags $OLDCONFIG

+
+test_begin_subtest "Xapian exception: read only files"
+chmod u-w  ${MAIL_DIR}/.notmuch/xapian/*.DB
+output=$(NOTMUCH_NEW 2>&1 | sed 's/: .*$//' )
+chmod u+w  ${MAIL_DIR}/.notmuch/xapian/*.DB
+test_expect_equal "$output" "A Xapian exception occurred opening database"
+
 test_done
diff --git a/test/T150-tagging.sh b/test/T150-tagging.sh
index 45471ac..4a2673d 100755
--- a/test/T150-tagging.sh
+++ b/test/T150-tagging.sh
@@ -261,4 +261,10 @@ test_expect_code 1 "Empty tag names" 'notmuch tag + One'

 test_expect_code 1 "Tag name beginning with -" 'notmuch tag +- One'

+test_begin_subtest "Xapian exception: read only files"
+chmod u-w  ${MAIL_DIR}/.notmuch/xapian/*.DB
+output=$(notmuch tag +something '*' 2>&1 | sed 's/: .*$//' )
+chmod u+w  ${MAIL_DIR}/.notmuch/xapian/*.DB
+test_expect_equal "$output" "A Xapian exception occurred opening database"
+
 test_done
-- 
2.1.4

[Patch v4 6/9] lib: add a log function with output to a string in notmuch_database_t

libnotmuch logging overhaul v4

[Patch v4 8/9] lib: replace almost all fprintfs in library with _n_d_log

[Patch v4 5/9] lib/database: add field for last error string

[Patch v4 3/9] test: add error reporting tests for lib/database.cc

[Patch v4 1/9] test: Add two tests for error output from notmuch_database_open

[Patch v4 7/9] lib: add private function to extract the database for a message.

[Patch v4 4/9] lib: add verbose versions of notmuch_database_{open,create}

libnotmuch logging overhaul v4

[Patch v4 8/9] lib: replace almost all fprintfs in library with _n_d_log

[Patch v4 2/9] test: add support for compiling and running C snippets

[Patch v4 6/9] lib: add a log function with output to a string in notmuch_database_t

[Patch v4 3/9] test: add error reporting tests for lib/database.cc

[Patch v4 7/9] lib: add private function to extract the database for a message.

[Patch v4 9/9] lib: eliminate fprintf from _notmuch_message_file_open

[Patch v4 5/9] lib/database: add field for last error string

[Patch v4 4/9] lib: add "verbose" versions of notmuch_database_{open,create}

[Patch v4 1/9] test: Add two tests for error output from notmuch_database_open

18 matches

Site Navigation

Mail list logo

Footer information