unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 1/3] lib: clarify description of notmuch_database_add_message
@ 2017-08-09  5:47 Daniel Kahn Gillmor
  2017-08-09  5:47 ` [PATCH 2/3] database: add n_d_index_file (deprecates n_d_add_message) Daniel Kahn Gillmor
  2017-08-09  5:47 ` [PATCH 3/3] reindex: drop notmuch_param_t, use notmuch_indexopts_t instead Daniel Kahn Gillmor
  0 siblings, 2 replies; 11+ messages in thread
From: Daniel Kahn Gillmor @ 2017-08-09  5:47 UTC (permalink / raw)
  To: Notmuch Mail

Since we're accumulating the index when we add a new file to the
message, the semantics have slightly changed.  This tries to align the
documentation with the actual functionality.
---
 lib/notmuch.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index f78b3473..02586a91 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -542,8 +542,10 @@ notmuch_database_get_directory (notmuch_database_t *database,
 				notmuch_directory_t **directory);
 
 /**
- * Add a new message to the given notmuch database or associate an
- * additional filename with an existing message.
+ * Add a message file to a database, indexing it for retrieval by
+ * future searches.  If a message already exists with the same message
+ * ID as the specified file, their indexes will be merged, and this
+ * new filename will also be associated with the existing message.
  *
  * Here, 'filename' should be a path relative to the path of
  * 'database' (see notmuch_database_get_path), or else should be an
@@ -556,8 +558,9 @@ notmuch_database_get_directory (notmuch_database_t *database,
  * entire contents of the file.
  *
  * If another message with the same message ID already exists in the
- * database, rather than creating a new message, this adds 'filename'
- * to the list of the filenames for the existing message.
+ * database, rather than creating a new message, this adds the search
+ * terms from the identified file to the existing message's index, and
+ * adds 'filename' to the list of filenames known for the message.
  *
  * If 'message' is not NULL, then, on successful return
  * (NOTMUCH_STATUS_SUCCESS or NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) '*message'
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] database: add n_d_index_file (deprecates n_d_add_message)
  2017-08-09  5:47 [PATCH 1/3] lib: clarify description of notmuch_database_add_message Daniel Kahn Gillmor
@ 2017-08-09  5:47 ` Daniel Kahn Gillmor
  2017-08-17 11:31   ` David Bremner
  2017-08-09  5:47 ` [PATCH 3/3] reindex: drop notmuch_param_t, use notmuch_indexopts_t instead Daniel Kahn Gillmor
  1 sibling, 1 reply; 11+ messages in thread
From: Daniel Kahn Gillmor @ 2017-08-09  5:47 UTC (permalink / raw)
  To: Notmuch Mail

We need a way to pass parameters to the indexing functionality on the
first index, not just on reindexing.  The obvious place is in
notmuch_database_add_message.  But since modifying the argument list
would break both API and ABI, we needed a new name.

I considered notmuch_database_add_message_with_params(), but the
functionality we're talking about doesn't always add a message.  It
tries to index a specific file, possibly adding a message, but
possibly doing other things, like adding terms to an existing message,
or failing to deal with message objects entirely (e.g. because the
file didn't contain a message).

So i chose the function name notmuch_database_index_file.

I confess i'm a little concerned about confusing future notmuch
developers with the new name, since we already have a private
_notmuch_message_index_file function, and the two do rather different
things.  But i think the added clarity for people linking against the
future libnotmuch and the capacity for using index parameters makes
this a worthwhile tradeoff.  (that said, if anyone has another name
that they strongly prefer, i'd be happy to go with it)

This changeset also adjusts the tests so that we test whether the new,
preferred function returns bad values (since the deprecated function
just calls the new one).

We can keep the deprecated n_d_add_message function around as long as
we like, but at the next place where we're forced to break API or ABI
we can probably choose to drop the name relatively safely.

NOTE: there is probably more cleanup to do in the ruby and go bindings
to complete the deprecation directly.  I don't know those languages
well enough to attempt a fix; i don't know how to test them; and i
don't know the culture around those languages about API additions or
deprecations.
---
 bindings/python/docs/source/database.rst |  2 +-
 bindings/python/notmuch/database.py      | 18 ++++++++++++------
 bindings/python/notmuch/directory.py     |  2 +-
 bindings/python/notmuch/message.py       |  2 +-
 bindings/ruby/database.c                 |  2 +-
 contrib/go/src/notmuch/notmuch.go        |  2 +-
 lib/add-message.cc                       | 18 +++++++++++++++---
 lib/notmuch-private.h                    |  1 +
 lib/notmuch.h                            | 27 +++++++++++++++++++++++----
 notmuch-insert.c                         |  2 +-
 notmuch-new.c                            |  2 +-
 test/T070-insert.sh                      | 10 +++++-----
 test/T560-lib-error.sh                   |  4 ++--
 13 files changed, 65 insertions(+), 27 deletions(-)

diff --git a/bindings/python/docs/source/database.rst b/bindings/python/docs/source/database.rst
index 5f1cdc14..079dc754 100644
--- a/bindings/python/docs/source/database.rst
+++ b/bindings/python/docs/source/database.rst
@@ -25,7 +25,7 @@
 
    .. automethod:: get_directory
 
-   .. automethod:: add_message
+   .. automethod:: index_file
 
    .. automethod:: remove_message
 
diff --git a/bindings/python/notmuch/database.py b/bindings/python/notmuch/database.py
index 8f918069..a2c025eb 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -285,7 +285,7 @@ class Database(object):
         """Does this database need to be upgraded before writing to it?
 
         If this function returns `True` then no functions that modify the
-        database (:meth:`add_message`,
+        database (:meth:`index_file`,
         :meth:`Message.add_tag`, :meth:`Directory.set_mtime`,
         etc.) will work unless :meth:`upgrade` is called successfully first.
 
@@ -399,12 +399,13 @@ class Database(object):
         # return the Directory, init it with the absolute path
         return Directory(abs_dirpath, dir_p, self)
 
-    _add_message = nmlib.notmuch_database_add_message
-    _add_message.argtypes = [NotmuchDatabaseP, c_char_p,
+    _index_file = nmlib.notmuch_database_index_file
+    _index_file.argtypes = [NotmuchDatabaseP, c_char_p,
+                             c_void_p,
                              POINTER(NotmuchMessageP)]
-    _add_message.restype = c_uint
+    _index_file.restype = c_uint
 
-    def add_message(self, filename, sync_maildir_flags=False):
+    def index_file(self, filename, sync_maildir_flags=False):
         """Adds a new message to the database
 
         :param filename: should be a path relative to the path of the
@@ -455,7 +456,7 @@ class Database(object):
         """
         self._assert_db_is_initialized()
         msg_p = NotmuchMessageP()
-        status = self._add_message(self._db, _str(filename), byref(msg_p))
+        status = self._index_file(self._db, _str(filename), c_void_p(None), byref(msg_p))
 
         if not status in [STATUS.SUCCESS, STATUS.DUPLICATE_MESSAGE_ID]:
             raise NotmuchError(status)
@@ -467,6 +468,11 @@ class Database(object):
             msg.maildir_flags_to_tags()
         return (msg, status)
 
+    def add_message(self, filename, sync_maildir_flags=False):
+        """Deprecated alias for :meth:`index_file`
+        """
+        self.index_file(self, filename, sync_maildir_flags=sync_maildir_flags)
+
     _remove_message = nmlib.notmuch_database_remove_message
     _remove_message.argtypes = [NotmuchDatabaseP, c_char_p]
     _remove_message.restype = c_uint
diff --git a/bindings/python/notmuch/directory.py b/bindings/python/notmuch/directory.py
index 7f86b1ac..b30c9e35 100644
--- a/bindings/python/notmuch/directory.py
+++ b/bindings/python/notmuch/directory.py
@@ -93,7 +93,7 @@ class Directory(object):
 
         * Read the mtime of a directory from the filesystem
 
-        * Call :meth:`Database.add_message` for all mail files in
+        * Call :meth:`Database.index_file` for all mail files in
           the directory
 
         * Call notmuch_directory_set_mtime with the mtime read from the
diff --git a/bindings/python/notmuch/message.py b/bindings/python/notmuch/message.py
index fc177eb2..cce377d0 100644
--- a/bindings/python/notmuch/message.py
+++ b/bindings/python/notmuch/message.py
@@ -566,7 +566,7 @@ class Message(Python3StringMixIn):
         logical OR operator.)
 
         As a convenience, you can set the sync_maildir_flags parameter in
-        :meth:`Database.add_message` to implicitly call this.
+        :meth:`Database.index_file` to implicitly call this.
 
         :returns: a :class:`STATUS`. In short, you want to see
             notmuch.STATUS.SUCCESS here. See there for details."""
diff --git a/bindings/ruby/database.c b/bindings/ruby/database.c
index 12e6bab7..416eb709 100644
--- a/bindings/ruby/database.c
+++ b/bindings/ruby/database.c
@@ -291,7 +291,7 @@ notmuch_rb_database_add_message (VALUE self, VALUE pathv)
     SafeStringValue (pathv);
     path = RSTRING_PTR (pathv);
 
-    ret = notmuch_database_add_message (db, path, &message);
+    ret = notmuch_database_index_file (db, path, NULL, &message);
     notmuch_rb_status_raise (ret);
     return rb_assoc_new (Data_Wrap_Struct (notmuch_rb_cMessage, NULL, NULL, message),
         (ret == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) ? Qtrue : Qfalse);
diff --git a/contrib/go/src/notmuch/notmuch.go b/contrib/go/src/notmuch/notmuch.go
index 2d684311..89093b2d 100644
--- a/contrib/go/src/notmuch/notmuch.go
+++ b/contrib/go/src/notmuch/notmuch.go
@@ -170,7 +170,7 @@ func (self *Database) GetVersion() uint {
 /* Does this database need to be upgraded before writing to it?
  *
  * If this function returns TRUE then no functions that modify the
- * database (notmuch_database_add_message, notmuch_message_add_tag,
+ * database (notmuch_database_index_file, notmuch_message_add_tag,
  * notmuch_directory_set_mtime, etc.) will work unless the function
  * notmuch_database_upgrade is called successfully first. */
 func (self *Database) NeedsUpgrade() bool {
diff --git a/lib/add-message.cc b/lib/add-message.cc
index 711ed9fa..4454b2b0 100644
--- a/lib/add-message.cc
+++ b/lib/add-message.cc
@@ -458,9 +458,10 @@ _notmuch_database_link_message (notmuch_database_t *notmuch,
 }
 
 notmuch_status_t
-notmuch_database_add_message (notmuch_database_t *notmuch,
-			      const char *filename,
-			      notmuch_message_t **message_ret)
+notmuch_database_index_file (notmuch_database_t *notmuch,
+			     const char *filename,
+			     notmuch_param_t unused (*indexopts),
+			     notmuch_message_t **message_ret)
 {
     notmuch_message_file_t *message_file;
     notmuch_message_t *message = NULL;
@@ -575,3 +576,14 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
 
     return ret;
 }
+
+notmuch_status_t
+notmuch_database_add_message (notmuch_database_t *notmuch,
+			      const char *filename,
+			      notmuch_message_t **message_ret)
+{
+    return notmuch_database_index_file (notmuch, filename,
+					NULL,
+					message_ret);
+    
+}
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index b187a80f..f6a497b5 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -632,6 +632,7 @@ _notmuch_thread_create (void *ctx,
 			notmuch_exclude_t omit_exclude,
 			notmuch_sort_t sort);
 
+
 NOTMUCH_END_DECLS
 
 #ifdef __cplusplus
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 02586a91..ed405dc5 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -237,7 +237,7 @@ typedef struct _notmuch_param notmuch_param_t;
  * The database will not yet have any data in it
  * (notmuch_database_create itself is a very cheap function). Messages
  * contained within 'path' can be added to the database by calling
- * notmuch_database_add_message.
+ * notmuch_database_index_file.
  *
  * In case of any failure, this function returns an error status and
  * sets *database to NULL (after printing an error message on stderr).
@@ -561,6 +561,10 @@ notmuch_database_get_directory (notmuch_database_t *database,
  * database, rather than creating a new message, this adds the search
  * terms from the identified file to the existing message's index, and
  * adds 'filename' to the list of filenames known for the message.
+ * 
+ * 'indexopts' can be NULL (meaning, use the indexing defaults from
+ * the database), or can be an explicit choice of indexing options
+ * that should govern the indexing of this specific 'filename'.
  *
  * If 'message' is not NULL, then, on successful return
  * (NOTMUCH_STATUS_SUCCESS or NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) '*message'
@@ -595,6 +599,21 @@ notmuch_database_get_directory (notmuch_database_t *database,
  * 	database to use this function.
  */
 notmuch_status_t
+notmuch_database_index_file (notmuch_database_t *database,
+			     const char *filename,
+			     notmuch_param_t *indexopts,
+			     notmuch_message_t **message);
+
+/**
+ * Deprecated alias for notmuch_database_index_file called with
+ * NULL indexopts.
+ *
+ * @deprecated Deprecated as of libnotmuch 5.1 (notmuch 0.26). Please
+ * use notmuch_database_index_file instead.
+ *
+ */
+NOTMUCH_DEPRECATED(5,1)
+notmuch_status_t
 notmuch_database_add_message (notmuch_database_t *database,
 			      const char *filename,
 			      notmuch_message_t **message);
@@ -1401,7 +1420,7 @@ notmuch_message_get_filenames (notmuch_message_t *message);
  * Re-index the e-mail corresponding to 'message' using the supplied index options
  *
  * Returns the status of the re-index operation.  (see the return
- * codes documented in notmuch_database_add_message)
+ * codes documented in notmuch_database_index_file)
  *
  * After reindexing, the user should discard the message object passed
  * in here by calling notmuch_message_destroy, since it refers to the
@@ -1584,7 +1603,7 @@ notmuch_message_remove_all_tags (notmuch_message_t *message);
  *
  * A client can ensure that notmuch database tags remain synchronized
  * with maildir flags by calling this function after each call to
- * notmuch_database_add_message. See also
+ * notmuch_database_index_file. See also
  * notmuch_message_tags_to_maildir_flags for synchronizing tag changes
  * back to maildir flags.
  */
@@ -1946,7 +1965,7 @@ notmuch_tags_destroy (notmuch_tags_t *tags);
  *
  *   o Read the mtime of a directory from the filesystem
  *
- *   o Call add_message for all mail files in the directory
+ *   o Call index_file for all mail files in the directory
  *
  *   o Call notmuch_directory_set_mtime with the mtime read from the
  *     filesystem.
diff --git a/notmuch-insert.c b/notmuch-insert.c
index bc96af0e..2f9d2634 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -383,7 +383,7 @@ add_file (notmuch_database_t *notmuch, const char *path, tag_op_list_t *tag_ops,
     notmuch_message_t *message;
     notmuch_status_t status;
 
-    status = notmuch_database_add_message (notmuch, path, &message);
+    status = notmuch_database_index_file (notmuch, path, NULL, &message);
     if (status == NOTMUCH_STATUS_SUCCESS) {
 	status = tag_op_list_apply (message, tag_ops, 0);
 	if (status) {
diff --git a/notmuch-new.c b/notmuch-new.c
index 3a60f7ca..99d8e709 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -261,7 +261,7 @@ add_file (notmuch_database_t *notmuch, const char *filename,
     if (status)
 	goto DONE;
 
-    status = notmuch_database_add_message (notmuch, filename, &message);
+    status = notmuch_database_index_file (notmuch, filename, NULL, &message);
     switch (status) {
     /* Success. */
     case NOTMUCH_STATUS_SUCCESS:
diff --git a/test/T070-insert.sh b/test/T070-insert.sh
index 48f212ee..fe691ea4 100755
--- a/test/T070-insert.sh
+++ b/test/T070-insert.sh
@@ -193,7 +193,7 @@ cat <<EOF > index-file-$code.gdb
 set breakpoint pending on
 set logging file index-file-$code.log
 set logging on
-break notmuch_database_add_message
+break notmuch_database_index_file
 commands
 return NOTMUCH_STATUS_$code
 continue
@@ -205,13 +205,13 @@ done
 gen_insert_msg
 
 for code in  FILE_NOT_EMAIL READ_ONLY_DATABASE UPGRADE_REQUIRED PATH_ERROR; do
-    test_begin_subtest "EXIT_FAILURE when add_message returns $code"
+    test_begin_subtest "EXIT_FAILURE when index_file returns $code"
     test_expect_code 1 \
          "${TEST_GDB} --batch-silent --return-child-result \
 	     -ex 'set args insert < $gen_msg_filename' \
 	     -x index-file-$code.gdb notmuch"
 
-    test_begin_subtest "success exit with --keep when add_message returns $code"
+    test_begin_subtest "success exit with --keep when index_file returns $code"
     test_expect_code 0 \
          "${TEST_GDB} --batch-silent --return-child-result \
 	     -ex 'set args insert --keep < $gen_msg_filename' \
@@ -219,13 +219,13 @@ for code in  FILE_NOT_EMAIL READ_ONLY_DATABASE UPGRADE_REQUIRED PATH_ERROR; do
 done
 
 for code in OUT_OF_MEMORY XAPIAN_EXCEPTION ; do
-    test_begin_subtest "EX_TEMPFAIL when add_message returns $code"
+    test_begin_subtest "EX_TEMPFAIL when index_file returns $code"
     test_expect_code 75 \
          "${TEST_GDB} --batch-silent --return-child-result \
 	     -ex 'set args insert < $gen_msg_filename' \
 	     -x index-file-$code.gdb notmuch"
 
-    test_begin_subtest "success exit with --keep when add_message returns $code"
+    test_begin_subtest "success exit with --keep when index_file returns $code"
     test_expect_code 0 \
          "${TEST_GDB} --batch-silent --return-child-result \
 	     -ex 'set args insert --keep < $gen_msg_filename' \
diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index 34d15698..a50eca80 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -127,7 +127,7 @@ int main (int argc, char** argv)
    if (stat != NOTMUCH_STATUS_SUCCESS) {
      fprintf (stderr, "error opening database: %d\n", stat);
    }
-   stat = notmuch_database_add_message (db, "/dev/null", NULL);
+   stat = notmuch_database_index_file (db, "/dev/null", NULL, NULL);
    if (stat)
        fputs (notmuch_database_status_string (db), stderr);
 
@@ -152,7 +152,7 @@ int main (int argc, char** argv)
    if (stat != NOTMUCH_STATUS_SUCCESS) {
      fprintf (stderr, "error opening database: %d\n", stat);
    }
-   stat = notmuch_database_add_message (db, "./nonexistent", NULL);
+   stat = notmuch_database_index_file (db, "./nonexistent", NULL, NULL);
    if (stat) {
        char *status_string = notmuch_database_status_string (db);
        if (status_string) fputs (status_string, stderr);
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] reindex: drop notmuch_param_t, use notmuch_indexopts_t instead
  2017-08-09  5:47 [PATCH 1/3] lib: clarify description of notmuch_database_add_message Daniel Kahn Gillmor
  2017-08-09  5:47 ` [PATCH 2/3] database: add n_d_index_file (deprecates n_d_add_message) Daniel Kahn Gillmor
@ 2017-08-09  5:47 ` Daniel Kahn Gillmor
  1 sibling, 0 replies; 11+ messages in thread
From: Daniel Kahn Gillmor @ 2017-08-09  5:47 UTC (permalink / raw)
  To: Notmuch Mail

There are at least three places in notmuch that can trigger an
indexing action:

 * notmuch new
 * notmuch insert
 * notmuch reindex

I have plans to add some indexing options (e.g. indexing the cleartext
of encrypted parts, external filters, automated property injection)
that should properly be available in all places where indexing
happens.

I also want those indexing options to be exposed by (and constrained
by) the libnotmuch C API.

This isn't yet an API break because we've never made a release with
notmuch_param_t.

These indexing options are relevant in the listed places (and in the
libnotmuch analogues), but they aren't relevant in the other kinds of
functionality that notmuch offers (e.g. dump/restore, tagging, search,
show, reply).

So i think a generic "param" object isn't well-suited for this case.
In particular:

 * a param object sounds like it could contain parameters for some
   other (non-indexing) operation.  This sounds confusing -- why would
   i pass non-indexing parameters to a function that only does
   indexing?

 * bremner suggests online a generic param object would actually be
   passed as a list of param objects, argv-style.  In this case (at
   least in the obvious argv implementation), the params might be some
   sort of generic string.  This introduces a problem where the API of
   the library doesn't grow as new options are added, which means that
   when code outside the library tries to use a feature, it first has
   to test for it, and have code to handle it not being available.
   The indexopts approach proposed here instead makes it clear at
   compile time and at dynamic link time that there is an explicit
   dependency on that feature, which allows automated tools to keep
   track of what's needed and keeps the actual code simple.

My proposal adds the notmuch_indexopts_t as an opaque struct, so that
we can extend the list of options without causing ABI breakage.

The cost of this proposal appears to be that the "boilerplate" API
increases a little bit, with a generic constructor and destructor
function for the indexopts struct.

More patches will follow that make use of this indexopts approach.
---
 lib/Makefile.local    |  1 +
 lib/add-message.cc    |  2 +-
 lib/indexopts.c       | 33 +++++++++++++++++++++++++++++++++
 lib/message.cc        |  2 +-
 lib/notmuch-private.h |  1 -
 lib/notmuch.h         | 31 ++++++++++++++++++++++++++++---
 notmuch-reindex.c     |  4 ++--
 7 files changed, 66 insertions(+), 8 deletions(-)
 create mode 100644 lib/indexopts.c

diff --git a/lib/Makefile.local b/lib/Makefile.local
index 0b5c4b08..93b08d15 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -43,6 +43,7 @@ libnotmuch_c_srcs =		\
 	$(dir)/sha1.c		\
 	$(dir)/built-with.c	\
 	$(dir)/string-map.c	\
+	$(dir)/indexopts.c	\
 	$(dir)/tags.c
 
 libnotmuch_cxx_srcs =		\
diff --git a/lib/add-message.cc b/lib/add-message.cc
index 4454b2b0..85505ec4 100644
--- a/lib/add-message.cc
+++ b/lib/add-message.cc
@@ -460,7 +460,7 @@ _notmuch_database_link_message (notmuch_database_t *notmuch,
 notmuch_status_t
 notmuch_database_index_file (notmuch_database_t *notmuch,
 			     const char *filename,
-			     notmuch_param_t unused (*indexopts),
+			     notmuch_indexopts_t unused (*indexopts),
 			     notmuch_message_t **message_ret)
 {
     notmuch_message_file_t *message_file;
diff --git a/lib/indexopts.c b/lib/indexopts.c
new file mode 100644
index 00000000..2f9b841b
--- /dev/null
+++ b/lib/indexopts.c
@@ -0,0 +1,33 @@
+/* indexopts.c - options for indexing messages (currently a stub)
+ *
+ * Copyright © 2017 Daniel Kahn Gillmor
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see https://www.gnu.org/licenses/ .
+ *
+ * Author: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
+ */
+
+#include "notmuch-private.h"
+
+notmuch_indexopts_t *
+notmuch_database_get_default_indexopts (notmuch_database_t unused (*db))
+{
+    return NULL;
+}
+
+void
+notmuch_indexopts_destroy (notmuch_indexopts_t *indexopts)
+{
+    talloc_free (indexopts);
+}
diff --git a/lib/message.cc b/lib/message.cc
index 539d3320..98a88d3a 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1939,7 +1939,7 @@ _notmuch_message_frozen (notmuch_message_t *message)
 
 notmuch_status_t
 notmuch_message_reindex (notmuch_message_t *message,
-			 notmuch_param_t unused (*indexopts))
+			 notmuch_indexopts_t unused (*indexopts))
 {
     notmuch_database_t *notmuch = NULL;
     notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index f6a497b5..b187a80f 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -632,7 +632,6 @@ _notmuch_thread_create (void *ctx,
 			notmuch_exclude_t omit_exclude,
 			notmuch_sort_t sort);
 
-
 NOTMUCH_END_DECLS
 
 #ifdef __cplusplus
diff --git a/lib/notmuch.h b/lib/notmuch.h
index ed405dc5..7b06cd12 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -219,7 +219,7 @@ typedef struct _notmuch_tags notmuch_tags_t;
 typedef struct _notmuch_directory notmuch_directory_t;
 typedef struct _notmuch_filenames notmuch_filenames_t;
 typedef struct _notmuch_config_list notmuch_config_list_t;
-typedef struct _notmuch_param notmuch_param_t;
+typedef struct _notmuch_indexopts notmuch_indexopts_t;
 #endif /* __DOXYGEN__ */
 
 /**
@@ -601,7 +601,7 @@ notmuch_database_get_directory (notmuch_database_t *database,
 notmuch_status_t
 notmuch_database_index_file (notmuch_database_t *database,
 			     const char *filename,
-			     notmuch_param_t *indexopts,
+			     notmuch_indexopts_t *indexopts,
 			     notmuch_message_t **message);
 
 /**
@@ -1428,7 +1428,7 @@ notmuch_message_get_filenames (notmuch_message_t *message);
  */
 notmuch_status_t
 notmuch_message_reindex (notmuch_message_t *message,
-			 notmuch_param_t *indexopts);
+			 notmuch_indexopts_t *indexopts);
 
 /**
  * Message flags.
@@ -2170,6 +2170,31 @@ notmuch_config_list_move_to_next (notmuch_config_list_t *config_list);
 void
 notmuch_config_list_destroy (notmuch_config_list_t *config_list);
 
+
+/**
+ * get the current default indexing options for a given database.  
+ * 
+ * This object will survive until the database itself is destroyed,
+ * but the caller may also release it earlier with
+ * notmuch_indexopts_destroy.
+ *
+ * This object represents a set of options on how a message can be
+ * added to the index.  At the moment it is a featureless stub.
+ * 
+ * @since libnotmuch 5.1 (notmuch 0.26)
+ */
+notmuch_indexopts_t *
+notmuch_database_get_default_indexopts (notmuch_database_t *db);
+
+/**
+ * Destroy a notmuch_indexopts_t object.
+ * 
+ * @since libnotmuch 5.1 (notmuch 0.26)
+ */
+void
+notmuch_indexopts_destroy (notmuch_indexopts_t *options);
+
+
 /**
  * interrogate the library for compile time features
  *
diff --git a/notmuch-reindex.c b/notmuch-reindex.c
index 44223042..bceac722 100644
--- a/notmuch-reindex.c
+++ b/notmuch-reindex.c
@@ -40,7 +40,7 @@ handle_sigint (unused (int sig))
  */
 static int
 reindex_query (notmuch_database_t *notmuch, const char *query_string,
-	       notmuch_param_t *indexopts)
+	       notmuch_indexopts_t *indexopts)
 {
     notmuch_query_t *query;
     notmuch_messages_t *messages;
@@ -89,7 +89,7 @@ notmuch_reindex_command (notmuch_config_t *config, int argc, char *argv[])
     struct sigaction action;
     int opt_index;
     int ret;
-    notmuch_param_t *indexopts = NULL;
+    notmuch_indexopts_t *indexopts = NULL;
 
     /* Set up our handler for SIGINT */
     memset (&action, 0, sizeof (struct sigaction));
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] database: add n_d_index_file (deprecates n_d_add_message)
  2017-08-09  5:47 ` [PATCH 2/3] database: add n_d_index_file (deprecates n_d_add_message) Daniel Kahn Gillmor
@ 2017-08-17 11:31   ` David Bremner
  2017-08-17 23:14     ` [PATCH v2 1/3] lib: clarify description of notmuch_database_add_message Daniel Kahn Gillmor
  0 siblings, 1 reply; 11+ messages in thread
From: David Bremner @ 2017-08-17 11:31 UTC (permalink / raw)
  To: Daniel Kahn Gillmor, Notmuch Mail

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> index b187a80f..f6a497b5 100644
> --- a/lib/notmuch-private.h
> +++ b/lib/notmuch-private.h
> @@ -632,6 +632,7 @@ _notmuch_thread_create (void *ctx,
>  			notmuch_exclude_t omit_exclude,
>  			notmuch_sort_t sort);
>  
> +
>  NOTMUCH_END_DECLS

mysterious extra whitespace

>  
>  #ifdef __cplusplus
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 02586a91..ed405dc5 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -237,7 +237,7 @@ typedef struct _notmuch_param notmuch_param_t;
>   * The database will not yet have any data in it
>   * (notmuch_database_create itself is a very cheap function). Messages
>   * contained within 'path' can be added to the database by calling
> - * notmuch_database_add_message.
> + * notmuch_database_index_file.
>   *
>   * In case of any failure, this function returns an error status and
>   * sets *database to NULL (after printing an error message on stderr).
> @@ -561,6 +561,10 @@ notmuch_database_get_directory (notmuch_database_t *database,
>   * database, rather than creating a new message, this adds the search
>   * terms from the identified file to the existing message's index, and
>   * adds 'filename' to the list of filenames known for the message.
> + * 
> + * 'indexopts' can be NULL (meaning, use the indexing defaults from
> + * the database), or can be an explicit choice of indexing options
> + * that should govern the indexing of this specific 'filename'.

Ideally this should have an @since line

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/3] lib: clarify description of notmuch_database_add_message
  2017-08-17 11:31   ` David Bremner
@ 2017-08-17 23:14     ` Daniel Kahn Gillmor
  2017-08-17 23:14       ` [PATCH v2 2/3] database: add n_d_index_file (deprecates n_d_add_message) Daniel Kahn Gillmor
                         ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Daniel Kahn Gillmor @ 2017-08-17 23:14 UTC (permalink / raw)
  To: Notmuch Mail

Since we're accumulating the index when we add a new file to the
message, the semantics have slightly changed.  This tries to align the
documentation with the actual functionality.
---
 lib/notmuch.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index f78b3473..02586a91 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -542,8 +542,10 @@ notmuch_database_get_directory (notmuch_database_t *database,
 				notmuch_directory_t **directory);
 
 /**
- * Add a new message to the given notmuch database or associate an
- * additional filename with an existing message.
+ * Add a message file to a database, indexing it for retrieval by
+ * future searches.  If a message already exists with the same message
+ * ID as the specified file, their indexes will be merged, and this
+ * new filename will also be associated with the existing message.
  *
  * Here, 'filename' should be a path relative to the path of
  * 'database' (see notmuch_database_get_path), or else should be an
@@ -556,8 +558,9 @@ notmuch_database_get_directory (notmuch_database_t *database,
  * entire contents of the file.
  *
  * If another message with the same message ID already exists in the
- * database, rather than creating a new message, this adds 'filename'
- * to the list of the filenames for the existing message.
+ * database, rather than creating a new message, this adds the search
+ * terms from the identified file to the existing message's index, and
+ * adds 'filename' to the list of filenames known for the message.
  *
  * If 'message' is not NULL, then, on successful return
  * (NOTMUCH_STATUS_SUCCESS or NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) '*message'
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/3] database: add n_d_index_file (deprecates n_d_add_message)
  2017-08-17 23:14     ` [PATCH v2 1/3] lib: clarify description of notmuch_database_add_message Daniel Kahn Gillmor
@ 2017-08-17 23:14       ` Daniel Kahn Gillmor
  2017-08-23 11:01         ` David Bremner
  2017-08-17 23:14       ` [PATCH v2 3/3] reindex: drop notmuch_param_t, use notmuch_indexopts_t instead Daniel Kahn Gillmor
  2017-08-20 12:10       ` [PATCH v2 1/3] lib: clarify description of notmuch_database_add_message David Bremner
  2 siblings, 1 reply; 11+ messages in thread
From: Daniel Kahn Gillmor @ 2017-08-17 23:14 UTC (permalink / raw)
  To: Notmuch Mail

We need a way to pass parameters to the indexing functionality on the
first index, not just on reindexing.  The obvious place is in
notmuch_database_add_message.  But since modifying the argument list
would break both API and ABI, we needed a new name.

I considered notmuch_database_add_message_with_params(), but the
functionality we're talking about doesn't always add a message.  It
tries to index a specific file, possibly adding a message, but
possibly doing other things, like adding terms to an existing message,
or failing to deal with message objects entirely (e.g. because the
file didn't contain a message).

So i chose the function name notmuch_database_index_file.

I confess i'm a little concerned about confusing future notmuch
developers with the new name, since we already have a private
_notmuch_message_index_file function, and the two do rather different
things.  But i think the added clarity for people linking against the
future libnotmuch and the capacity for using index parameters makes
this a worthwhile tradeoff.  (that said, if anyone has another name
that they strongly prefer, i'd be happy to go with it)

This changeset also adjusts the tests so that we test whether the new,
preferred function returns bad values (since the deprecated function
just calls the new one).

We can keep the deprecated n_d_add_message function around as long as
we like, but at the next place where we're forced to break API or ABI
we can probably choose to drop the name relatively safely.

NOTE: there is probably more cleanup to do in the ruby and go bindings
to complete the deprecation directly.  I don't know those languages
well enough to attempt a fix; i don't know how to test them; and i
don't know the culture around those languages about API additions or
deprecations.
---
 bindings/python/docs/source/database.rst |  2 +-
 bindings/python/notmuch/database.py      | 18 ++++++++++++------
 bindings/python/notmuch/directory.py     |  2 +-
 bindings/python/notmuch/message.py       |  2 +-
 bindings/ruby/database.c                 |  2 +-
 contrib/go/src/notmuch/notmuch.go        |  2 +-
 lib/add-message.cc                       | 18 +++++++++++++++---
 lib/notmuch.h                            | 29 +++++++++++++++++++++++++----
 notmuch-insert.c                         |  2 +-
 notmuch-new.c                            |  2 +-
 test/T070-insert.sh                      | 10 +++++-----
 test/T560-lib-error.sh                   |  4 ++--
 12 files changed, 66 insertions(+), 27 deletions(-)

diff --git a/bindings/python/docs/source/database.rst b/bindings/python/docs/source/database.rst
index 5f1cdc14..079dc754 100644
--- a/bindings/python/docs/source/database.rst
+++ b/bindings/python/docs/source/database.rst
@@ -25,7 +25,7 @@
 
    .. automethod:: get_directory
 
-   .. automethod:: add_message
+   .. automethod:: index_file
 
    .. automethod:: remove_message
 
diff --git a/bindings/python/notmuch/database.py b/bindings/python/notmuch/database.py
index 8f918069..a2c025eb 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -285,7 +285,7 @@ class Database(object):
         """Does this database need to be upgraded before writing to it?
 
         If this function returns `True` then no functions that modify the
-        database (:meth:`add_message`,
+        database (:meth:`index_file`,
         :meth:`Message.add_tag`, :meth:`Directory.set_mtime`,
         etc.) will work unless :meth:`upgrade` is called successfully first.
 
@@ -399,12 +399,13 @@ class Database(object):
         # return the Directory, init it with the absolute path
         return Directory(abs_dirpath, dir_p, self)
 
-    _add_message = nmlib.notmuch_database_add_message
-    _add_message.argtypes = [NotmuchDatabaseP, c_char_p,
+    _index_file = nmlib.notmuch_database_index_file
+    _index_file.argtypes = [NotmuchDatabaseP, c_char_p,
+                             c_void_p,
                              POINTER(NotmuchMessageP)]
-    _add_message.restype = c_uint
+    _index_file.restype = c_uint
 
-    def add_message(self, filename, sync_maildir_flags=False):
+    def index_file(self, filename, sync_maildir_flags=False):
         """Adds a new message to the database
 
         :param filename: should be a path relative to the path of the
@@ -455,7 +456,7 @@ class Database(object):
         """
         self._assert_db_is_initialized()
         msg_p = NotmuchMessageP()
-        status = self._add_message(self._db, _str(filename), byref(msg_p))
+        status = self._index_file(self._db, _str(filename), c_void_p(None), byref(msg_p))
 
         if not status in [STATUS.SUCCESS, STATUS.DUPLICATE_MESSAGE_ID]:
             raise NotmuchError(status)
@@ -467,6 +468,11 @@ class Database(object):
             msg.maildir_flags_to_tags()
         return (msg, status)
 
+    def add_message(self, filename, sync_maildir_flags=False):
+        """Deprecated alias for :meth:`index_file`
+        """
+        self.index_file(self, filename, sync_maildir_flags=sync_maildir_flags)
+
     _remove_message = nmlib.notmuch_database_remove_message
     _remove_message.argtypes = [NotmuchDatabaseP, c_char_p]
     _remove_message.restype = c_uint
diff --git a/bindings/python/notmuch/directory.py b/bindings/python/notmuch/directory.py
index 7f86b1ac..b30c9e35 100644
--- a/bindings/python/notmuch/directory.py
+++ b/bindings/python/notmuch/directory.py
@@ -93,7 +93,7 @@ class Directory(object):
 
         * Read the mtime of a directory from the filesystem
 
-        * Call :meth:`Database.add_message` for all mail files in
+        * Call :meth:`Database.index_file` for all mail files in
           the directory
 
         * Call notmuch_directory_set_mtime with the mtime read from the
diff --git a/bindings/python/notmuch/message.py b/bindings/python/notmuch/message.py
index fc177eb2..cce377d0 100644
--- a/bindings/python/notmuch/message.py
+++ b/bindings/python/notmuch/message.py
@@ -566,7 +566,7 @@ class Message(Python3StringMixIn):
         logical OR operator.)
 
         As a convenience, you can set the sync_maildir_flags parameter in
-        :meth:`Database.add_message` to implicitly call this.
+        :meth:`Database.index_file` to implicitly call this.
 
         :returns: a :class:`STATUS`. In short, you want to see
             notmuch.STATUS.SUCCESS here. See there for details."""
diff --git a/bindings/ruby/database.c b/bindings/ruby/database.c
index 12e6bab7..416eb709 100644
--- a/bindings/ruby/database.c
+++ b/bindings/ruby/database.c
@@ -291,7 +291,7 @@ notmuch_rb_database_add_message (VALUE self, VALUE pathv)
     SafeStringValue (pathv);
     path = RSTRING_PTR (pathv);
 
-    ret = notmuch_database_add_message (db, path, &message);
+    ret = notmuch_database_index_file (db, path, NULL, &message);
     notmuch_rb_status_raise (ret);
     return rb_assoc_new (Data_Wrap_Struct (notmuch_rb_cMessage, NULL, NULL, message),
         (ret == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) ? Qtrue : Qfalse);
diff --git a/contrib/go/src/notmuch/notmuch.go b/contrib/go/src/notmuch/notmuch.go
index 2d684311..89093b2d 100644
--- a/contrib/go/src/notmuch/notmuch.go
+++ b/contrib/go/src/notmuch/notmuch.go
@@ -170,7 +170,7 @@ func (self *Database) GetVersion() uint {
 /* Does this database need to be upgraded before writing to it?
  *
  * If this function returns TRUE then no functions that modify the
- * database (notmuch_database_add_message, notmuch_message_add_tag,
+ * database (notmuch_database_index_file, notmuch_message_add_tag,
  * notmuch_directory_set_mtime, etc.) will work unless the function
  * notmuch_database_upgrade is called successfully first. */
 func (self *Database) NeedsUpgrade() bool {
diff --git a/lib/add-message.cc b/lib/add-message.cc
index 711ed9fa..4454b2b0 100644
--- a/lib/add-message.cc
+++ b/lib/add-message.cc
@@ -458,9 +458,10 @@ _notmuch_database_link_message (notmuch_database_t *notmuch,
 }
 
 notmuch_status_t
-notmuch_database_add_message (notmuch_database_t *notmuch,
-			      const char *filename,
-			      notmuch_message_t **message_ret)
+notmuch_database_index_file (notmuch_database_t *notmuch,
+			     const char *filename,
+			     notmuch_param_t unused (*indexopts),
+			     notmuch_message_t **message_ret)
 {
     notmuch_message_file_t *message_file;
     notmuch_message_t *message = NULL;
@@ -575,3 +576,14 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
 
     return ret;
 }
+
+notmuch_status_t
+notmuch_database_add_message (notmuch_database_t *notmuch,
+			      const char *filename,
+			      notmuch_message_t **message_ret)
+{
+    return notmuch_database_index_file (notmuch, filename,
+					NULL,
+					message_ret);
+    
+}
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 02586a91..9f6f0fdd 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -237,7 +237,7 @@ typedef struct _notmuch_param notmuch_param_t;
  * The database will not yet have any data in it
  * (notmuch_database_create itself is a very cheap function). Messages
  * contained within 'path' can be added to the database by calling
- * notmuch_database_add_message.
+ * notmuch_database_index_file.
  *
  * In case of any failure, this function returns an error status and
  * sets *database to NULL (after printing an error message on stderr).
@@ -561,6 +561,10 @@ notmuch_database_get_directory (notmuch_database_t *database,
  * database, rather than creating a new message, this adds the search
  * terms from the identified file to the existing message's index, and
  * adds 'filename' to the list of filenames known for the message.
+ * 
+ * 'indexopts' can be NULL (meaning, use the indexing defaults from
+ * the database), or can be an explicit choice of indexing options
+ * that should govern the indexing of this specific 'filename'.
  *
  * If 'message' is not NULL, then, on successful return
  * (NOTMUCH_STATUS_SUCCESS or NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) '*message'
@@ -593,7 +597,24 @@ notmuch_database_get_directory (notmuch_database_t *database,
  *
  * NOTMUCH_STATUS_UPGRADE_REQUIRED: The caller must upgrade the
  * 	database to use this function.
+ *
+ * @since libnotmuch 5.1 (notmuch 0.26)
+ */
+notmuch_status_t
+notmuch_database_index_file (notmuch_database_t *database,
+			     const char *filename,
+			     notmuch_param_t *indexopts,
+			     notmuch_message_t **message);
+
+/**
+ * Deprecated alias for notmuch_database_index_file called with
+ * NULL indexopts.
+ *
+ * @deprecated Deprecated as of libnotmuch 5.1 (notmuch 0.26). Please
+ * use notmuch_database_index_file instead.
+ *
  */
+NOTMUCH_DEPRECATED(5,1)
 notmuch_status_t
 notmuch_database_add_message (notmuch_database_t *database,
 			      const char *filename,
@@ -1401,7 +1422,7 @@ notmuch_message_get_filenames (notmuch_message_t *message);
  * Re-index the e-mail corresponding to 'message' using the supplied index options
  *
  * Returns the status of the re-index operation.  (see the return
- * codes documented in notmuch_database_add_message)
+ * codes documented in notmuch_database_index_file)
  *
  * After reindexing, the user should discard the message object passed
  * in here by calling notmuch_message_destroy, since it refers to the
@@ -1584,7 +1605,7 @@ notmuch_message_remove_all_tags (notmuch_message_t *message);
  *
  * A client can ensure that notmuch database tags remain synchronized
  * with maildir flags by calling this function after each call to
- * notmuch_database_add_message. See also
+ * notmuch_database_index_file. See also
  * notmuch_message_tags_to_maildir_flags for synchronizing tag changes
  * back to maildir flags.
  */
@@ -1946,7 +1967,7 @@ notmuch_tags_destroy (notmuch_tags_t *tags);
  *
  *   o Read the mtime of a directory from the filesystem
  *
- *   o Call add_message for all mail files in the directory
+ *   o Call index_file for all mail files in the directory
  *
  *   o Call notmuch_directory_set_mtime with the mtime read from the
  *     filesystem.
diff --git a/notmuch-insert.c b/notmuch-insert.c
index bc96af0e..2f9d2634 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -383,7 +383,7 @@ add_file (notmuch_database_t *notmuch, const char *path, tag_op_list_t *tag_ops,
     notmuch_message_t *message;
     notmuch_status_t status;
 
-    status = notmuch_database_add_message (notmuch, path, &message);
+    status = notmuch_database_index_file (notmuch, path, NULL, &message);
     if (status == NOTMUCH_STATUS_SUCCESS) {
 	status = tag_op_list_apply (message, tag_ops, 0);
 	if (status) {
diff --git a/notmuch-new.c b/notmuch-new.c
index 16b4d022..a4829327 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -261,7 +261,7 @@ add_file (notmuch_database_t *notmuch, const char *filename,
     if (status)
 	goto DONE;
 
-    status = notmuch_database_add_message (notmuch, filename, &message);
+    status = notmuch_database_index_file (notmuch, filename, NULL, &message);
     switch (status) {
     /* Success. */
     case NOTMUCH_STATUS_SUCCESS:
diff --git a/test/T070-insert.sh b/test/T070-insert.sh
index 48f212ee..fe691ea4 100755
--- a/test/T070-insert.sh
+++ b/test/T070-insert.sh
@@ -193,7 +193,7 @@ cat <<EOF > index-file-$code.gdb
 set breakpoint pending on
 set logging file index-file-$code.log
 set logging on
-break notmuch_database_add_message
+break notmuch_database_index_file
 commands
 return NOTMUCH_STATUS_$code
 continue
@@ -205,13 +205,13 @@ done
 gen_insert_msg
 
 for code in  FILE_NOT_EMAIL READ_ONLY_DATABASE UPGRADE_REQUIRED PATH_ERROR; do
-    test_begin_subtest "EXIT_FAILURE when add_message returns $code"
+    test_begin_subtest "EXIT_FAILURE when index_file returns $code"
     test_expect_code 1 \
          "${TEST_GDB} --batch-silent --return-child-result \
 	     -ex 'set args insert < $gen_msg_filename' \
 	     -x index-file-$code.gdb notmuch"
 
-    test_begin_subtest "success exit with --keep when add_message returns $code"
+    test_begin_subtest "success exit with --keep when index_file returns $code"
     test_expect_code 0 \
          "${TEST_GDB} --batch-silent --return-child-result \
 	     -ex 'set args insert --keep < $gen_msg_filename' \
@@ -219,13 +219,13 @@ for code in  FILE_NOT_EMAIL READ_ONLY_DATABASE UPGRADE_REQUIRED PATH_ERROR; do
 done
 
 for code in OUT_OF_MEMORY XAPIAN_EXCEPTION ; do
-    test_begin_subtest "EX_TEMPFAIL when add_message returns $code"
+    test_begin_subtest "EX_TEMPFAIL when index_file returns $code"
     test_expect_code 75 \
          "${TEST_GDB} --batch-silent --return-child-result \
 	     -ex 'set args insert < $gen_msg_filename' \
 	     -x index-file-$code.gdb notmuch"
 
-    test_begin_subtest "success exit with --keep when add_message returns $code"
+    test_begin_subtest "success exit with --keep when index_file returns $code"
     test_expect_code 0 \
          "${TEST_GDB} --batch-silent --return-child-result \
 	     -ex 'set args insert --keep < $gen_msg_filename' \
diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index 34d15698..a50eca80 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -127,7 +127,7 @@ int main (int argc, char** argv)
    if (stat != NOTMUCH_STATUS_SUCCESS) {
      fprintf (stderr, "error opening database: %d\n", stat);
    }
-   stat = notmuch_database_add_message (db, "/dev/null", NULL);
+   stat = notmuch_database_index_file (db, "/dev/null", NULL, NULL);
    if (stat)
        fputs (notmuch_database_status_string (db), stderr);
 
@@ -152,7 +152,7 @@ int main (int argc, char** argv)
    if (stat != NOTMUCH_STATUS_SUCCESS) {
      fprintf (stderr, "error opening database: %d\n", stat);
    }
-   stat = notmuch_database_add_message (db, "./nonexistent", NULL);
+   stat = notmuch_database_index_file (db, "./nonexistent", NULL, NULL);
    if (stat) {
        char *status_string = notmuch_database_status_string (db);
        if (status_string) fputs (status_string, stderr);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/3] reindex: drop notmuch_param_t, use notmuch_indexopts_t instead
  2017-08-17 23:14     ` [PATCH v2 1/3] lib: clarify description of notmuch_database_add_message Daniel Kahn Gillmor
  2017-08-17 23:14       ` [PATCH v2 2/3] database: add n_d_index_file (deprecates n_d_add_message) Daniel Kahn Gillmor
@ 2017-08-17 23:14       ` Daniel Kahn Gillmor
  2017-08-20 12:10       ` [PATCH v2 1/3] lib: clarify description of notmuch_database_add_message David Bremner
  2 siblings, 0 replies; 11+ messages in thread
From: Daniel Kahn Gillmor @ 2017-08-17 23:14 UTC (permalink / raw)
  To: Notmuch Mail

There are at least three places in notmuch that can trigger an
indexing action:

 * notmuch new
 * notmuch insert
 * notmuch reindex

I have plans to add some indexing options (e.g. indexing the cleartext
of encrypted parts, external filters, automated property injection)
that should properly be available in all places where indexing
happens.

I also want those indexing options to be exposed by (and constrained
by) the libnotmuch C API.

This isn't yet an API break because we've never made a release with
notmuch_param_t.

These indexing options are relevant in the listed places (and in the
libnotmuch analogues), but they aren't relevant in the other kinds of
functionality that notmuch offers (e.g. dump/restore, tagging, search,
show, reply).

So i think a generic "param" object isn't well-suited for this case.
In particular:

 * a param object sounds like it could contain parameters for some
   other (non-indexing) operation.  This sounds confusing -- why would
   i pass non-indexing parameters to a function that only does
   indexing?

 * bremner suggests online a generic param object would actually be
   passed as a list of param objects, argv-style.  In this case (at
   least in the obvious argv implementation), the params might be some
   sort of generic string.  This introduces a problem where the API of
   the library doesn't grow as new options are added, which means that
   when code outside the library tries to use a feature, it first has
   to test for it, and have code to handle it not being available.
   The indexopts approach proposed here instead makes it clear at
   compile time and at dynamic link time that there is an explicit
   dependency on that feature, which allows automated tools to keep
   track of what's needed and keeps the actual code simple.

My proposal adds the notmuch_indexopts_t as an opaque struct, so that
we can extend the list of options without causing ABI breakage.

The cost of this proposal appears to be that the "boilerplate" API
increases a little bit, with a generic constructor and destructor
function for the indexopts struct.

More patches will follow that make use of this indexopts approach.
---
 lib/Makefile.local |  1 +
 lib/add-message.cc |  2 +-
 lib/indexopts.c    | 33 +++++++++++++++++++++++++++++++++
 lib/message.cc     |  2 +-
 lib/notmuch.h      | 31 ++++++++++++++++++++++++++++---
 notmuch-reindex.c  |  4 ++--
 6 files changed, 66 insertions(+), 7 deletions(-)
 create mode 100644 lib/indexopts.c

diff --git a/lib/Makefile.local b/lib/Makefile.local
index 0b5c4b08..93b08d15 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -43,6 +43,7 @@ libnotmuch_c_srcs =		\
 	$(dir)/sha1.c		\
 	$(dir)/built-with.c	\
 	$(dir)/string-map.c	\
+	$(dir)/indexopts.c	\
 	$(dir)/tags.c
 
 libnotmuch_cxx_srcs =		\
diff --git a/lib/add-message.cc b/lib/add-message.cc
index 4454b2b0..85505ec4 100644
--- a/lib/add-message.cc
+++ b/lib/add-message.cc
@@ -460,7 +460,7 @@ _notmuch_database_link_message (notmuch_database_t *notmuch,
 notmuch_status_t
 notmuch_database_index_file (notmuch_database_t *notmuch,
 			     const char *filename,
-			     notmuch_param_t unused (*indexopts),
+			     notmuch_indexopts_t unused (*indexopts),
 			     notmuch_message_t **message_ret)
 {
     notmuch_message_file_t *message_file;
diff --git a/lib/indexopts.c b/lib/indexopts.c
new file mode 100644
index 00000000..2f9b841b
--- /dev/null
+++ b/lib/indexopts.c
@@ -0,0 +1,33 @@
+/* indexopts.c - options for indexing messages (currently a stub)
+ *
+ * Copyright © 2017 Daniel Kahn Gillmor
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see https://www.gnu.org/licenses/ .
+ *
+ * Author: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
+ */
+
+#include "notmuch-private.h"
+
+notmuch_indexopts_t *
+notmuch_database_get_default_indexopts (notmuch_database_t unused (*db))
+{
+    return NULL;
+}
+
+void
+notmuch_indexopts_destroy (notmuch_indexopts_t *indexopts)
+{
+    talloc_free (indexopts);
+}
diff --git a/lib/message.cc b/lib/message.cc
index 539d3320..98a88d3a 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1939,7 +1939,7 @@ _notmuch_message_frozen (notmuch_message_t *message)
 
 notmuch_status_t
 notmuch_message_reindex (notmuch_message_t *message,
-			 notmuch_param_t unused (*indexopts))
+			 notmuch_indexopts_t unused (*indexopts))
 {
     notmuch_database_t *notmuch = NULL;
     notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 9f6f0fdd..85ea4ff9 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -219,7 +219,7 @@ typedef struct _notmuch_tags notmuch_tags_t;
 typedef struct _notmuch_directory notmuch_directory_t;
 typedef struct _notmuch_filenames notmuch_filenames_t;
 typedef struct _notmuch_config_list notmuch_config_list_t;
-typedef struct _notmuch_param notmuch_param_t;
+typedef struct _notmuch_indexopts notmuch_indexopts_t;
 #endif /* __DOXYGEN__ */
 
 /**
@@ -603,7 +603,7 @@ notmuch_database_get_directory (notmuch_database_t *database,
 notmuch_status_t
 notmuch_database_index_file (notmuch_database_t *database,
 			     const char *filename,
-			     notmuch_param_t *indexopts,
+			     notmuch_indexopts_t *indexopts,
 			     notmuch_message_t **message);
 
 /**
@@ -1430,7 +1430,7 @@ notmuch_message_get_filenames (notmuch_message_t *message);
  */
 notmuch_status_t
 notmuch_message_reindex (notmuch_message_t *message,
-			 notmuch_param_t *indexopts);
+			 notmuch_indexopts_t *indexopts);
 
 /**
  * Message flags.
@@ -2172,6 +2172,31 @@ notmuch_config_list_move_to_next (notmuch_config_list_t *config_list);
 void
 notmuch_config_list_destroy (notmuch_config_list_t *config_list);
 
+
+/**
+ * get the current default indexing options for a given database.  
+ * 
+ * This object will survive until the database itself is destroyed,
+ * but the caller may also release it earlier with
+ * notmuch_indexopts_destroy.
+ *
+ * This object represents a set of options on how a message can be
+ * added to the index.  At the moment it is a featureless stub.
+ * 
+ * @since libnotmuch 5.1 (notmuch 0.26)
+ */
+notmuch_indexopts_t *
+notmuch_database_get_default_indexopts (notmuch_database_t *db);
+
+/**
+ * Destroy a notmuch_indexopts_t object.
+ * 
+ * @since libnotmuch 5.1 (notmuch 0.26)
+ */
+void
+notmuch_indexopts_destroy (notmuch_indexopts_t *options);
+
+
 /**
  * interrogate the library for compile time features
  *
diff --git a/notmuch-reindex.c b/notmuch-reindex.c
index 44223042..bceac722 100644
--- a/notmuch-reindex.c
+++ b/notmuch-reindex.c
@@ -40,7 +40,7 @@ handle_sigint (unused (int sig))
  */
 static int
 reindex_query (notmuch_database_t *notmuch, const char *query_string,
-	       notmuch_param_t *indexopts)
+	       notmuch_indexopts_t *indexopts)
 {
     notmuch_query_t *query;
     notmuch_messages_t *messages;
@@ -89,7 +89,7 @@ notmuch_reindex_command (notmuch_config_t *config, int argc, char *argv[])
     struct sigaction action;
     int opt_index;
     int ret;
-    notmuch_param_t *indexopts = NULL;
+    notmuch_indexopts_t *indexopts = NULL;
 
     /* Set up our handler for SIGINT */
     memset (&action, 0, sizeof (struct sigaction));
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/3] lib: clarify description of notmuch_database_add_message
  2017-08-17 23:14     ` [PATCH v2 1/3] lib: clarify description of notmuch_database_add_message Daniel Kahn Gillmor
  2017-08-17 23:14       ` [PATCH v2 2/3] database: add n_d_index_file (deprecates n_d_add_message) Daniel Kahn Gillmor
  2017-08-17 23:14       ` [PATCH v2 3/3] reindex: drop notmuch_param_t, use notmuch_indexopts_t instead Daniel Kahn Gillmor
@ 2017-08-20 12:10       ` David Bremner
  2 siblings, 0 replies; 11+ messages in thread
From: David Bremner @ 2017-08-20 12:10 UTC (permalink / raw)
  To: Daniel Kahn Gillmor, Notmuch Mail

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> Since we're accumulating the index when we add a new file to the
> message, the semantics have slightly changed.  This tries to align the
> documentation with the actual functionality.

pushed this one patch

d

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/3] database: add n_d_index_file (deprecates n_d_add_message)
  2017-08-17 23:14       ` [PATCH v2 2/3] database: add n_d_index_file (deprecates n_d_add_message) Daniel Kahn Gillmor
@ 2017-08-23 11:01         ` David Bremner
  2017-09-17 20:53           ` Daniel Kahn Gillmor
  0 siblings, 1 reply; 11+ messages in thread
From: David Bremner @ 2017-08-23 11:01 UTC (permalink / raw)
  To: Daniel Kahn Gillmor, Notmuch Mail

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> We need a way to pass parameters to the indexing functionality on the
> first index, not just on reindexing.  The obvious place is in
> notmuch_database_add_message.  But since modifying the argument list
> would break both API and ABI, we needed a new name.
>

Pushed 2 and 3, but with both amended to fix whitespace errors. Please
_do_ adjust your settings, this is not a test ;).

d

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/3] database: add n_d_index_file (deprecates n_d_add_message)
  2017-08-23 11:01         ` David Bremner
@ 2017-09-17 20:53           ` Daniel Kahn Gillmor
  2017-09-18 10:55             ` David Bremner
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel Kahn Gillmor @ 2017-09-17 20:53 UTC (permalink / raw)
  To: David Bremner, Notmuch Mail

[-- Attachment #1: Type: text/plain, Size: 484 bytes --]

On Wed 2017-08-23 08:01:22 -0300, David Bremner wrote:
> Pushed 2 and 3, but with both amended to fix whitespace errors. Please
> _do_ adjust your settings, this is not a test ;).

thanks for this push, bremner.

if there are settings that need to be adjusted, i believe .dir-locals.el
is the place to adjust them for those of us who use emacs as their text
editor, right?  i'm happy to pick them up from there.

any suggestions on what specific settings need adjustment?

     --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/3] database: add n_d_index_file (deprecates n_d_add_message)
  2017-09-17 20:53           ` Daniel Kahn Gillmor
@ 2017-09-18 10:55             ` David Bremner
  0 siblings, 0 replies; 11+ messages in thread
From: David Bremner @ 2017-09-18 10:55 UTC (permalink / raw)
  To: Daniel Kahn Gillmor, Notmuch Mail

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> On Wed 2017-08-23 08:01:22 -0300, David Bremner wrote:
>> Pushed 2 and 3, but with both amended to fix whitespace errors. Please
>> _do_ adjust your settings, this is not a test ;).
>
> thanks for this push, bremner.
>
> if there are settings that need to be adjusted, i believe .dir-locals.el
> is the place to adjust them for those of us who use emacs as their text
> editor, right?  i'm happy to pick them up from there.
>
> any suggestions on what specific settings need adjustment?
>
>      --dkg

I'm not sure about an emacs-specific fix; I just rename
.git/hooks/pre-commit.sample to .git/hooks/pre-commit to enable checking
for whitespace issues before committing.

d

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-09-18 10:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-09  5:47 [PATCH 1/3] lib: clarify description of notmuch_database_add_message Daniel Kahn Gillmor
2017-08-09  5:47 ` [PATCH 2/3] database: add n_d_index_file (deprecates n_d_add_message) Daniel Kahn Gillmor
2017-08-17 11:31   ` David Bremner
2017-08-17 23:14     ` [PATCH v2 1/3] lib: clarify description of notmuch_database_add_message Daniel Kahn Gillmor
2017-08-17 23:14       ` [PATCH v2 2/3] database: add n_d_index_file (deprecates n_d_add_message) Daniel Kahn Gillmor
2017-08-23 11:01         ` David Bremner
2017-09-17 20:53           ` Daniel Kahn Gillmor
2017-09-18 10:55             ` David Bremner
2017-08-17 23:14       ` [PATCH v2 3/3] reindex: drop notmuch_param_t, use notmuch_indexopts_t instead Daniel Kahn Gillmor
2017-08-20 12:10       ` [PATCH v2 1/3] lib: clarify description of notmuch_database_add_message David Bremner
2017-08-09  5:47 ` [PATCH 3/3] reindex: drop notmuch_param_t, use notmuch_indexopts_t instead Daniel Kahn Gillmor

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).