Re: [PATCH 2/4] lib: catch error from closed db in n_m_get_message_id

2020-07-04 Thread David Bremner
Floris Bruynooghe  writes:


>> - * This function will not return NULL since Notmuch ensures that every
>> - * message has a unique message ID, (Notmuch will generate an ID for a
>> - * message if the original file does not contain one).
>> + * This function will return NULL if triggers an unhandled Xapian
>> + * exception.

> How much of a departure from the existing API is this?  Will this be
> possible with all functions?  I had a quick look and tried some other
> functions that don't return notmuch_status_t:

It's upward compatible in that any code which crashes because it was not
expecting a NULL pointer, will already be crashing in the same
circumstances because of an uncaught exception / call to abort.

> notmuch_database_get_version currently returns and unsigned int and
> segfaults on use with a closed db.

Yes, the ones without a proper status value are going to be a bit work.

In the next series I just posted [1], I started providing status value
returning version (see notmuch_message_get_flag_st). We've been through
a few of these migrations and it has not been too painful.

> I wonder if a backwards-compatible errno-style API could work,
> notmuch_last_status(notmuch_database_t* database) or so.  This kind of
> thing is probably easy to adopt in bindings but harder for direct users
> of the API.  It's also an extra API call for everything that doesn't
> return notmuch_status_t.  But I'll leave the judgement to you, I'm not
> as experienced with the API.

I think my main objection to this is that there is no out-of-band value
to tell the caller they need to check errno. So basically every call to
to one of the relevant functions would need be followed by a call to
checking the error number. I don't think that's less work than switching
to a new API. Of course it's less work for me, and we already sort of
made that choice with notmuch_database_status_string. In that case it
was a matter of changing the entire API.  Here we're talking about 10
functions, and I'm not sure if they all need to be changed. For example
several of the notmuch_foo_valid functions just check pointers for being
NULL and can't generate I/O or exceptions.

d

[1]: id:20200704151805.3717715-1-da...@tethera.net
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: [RFC PATCH] lib: document new database_open API

2020-07-04 Thread David Bremner
Floris Bruynooghe  writes:

>> + *
>> + * - in the environment variable NOTMUCH_DATABASE, if non-empty
>> + *
>> + * - by $XDG_DATA_HOME/notmuch/$NOTMUCH_PROFILE where XDG_DATA_HOME
>> + *   defaults to "$HOME/.local/share" and NOTMUCH_PROFILE defaults to
>> + *   "default"
>
> I like the profile support, is the plan for
> $XDG_DATA_HOME/notmuch/$NOTMUCH_PROFILE to be written when creating the
> database?

Yes, with "NOTMUCH_PROFILE=default" by default.

> It's nice that the environment variable handling is done in the library,
> should make it more consistent for bindings.  As long as it can be
> overwritten I guess.

Overwritten how? By passing parameters?

> The API is rather complex though, perhaps easier when split across
> several functions?  Maybe a notmuch_database_open_profile(const char
> *profile, notmuch_database_t**) is useful as the simple one which always
> does the right thing when called with NULL for profile.  Not sure what
> other combinations would be needed.

I have no objections to a "do the write thing" wrapper or two. I don't
think that increases maintence cost too much.

d
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: [RFC PATCH] lib: document new database_open API

2020-07-04 Thread Floris Bruynooghe
On Fri 03 Jul 2020 at 10:43 -0300, David Bremner wrote:

> Several aspects of this are potentially controversial:
>
> 1) The use of environment variables as fallback. I understand the
> discomfort with having a library function check the environment, but
> this seems to be functionality people want, and it is better to
> implement it once.
>
> 2) The use of both NULL and "" to do different things for config_path.
>
> 3) The new API is pretty complex, compared to the previous one.
> ---
>
> There is not much code to back this so far. This is just me thinking
> out loud at this point.  The location calculation is done (and also
> easy).  The challenging part is probably updating
> notmuch_database_get_config to do what this comment promises.
>
> I suspect database_create will probably need to be updated to match.
>
> Another question is if we should have an opaque set of options to pass
> to open (in the style notmuch_indexopts_t).  Currently this is just
> future proofing as far as I know.
>
>  lib/notmuch.h | 138 --
>  1 file changed, 111 insertions(+), 27 deletions(-)
>
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index ceb5a018..6a46b80a 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -301,52 +301,136 @@ typedef enum {
>  } notmuch_database_mode_t;
>  
>  /**
> - * Open an existing notmuch database located at 'path'.
> + * Deprecated alias for notmuch_database_open_with_config with
> + * config_path=error_message=NULL
> + * @deprecated Deprecated as of libnotmuch 5.2 (notmuch 0.31)
> + */
> +NOTMUCH_DEPRECATED(5, 2)
> +notmuch_status_t
> +notmuch_database_open (const char *path,
> +notmuch_database_mode_t mode,
> +notmuch_database_t **database);
> +/**
> + * Deprecated alias for notmuch_database_open_with_config with
> + * config_path=NULL
> + *
> + * @deprecated Deprecated as of libnotmuch 5.2 (notmuch 0.31)
> + *
> + */
> +NOTMUCH_DEPRECATED(5, 2)
> +notmuch_status_t
> +notmuch_database_open_verbose (const char *path,
> +notmuch_database_mode_t mode,
> +notmuch_database_t **database,
> +char **error_message);
> +
> +/**
> + * Open an existing notmuch database located at 'database_path', using
> + * configuration in 'config_path'.
> + *
> + * @param[in]database_path
> + * @parblock
> + * Path to existing database.
> + *
> + * A notmuch database is a Xapian database containing appropriate
> + * metadata.
>   *
>   * The database should have been created at some time in the past,
>   * (not necessarily by this process), by calling
> - * notmuch_database_create with 'path'. By default the database should be
> - * opened for reading only. In order to write to the database you need to
> - * pass the NOTMUCH_DATABASE_MODE_READ_WRITE mode.
> + * notmuch_database_create.
> + *
> + * If 'database_path' is NULL, use the location specified
> + *
> + * - in the environment variable NOTMUCH_DATABASE, if non-empty
> + *
> + * - by $XDG_DATA_HOME/notmuch/$NOTMUCH_PROFILE where XDG_DATA_HOME
> + *   defaults to "$HOME/.local/share" and NOTMUCH_PROFILE defaults to
> + *   "default"

I like the profile support, is the plan for
$XDG_DATA_HOME/notmuch/$NOTMUCH_PROFILE to be written when creating the
database?

> + *
> + * If 'database_path' is non-NULL, but does not appear to be a Xapian
> + * database, check for a directory '.notmuch/xapian' below
> + * 'database_path'.
> + *
> + * @endparblock
> + * @param[in]mode
> + * @parblock
> + * Mode to open database
> + *
> + * By default the database will be opened for reading only. In order
> + * to write to the database you need to pass the
> + * #NOTMUCH_DATABASE_MODE_READ_WRITE mode.
> + *
> + * @endparblock
> + * @param[in]  config_path
> + * @parblock
> + * Path to config file.
> + *
> + * Config file is key-value, with mandatory sections. See
> + * notmuch-config(5) for more information. The key-value pair
> + * overrides the corresponding configuration data stored in the
> + * database (see notmuch_database_get_config)
>   *
> - * An existing notmuch database can be identified by the presence of a
> - * directory named ".notmuch" below 'path'.
> + * If config_path is NULL use the path specified
> + *
> + * - in environment variable NOTMUCH_CONFIG, if non-empty
> + *
> + * - by  XDG_CONFIG_HOME/notmuch/ where
> + *   XDG_CONFIG_HOME defaults to "$HOME/.config".
> + *
> + * - by $HOME/.notmuch-config
> + *
> + * If config_path is "" (empty string) then do not
> + * open any configuration file.
> + * @endparblock
> + * @param[in] profile:
> + * @parblock
> + * Name of profile (configuration/database variant).
> + *
> + * If non-NULL, append to the directory / file path determined by
> + * config_path.
> + *
> + * If NULL then use
> + * - environment variable NOTMUCH_PROFILE if defined,
> + * - otherwise "default" for directories and "" (empty string) for paths.
> + *
> + * 

Re: [PATCH 2/4] lib: catch error from closed db in n_m_get_message_id

2020-07-04 Thread Floris Bruynooghe
Nice.

On Mon 29 Jun 2020 at 22:14 -0300, David Bremner wrote:
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index ceb5a018..0dc89547 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -1363,9 +1363,8 @@ notmuch_message_get_database (const notmuch_message_t 
> *message);
>   * message is valid, (which is until the query from which it derived
>   * is destroyed).
>   *
> - * This function will not return NULL since Notmuch ensures that every
> - * message has a unique message ID, (Notmuch will generate an ID for a
> - * message if the original file does not contain one).
> + * This function will return NULL if triggers an unhandled Xapian
> + * exception.
>   */
>  const char *
>  notmuch_message_get_message_id (notmuch_message_t *message);

How much of a departure from the existing API is this?  Will this be
possible with all functions?  I had a quick look and tried some other
functions that don't return notmuch_status_t:

notmuch_database_get_version currently returns and unsigned int and
segfaults on use with a closed db.

notmuch_needs_upgrade returns notmuch_bool_t and seems to return a valid
bool with a closed db.  I'm not sure if this is always the right answer
though.

I wonder if a backwards-compatible errno-style API could work,
notmuch_last_status(notmuch_database_t* database) or so.  This kind of
thing is probably easy to adopt in bindings but harder for direct users
of the API.  It's also an extra API call for everything that doesn't
return notmuch_status_t.  But I'll leave the judgement to you, I'm not
as experienced with the API.
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


[PATCH 01/11] test: remove unused backup_database calls

2020-07-04 Thread David Bremner
Since these backups are never restored, they should be safe to remove.
---
 test/T560-lib-error.sh | 2 --
 1 file changed, 2 deletions(-)

diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index 81500536..5af3eab2 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -341,7 +341,6 @@ int main (int argc, char** argv)
EXPECT0(notmuch_database_close (db));
 EOF
 
-backup_database
 test_begin_subtest "Handle getting message-id from closed database"
 cat c_head2 - c_tail <<'EOF' | test_C ${MAIL_DIR}
 {
@@ -358,7 +357,6 @@ cat < EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
-backup_database
 test_begin_subtest "Handle getting thread-id from closed database"
 cat c_head2 - c_tail <<'EOF' | test_C ${MAIL_DIR}
 {
-- 
2.27.0
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


[PATCH 07/11] lib/n_m_g_filename: catch Xapian exceptions, document NULL return

2020-07-04 Thread David Bremner
This is the same machinery as applied for

 notmuch_message_get_{thread,message}_id
---
 lib/message.cc | 7 ++-
 lib/notmuch.h  | 2 ++
 test/T560-lib-error.sh | 1 -
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index 0551a427..1a9eaffe 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1122,7 +1122,12 @@ _notmuch_message_ensure_filename_list (notmuch_message_t 
*message)
 const char *
 notmuch_message_get_filename (notmuch_message_t *message)
 {
-_notmuch_message_ensure_filename_list (message);
+try {
+   _notmuch_message_ensure_filename_list (message);
+} catch (Xapian::Error ) {
+   LOG_XAPIAN_EXCEPTION (message, error);
+   return NULL;
+}
 
 if (message->filename_list == NULL)
return NULL;
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 037913f4..5c17ec7c 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1433,6 +1433,8 @@ notmuch_message_count_files (notmuch_message_t *message);
  * this function will arbitrarily return a single one of those
  * filenames. See notmuch_message_get_filenames for returning the
  * complete list of filenames.
+ *
+ * This function returns NULL if it triggers a Xapian exception.
  */
 const char *
 notmuch_message_get_filename (notmuch_message_t *message);
diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index 7555197f..57c74a2b 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -406,7 +406,6 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "Handle getting message filename from closed database"
-test_subtest_known_broken
 cat c_head2 - c_tail <<'EOF' | test_C ${MAIL_DIR}
 {
 const char *filename;
-- 
2.27.0
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


[PATCH 04/11] test: add regression test for n_m_get_header

2020-07-04 Thread David Bremner
This function already catches Xapian exceptions, and we want to make
sure it stays that way.
---
 test/T560-lib-error.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index 6432faaa..777eb375 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -371,4 +371,20 @@ cat < EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "Handle getting header from closed database"
+cat c_head2 - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+const char *from;
+from=notmuch_message_get_header (message, "from");
+printf("%s\n%d\n", id, from == NULL);
+}
+EOF
+cat < EXPECTED
+== stdout ==
+1258471718-6781-1-git-send-email-dotted...@dottedmag.net
+1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.27.0
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Second batch of API cleanup for exception safety.

2020-07-04 Thread David Bremner
This is going to be a bit of a slog, so I'm going to send the patches
in batches, rather than waiting until I have one giant series.

I'm interested in feedback on the last two patches in particular
before I adjust notmuch to use the new API itself. An alternative
approach would be treat being a ghost_message as a seperate property
(and leave the old notmuch_bool_t API for the other flags). That seems
harder to migrate to cleanly, but I'm open to ideas.

___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


[PATCH 08/11] test: add known broken test for n_m_get_filenames

2020-07-04 Thread David Bremner
This will be fixed in the next commit
---
 test/T560-lib-error.sh | 17 +
 1 file changed, 17 insertions(+)

diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index 57c74a2b..8d9b4cc0 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -421,4 +421,21 @@ cat < EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "Handle getting all message filenames from closed database"
+test_subtest_known_broken
+cat c_head2 - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+notmuch_filenames_t *filenames;
+filenames = notmuch_message_get_filenames (message);
+printf("%d\n%d\n", message != NULL, filenames == NULL);
+}
+EOF
+cat < EXPECTED
+== stdout ==
+1
+1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.27.0
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


[PATCH 10/11] test: add known broken for n_m_get_flag on closed db

2020-07-04 Thread David Bremner
Exception caught in next commit. Note that FLAG_GHOST is the only one
that triggers the I/O code path.
---
 test/T560-lib-error.sh | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index ce3526a4..68aadcdb 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -437,4 +437,22 @@ cat < EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "Handle getting ghost flag from closed database"
+test_subtest_known_broken
+cat c_head2 - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+notmuch_bool_t result;
+result = notmuch_message_get_flag (message, 
NOTMUCH_MESSAGE_FLAG_GHOST);
+printf("%d\n%d\n", message != NULL, result == FALSE);
+}
+EOF
+cat < EXPECTED
+== stdout ==
+1
+1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+
 test_done
-- 
2.27.0
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


[PATCH 06/11] lib: add known broken test for notmuch_message_get_filename

2020-07-04 Thread David Bremner
This will be fixed in the next commit
---
 test/T560-lib-error.sh | 17 +
 1 file changed, 17 insertions(+)

diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index d7caed5a..7555197f 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -405,4 +405,21 @@ cat < EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "Handle getting message filename from closed database"
+test_subtest_known_broken
+cat c_head2 - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+const char *filename;
+filename = notmuch_message_get_filename (message);
+printf("%d\n%d\n", message != NULL, filename == NULL);
+}
+EOF
+cat < EXPECTED
+== stdout ==
+1
+1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.27.0
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


[PATCH 09/11] lib: catch exceptions in n_m_get_filenames

2020-07-04 Thread David Bremner
This is essentially copied from the change to notmuch_message_get_filename
---
 lib/message.cc | 7 ++-
 lib/notmuch.h  | 2 ++
 test/T560-lib-error.sh | 1 -
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index 1a9eaffe..8e43a393 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1143,7 +1143,12 @@ notmuch_message_get_filename (notmuch_message_t *message)
 notmuch_filenames_t *
 notmuch_message_get_filenames (notmuch_message_t *message)
 {
-_notmuch_message_ensure_filename_list (message);
+try {
+   _notmuch_message_ensure_filename_list (message);
+} catch (Xapian::Error ) {
+   LOG_XAPIAN_EXCEPTION (message, error);
+   return NULL;
+}
 
 return _notmuch_filenames_create (message, message->filename_list);
 }
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 5c17ec7c..e544aafd 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1448,6 +1448,8 @@ notmuch_message_get_filename (notmuch_message_t *message);
  *
  * Each filename in the iterator is an absolute filename, (the initial
  * component will match notmuch_database_get_path() ).
+ *
+ * This function returns NULL if it triggers a Xapian exception.
  */
 notmuch_filenames_t *
 notmuch_message_get_filenames (notmuch_message_t *message);
diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index 8d9b4cc0..ce3526a4 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -422,7 +422,6 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "Handle getting all message filenames from closed database"
-test_subtest_known_broken
 cat c_head2 - c_tail <<'EOF' | test_C ${MAIL_DIR}
 {
 notmuch_filenames_t *filenames;
-- 
2.27.0
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


[PATCH 11/11] lib: catch exceptions in n_m_get_flag, provide n_m_get_flag_st

2020-07-04 Thread David Bremner
It's not very nice to return FALSE for an error, so provide
notmuch_message_get_flag_st as a migration path.
---
 lib/message.cc | 33 +
 lib/notmuch.h  | 25 +
 test/T560-lib-error.sh | 18 +-
 3 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index 8e43a393..81278d5e 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1161,15 +1161,40 @@ notmuch_message_count_files (notmuch_message_t *message)
 return _notmuch_string_list_length (message->filename_list);
 }
 
+notmuch_status_t
+notmuch_message_get_flag_st (notmuch_message_t *message,
+notmuch_message_flag_t flag,
+notmuch_bool_t *is_set)
+{
+if (! is_set)
+   return NOTMUCH_STATUS_NULL_POINTER;
+
+try {
+   if (flag == NOTMUCH_MESSAGE_FLAG_GHOST &&
+   ! NOTMUCH_TEST_BIT (message->lazy_flags, flag))
+   _notmuch_message_ensure_metadata (message, NULL);
+} catch (Xapian::Error ) {
+   LOG_XAPIAN_EXCEPTION (message, error);
+   return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
+}
+
+*is_set = NOTMUCH_TEST_BIT (message->flags, flag);
+return NOTMUCH_STATUS_SUCCESS;
+}
+
 notmuch_bool_t
 notmuch_message_get_flag (notmuch_message_t *message,
  notmuch_message_flag_t flag)
 {
-if (flag == NOTMUCH_MESSAGE_FLAG_GHOST &&
-   ! NOTMUCH_TEST_BIT (message->lazy_flags, flag))
-   _notmuch_message_ensure_metadata (message, NULL);
+notmuch_bool_t is_set;
+notmuch_status_t status;
+
+status = notmuch_message_get_flag_st (message, flag, _set);
 
-return NOTMUCH_TEST_BIT (message->flags, flag);
+if (status)
+   return FALSE;
+else
+   return is_set;
 }
 
 void
diff --git a/lib/notmuch.h b/lib/notmuch.h
index e544aafd..c7b21060 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1485,11 +1485,36 @@ typedef enum _notmuch_message_flag {
 
 /**
  * Get a value of a flag for the email corresponding to 'message'.
+ *
+ * returns FALSE in case of errors.
+ *
+ * @deprecated Deprecated as of libnotmuch 5.2 (notmuch 0.31). Please
+ * use notmuch_message_get_flag_st instead.
  */
+NOTMUCH_DEPRECATED(5,2)
 notmuch_bool_t
 notmuch_message_get_flag (notmuch_message_t *message,
  notmuch_message_flag_t flag);
 
+/**
+ * Get a value of a flag for the email corresponding to 'message'.
+ *
+ * @param message a message object
+ * @param flag flag to check
+ * @param is_set pointer to boolean to store flag value.
+ *
+ * @retval #NOTMUCH_STATUS_SUCCESS
+ * @retval #NOTMUCH_STATUS_NULL_POINTER is_set is NULL
+ * @retval #NOTMUCH_STATUS_XAPIAN_EXCEPTION Accessing the database
+ * triggered an exception.
+ *
+ * @since libnotmuch 5.2 (notmuch 0.31)
+ */
+notmuch_status_t
+notmuch_message_get_flag_st (notmuch_message_t *message,
+notmuch_message_flag_t flag,
+notmuch_bool_t *is_set);
+
 /**
  * Set a value of a flag for the email corresponding to 'message'.
  */
diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index 68aadcdb..06b43ef2 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -438,7 +438,6 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "Handle getting ghost flag from closed database"
-test_subtest_known_broken
 cat c_head2 - c_tail <<'EOF' | test_C ${MAIL_DIR}
 {
 notmuch_bool_t result;
@@ -454,5 +453,22 @@ cat < EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "Handle getting ghost flag from closed database (new API)"
+cat c_head2 - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+notmuch_bool_t result;
+notmuch_status_t status;
+status = notmuch_message_get_flag_st (message, 
NOTMUCH_MESSAGE_FLAG_GHOST, );
+printf("%d\n%d\n", message != NULL, status == 
NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+}
+EOF
+cat < EXPECTED
+== stdout ==
+1
+1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 
 test_done
-- 
2.27.0
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


[PATCH 02/11] test: drop use of assert in closed db tests

2020-07-04 Thread David Bremner
Instead of printing the same static string for each test, can replace
the assert with something simpler (or at least easier to integrate
into the test suite).
---
 test/T560-lib-error.sh | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index 5af3eab2..6432faaa 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -322,7 +322,6 @@ cat < c_head2
 #include 
 #include 
 #include 
-#include 
 int main (int argc, char** argv)
 {
notmuch_database_t *db;
@@ -337,7 +336,6 @@ int main (int argc, char** argv)
  exit (1);
}
EXPECT0(notmuch_database_find_message (db, id, ));
-   assert(message != NULL);
EXPECT0(notmuch_database_close (db));
 EOF
 
@@ -346,12 +344,12 @@ cat c_head2 - c_tail <<'EOF' | test_C ${MAIL_DIR}
 {
 const char *id2;
 id2=notmuch_message_get_message_id (message);
-printf("%s\n%d\n", id, id2==NULL);
+printf("%d\n%d\n", message != NULL, id2==NULL);
 }
 EOF
 cat < EXPECTED
 == stdout ==
-1258471718-6781-1-git-send-email-dotted...@dottedmag.net
+1
 1
 == stderr ==
 EOF
@@ -362,12 +360,12 @@ cat c_head2 - c_tail <<'EOF' | test_C ${MAIL_DIR}
 {
 const char *id2;
 id2=notmuch_message_get_thread_id (message);
-printf("%s\n%d\n", id, id2==NULL);
+printf("%d\n%d\n", message != NULL, id2==NULL);
 }
 EOF
 cat < EXPECTED
 == stdout ==
-1258471718-6781-1-git-send-email-dotted...@dottedmag.net
+1
 1
 == stderr ==
 EOF
-- 
2.27.0
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


[PATCH 03/11] lib/message: use LOG_XAPIAN_EXCEPTION in n_m_get_header

2020-07-04 Thread David Bremner
This is just for consistency, and a small reduction in the amount of
boilerplate.
---
 lib/message.cc | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index 3ca7b902..0551a427 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -572,9 +572,7 @@ notmuch_message_get_header (notmuch_message_t *message, 
const char *header)
return talloc_strdup (message, value.c_str ());
 
} catch (Xapian::Error ) {
-   _notmuch_database_log (notmuch_message_get_database (message), "A 
Xapian exception occurred when reading header: %s\n",
-  error.get_msg ().c_str ());
-   message->notmuch->exception_reported = true;
+   LOG_XAPIAN_EXCEPTION (message, error);
return NULL;
}
 }
-- 
2.27.0
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


[PATCH 05/11] lib/n_m_get_replies: doc return, initial regression test

2020-07-04 Thread David Bremner
We need to to set a query and retrieve the threads to meaningfully
test this function.
---
 lib/notmuch.h  |  5 -
 test/T560-lib-error.sh | 18 ++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 0dc89547..037913f4 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1402,7 +1402,10 @@ notmuch_message_get_thread_id (notmuch_message_t 
*message);
  * NULL. (Note that notmuch_messages_valid will accept that NULL
  * value as legitimate, and simply return FALSE for it.)
  *
- * The returned list will be destroyed when the thread is destroyed.
+ * This function also returns NULL if it triggers a Xapian exception.
+ *
+ * The returned list will be destroyed when the thread is
+ * destroyed.
  */
 notmuch_messages_t *
 notmuch_message_get_replies (notmuch_message_t *message);
diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index 777eb375..d7caed5a 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -387,4 +387,22 @@ cat < EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+# XXX TODO: test on a message from notmuch_thread_get_toplevel_messages
+# XXX this test only tests the trivial code path
+test_begin_subtest "Handle getting replies from closed database"
+cat c_head2 - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+notmuch_messages_t *replies;
+replies = notmuch_message_get_replies (message);
+printf("%d\n%d\n", message != NULL, replies==NULL);
+}
+EOF
+cat < EXPECTED
+== stdout ==
+1
+1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.27.0
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: [PATCH] emacs: Add notmuch-show-local-dates option

2020-07-04 Thread Tomi Ollila
On Fri, Jul 03 2020, David Bremner wrote:

> Daniel Kahn Gillmor  writes:
>
>> But if the sender is in TZ=Europe/Berlin, i would see:
>>
>> Date: Fri, 03 Jul 2020 13:22:36 -0400 [Fri, 03 Jul 2020 19:22:36 +0200]
>>
>> (Note that RFC 5322 Date format shows the hour offset, but not the
>> actual TZ -- i can't tell from -0400 whether someone is in
>> TZ=America/New_York or TZ=America/Manaus)
>>
>> Is there anyone who would complain about this just being the default
>> behavior -- with no additional settings to change?
>
> The bikeshed must be blue! Uh, I mean what about narrowish screens (80
> columns or so) and or deeply indented threads?

After very casual read-thru we have had similar discussion in thread starting

id:1427132722-20346-1-git-send-email-tomi.oll...@iki.fi

https://nmbug.notmuchmail.org/nmweb/search/id%3A1427132722-20346-1-git-send-email-tomi.ollila%40iki.fi

I've been patching my notmuch-emacs interface with that ever since, using 

https://github.com/domo141/nottoomuch/blob/master/build/01-date-local.patch

The Date header from David's email looked to me as

Date: Fri, 03 Jul 2020 19:58:23 -0300  (Sat, 04 Jul 2020 01:58:23 +0300)

(and fits fine in 80-column terminal window ;D -- 72 chars used -- when
 indent or replies with > goes too deep, everything wraps (or gets cut)...) 

Anyway, there are good comments in that thread -- and every now and then
I've tried to figure out a bit different way to "solve" this -- but as
my workaround work I've usually stopped short on that.

For me the interests are to know what time the user sent the email (their
timezone) and how that relates the time the other emails I've received
(my timezone) -- not too much the (exact) location sender is...

>
> d

Tomi
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org