unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* batch 6, API exception handling cleanup.
@ 2020-07-18 15:07 David Bremner
  2020-07-18 15:07 ` [PATCH 01/10] test: regression test for closing a closed database David Bremner
                   ` (10 more replies)
  0 siblings, 11 replies; 14+ messages in thread
From: David Bremner @ 2020-07-18 15:07 UTC (permalink / raw)
  To: notmuch

These follow the series at

      id:20200714224119.717845-1-david@tethera.net

Probably most interesting is 

     [PATCH 10/10] test: add known broken test for indexing relative path

That test is also part of the bug report
          
     id:87sgdqo0rz.fsf@tethera.net

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

* [PATCH 01/10] test: regression test for closing a closed database
  2020-07-18 15:07 batch 6, API exception handling cleanup David Bremner
@ 2020-07-18 15:07 ` David Bremner
  2020-07-18 15:07 ` [PATCH 02/10] test: add regression tests for notmuch database destroy David Bremner
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: David Bremner @ 2020-07-18 15:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

This does not currently throw an error, and it should stay that way.
---
 test/T562-lib-database.sh | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index c9705b13..0dae0096 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -83,4 +83,19 @@ cat <<EOF > EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "re-close a closed db"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        EXPECT0(notmuch_database_close (db));
+        stat = notmuch_database_close (db);
+        printf ("%d\n", stat);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+0
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.27.0

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

* [PATCH 02/10] test: add regression tests for notmuch database destroy
  2020-07-18 15:07 batch 6, API exception handling cleanup David Bremner
  2020-07-18 15:07 ` [PATCH 01/10] test: regression test for closing a closed database David Bremner
@ 2020-07-18 15:07 ` David Bremner
  2020-07-18 15:07 ` [PATCH 03/10] test: add known broken test for n_d_needs_upgrade David Bremner
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: David Bremner @ 2020-07-18 15:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Either an open or closed database should be ok to destroy
---
 test/T562-lib-database.sh | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index 0dae0096..db7a34f3 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -98,4 +98,35 @@ cat <<EOF > EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "destroy a closed db"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        unsigned int version;
+        EXPECT0(notmuch_database_close (db));
+        stat = notmuch_database_destroy (db);
+        printf ("%d\n", stat);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+0
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "destroy an open db"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        unsigned int version;
+        stat = notmuch_database_destroy (db);
+        printf ("%d\n", stat);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+0
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.27.0

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

* [PATCH 03/10] test: add known broken test for n_d_needs_upgrade
  2020-07-18 15:07 batch 6, API exception handling cleanup David Bremner
  2020-07-18 15:07 ` [PATCH 01/10] test: regression test for closing a closed database David Bremner
  2020-07-18 15:07 ` [PATCH 02/10] test: add regression tests for notmuch database destroy David Bremner
@ 2020-07-18 15:07 ` David Bremner
  2020-07-18 15:07 ` [PATCH 04/10] lib/n_d_needs_upgrade: handle error return from n_d_get_version David Bremner
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: David Bremner @ 2020-07-18 15:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

It's a bit arbitrary which value to return for errors, but the same
argument as for read only databases applies for errors.
---
 test/T562-lib-database.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index db7a34f3..0af7587a 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -129,4 +129,22 @@ cat <<EOF > EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "check a closed db for upgrade"
+test_subtest_known_broken
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        notmuch_bool_t ret;
+
+        EXPECT0(notmuch_database_close (db));
+        ret = notmuch_database_needs_upgrade (db);
+        printf ("%d\n", ret == FALSE);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.27.0

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

* [PATCH 04/10] lib/n_d_needs_upgrade: handle error return from n_d_get_version
  2020-07-18 15:07 batch 6, API exception handling cleanup David Bremner
                   ` (2 preceding siblings ...)
  2020-07-18 15:07 ` [PATCH 03/10] test: add known broken test for n_d_needs_upgrade David Bremner
@ 2020-07-18 15:07 ` David Bremner
  2020-07-18 15:07 ` [PATCH 05/10] test: regression test for n_d_upgrade David Bremner
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: David Bremner @ 2020-07-18 15:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Also clarify documentation of error return from n_d_needs_upgrade.
---
 lib/database.cc           | 14 +++++++++++---
 lib/notmuch.h             |  2 ++
 test/T562-lib-database.sh |  1 -
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 27861970..25d34253 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1395,9 +1395,17 @@ notmuch_database_get_version (notmuch_database_t *notmuch)
 notmuch_bool_t
 notmuch_database_needs_upgrade (notmuch_database_t *notmuch)
 {
-    return notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE &&
-	   ((NOTMUCH_FEATURES_CURRENT & ~notmuch->features) ||
-	    (notmuch_database_get_version (notmuch) < NOTMUCH_DATABASE_VERSION));
+    unsigned int version;
+
+    if (notmuch->mode != NOTMUCH_DATABASE_MODE_READ_WRITE)
+	return FALSE;
+
+    if (NOTMUCH_FEATURES_CURRENT & ~notmuch->features)
+	return TRUE;
+
+    version = notmuch_database_get_version (notmuch);
+
+    return (version > 0 && version < NOTMUCH_DATABASE_VERSION);
 }
 
 static volatile sig_atomic_t do_progress_notify = 0;
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 45ecc59b..b8b89a4a 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -447,6 +447,8 @@ notmuch_database_get_version (notmuch_database_t *database);
  * FALSE for a read-only database because there's no way to upgrade a
  * read-only database.
  *
+ * Also returns FALSE if an error occurs accessing the database.
+ *
  */
 notmuch_bool_t
 notmuch_database_needs_upgrade (notmuch_database_t *database);
diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index 0af7587a..4bcbc805 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -130,7 +130,6 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "check a closed db for upgrade"
-test_subtest_known_broken
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
         notmuch_bool_t ret;
-- 
2.27.0

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

* [PATCH 05/10] test: regression test for n_d_upgrade
  2020-07-18 15:07 batch 6, API exception handling cleanup David Bremner
                   ` (3 preceding siblings ...)
  2020-07-18 15:07 ` [PATCH 04/10] lib/n_d_needs_upgrade: handle error return from n_d_get_version David Bremner
@ 2020-07-18 15:07 ` David Bremner
  2020-07-18 15:07 ` [PATCH 06/10] test: add regression test for n_d_{begin,end}_atomic David Bremner
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: David Bremner @ 2020-07-18 15:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

The logic is that if it's acceptable to return SUCCESS for read only
database, it's acceptable for a closed one.
---
 test/T562-lib-database.sh | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index 4bcbc805..d975f109 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -146,4 +146,21 @@ cat <<EOF > EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "upgrade a closed db"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        notmuch_bool_t ret;
+
+        EXPECT0(notmuch_database_close (db));
+        stat = notmuch_database_upgrade (db, NULL, NULL);
+        printf ("%d\n", ret == NOTMUCH_STATUS_SUCCESS);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.27.0

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

* [PATCH 06/10] test: add regression test for n_d_{begin,end}_atomic
  2020-07-18 15:07 batch 6, API exception handling cleanup David Bremner
                   ` (4 preceding siblings ...)
  2020-07-18 15:07 ` [PATCH 05/10] test: regression test for n_d_upgrade David Bremner
@ 2020-07-18 15:07 ` David Bremner
  2020-07-18 15:07 ` [PATCH 07/10] test: regression test for n_d_get_revision David Bremner
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: David Bremner @ 2020-07-18 15:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Xapian currently succeeds to begin/end a transaction on a closed database,
or at least does not throw an exception. Make the test robust against
this changing.
---
 test/T562-lib-database.sh | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index d975f109..94e0e06a 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -163,4 +163,37 @@ cat <<EOF > EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "begin atomic section for a closed db"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        EXPECT0(notmuch_database_close (db));
+        stat = notmuch_database_begin_atomic (db);
+        printf ("%d\n", stat == NOTMUCH_STATUS_SUCCESS ||
+                        stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "end atomic section for a closed db"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        EXPECT0(notmuch_database_close (db));
+        EXPECT0(notmuch_database_begin_atomic (db));
+        stat = notmuch_database_end_atomic (db);
+        printf ("%d\n", stat == NOTMUCH_STATUS_SUCCESS ||
+                        stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.27.0

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

* [PATCH 07/10] test: regression test for n_d_get_revision
  2020-07-18 15:07 batch 6, API exception handling cleanup David Bremner
                   ` (5 preceding siblings ...)
  2020-07-18 15:07 ` [PATCH 06/10] test: add regression test for n_d_{begin,end}_atomic David Bremner
@ 2020-07-18 15:07 ` David Bremner
  2020-07-18 15:07 ` [PATCH 08/10] test: add regression test for n_d_get_directory David Bremner
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: David Bremner @ 2020-07-18 15:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

This function only accesses data cached by notmuch, so being closed is
not a problem.
---
 test/T562-lib-database.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index 94e0e06a..e9f00726 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -196,4 +196,22 @@ cat <<EOF > EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "get revision for a closed db"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        const char *uuid;
+        unsigned long rev;
+
+        EXPECT0(notmuch_database_close (db));
+        rev = notmuch_database_get_revision (db, &uuid);
+        printf ("%d\n", rev, uuid);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+53
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.27.0

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

* [PATCH 08/10] test: add regression test for n_d_get_directory
  2020-07-18 15:07 batch 6, API exception handling cleanup David Bremner
                   ` (6 preceding siblings ...)
  2020-07-18 15:07 ` [PATCH 07/10] test: regression test for n_d_get_revision David Bremner
@ 2020-07-18 15:07 ` David Bremner
  2020-07-18 15:07 ` [PATCH 09/10] test: regression test for n_d_index_file closed db David Bremner
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: David Bremner @ 2020-07-18 15:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

At least this exception is already handled correctly.
---
 test/T562-lib-database.sh | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index e9f00726..06569291 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -214,4 +214,21 @@ cat <<EOF > EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "get directory for a closed db"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        notmuch_directory_t *dir;
+        EXPECT0(notmuch_database_close (db));
+        stat = notmuch_database_get_directory (db, "/nonexistent", &dir);
+        printf ("%d\n", stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+A Xapian exception occurred creating a directory: Database has been closed.
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.27.0

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

* [PATCH 09/10] test: regression test for n_d_index_file closed db
  2020-07-18 15:07 batch 6, API exception handling cleanup David Bremner
                   ` (7 preceding siblings ...)
  2020-07-18 15:07 ` [PATCH 08/10] test: add regression test for n_d_get_directory David Bremner
@ 2020-07-18 15:07 ` David Bremner
  2020-07-18 15:07 ` [PATCH 10/10] test: add known broken test for indexing relative path David Bremner
  2020-07-23 10:51 ` batch 6, API exception handling cleanup David Bremner
  10 siblings, 0 replies; 14+ messages in thread
From: David Bremner @ 2020-07-18 15:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Current exception handling seems OK, at least for this case.
---
 test/T562-lib-database.sh | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index 06569291..6a51002a 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -12,6 +12,7 @@ cat <<EOF > c_head
 #include <stdio.h>
 #include <notmuch.h>
 #include <notmuch-test.h>
+#include <talloc.h>
 int main (int argc, char** argv)
 {
    notmuch_database_t *db;
@@ -231,4 +232,23 @@ A Xapian exception occurred creating a directory: Database has been closed.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+# XXX TODO: test with relative path
+test_begin_subtest "index file with a closed db"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        notmuch_message_t *msg;
+        const char *path = talloc_asprintf(db, "%s/01:2,", argv[1]);
+        EXPECT0(notmuch_database_close (db));
+        stat = notmuch_database_index_file (db, path, NULL, &msg);
+        printf ("%d\n", stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+A Xapian exception occurred finding message: Database has been closed.
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.27.0

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

* [PATCH 10/10] test: add known broken test for indexing relative path
  2020-07-18 15:07 batch 6, API exception handling cleanup David Bremner
                   ` (8 preceding siblings ...)
  2020-07-18 15:07 ` [PATCH 09/10] test: regression test for n_d_index_file closed db David Bremner
@ 2020-07-18 15:07 ` David Bremner
  2020-07-19 10:05   ` [PATCH] lib: convert relative filenames to absolute in n_d_index_file David Bremner
  2020-07-25 11:17   ` [PATCH 10/10] test: add known broken test for indexing relative path David Bremner
  2020-07-23 10:51 ` batch 6, API exception handling cleanup David Bremner
  10 siblings, 2 replies; 14+ messages in thread
From: David Bremner @ 2020-07-18 15:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

The current code seems to look for the path relative to the current
working directory, rather than the mail store root.
---
 test/T562-lib-database.sh | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index 6a51002a..c9d7c748 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -251,4 +251,21 @@ A Xapian exception occurred finding message: Database has been closed.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+generate_message '[filename]=relative_path'
+test_subtest_known_broken
+test_begin_subtest "index file (relative path)"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        notmuch_message_t *msg;
+        stat = notmuch_database_index_file (db, "relative_path", NULL, &msg);
+        printf ("%d\n", stat == NOTMUCH_STATUS_SUCCESS);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.27.0

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

* [PATCH] lib: convert relative filenames to absolute in n_d_index_file
  2020-07-18 15:07 ` [PATCH 10/10] test: add known broken test for indexing relative path David Bremner
@ 2020-07-19 10:05   ` David Bremner
  2020-07-25 11:17   ` [PATCH 10/10] test: add known broken test for indexing relative path David Bremner
  1 sibling, 0 replies; 14+ messages in thread
From: David Bremner @ 2020-07-19 10:05 UTC (permalink / raw)
  To: David Bremner, notmuch

The API docs promise to handle relative filenames, but the code did
not do it.

Also check for files outside the mail root, as implied by the API
description.

This fixes the bug reported at

     id:87sgdqo0rz.fsf@tethera.net
---
 lib/message-file.c        | 24 ++++++++++++++++++++----
 test/T562-lib-database.sh | 18 +++++++++++++++++-
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/lib/message-file.c b/lib/message-file.c
index e1db26fb..311bd478 100644
--- a/lib/message-file.c
+++ b/lib/message-file.c
@@ -64,21 +64,37 @@ _notmuch_message_file_open_ctx (notmuch_database_t *notmuch,
     if (unlikely (message == NULL))
 	return NULL;
 
-    message->filename = talloc_strdup (message, filename);
+    const char *prefix = notmuch_database_get_path (notmuch);
+    if (prefix == NULL)
+	goto FAIL;
+
+    if (*filename == '/') {
+	if (strncmp (filename, prefix, strlen(prefix)) != 0) {
+	    _notmuch_database_log (notmuch, "Error opening %s: path outside mail root\n",
+				   filename);
+	    errno = 0;
+	    goto FAIL;
+	}
+	message->filename = talloc_strdup (message, filename);
+    } else {
+	message->filename = talloc_asprintf(message, "%s/%s", prefix, filename);
+    }
+
     if (message->filename == NULL)
 	goto FAIL;
 
     talloc_set_destructor (message, _notmuch_message_file_destructor);
 
-    message->stream = g_mime_stream_gzfile_open (filename);
+    message->stream = g_mime_stream_gzfile_open (message->filename);
     if (message->stream == NULL)
 	goto FAIL;
 
     return message;
 
   FAIL:
-    _notmuch_database_log (notmuch, "Error opening %s: %s\n",
-			   filename, strerror (errno));
+    if (errno)
+	_notmuch_database_log (notmuch, "Error opening %s: %s\n",
+			       filename, strerror (errno));
     _notmuch_message_file_close (message);
 
     return NULL;
diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index 4e225ccc..301eeb73 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -253,7 +253,6 @@ test_expect_equal_file EXPECTED OUTPUT
 
 generate_message '[filename]=relative_path'
 test_begin_subtest "index file (relative path)"
-test_subtest_known_broken
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
         notmuch_message_t *msg;
@@ -268,4 +267,21 @@ cat <<EOF > EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "index file (absolute path outside mail root)"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        notmuch_message_t *msg;
+        stat = notmuch_database_index_file (db, "/dev/zero", NULL, &msg);
+        printf ("%d\n", stat == NOTMUCH_STATUS_FILE_ERROR);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+Error opening /dev/zero: path outside mail root
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+
 test_done
-- 
2.27.0

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

* Re: batch 6, API exception handling cleanup.
  2020-07-18 15:07 batch 6, API exception handling cleanup David Bremner
                   ` (9 preceding siblings ...)
  2020-07-18 15:07 ` [PATCH 10/10] test: add known broken test for indexing relative path David Bremner
@ 2020-07-23 10:51 ` David Bremner
  10 siblings, 0 replies; 14+ messages in thread
From: David Bremner @ 2020-07-23 10:51 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

> These follow the series at
>
>       id:20200714224119.717845-1-david@tethera.net
>
> Probably most interesting is 
>
>      [PATCH 10/10] test: add known broken test for indexing relative path
>
> That test is also part of the bug report
>           
>      id:87sgdqo0rz.fsf@tethera.net

Queued, with some minor updates:

diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index d2749a80..e64f0f12 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -140,12 +140,14 @@ cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
         EXPECT0(notmuch_database_close (db));
         ret = notmuch_database_needs_upgrade (db);
         printf ("%d\n", ret == FALSE);
+        stat = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
     }
 EOF
 cat <<EOF > EXPECTED
 == stdout ==
 1
 == stderr ==
+A Xapian exception occurred at lib/database.cc:XXX: Database has been closed
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
@@ -173,6 +175,7 @@ cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
         stat = notmuch_database_begin_atomic (db);
         printf ("%d\n", stat == NOTMUCH_STATUS_SUCCESS ||
                         stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+        stat = NOTMUCH_STATUS_SUCCESS;
     }
 EOF
 cat <<EOF > EXPECTED
@@ -190,6 +193,7 @@ cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
         stat = notmuch_database_end_atomic (db);
         printf ("%d\n", stat == NOTMUCH_STATUS_SUCCESS ||
                         stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+        stat = NOTMUCH_STATUS_SUCCESS;
     }
 EOF
 cat <<EOF > EXPECTED
@@ -234,7 +238,6 @@ A Xapian exception occurred creating a directory: Database has been closed.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
-# XXX TODO: test with relative path
 test_begin_subtest "index file with a closed db"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {

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

* Re: [PATCH 10/10] test: add known broken test for indexing relative path
  2020-07-18 15:07 ` [PATCH 10/10] test: add known broken test for indexing relative path David Bremner
  2020-07-19 10:05   ` [PATCH] lib: convert relative filenames to absolute in n_d_index_file David Bremner
@ 2020-07-25 11:17   ` David Bremner
  1 sibling, 0 replies; 14+ messages in thread
From: David Bremner @ 2020-07-25 11:17 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

> The current code seems to look for the path relative to the current
> working directory, rather than the mail store root.
> ---
>  test/T562-lib-database.sh | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)

Series applied to master

d

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

end of thread, other threads:[~2020-07-25 11:17 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-18 15:07 batch 6, API exception handling cleanup David Bremner
2020-07-18 15:07 ` [PATCH 01/10] test: regression test for closing a closed database David Bremner
2020-07-18 15:07 ` [PATCH 02/10] test: add regression tests for notmuch database destroy David Bremner
2020-07-18 15:07 ` [PATCH 03/10] test: add known broken test for n_d_needs_upgrade David Bremner
2020-07-18 15:07 ` [PATCH 04/10] lib/n_d_needs_upgrade: handle error return from n_d_get_version David Bremner
2020-07-18 15:07 ` [PATCH 05/10] test: regression test for n_d_upgrade David Bremner
2020-07-18 15:07 ` [PATCH 06/10] test: add regression test for n_d_{begin,end}_atomic David Bremner
2020-07-18 15:07 ` [PATCH 07/10] test: regression test for n_d_get_revision David Bremner
2020-07-18 15:07 ` [PATCH 08/10] test: add regression test for n_d_get_directory David Bremner
2020-07-18 15:07 ` [PATCH 09/10] test: regression test for n_d_index_file closed db David Bremner
2020-07-18 15:07 ` [PATCH 10/10] test: add known broken test for indexing relative path David Bremner
2020-07-19 10:05   ` [PATCH] lib: convert relative filenames to absolute in n_d_index_file David Bremner
2020-07-25 11:17   ` [PATCH 10/10] test: add known broken test for indexing relative path David Bremner
2020-07-23 10:51 ` batch 6, API exception handling cleanup David Bremner

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).