[PATCH 0/4] fix "no such file" problem for emails send by emacs

[PATCH 0/4] fix "no such file" problem for emails send by emacs

[Yuri Volchkov]

Hi,

I have faced a problem, that messages sent by emacs could not be shown
or found later. The "notmuch show id:<msg_id>" says "no such file or
directory".

This patch series fixes the problem related to my use case. The
detailed description of the root cause is provided in the related
patch.

Yuri Volchkov (4):
  test: insert into the folder with trailing /
  insert: strip trailing / in folder path
  test: show id:<> works even if the first duplicated is deleted
  show: workaround for the missing file problem

 lib/database.cc            |  3 +--
 mime-node.c                | 28 ++++++++++++++++++++++++----
 notmuch-insert.c           |  4 +++-
 test/T070-insert.sh        |  7 +++++++
 test/T670-duplicate-mid.sh | 25 +++++++++++++++++++++++++
 util/string-util.c         | 13 +++++++++++++
 util/string-util.h         |  2 ++
 7 files changed, 75 insertions(+), 7 deletions(-)

-- 
2.7.4

[PATCH 1/4] test: insert into the folder with trailing /

[Yuri Volchkov]

From database's point of view, "Drafts" and "Drafts/" are different
folders

Signed-off-by: Yuri Volchkov <yuri.volchkov@gmail.com>
---
 test/T070-insert.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/test/T070-insert.sh b/test/T070-insert.sh
index 48f212e..380934a 100755
--- a/test/T070-insert.sh
+++ b/test/T070-insert.sh
@@ -132,6 +132,13 @@ output=$(notmuch search --output=files path:Drafts/new)
 dirname=$(dirname "$output")
 test_expect_equal "$dirname" "$MAIL_DIR/Drafts/new"
 
+test_begin_subtest "Insert message into folder with trailing /"
+gen_insert_msg
+notmuch insert --folder=Drafts/ < "$gen_msg_filename"
+output=$(notmuch search --output=files id:${gen_msg_id})
+dirname=$(dirname "$output")
+test_expect_equal "$dirname" "$MAIL_DIR/Drafts/new"
+
 test_begin_subtest "Insert message into folder, add/remove tags"
 gen_insert_msg
 notmuch insert --folder=Drafts +draft -unread < "$gen_msg_filename"
-- 
2.7.4

Re: [PATCH 1/4] test: insert into the folder with trailing /

[David Bremner]

Yuri Volchkov <yuri.volchkov@gmail.com> writes:

> From database's point of view, "Drafts" and "Drafts/" are different
> folders
>
> Signed-off-by: Yuri Volchkov <yuri.volchkov@gmail.com>
> ---
>  test/T070-insert.sh | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/test/T070-insert.sh b/test/T070-insert.sh
> index 48f212e..380934a 100755
> --- a/test/T070-insert.sh
> +++ b/test/T070-insert.sh
> @@ -132,6 +132,13 @@ output=$(notmuch search --output=files path:Drafts/new)
>  dirname=$(dirname "$output")
>  test_expect_equal "$dirname" "$MAIL_DIR/Drafts/new"
>  
> +test_begin_subtest "Insert message into folder with trailing /"
> +gen_insert_msg
> +notmuch insert --folder=Drafts/ < "$gen_msg_filename"
> +output=$(notmuch search --output=files id:${gen_msg_id})
> +dirname=$(dirname "$output")
> +test_expect_equal "$dirname" "$MAIL_DIR/Drafts/new"
> +

This is basically OK, but for future reference you can add
'test_subtest_known_broken' when adding a test, and remove it in the
patch providing a fix. The preserves the nice property that the test
suite passes after every commit.

d

Re: [PATCH 1/4] test: insert into the folder with trailing /

[David Bremner]

Yuri Volchkov <yuri.volchkov@gmail.com> writes:

> From database's point of view, "Drafts" and "Drafts/" are different
> folders
>
> Signed-off-by: Yuri Volchkov <yuri.volchkov@gmail.com>

Merged this test, marked as known-broken

[PATCH 2/4] insert: strip trailing / in folder path

[Yuri Volchkov]

I have faced a problem, that messages sent by emacs could not be shown
or found later. The "notmuch show id:<msg_id>" says "no such file or
directory".

The reason of this behavior is the following chain of events:
1) While sending a message, emacs calls
      notmuch insert --folder=maildir/Sent/ < test.msg

   From database's point of view, "Sent" and "Sent/" are different
   folders, and the separate record is created for the "Sent/".

   For the sake of simplicity let's assume inserted message will be
   stored as "maildir/Sent//new/msg_file_orig" (note the double slash)

2) During next sync, offlineimap uploads this message to the server,
   renames the source file according to his or servers naming
   convention. Let's say it is renamed to
   "maildir/Sent/new/msg_file_renamed"

3) The "notmuch new" command, composes a list of folders which has
   been modified. Obviously, the "Sent" dir is going to be in this
   list, but not the "Sent/".

   When notmuch scans the "Sent" folder, it finds the new file
   "maildir/Sent/new/msg_file_renamed", and adds it to the database as
   a duplicated of the message inserted in the step 1.

   But, notmuch does not notice that the file
   "maildir/Sent//new/msg_file_orig" has been deleted. So it
   erroneously stays in the database.

4) Next time, the user wants to see this email, the command "notmuch
   show id:<inserted_msg_id>" picks the first message file, stored in
   the database record, which happens to be the non-existing
   "maildir/Sent//new/msg_file_orig".

The solution is simple, we have to strip trailing '/' from the insert
path.

Signed-off-by: Yuri Volchkov <yuri.volchkov@gmail.com>
---
 lib/database.cc    |  3 +--
 notmuch-insert.c   |  4 +++-
 util/string-util.c | 13 +++++++++++++
 util/string-util.h |  2 ++
 4 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 8f0e22a..79eb3d6 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -858,8 +858,7 @@ notmuch_database_open_verbose (const char *path,
     notmuch->status_string = NULL;
     notmuch->path = talloc_strdup (notmuch, path);
 
-    if (notmuch->path[strlen (notmuch->path) - 1] == '/')
-	notmuch->path[strlen (notmuch->path) - 1] = '\0';
+    strip_trailing(notmuch->path, '/');
 
     notmuch->mode = mode;
     notmuch->atomic_nesting = 0;
diff --git a/notmuch-insert.c b/notmuch-insert.c
index bc96af0..2590e83 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -27,6 +27,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
+#include "string-util.h"
 
 static volatile sig_atomic_t interrupted;
 
@@ -451,7 +452,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
     size_t new_tags_length;
     tag_op_list_t *tag_ops;
     char *query_string = NULL;
-    const char *folder = NULL;
+    char *folder = NULL;
     notmuch_bool_t create_folder = FALSE;
     notmuch_bool_t keep = FALSE;
     notmuch_bool_t no_hooks = FALSE;
@@ -511,6 +512,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
     if (folder == NULL) {
 	maildir = db_path;
     } else {
+	strip_trailing (folder, '/');
 	if (! is_valid_folder_name (folder)) {
 	    fprintf (stderr, "Error: invalid folder name: '%s'\n", folder);
 	    return EXIT_FAILURE;
diff --git a/util/string-util.c b/util/string-util.c
index 1812530..b010881 100644
--- a/util/string-util.c
+++ b/util/string-util.c
@@ -255,3 +255,16 @@ strcase_hash (const void *ptr)
 
     return hash;
 }
+
+void
+strip_trailing (char *str, char ch)
+{
+    int i;
+
+    for (i = strlen (str) - 1; i >= 0; i--) {
+	if (str[i] == ch)
+	    str[i] = '\0';
+	else
+	    break;
+    }
+}
diff --git a/util/string-util.h b/util/string-util.h
index 87917b8..9777061 100644
--- a/util/string-util.h
+++ b/util/string-util.h
@@ -75,6 +75,8 @@ int strcase_equal (const void *a, const void *b);
 /* GLib GHashFunc compatible case insensitive hash function */
 unsigned int strcase_hash (const void *ptr);
 
+void strip_trailing (char *str, char ch);
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.7.4

Re: [PATCH 2/4] insert: strip trailing / in folder path

[David Bremner]

Yuri Volchkov <yuri.volchkov@gmail.com> writes:

> I have faced a problem, that messages sent by emacs could not be shown
> or found later. The "notmuch show id:<msg_id>" says "no such file or
> directory".
>
> The reason of this behavior is the following chain of events:
> 1) While sending a message, emacs calls
>       notmuch insert --folder=maildir/Sent/ < test.msg
>

I think it will be less confusing if you give a more reduced description
of the bug fix here. In particular the test in your previous patch shows
that the neither offlineimap nor multiple copies of a message are needed
to trigger this bug; rather it has to do with insufficient path
canonicalization.
>
> The solution is simple, we have to strip trailing '/' from the insert
> path.
>
> diff --git a/lib/database.cc b/lib/database.cc
> index 8f0e22a..79eb3d6 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -858,8 +858,7 @@ notmuch_database_open_verbose (const char *path,
>      notmuch->status_string = NULL;
>      notmuch->path = talloc_strdup (notmuch, path);
>  
> -    if (notmuch->path[strlen (notmuch->path) - 1] == '/')
> -	notmuch->path[strlen (notmuch->path) - 1] = '\0';
> +    strip_trailing(notmuch->path, '/');

These seems like a reasonable change, but I don't see the connection
with the notmuch-insert problem. Please either explain the connection in
the commit message or split the change out into a different commit with
an explanation of what it is fixing (and perhaps a seperate test, if
there's a real problem there, rather than just tidying up).

Re: [PATCH 2/4] insert: strip trailing / in folder path

[Yuri Volchkov]

David Bremner <david@tethera.net> writes:

> Yuri Volchkov <yuri.volchkov@gmail.com> writes:
>
>> I have faced a problem, that messages sent by emacs could not be shown
>> or found later. The "notmuch show id:<msg_id>" says "no such file or
>> directory".
>>
>> The reason of this behavior is the following chain of events:
>> 1) While sending a message, emacs calls
>>       notmuch insert --folder=maildir/Sent/ < test.msg
>>
>
> I think it will be less confusing if you give a more reduced description
> of the bug fix here. In particular the test in your previous patch shows
> that the neither offlineimap nor multiple copies of a message are needed
> to trigger this bug; rather it has to do with insufficient path
> canonicalization.

You are probably right, I was too obsessed with the problem blocking my
switch-to-notmuch project . I'll try to reduce the commit message.

>>
>> The solution is simple, we have to strip trailing '/' from the insert
>> path.
>>
>> diff --git a/lib/database.cc b/lib/database.cc
>> index 8f0e22a..79eb3d6 100644
>> --- a/lib/database.cc
>> +++ b/lib/database.cc
>> @@ -858,8 +858,7 @@ notmuch_database_open_verbose (const char *path,
>>      notmuch->status_string = NULL;
>>      notmuch->path = talloc_strdup (notmuch, path);
>>  
>> -    if (notmuch->path[strlen (notmuch->path) - 1] == '/')
>> -	notmuch->path[strlen (notmuch->path) - 1] = '\0';
>> +    strip_trailing(notmuch->path, '/');
>
> These seems like a reasonable change, but I don't see the connection
> with the notmuch-insert problem. Please either explain the connection in
> the commit message or split the change out into a different commit with
> an explanation of what it is fixing (and perhaps a seperate test, if
> there's a real problem there, rather than just tidying up).

This is just for consistency reasons. Fixing my problem required the
same piece of code, which was used here. Duplicating is not nice, so I
made a function around this code. That's why it feels atomic change
to me.

I think, explanation like this is superfluously detailed.

I can move the introduction of the strip_trailing and cleaning
notmuch_database_open_verbose into separate patch, but the fix then will
be just a one line. And I'll get unnecessarily tiny patches.

What do you think?

Re: [PATCH 2/4] insert: strip trailing / in folder path

[David Bremner]

Yuri Volchkov <yuri.volchkov@gmail.com> writes:

>
> This is just for consistency reasons. Fixing my problem required the
> same piece of code, which was used here. Duplicating is not nice, so I
> made a function around this code. That's why it feels atomic change
> to me.
>
> I think, explanation like this is superfluously detailed.
>
> I can move the introduction of the strip_trailing and cleaning
> notmuch_database_open_verbose into separate patch, but the fix then will
> be just a one line. And I'll get unnecessarily tiny patches.

I prefer the smaller commits in case there is some unforseen effect of
changing notmuch_database_open_verbose; it makes it easier to use git
bisect. As a commit message, it almost suffices to mention reducing code
duplication, although do note this is really a change in beheviour: your
new code strips multiple trailing /, while the old code did not.

d

[PATCH 3/4] test: show id:<> works even if the first duplicated is deleted

[Yuri Volchkov]

Signed-off-by: Yuri Volchkov <yuri.volchkov@gmail.com>
---
 test/T670-duplicate-mid.sh | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/test/T670-duplicate-mid.sh b/test/T670-duplicate-mid.sh
index ea5e1d6..decbc0a 100755
--- a/test/T670-duplicate-mid.sh
+++ b/test/T670-duplicate-mid.sh
@@ -37,4 +37,29 @@ notmuch reindex '*'
 notmuch search --output=files "sekrit" | notmuch_dir_sanitize > OUTPUT
 test_expect_equal_file EXPECTED OUTPUT
 
+rm ${MAIL_DIR}/copy1
+test_begin_subtest 'Deleted first duplicate file does not stop notmuch show from working'
+output=$(notmuch show --body=false --format=json id:duplicate)
+expected='[[[{
+    "id": "'duplicate'",
+    "match": true,
+    "excluded": false,
+    "filename": [
+        "'"${MAIL_DIR}"/copy1'",
+        "'"${MAIL_DIR}"/copy2'"
+    ],
+    "timestamp": 978709435,
+    "date_relative": "2001-01-05",
+    "tags": ["inbox","unread"],
+    "headers": {
+        "Subject": "message 2",
+        "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
+        "To": "Notmuch Test Suite <test_suite@notmuchmail.org>",
+        "Date": "Fri, 05 Jan 2001 15:43:55 +0000"
+    }
+ },
+[]]]]'
+
+test_expect_equal_json "$output" "$expected"
+
 test_done
-- 
2.7.4

[PATCH 4/4] show: workaround for the missing file problem

[Yuri Volchkov]

If a message to be shown has several duplicated files, and for some
reason the first file in the list is not available anymore, notmuch
will exit with error.

This is clearly a problem in the database, but we are not going to let
this problem be a show-stopper. Let's walk through the list, and show
the first existing file.

Signed-off-by: Yuri Volchkov <yuri.volchkov@gmail.com>
---
 mime-node.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index bb0870d..24d73af 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -79,12 +79,32 @@ mime_node_open (const void *ctx, notmuch_message_t *message,
     }
     talloc_set_destructor (mctx, _mime_node_context_free);
 
+    /* Fast path */
     mctx->file = fopen (filename, "r");
     if (! mctx->file) {
-	fprintf (stderr, "Error opening %s: %s\n", filename, strerror (errno));
-	status = NOTMUCH_STATUS_FILE_ERROR;
-	goto DONE;
-    }
+	/* Slow path - for some reason the first file in the list is
+	 * not available anymore. This is clearly a problem in the
+	 * database, but we are not going to let this problem be a
+	 * show stopper */
+	notmuch_filenames_t *filenames;
+	for (filenames = notmuch_message_get_filenames (message);
+	     notmuch_filenames_valid (filenames);
+	     notmuch_filenames_move_to_next (filenames))
+	{
+	    filename = notmuch_filenames_get (filenames);
+	    mctx->file = fopen (filename, "r");
+	    if (mctx->file)
+		break;
+	}
+
+	talloc_free (filenames);
+	if (! mctx->file) {
+	    /* Give up */
+	    fprintf (stderr, "Error opening %s: %s\n", filename, strerror (errno));
+		status = NOTMUCH_STATUS_FILE_ERROR;
+		goto DONE;
+	    }
+	}
 
     mctx->stream = g_mime_stream_file_new (mctx->file);
     if (!mctx->stream) {
-- 
2.7.4

Re: [PATCH 0/4] fix "no such file" problem for emails send by emacs

[David Bremner]

Yuri Volchkov <yuri.volchkov@gmail.com> writes:

> Hi,
>
> I have faced a problem, that messages sent by emacs could not be shown
> or found later. The "notmuch show id:<msg_id>" says "no such file or
> directory".
>
> This patch series fixes the problem related to my use case. The
> detailed description of the root cause is provided in the related
> patch.

I haven't had a chance to look carefully at your series, but I would be
curious if "notmuch reindex id:the-msg-id" fixes the database problem.
notmuch reindex is only in git master at this point.

d

Re: [PATCH 0/4] fix "no such file" problem for emails send by emacs

[Yuri Volchkov]

Hi,

I have tried the command "notmuch reindex id:the-msg-id". Yes, it does
fix the database problem. However, as far as I understood your
reindex-patches, the command does not touch 'XDIRECTORY' records. So
the record about folder "new/" will remain in the database. But it is
very minor thing, because it will not affect the work of notmuch at all.

So my patch-set fixes the bug, but does not fix the database. And 'reindex'
does fix the consequences of the bug. Right in time, because I was going
to fix my broken records manually :).

Buy the way, I'm not sure I understood the purpose of the 'reindex'. Is
it for situations like this? For fixing broken database? Or something more?


David Bremner <david@tethera.net> writes:

> Yuri Volchkov <yuri.volchkov@gmail.com> writes:
>
>> Hi,
>>
>> I have faced a problem, that messages sent by emacs could not be shown
>> or found later. The "notmuch show id:<msg_id>" says "no such file or
>> directory".
>>
>> This patch series fixes the problem related to my use case. The
>> detailed description of the root cause is provided in the related
>> patch.
>
> I haven't had a chance to look carefully at your series, but I would be
> curious if "notmuch reindex id:the-msg-id" fixes the database problem.
> notmuch reindex is only in git master at this point.
>
> d

Re: [PATCH 0/4] fix "no such file" problem for emails send by emacs

[David Bremner]

Yuri Volchkov <yuri.volchkov@gmail.com> writes:

>
> Buy the way, I'm not sure I understood the purpose of the 'reindex'. Is
> it for situations like this? For fixing broken database? Or something more?

The original motivation has to do with removing duplicate files (with
the same message-id) and adjusting the search results. I do intend for
it to be a fairly generic tool though. I know dkg has some plans related
to de-crypting and re-encrypting files.

d

[PATCH v2 0/4] fix insufficient path canonization

[Yuri Volchkov]

Former subject was 'fix "no such file" problem for emails send by
emacs'

Patches 1-2 fix the problem, so new broken records do not appear
anymore.
Patches 3-4 make notmuch able to work with such brocken records

Changes in v2:
1) Separate patch for introduction of the strip_trailing() 
2) Reduced commit description for the patch "insert: strip trailing /
   in folder path"
3) Proper use of 'test_subtest_known_broken' in the related tests

Yuri Volchkov (4):
  database: move striping of trailing '/' into helper function
  insert: strip trailing / in folder path
  test: show id:<> works even if the first duplicate is deleted
  show: workaround for the missing file problem

 lib/database.cc            |  3 +--
 mime-node.c                | 28 ++++++++++++++++++++++++----
 notmuch-insert.c           |  4 +++-
 test/T070-insert.sh        |  1 -
 test/T670-duplicate-mid.sh | 25 +++++++++++++++++++++++++
 util/string-util.c         | 13 +++++++++++++
 util/string-util.h         |  2 ++
 7 files changed, 68 insertions(+), 8 deletions(-)

-- 
2.7.4

[PATCH v2 1/4] database: move striping of trailing '/' into helper function

[Yuri Volchkov]

Stripping trailing character is not that uncommon
operation. Particularly, the next patch has to perform it as
well. Lets move it to the separate function to avoid code duplication.

Also the new function has a little improvement: if the character to
strip is repeated several times in the end of a string, function
strips them all.

Signed-off-by: Yuri Volchkov <yuri.volchkov@gmail.com>
---
 lib/database.cc    |  3 +--
 util/string-util.c | 13 +++++++++++++
 util/string-util.h |  2 ++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 8f0e22a..79eb3d6 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -858,8 +858,7 @@ notmuch_database_open_verbose (const char *path,
     notmuch->status_string = NULL;
     notmuch->path = talloc_strdup (notmuch, path);
 
-    if (notmuch->path[strlen (notmuch->path) - 1] == '/')
-	notmuch->path[strlen (notmuch->path) - 1] = '\0';
+    strip_trailing(notmuch->path, '/');
 
     notmuch->mode = mode;
     notmuch->atomic_nesting = 0;
diff --git a/util/string-util.c b/util/string-util.c
index 1812530..b010881 100644
--- a/util/string-util.c
+++ b/util/string-util.c
@@ -255,3 +255,16 @@ strcase_hash (const void *ptr)
 
     return hash;
 }
+
+void
+strip_trailing (char *str, char ch)
+{
+    int i;
+
+    for (i = strlen (str) - 1; i >= 0; i--) {
+	if (str[i] == ch)
+	    str[i] = '\0';
+	else
+	    break;
+    }
+}
diff --git a/util/string-util.h b/util/string-util.h
index 87917b8..9777061 100644
--- a/util/string-util.h
+++ b/util/string-util.h
@@ -75,6 +75,8 @@ int strcase_equal (const void *a, const void *b);
 /* GLib GHashFunc compatible case insensitive hash function */
 unsigned int strcase_hash (const void *ptr);
 
+void strip_trailing (char *str, char ch);
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.7.4

[PATCH v2 2/4] insert: strip trailing / in folder path

[Yuri Volchkov]

This patch fixes the "Insert message into folder with trailing /"
test. The problem was insufficient path canonization.

From database's point of view, "Sent" and "Sent/" are different
folders. If user runs (note the last '/'):

    notmuch insert --folder=maildir/Sent/ < test.msg

notmuch will create an extra XDIRECTORY record for the folder
'Sent/'. This means that database will have _TWO_ records for _ONE_
physical folder: 'Sent' and 'Sent/'. However, the 'notmuch new'
command will update only records related to the first one (the correct
one).

Now, if user moved the email file (e.g. from 'Sent/new' to
'Sent/cur'), 'notmuch new' will add a record about the new file, but
will not delete the old record.

Signed-off-by: Yuri Volchkov <yuri.volchkov@gmail.com>
---
 notmuch-insert.c    | 4 +++-
 test/T070-insert.sh | 1 -
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index bc96af0..2590e83 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -27,6 +27,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
+#include "string-util.h"
 
 static volatile sig_atomic_t interrupted;
 
@@ -451,7 +452,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
     size_t new_tags_length;
     tag_op_list_t *tag_ops;
     char *query_string = NULL;
-    const char *folder = NULL;
+    char *folder = NULL;
     notmuch_bool_t create_folder = FALSE;
     notmuch_bool_t keep = FALSE;
     notmuch_bool_t no_hooks = FALSE;
@@ -511,6 +512,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
     if (folder == NULL) {
 	maildir = db_path;
     } else {
+	strip_trailing (folder, '/');
 	if (! is_valid_folder_name (folder)) {
 	    fprintf (stderr, "Error: invalid folder name: '%s'\n", folder);
 	    return EXIT_FAILURE;
diff --git a/test/T070-insert.sh b/test/T070-insert.sh
index 187dfd3..380934a 100755
--- a/test/T070-insert.sh
+++ b/test/T070-insert.sh
@@ -133,7 +133,6 @@ dirname=$(dirname "$output")
 test_expect_equal "$dirname" "$MAIL_DIR/Drafts/new"
 
 test_begin_subtest "Insert message into folder with trailing /"
-test_subtest_known_broken
 gen_insert_msg
 notmuch insert --folder=Drafts/ < "$gen_msg_filename"
 output=$(notmuch search --output=files id:${gen_msg_id})
-- 
2.7.4

[PATCH v2 3/4] test: show id:<> works even if the first duplicate is deleted

[Yuri Volchkov]

Signed-off-by: Yuri Volchkov <yuri.volchkov@gmail.com>
---
 test/T670-duplicate-mid.sh | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/test/T670-duplicate-mid.sh b/test/T670-duplicate-mid.sh
index ea5e1d6..f6f1602 100755
--- a/test/T670-duplicate-mid.sh
+++ b/test/T670-duplicate-mid.sh
@@ -37,4 +37,30 @@ notmuch reindex '*'
 notmuch search --output=files "sekrit" | notmuch_dir_sanitize > OUTPUT
 test_expect_equal_file EXPECTED OUTPUT
 
+rm ${MAIL_DIR}/copy1
+test_begin_subtest 'Deleted first duplicate file does not stop notmuch show from working'
+test_subtest_known_broken
+output=$(notmuch show --body=false --format=json id:duplicate)
+expected='[[[{
+    "id": "'duplicate'",
+    "match": true,
+    "excluded": false,
+    "filename": [
+        "'"${MAIL_DIR}"/copy1'",
+        "'"${MAIL_DIR}"/copy2'"
+    ],
+    "timestamp": 978709435,
+    "date_relative": "2001-01-05",
+    "tags": ["inbox","unread"],
+    "headers": {
+        "Subject": "message 2",
+        "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
+        "To": "Notmuch Test Suite <test_suite@notmuchmail.org>",
+        "Date": "Fri, 05 Jan 2001 15:43:55 +0000"
+    }
+ },
+[]]]]'
+
+test_expect_equal_json "$output" "$expected"
+
 test_done
-- 
2.7.4

[PATCH v2 4/4] show: workaround for the missing file problem

[Yuri Volchkov]

This patch fixes the 'Deleted first duplicate file does not stop
notmuch show from working' test.

If a message to be shown has several duplicated files, and for some
reason the first file in the list is not available anymore, notmuch
will exit with an error.

This is clearly a problem in the database, but we are not going to let
this problem be a show-stopper. Let's walk through the list, and show
the first existing file.

Signed-off-by: Yuri Volchkov <yuri.volchkov@gmail.com>
---
 mime-node.c                | 28 ++++++++++++++++++++++++----
 test/T670-duplicate-mid.sh |  1 -
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index bb0870d..24d73af 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -79,12 +79,32 @@ mime_node_open (const void *ctx, notmuch_message_t *message,
     }
     talloc_set_destructor (mctx, _mime_node_context_free);
 
+    /* Fast path */
     mctx->file = fopen (filename, "r");
     if (! mctx->file) {
-	fprintf (stderr, "Error opening %s: %s\n", filename, strerror (errno));
-	status = NOTMUCH_STATUS_FILE_ERROR;
-	goto DONE;
-    }
+	/* Slow path - for some reason the first file in the list is
+	 * not available anymore. This is clearly a problem in the
+	 * database, but we are not going to let this problem be a
+	 * show stopper */
+	notmuch_filenames_t *filenames;
+	for (filenames = notmuch_message_get_filenames (message);
+	     notmuch_filenames_valid (filenames);
+	     notmuch_filenames_move_to_next (filenames))
+	{
+	    filename = notmuch_filenames_get (filenames);
+	    mctx->file = fopen (filename, "r");
+	    if (mctx->file)
+		break;
+	}
+
+	talloc_free (filenames);
+	if (! mctx->file) {
+	    /* Give up */
+	    fprintf (stderr, "Error opening %s: %s\n", filename, strerror (errno));
+		status = NOTMUCH_STATUS_FILE_ERROR;
+		goto DONE;
+	    }
+	}
 
     mctx->stream = g_mime_stream_file_new (mctx->file);
     if (!mctx->stream) {
diff --git a/test/T670-duplicate-mid.sh b/test/T670-duplicate-mid.sh
index f6f1602..decbc0a 100755
--- a/test/T670-duplicate-mid.sh
+++ b/test/T670-duplicate-mid.sh
@@ -39,7 +39,6 @@ test_expect_equal_file EXPECTED OUTPUT
 
 rm ${MAIL_DIR}/copy1
 test_begin_subtest 'Deleted first duplicate file does not stop notmuch show from working'
-test_subtest_known_broken
 output=$(notmuch show --body=false --format=json id:duplicate)
 expected='[[[{
     "id": "'duplicate'",
-- 
2.7.4

Re: [PATCH v2 0/4] fix insufficient path canonization

[David Bremner]

Yuri Volchkov <yuri.volchkov@gmail.com> writes:

> Former subject was 'fix "no such file" problem for emails send by
> emacs'
>
> Patches 1-2 fix the problem, so new broken records do not appear
> anymore.
> Patches 3-4 make notmuch able to work with such brocken records

pushed to master, thanks

d