[PATCH 0/4] Maildir synchronization

[PATCH 0/4] Maildir synchronization

[Michal Sojka]

Hi,

these patches implement synchronization between maildir flags and
notmuch tags. The synchronization can be configured to not happen at
all (default), to set/unset tags when importing new (or new and
renamed) messages and to happen in both directions - set/unset tags
during importing and change maildir flags during tagging.

The previous version if this feature was present in mailstore
abstraction patches. I've extracted it from there and I'm sending it
separately. I'm quite heavily dependent on this feature so I hope this
version is closer to merging than the previous one.

The last patch needs git-based test suite patches sent yesterday.

Michal Sojka (4):
  lib: Return added message even if it already was in the database
  Maildir synchronization
  Make maildir synchronization configurable
  Tests for maildir synchronization

 lib/database-private.h     |    2 +-
 lib/database.cc            |   19 ++++-
 lib/message.cc             |  226 ++++++++++++++++++++++++++++++++++++++++++++
 lib/notmuch-private.h      |    4 +
 lib/notmuch.h              |   29 ++++++-
 notmuch-client.h           |    7 ++
 notmuch-config.c           |   48 +++++++++
 notmuch-new.c              |    7 +-
 notmuch-restore.c          |    2 +
 notmuch-setup.c            |   17 ++++
 notmuch-tag.c              |    2 +
 test/t0011-maildir-sync.sh |  216 ++++++++++++++++++++++++++++++++++++++++++
 test/test-lib.sh           |   11 ++-
 13 files changed, 582 insertions(+), 8 deletions(-)
 create mode 100755 test/t0011-maildir-sync.sh

[PATCH 1/4] lib: Return added message even if it already was in the database

[Michal Sojka]

---
 lib/database.cc |    3 ++-
 lib/notmuch.h   |    3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 6afc8d9..694b7ec 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1613,7 +1613,8 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
 
   DONE:
     if (message) {
-	if (ret == NOTMUCH_STATUS_SUCCESS && message_ret)
+	if ((ret == NOTMUCH_STATUS_SUCCESS ||
+	     ret == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) && message_ret)
 	    *message_ret = message;
 	else
 	    notmuch_message_destroy (message);
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 505ad19..0ba1416 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -238,7 +238,8 @@ notmuch_database_get_directory (notmuch_database_t *database,
  * notmuch database will reference the filename, and will not copy the
  * entire contents of the file.
  *
- * If 'message' is not NULL, then, on successful return '*message'
+ * If 'message' is not NULL, then, on successful return
+ * (NOTMUCH_STATUS_SUCCESS or NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) '*message'
  * will be initialized to a message object that can be used for things
  * such as adding tags to the just-added message. The user should call
  * notmuch_message_destroy when done with the message. On any failure
-- 
1.7.1

[PATCH 2/4] Maildir synchronization

[Michal Sojka]

This patch allows bi-directional synchronization between maildir
flags and certain tags. The flag-to-tag mapping is defined by flag2tag
array.

The synchronization works this way:

1) Whenever notmuch new is executed, the following happens:
   o New messages are tagged with configured new_tags.
   o For new or renamed messages with maildir info present in the file
     name, the tags defined in flag2tag are either added or removed
     depending on the flags from the file name.

2) Whenever notmuch tag (or notmuch restore) is executed, a new set of
   flags based on the tags is constructed for every message and a new
   file name is prepared based on the old file name but with the new
   flags. If the flags differs and the old message was in 'new'
   directory then this is replaced with 'cur' in the new file name. If
   the new and old file names differ, the file is renamed and notmuch
   database is updated accordingly.

   The rename happens before the database is updated. In case of crash
   between rename and database update, the next run of notmuch new
   brings the database in sync with the mail store again.

There is currenlty one known issue: Viewing/storing of attachments of
unread messages doesn't work. The reason is that when you view the
message its unread tag is removed which leads to rename of the file,
but Emacs still uses the original name to access the attachment.

Workaround: close the message and open it again.
---
 lib/database.cc       |    7 ++
 lib/message.cc        |  226 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/notmuch-private.h |    4 +
 lib/notmuch.h         |    7 ++
 notmuch-new.c         |    3 +-
 5 files changed, 246 insertions(+), 1 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 694b7ec..908bbaa 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1585,6 +1585,13 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
 
 	_notmuch_message_add_filename (message, filename);
 
+	/* This is a new message or it has a new filename and as such,
+	 * its tags in database either do not exists or might be out
+	 * of date. We assign the tags later in notmuch new, but until
+	 * then we should not synchronize the tags back to the maildir
+	 * flags (if notmuch is configured to do so). */
+	notmuch_message_set_flag(message, NOTMUCH_MESSAGE_FLAG_TAGS_INVALID, TRUE);
+
 	/* Is this a newly created message object? */
 	if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
 	    _notmuch_message_add_term (message, "type", "mail");
diff --git a/lib/message.cc b/lib/message.cc
index 4b2f98f..0de3f01 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -43,6 +43,24 @@ struct _notmuch_message {
     Xapian::Document doc;
 };
 
+#define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))
+
+struct maildir_flag_tag {
+    char flag;
+    const char *tag;
+    bool inverse;
+};
+
+/* ASCII ordered table of Maildir flags and associated tags */
+struct maildir_flag_tag flag2tag[] = {
+    { 'D', "draft",   false},
+    { 'F', "flagged", false},
+    { 'P', "passed",  false},
+    { 'R', "replied", false},
+    { 'S', "unread",  true },
+    { 'T', "delete",  false},
+};
+
 /* We end up having to call the destructor explicitly because we had
  * to use "placement new" in order to initialize C++ objects within a
  * block that we allocated with talloc. So C++ is making talloc
@@ -568,15 +586,29 @@ _notmuch_message_set_date (notmuch_message_t *message,
 			    Xapian::sortable_serialise (time_value));
 }
 
+static notmuch_private_status_t
+_notmuch_message_tags_to_maildir (notmuch_message_t *message);
+
 /* Synchronize changes made to message->doc out into the database. */
 void
 _notmuch_message_sync (notmuch_message_t *message)
 {
     Xapian::WritableDatabase *db;
+    notmuch_private_status_t status;
 
     if (message->notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY)
 	return;
 
+    if (// todo_sync_enabled &&
+	!notmuch_message_get_flag(message, NOTMUCH_MESSAGE_FLAG_TAGS_INVALID)) {
+ 	status = _notmuch_message_tags_to_maildir (message);
+	if (status != NOTMUCH_PRIVATE_STATUS_SUCCESS) {
+	    fprintf (stderr, "Error: Cannot sync tags to maildir (%s)\n",
+		     notmuch_status_to_string ((notmuch_status_t)status));
+	    /* Exit to avoid unsynchronized mailstore. */
+	    exit(1);
+	}
+    }
     db = static_cast <Xapian::WritableDatabase *> (message->notmuch->xapian_db);
     db->replace_document (message->doc_id, message->doc);
 }
@@ -688,6 +720,44 @@ _notmuch_message_remove_term (notmuch_message_t *message,
     return NOTMUCH_PRIVATE_STATUS_SUCCESS;
 }
 
+/* Change the message filename stored in the database.
+ *
+ * This change will not be reflected in the database until the next
+ * call to _notmuch_message_sync.
+ */
+notmuch_private_status_t
+_notmuch_message_rename (notmuch_message_t *message,
+			 const char *new_filename)
+{
+    void *local = talloc_new (message);
+    char *direntry;
+    Xapian::PostingIterator i, end;
+    Xapian::Document document;
+    notmuch_private_status_t pstatus;
+    notmuch_status_t status;
+    const char *old_filename;
+
+    old_filename = notmuch_message_get_filename(message);
+    old_filename = talloc_reference(local, old_filename);
+    if (unlikely(!old_filename))
+	return NOTMUCH_PRIVATE_STATUS_OUT_OF_MEMORY;
+
+    status = _notmuch_message_add_filename (message, new_filename);
+    if (status)
+	return (notmuch_private_status_t)status;
+
+    status = _notmuch_database_filename_to_direntry (local, message->notmuch,
+						     old_filename, &direntry);
+    if (status)
+	return (notmuch_private_status_t)status;
+
+    pstatus = _notmuch_message_remove_term (message, "file-direntry", direntry);
+
+    talloc_free (local);
+
+    return pstatus;
+}
+
 notmuch_status_t
 notmuch_message_add_tag (notmuch_message_t *message, const char *tag)
 {
@@ -745,6 +815,162 @@ notmuch_message_remove_tag (notmuch_message_t *message, const char *tag)
 }
 
 notmuch_status_t
+notmuch_message_maildir_to_tags (notmuch_message_t *message, const char *filename)
+{
+    const char *flags, *p;
+    char f;
+    bool valid, unread;
+    unsigned i;
+    notmuch_status_t status;
+
+    flags = strstr (filename, ":2,");
+    if (!flags)
+	return NOTMUCH_STATUS_FILE_NOT_EMAIL;
+    flags += 3;
+
+    /*  Check that the letters are valid Maildir flags */
+    f = 0;
+    valid = true;
+    for (p=flags; valid && *p; p++) {
+	switch (*p) {
+	case 'P':
+	case 'R':
+	case 'S':
+	case 'T':
+	case 'D':
+	case 'F':
+	    if (*p > f) f=*p;
+	    else valid = false;
+	break;
+	default:
+	    valid = false;
+	}
+    }
+    if (!valid) {
+	fprintf (stderr, "Warning: Invalid maildir flags in filename %s\n", filename);
+	return NOTMUCH_STATUS_FILE_NOT_EMAIL;
+    }
+
+    status = notmuch_message_freeze (message);
+    if (status)
+	return status;
+    unread = true;
+    for (i = 0; i < ARRAY_SIZE(flag2tag); i++) {
+	if ((strchr (flags, flag2tag[i].flag) != NULL) ^ flag2tag[i].inverse) {
+	    status = notmuch_message_add_tag (message, flag2tag[i].tag);
+	} else {
+	    status = notmuch_message_remove_tag (message, flag2tag[i].tag);
+	}
+	if (status)
+	    return status;
+    }
+    status = notmuch_message_thaw (message);
+
+    /* From now on, we can synchronize the tags from the database to
+     * the mailstore. */
+    notmuch_message_set_flag (message, NOTMUCH_MESSAGE_FLAG_TAGS_INVALID, FALSE);
+    return status;
+}
+
+static void
+maildir_get_new_flags(notmuch_message_t *message, char *flags)
+{
+    notmuch_tags_t *tags;
+    const char *tag;
+    unsigned i;
+    char *p;
+
+    for (i = 0; i < ARRAY_SIZE(flag2tag); i++)
+	flags[i] = flag2tag[i].inverse ? flag2tag[i].flag : '\0';
+
+    for (tags = notmuch_message_get_tags (message);
+	 notmuch_tags_valid (tags);
+	 notmuch_tags_move_to_next (tags))
+    {
+	tag = notmuch_tags_get (tags);
+	for (i = 0; i < ARRAY_SIZE(flag2tag); i++) {
+	    if (strcmp(tag, flag2tag[i].tag) == 0)
+		flags[i] = flag2tag[i].inverse ? '\0' : flag2tag[i].flag;
+	}
+    }
+
+    p = flags;
+    for (i = 0; i < ARRAY_SIZE(flag2tag); i++) {
+	if (flags[i])
+	    *p++ = flags[i];
+    }
+    *p = '\0';
+}
+
+static char *
+maildir_get_subdir (char *filename)
+{
+    char *p, *subdir = NULL;
+
+    p = filename + strlen (filename) - 1;
+    while (p > filename + 3 && *p != '/')
+	p--;
+    if (*p == '/') {
+	subdir = p - 3;
+	if (subdir > filename && *(subdir - 1) != '/')
+	    subdir = NULL;
+    }
+    return subdir;
+}
+
+/* Rename the message file so that maildir flags corresponds to the
+ * tags and, if aplicable, move the message from new/ to cur/. */
+static notmuch_private_status_t
+_notmuch_message_tags_to_maildir (notmuch_message_t *message)
+{
+    char flags[ARRAY_SIZE(flag2tag)+1];
+    const char *filename, *p;
+    char *filename_new, *subdir = NULL;
+    int ret;
+
+    maildir_get_new_flags (message, flags);
+
+    filename = notmuch_message_get_filename (message);
+    /* TODO: Iterate over all file names. */
+    p = strstr(filename, ":2,");
+    if ((p && strcmp (p+3, flags) == 0) ||
+	(!p && flags[0] == '\0')) {
+	// Return if flags are not to be changed - this suppresses
+	// moving the message from new/ to cur/ during initial
+	// tagging.
+	return NOTMUCH_PRIVATE_STATUS_SUCCESS;
+    }
+    if (!p)
+	p = filename + strlen(filename);
+
+    filename_new = (char*)talloc_size(message, (p-filename) + 3 + sizeof(flags));
+    if (unlikely (filename_new == NULL))
+	return NOTMUCH_PRIVATE_STATUS_OUT_OF_MEMORY;
+    memcpy(filename_new, filename, p-filename);
+    filename_new[p-filename] = '\0';
+
+    /* If message is in new/ move it under cur/. */
+    subdir = maildir_get_subdir (filename_new);
+    if (subdir && memcmp (subdir, "new/", 4) == 0)
+	memcpy (subdir, "cur/", 4);
+
+    strcpy (filename_new+(p-filename), ":2,");
+    strcpy (filename_new+(p-filename)+3, flags);
+
+    if (strcmp (filename, filename_new) != 0) {
+	ret = rename (filename, filename_new);
+	if (ret == -1) {
+	    perror (talloc_asprintf (message, "rename of %s to %s failed",
+				     filename, filename_new));
+	    exit (1);
+	}
+	return _notmuch_message_rename (message, filename_new);
+	/* _notmuch_message_sync is our caller. Do not call it here. */
+    }
+    return NOTMUCH_PRIVATE_STATUS_SUCCESS;
+}
+
+notmuch_status_t
 notmuch_message_remove_all_tags (notmuch_message_t *message)
 {
     notmuch_private_status_t private_status;
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 3768d6f..ba11498 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -246,6 +246,10 @@ notmuch_status_t
 _notmuch_message_add_filename (notmuch_message_t *message,
 			       const char *filename);
 
+notmuch_private_status_t
+_notmuch_message_rename (notmuch_message_t *message,
+			 const char *new_filename);
+
 void
 _notmuch_message_ensure_thread_id (notmuch_message_t *message);
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 0ba1416..408d633 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -770,6 +770,7 @@ notmuch_message_get_filename (notmuch_message_t *message);
 /* Message flags */
 typedef enum _notmuch_message_flag {
     NOTMUCH_MESSAGE_FLAG_MATCH,
+    NOTMUCH_MESSAGE_FLAG_TAGS_INVALID,
 } notmuch_message_flag_t;
 
 /* Get a value of a flag for the email corresponding to 'message'. */
@@ -886,6 +887,12 @@ notmuch_message_remove_tag (notmuch_message_t *message, const char *tag);
 notmuch_status_t
 notmuch_message_remove_all_tags (notmuch_message_t *message);
 
+/* Add or remove tags based on the maildir flags in the file name.
+ */
+notmuch_status_t
+notmuch_message_maildir_to_tags (notmuch_message_t *message,
+				 const char *filename);
+
 /* Freeze the current state of 'message' within the database.
  *
  * This means that changes to the message state, (via
diff --git a/notmuch-new.c b/notmuch-new.c
index 8818728..ed3f944 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -410,10 +410,11 @@ add_files_recursive (notmuch_database_t *notmuch,
 	    state->added_messages++;
 	    for (tag=state->new_tags; *tag != NULL; tag++)
 	        notmuch_message_add_tag (message, *tag);
+	    notmuch_message_maildir_to_tags (message, next);
 	    break;
 	/* Non-fatal issues (go on to next file) */
 	case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
-	    /* Stay silent on this one. */
+	    notmuch_message_maildir_to_tags (message, next);
 	    break;
 	case NOTMUCH_STATUS_FILE_NOT_EMAIL:
 	    fprintf (stderr, "Note: Ignoring non-mail file: %s\n",
-- 
1.7.1

[PATCH 3/4] Make maildir synchronization configurable

[Michal Sojka]

This adds group [maildir] and key 'sync_level' to the configuration file.
The value of sync_level is used to control how the synchronization happens.
The default value is no synchronization.
---
 lib/database-private.h |    2 +-
 lib/database.cc        |    9 +++++++++
 lib/message.cc         |    2 +-
 lib/notmuch.h          |   19 +++++++++++++++++++
 notmuch-client.h       |    7 +++++++
 notmuch-config.c       |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 notmuch-new.c          |    8 ++++++--
 notmuch-restore.c      |    2 ++
 notmuch-setup.c        |   17 +++++++++++++++++
 notmuch-tag.c          |    2 ++
 10 files changed, 112 insertions(+), 4 deletions(-)

diff --git a/lib/database-private.h b/lib/database-private.h
index 41918d7..370d779 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -48,7 +48,7 @@ struct _notmuch_database {
     Xapian::QueryParser *query_parser;
     Xapian::TermGenerator *term_gen;
     Xapian::ValueRangeProcessor *value_range_processor;
-
+    enum notmuch_maildir_sync maildir_sync;
 };
 
 /* Convert tags from Xapian internal format to notmuch format.
diff --git a/lib/database.cc b/lib/database.cc
index 908bbaa..bf645d0 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -662,6 +662,8 @@ notmuch_database_open (const char *path,
 	notmuch = NULL;
     }
 
+    notmuch_database_set_maildir_sync (notmuch, NOTMUCH_MAILDIR_SYNC_NONE);
+    
   DONE:
     if (notmuch_path)
 	free (notmuch_path);
@@ -691,6 +693,13 @@ notmuch_database_close (notmuch_database_t *notmuch)
     talloc_free (notmuch);
 }
 
+void
+notmuch_database_set_maildir_sync (notmuch_database_t *database,
+				   enum notmuch_maildir_sync maildir_sync)
+{
+    database->maildir_sync = maildir_sync;
+}
+
 const char *
 notmuch_database_get_path (notmuch_database_t *notmuch)
 {
diff --git a/lib/message.cc b/lib/message.cc
index 0de3f01..758e6fa 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -599,7 +599,7 @@ _notmuch_message_sync (notmuch_message_t *message)
     if (message->notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY)
 	return;
 
-    if (// todo_sync_enabled &&
+    if (message->notmuch->maildir_sync == NOTMUCH_MAILDIR_SYNC_NEW_RENAMED_TAGGED &&
 	!notmuch_message_get_flag(message, NOTMUCH_MESSAGE_FLAG_TAGS_INVALID)) {
  	status = _notmuch_message_tags_to_maildir (message);
 	if (status != NOTMUCH_PRIVATE_STATUS_SUCCESS) {
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 408d633..34b3bcf 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -108,6 +108,19 @@ typedef enum _notmuch_status {
 const char *
 notmuch_status_to_string (notmuch_status_t status);
 
+/* Level of synchronization between notmuch tags and maildir flags. */
+enum notmuch_maildir_sync {
+    NOTMUCH_MAILDIR_SYNC_INVALID = 0,
+    /* No synchronization */
+    NOTMUCH_MAILDIR_SYNC_NONE,
+    /* Tag new messages accoring to maildir flags */
+    NOTMUCH_MAILDIR_SYNC_NEW,
+    /* The above + update tags for renamed messages */
+    NOTMUCH_MAILDIR_SYNC_NEW_RENAMED,
+    /* The above + update flags when tags are added/removed. */
+    NOTMUCH_MAILDIR_SYNC_NEW_RENAMED_TAGGED,
+};
+
 /* Various opaque data types. For each notmuch_<foo>_t see the various
  * notmuch_<foo> functions below. */
 typedef struct _notmuch_database notmuch_database_t;
@@ -176,6 +189,12 @@ notmuch_database_open (const char *path,
 void
 notmuch_database_close (notmuch_database_t *database);
 
+/* Sets the level of synchronization between maildir flags and notmuch
+ * tags. */
+void
+notmuch_database_set_maildir_sync (notmuch_database_t *database,
+				   enum notmuch_maildir_sync maildir_sync);
+
 /* Return the database path of the given database.
  *
  * The return value is a string owned by notmuch so should not be
diff --git a/notmuch-client.h b/notmuch-client.h
index 20be43b..50be95c 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -191,6 +191,13 @@ notmuch_config_set_new_tags (notmuch_config_t *config,
 			     const char *new_tags[],
 			     size_t length);
 
+enum notmuch_maildir_sync
+notmuch_config_get_maildir_sync (notmuch_config_t *config);
+
+void
+notmuch_config_set_maildir_sync (notmuch_config_t *config,
+				 enum notmuch_maildir_sync maildir_sync);
+
 notmuch_bool_t
 debugger_is_active (void);
 
diff --git a/notmuch-config.c b/notmuch-config.c
index 58f83b0..5ac5f7b 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -61,6 +61,21 @@ static const char user_config_comment[] =
     " recipient list of replies, and will set the From address based on the\n"
     " address to which the original email was addressed.\n";
 
+static const char maildir_config_comment[] =
+    " Maildir compatibility configuration\n"
+    "\n"
+    " Here you can configure whether and how will notmuch synchronize its\n"
+    " tags with maildir flags."
+    "\n"
+    "\tsync_level      Integer in the range 1 - 4 with the following meaning:\n"
+    "\t\t1 - No synchronization at all.\n"
+    "\t\t2 - 'notmuch new' tags the messages based on their maildir flags\n"
+    "\t\t    only when it sees them for the first time.\n"
+    "\t\t3 - Same as 2 plus 'notmuch new' updates tags when it detects the\n"
+    "\t\t    message was renamed.\n"
+    "\t\t4 - Same as 3 plus whenever message tags are changed, maildir\n"
+    "\t\t    flags are updated accordingly.\n";
+
 struct _notmuch_config {
     char *filename;
     GKeyFile *key_file;
@@ -72,6 +87,7 @@ struct _notmuch_config {
     size_t user_other_email_length;
     const char **new_tags;
     size_t new_tags_length;
+    enum notmuch_maildir_sync maildir_sync;
 };
 
 static int
@@ -184,6 +200,7 @@ notmuch_config_open (void *ctx,
     int file_had_database_group;
     int file_had_new_group;
     int file_had_user_group;
+    int file_had_maildir_group;
 
     if (is_new_ret)
 	*is_new_ret = 0;
@@ -214,6 +231,7 @@ notmuch_config_open (void *ctx,
     config->user_other_email_length = 0;
     config->new_tags = NULL;
     config->new_tags_length = 0;
+    config->maildir_sync = NOTMUCH_MAILDIR_SYNC_INVALID;
 
     if (! g_key_file_load_from_file (config->key_file,
 				     config->filename,
@@ -254,6 +272,7 @@ notmuch_config_open (void *ctx,
 						    "database");
     file_had_new_group = g_key_file_has_group (config->key_file, "new");
     file_had_user_group = g_key_file_has_group (config->key_file, "user");
+    file_had_maildir_group = g_key_file_has_group (config->key_file, "maildir");
 
 
     if (notmuch_config_get_database_path (config) == NULL) {
@@ -304,6 +323,10 @@ notmuch_config_open (void *ctx,
 	notmuch_config_set_new_tags (config, tags, 2);
     }
 
+    if (notmuch_config_get_maildir_sync (config) == NOTMUCH_MAILDIR_SYNC_INVALID) {
+	notmuch_config_set_maildir_sync (config, NOTMUCH_MAILDIR_SYNC_NONE);
+    }
+
     /* Whenever we know of configuration sections that don't appear in
      * the configuration file, we add some comments to help the user
      * understand what can be done. */
@@ -331,6 +354,12 @@ notmuch_config_open (void *ctx,
 				user_config_comment, NULL);
     }
 
+    if (! file_had_maildir_group)
+    {
+	g_key_file_set_comment (config->key_file, "maildir", NULL,
+				maildir_config_comment, NULL);
+    }
+
     if (is_new_ret)
 	*is_new_ret = is_new;
 
@@ -553,3 +582,22 @@ notmuch_config_set_new_tags (notmuch_config_t *config,
     config->new_tags = NULL;
 }
 
+enum notmuch_maildir_sync
+notmuch_config_get_maildir_sync (notmuch_config_t *config)
+{
+    if (config->maildir_sync == NOTMUCH_MAILDIR_SYNC_INVALID) {
+	config->maildir_sync = 
+	    g_key_file_get_integer (config->key_file,
+				    "maildir", "sync_level", NULL);
+    }
+    return config->maildir_sync;
+}
+
+void
+notmuch_config_set_maildir_sync (notmuch_config_t *config,
+				 enum notmuch_maildir_sync maildir_sync)
+{
+    g_key_file_set_integer (config->key_file,
+			    "maildir", "sync_level", maildir_sync);
+    config->maildir_sync = maildir_sync;
+}
diff --git a/notmuch-new.c b/notmuch-new.c
index ed3f944..40ea610 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -45,6 +45,7 @@ typedef struct {
 
     _filename_list_t *removed_files;
     _filename_list_t *removed_directories;
+    enum notmuch_maildir_sync maildir_sync;
 } add_files_state_t;
 
 static volatile sig_atomic_t do_add_files_print_progress = 0;
@@ -410,11 +411,13 @@ add_files_recursive (notmuch_database_t *notmuch,
 	    state->added_messages++;
 	    for (tag=state->new_tags; *tag != NULL; tag++)
 	        notmuch_message_add_tag (message, *tag);
-	    notmuch_message_maildir_to_tags (message, next);
+	    if (state->maildir_sync >= NOTMUCH_MAILDIR_SYNC_NEW)
+		notmuch_message_maildir_to_tags (message, next);
 	    break;
 	/* Non-fatal issues (go on to next file) */
 	case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
-	    notmuch_message_maildir_to_tags (message, next);
+	    if (state->maildir_sync >= NOTMUCH_MAILDIR_SYNC_NEW_RENAMED)
+		notmuch_message_maildir_to_tags (message, next);
 	    break;
 	case NOTMUCH_STATUS_FILE_NOT_EMAIL:
 	    fprintf (stderr, "Note: Ignoring non-mail file: %s\n",
@@ -738,6 +741,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 	return 1;
 
     add_files_state.new_tags = notmuch_config_get_new_tags (config, &add_files_state.new_tags_length);
+    add_files_state.maildir_sync = notmuch_config_get_maildir_sync (config);
     db_path = notmuch_config_get_database_path (config);
 
     dot_notmuch_path = talloc_asprintf (ctx, "%s/%s", db_path, ".notmuch");
diff --git a/notmuch-restore.c b/notmuch-restore.c
index b0a4e1c..b5c5c48 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -41,6 +41,8 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
     if (notmuch == NULL)
 	return 1;
 
+    notmuch_database_set_maildir_sync (notmuch,
+				       notmuch_config_get_maildir_sync (config));
     if (argc) {
 	input = fopen (argv[0], "r");
 	if (input == NULL) {
diff --git a/notmuch-setup.c b/notmuch-setup.c
index 955deb7..8364854 100644
--- a/notmuch-setup.c
+++ b/notmuch-setup.c
@@ -195,6 +195,23 @@ notmuch_setup_command (unused (void *ctx),
 	g_ptr_array_free (tags, TRUE);
     }
 
+    printf ("\n"
+	    "Notmuch can synchronize certain tags with maildir flags. You can\n"
+	    "select between several levels of synchronization:\n"
+	    "1 - No synchronization at all.\n"
+	    "2 - 'notmuch new' tags the messages based on their maildir flags\n"
+	    "    only when it sees them for the first time.\n"
+	    "3 - Same as 2 plus 'notmuch new' updates tags when it detects the\n"
+	    "    message was renamed.\n"
+	    "4 - Same as 3 plus whenever message tags are changed, maildir\n"
+	    "    flags are updated accordingly.\n");
+
+    prompt ("Maildir synchronization level [%d]: ", notmuch_config_get_maildir_sync (config));
+    if (strlen (response) == 1 &&
+	response[0] >= '1' &&
+	response[0] <= '4')
+	notmuch_config_set_maildir_sync (config, atoi (response));
+
     if (! notmuch_config_save (config)) {
 	if (is_new)
 	  welcome_message_post_setup ();
diff --git a/notmuch-tag.c b/notmuch-tag.c
index fd54bc7..3a489a9 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -100,6 +100,8 @@ notmuch_tag_command (void *ctx, unused (int argc), unused (char *argv[]))
 				     NOTMUCH_DATABASE_MODE_READ_WRITE);
     if (notmuch == NULL)
 	return 1;
+    notmuch_database_set_maildir_sync (notmuch,
+				       notmuch_config_get_maildir_sync (config));
 
     query = notmuch_query_create (notmuch, query_string);
     if (query == NULL) {
-- 
1.7.1

[PATCH 4/4] Tests for maildir synchronization

[Michal Sojka]

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 test/t0011-maildir-sync.sh |  216 ++++++++++++++++++++++++++++++++++++++++++++
 test/test-lib.sh           |   11 ++-
 2 files changed, 223 insertions(+), 4 deletions(-)
 create mode 100755 test/t0011-maildir-sync.sh

diff --git a/test/t0011-maildir-sync.sh b/test/t0011-maildir-sync.sh
new file mode 100755
index 0000000..6bdee2d
--- /dev/null
+++ b/test/t0011-maildir-sync.sh
@@ -0,0 +1,216 @@
+#!/bin/bash
+
+test_description="Test maildir synchronization"
+
+. ./test-lib.sh
+
+filter_show() {
+    sed -e 's/, /,\n/g'|sed -e "s|${MAIL_DIR}/||" -e '/^"tags"/d'
+    echo
+}
+
+cat >> "$NOTMUCH_CONFIG" <<EOF
+[maildir]
+sync_level=4
+EOF
+
+test_expect_success "No new mail" '
+output=$(NOTMUCH_NEW) &&
+pass_if_equal "$output" "No new mail."
+'
+cat > expected <<EOF
+Added 1 new message to the database.
+EOF
+test_expect_success "Add a message, no flags" '
+generate_message [subject]="\"test message\"" [date]="\"Sat, 01 Jan 2000 12:00:00 -0000\"" [filename]="\"msg-001:2,\"" &&
+NOTMUCH_NEW > actual &&
+test_cmp expected actual
+#emacs --eval "(gdb \"gdb --annotate=3 --args $(which notmuch) new\")"
+'
+cat > expected <<EOF
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message (inbox unread)
+EOF
+test_expect_success 'Search for the message' '
+notmuch search tag:inbox and tag:unread | notmuch_search_sanitize > actual &&
+test_cmp expected actual
+'
+cat > expected <<EOF
+No new mail. Detected 1 file rename.
+EOF
+test_expect_success 'Add seen flag' '
+mv "${gen_msg_filename}" "${gen_msg_filename}S" &&
+increment_mtime "$(dirname "${gen_msg_filename}")" &&
+NOTMUCH_NEW > actual &&
+test_cmp expected actual
+'
+cat > expected <<EOF
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message (inbox)
+EOF
+test_expect_success 'Check that tags were updated' '
+notmuch search tag:inbox and not tag:unread | notmuch_search_sanitize > actual &&
+test_cmp expected actual
+'
+cat > expected <<EOF
+Added 1 new message to the database.
+EOF
+test_expect_success "Add a seen message" '
+generate_message [subject]="\"test message 2\"" [date]="\"Sat, 01 Jan 2000 12:00:00 -0000\"" [filename]="\"msg-002:2,S\"" &&
+NOTMUCH_NEW > actual &&
+test_cmp expected actual
+'
+cat > expected <<EOF
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message (inbox)
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message 2 (inbox)
+EOF
+test_expect_success 'Check that the seen message is not tagged unread' '
+notmuch search tag:inbox and not tag:unread | notmuch_search_sanitize > actual &&
+test_cmp expected actual
+'
+test_expect_success 'Tag the seen messages as replied' '
+notmuch tag +replied -inbox tag:inbox and not tag:unread
+'
+
+cat > expected <<EOF
+msg-001:2,RS
+msg-002:2,RS
+EOF
+test_expect_success 'Check that R flag was added' '
+ls -1 "${MAIL_DIR}" > actual &&
+test_cmp expected actual
+'
+cat <<EOF > show-expected
+[[[{"id": "msg-001@notmuch-test-suite",
+"match": true,
+"filename": "msg-001:2,RS",
+"timestamp": 946728000,
+"date_relative": "2000-01-01",
+"headers": {"Subject": "test message",
+"From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
+"To": "Notmuch Test Suite <test_suite@notmuchmail.org>",
+"Cc": "",
+"Bcc": "",
+"Date": "Sat,
+01 Jan 2000 12:00:00 -0000"},
+"body": [{"id": 1,
+"content-type": "text/plain",
+"content": "This is just a test message (#1)\n"}]},
+[]]]]
+EOF
+
+test_expect_success 'Renamed message can be shown without running notmuch new' '
+notmuch show --format=json id:msg-001@notmuch-test-suite | filter_show > show-actual &&
+test_cmp show-expected show-actual
+'
+
+test_expect_success 'Test that we can reply to the renamed message' '
+notmuch reply id:msg-001@notmuch-test-suite
+'
+
+echo "No new mail." > expected
+test_expect_success 'No rename should be detected by notmuch new' '
+increment_mtime "$(dirname "${gen_msg_filename}")" &&
+notmuch new > actual &&
+test_cmp expected actual
+'
+test_expect_success "Add a message to new/ without info" '
+generate_message [subject]="\"test message 3\"" [date]="\"Sat, 01 Jan 2000 12:00:00 -0000\"" [dir]=new &&
+NOTMUCH_NEW > actual &&
+test_cmp - actual <<EOF
+Added 1 new message to the database.
+EOF
+'
+test_expect_success "Check that the message has inbox and unread tags" '
+notmuch search tag:inbox and tag:unread | notmuch_search_sanitize > actual &&
+test_cmp - actual <<EOF
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message 3 (inbox unread)
+EOF
+'
+test_expect_success "Tag the message with 'tmp' tag" '
+notmuch tag +tmp tag:inbox and tag:unread'
+
+test_expect_success "Check that the message was not moved from new/ to cur/" '
+echo filename:$gen_msg_filename > expected &&
+notmuch show id:$gen_msg_id|grep -o "filename:.*$" > actual &&
+test_cmp expected actual &&
+test -f "$gen_msg_filename"
+'
+test_expect_success "Check that the message was not renamed" '
+ls "${MAIL_DIR}/new" > actual &&
+test_cmp - actual <<EOF
+msg-003
+EOF
+'
+test_expect_success 'Removing of unread tag should fail without cur/' '
+test_must_fail notmuch tag -unread tag:inbox and tag:unread
+'
+test_expect_success "Check that the tags were not changed" '
+notmuch search tag:inbox and tag:unread | notmuch_search_sanitize > actual &&
+test_cmp - actual <<EOF
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message 3 (inbox tmp unread)
+EOF
+'
+
+# notmuch new is not necessary here, but we run it in order to check
+# for 'no rename' later (*).
+test_expect_success 'Create cur/ and let notmuch know about it' '
+mkdir "$MAIL_DIR/cur" &&
+notmuch new
+'
+test_expect_success 'Removing of unread tag should pass with cur/' '
+notmuch tag -unread tag:inbox and tag:unread
+'
+test_expect_success 'Check that the message was moved to cur/' '\
+ls "$MAIL_DIR/cur" > actual &&
+test_cmp - actual <<EOF
+msg-003:2,S
+EOF
+'
+test_expect_success 'No rename should be detected by notmuch new' '
+increment_mtime "$MAIL_DIR/cur" &&
+notmuch new > actual &&
+test_cmp - actual <<EOF
+No new mail.
+EOF
+'
+# (*) If notmuch new was not run we've got "Processed 1 file in almost
+# no time" here. The reason is that removing unread tag in a previous
+# test created directory document in the database but this document
+# was not linked as subdirectory of $MAIL_DIR. Therefore notmuch new
+# could not reach the cur/ directory and its files in it during
+# recurive traversal.
+test_expect_success 'Remove info from file name' '
+mv "$MAIL_DIR/cur/msg-003:2,S" "$MAIL_DIR/cur/msg-003" &&
+increment_mtime "$MAIL_DIR/cur" &&
+NOTMUCH_NEW > actual
+test_cmp - actual <<EOF
+No new mail. Detected 1 file rename.
+EOF
+'
+test_expect_success "Check that removing info did not change tags" '
+notmuch search tag:inbox | notmuch_search_sanitize > actual &&
+test_cmp - actual <<EOF
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message 3 (inbox tmp)
+EOF
+'
+test_expect_success "Add a message to fakenew/ without info" '
+generate_message [subject]="\"test message 4\"" [date]="\"Sat, 01 Jan 2000 12:00:00 -0000\"" [dir]=fakenew &&
+NOTMUCH_NEW > actual &&
+test_cmp - actual <<EOF
+Added 1 new message to the database.
+EOF
+'
+test_expect_success "Check that the message has inbox and unread tags" '
+notmuch search tag:inbox and tag:unread | notmuch_search_sanitize > actual &&
+test_cmp - actual <<EOF
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message 4 (inbox unread)
+EOF
+'
+test_expect_success 'Removing of unread tag should leave the message in fakenew/' '
+notmuch tag -unread tag:inbox and tag:unread &&
+ls "$MAIL_DIR/fakenew" > actual &&
+test_cmp - actual <<EOF
+msg-004:2,S
+EOF
+'
+
+test_done
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 06559cd..3c288a5 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -276,12 +276,15 @@ generate_message ()
     local -A template="($@)"
     local additional_headers
 
+    gen_msg_cnt=$((gen_msg_cnt + 1))
+    if [ -z "${template[filename]}" ]; then
+	template[filename]="msg-$(printf "%03d" $gen_msg_cnt)"
+    fi
+    gen_msg_name=${template[filename]}
+
     if [ -z "${template[id]}" ]; then
-	gen_msg_cnt=$((gen_msg_cnt + 1))
-	gen_msg_name=msg-$(printf "%03d" $gen_msg_cnt)
-	gen_msg_id="${gen_msg_name}@notmuch-test-suite"
+	gen_msg_id="${gen_msg_name%:2,*}@notmuch-test-suite"
     else
-	gen_msg_name="msg-${template[id]}"
 	gen_msg_id="${template[id]}"
     fi
 
-- 
1.7.1

Re: [PATCH 0/4] Maildir synchronization

[Matt Fleming]

On Tue, 11 May 2010 14:14:17 +0200, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> Hi,
> 
> these patches implement synchronization between maildir flags and
> notmuch tags. The synchronization can be configured to not happen at
> all (default), to set/unset tags when importing new (or new and
> renamed) messages and to happen in both directions - set/unset tags
> during importing and change maildir flags during tagging.

I finally got around to testing these patches and I think they're great!
It would be fantastic if these could be merged into the main notmuch
tree.

FWIW, I've been using some elisp to emulate the behaviour of these
patches ever since I started using notmuch - it's a necessity for my
workflow. Your patches provide a much more complete solution than the
bit of elisp I hacked up so I'll be using these from now on.

Re: [PATCH 0/4] Maildir synchronization

[Dirk Hohndel]

On Wed, 09 Jun 2010 19:52:42 +0100, Matt Fleming <matt@console-pimps.org> wrote:
> On Tue, 11 May 2010 14:14:17 +0200, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> > Hi,
> > 
> > these patches implement synchronization between maildir flags and
> > notmuch tags. The synchronization can be configured to not happen at
> > all (default), to set/unset tags when importing new (or new and
> > renamed) messages and to happen in both directions - set/unset tags
> > during importing and change maildir flags during tagging.
> 
> I finally got around to testing these patches and I think they're great!
> It would be fantastic if these could be merged into the main notmuch
> tree.

I've been using them for a while as well and love the feature, but I
keep running into situations where notmuch seems to get out of sync
regarding file names on disk. I haven't done a lot of searching on what
exactly causes this - but the symptom is that you'll open a message,
read it and then try to do something on it (like, save an attachment)
and suddenly are told that the message file on disk can't be found.
Or that you reply to a message and just as you are trying to send the
reply things fail for the same reason.

Next time this happens I'll take a closer look at the filenames
displayed to see what's going on.

/D

-- 
Dirk Hohndel
Intel Open Source Technology Center

Re: [PATCH 0/4] Maildir synchronization

[Michal Sojka]

On Thu, 10 Jun 2010, Dirk Hohndel wrote:
> On Wed, 09 Jun 2010 19:52:42 +0100, Matt Fleming <matt@console-pimps.org> wrote:
> I've been using them for a while as well and love the feature, but I
> keep running into situations where notmuch seems to get out of sync
> regarding file names on disk. I haven't done a lot of searching on what
> exactly causes this - but the symptom is that you'll open a message,
> read it and then try to do something on it (like, save an attachment)
> and suddenly are told that the message file on disk can't be found.
> Or that you reply to a message and just as you are trying to send the
> reply things fail for the same reason.

This is a known limitation.
From id:1273580061-22580-3-git-send-email-sojkam1@fel.cvut.cz:

   The reason is that when you view the message its unread tag is
   removed which leads to rename of the file, but Emacs still uses the
   original name to access the attachment.

   Workaround: close the message and open it again.

IMHO, the final solution to this issue would be the "notmuch cat"
command. With this command, emacs would not access the messages by file
name, but by message id. My previous notmuch cat patches didn't have
support for message id and I plan to work on this but not earlier than
the next month.

Bye
Michal

Re: [PATCH 0/4] Maildir synchronization

[Carl Worth]

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

On Thu, 10 Jun 2010 06:59:02 +0200, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> This is a known limitation.
> From id:1273580061-22580-3-git-send-email-sojkam1@fel.cvut.cz:
> 
>    The reason is that when you view the message its unread tag is
>    removed which leads to rename of the file, but Emacs still uses the
>    original name to access the attachment.
> 
>    Workaround: close the message and open it again.

Hi Michal,

These patches do indeed look very interesting. But the above limitation
is really too severe. It just breaks things to much. Let's get that
fixed first.

> IMHO, the final solution to this issue would be the "notmuch cat"
> command. With this command, emacs would not access the messages by file
> name, but by message id.

Sounds like a great idea. Instead of "notmuch cat", how about we name
this "notmuch show --format=raw"? That should be even easier to
implement, too.

Then, I think I'll be very interested in the maildir-synchronization
patches, and further I'd like to have these enabled by default. After,
all the #1 item in the TODO list for quite some time has been:

1. A new import is tagging all messages as "inbox" -- total pain

And this has the potential to finally fix that.

Finally, as for configuration, I don't like the numeric codes for this
feature. Do we really need that much granularity in the functionality
here? Other mail clients certainly don't. From what I can see, most mail
clients just twiddle these flags unconditionally.

I can imagine some people might want to be able to turn the feature off
entirely, so maybe we'll need that.

Or perhaps more importantly than configuration, we need the ability to
easily migrate people to a synchronized state. For example, in my
current mail store, most filenames have never been changed, so I've got
a lot of files with flags that don't match my tags. What do you think
would be the best way to resolve a situation like that?

Looking forward to more here,

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 0/4] Maildir synchronization

[Michal Sojka]

On Sat, 30 Oct 2010, Carl Worth wrote:
> On Thu, 10 Jun 2010 06:59:02 +0200, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> > This is a known limitation.
> > From id:1273580061-22580-3-git-send-email-sojkam1@fel.cvut.cz:
> > 
> >    The reason is that when you view the message its unread tag is
> >    removed which leads to rename of the file, but Emacs still uses the
> >    original name to access the attachment.
> > 
> >    Workaround: close the message and open it again.
> 
> These patches do indeed look very interesting. But the above limitation
> is really too severe. It just breaks things to much. Let's get that
> fixed first.
> 
> > IMHO, the final solution to this issue would be the "notmuch cat"
> > command. With this command, emacs would not access the messages by file
> > name, but by message id.
> 
> Sounds like a great idea. Instead of "notmuch cat", how about we name
> this "notmuch show --format=raw"? That should be even easier to
> implement, too.

See id:1287739684-26188-1-git-send-email-sojkam1@fel.cvut.cz for my last
attempt to implement this. I didn't implement it as --format=raw because
it seems that notmuch show always constructs threads even if they are
not used. I didn't check the code carefully so I may be wrong. Let me
know if you really prefer raw format over cat.

> Finally, as for configuration, I don't like the numeric codes for this
> feature. Do we really need that much granularity in the functionality
> here? Other mail clients certainly don't. From what I can see, most mail
> clients just twiddle these flags unconditionally.
> 
> I can imagine some people might want to be able to turn the feature off
> entirely, so maybe we'll need that.>

I agree that having only on/off settings should be sufficient for most
users. I'll send updated patches in a while.
 
> Or perhaps more importantly than configuration, we need the ability to
> easily migrate people to a synchronized state. For example, in my
> current mail store, most filenames have never been changed, so I've got
> a lot of files with flags that don't match my tags. What do you think
> would be the best way to resolve a situation like that?

One thing is that if you simply enable maildir synchronization and run
notmuch new, it should not touch your tags as the tags are modified only
when a new or renamed message is found. So if one doesn't modify the
flags by other programs it is safe to enable maildir synchronization.

Then, if other programs that may modify the flags are used, the mail
store flags should be made synchronized. One way of doing this manually
is to execute the following sequence of commands after enabling maildir
synchronization.

notmuch dump > x    # stores the tags
notmuch new         # makes sure that the file names in the database are up to date
notmuch restore < x # sets maildir flags to match the tags

If you want this to happen automatically, it might be possible to
modify notmuch new to use something like
notmuch->xapian_db->get_metadata("maildir_in_sync") and if this metadata
is not found and the maildir synchronization is enabled then it would
synchronize the flags and set the database metadata.

-Michal

[PATCH v4 0/4] Maildir synchronization

[Michal Sojka]

This is the next iteration of maildir synchronization patches. The
changes are:
- Configuration is now simplified. The synchronization can only be
  full enabled or disabled. By default it is still disabled.
- Added test for notmuch restore (with enabled synchronization)
- Rebased to the current master
- 'D' flag is mapped to 'deleted' tag instead of 'delete' (this has
  already been in v3)

Can be pulled by
git pull git://rtime.felk.cvut.cz/notmuch tags/maildir-sync-v4

Michal Sojka (4):
  lib: Return added message even if it already was in the database
  Maildir synchronization
  Make maildir synchronization configurable
  Tests for maildir synchronization

 lib/database-private.h |    2 +-
 lib/database.cc        |   19 ++++-
 lib/message.cc         |  226 ++++++++++++++++++++++++++++++++++++++++++++++
 lib/notmuch-private.h  |    4 +
 lib/notmuch.h          |   16 +++-
 notmuch-client.h       |    7 ++
 notmuch-config.c       |   49 ++++++++++
 notmuch-new.c          |    7 ++-
 notmuch-restore.c      |    2 +
 notmuch-setup.c        |   10 ++
 notmuch-tag.c          |    2 +
 test/maildir-sync      |  231 ++++++++++++++++++++++++++++++++++++++++++++++++
 test/notmuch-test      |    3 +-
 test/test-lib.sh       |   14 +++-
 14 files changed, 585 insertions(+), 7 deletions(-)
 create mode 100755 test/maildir-sync

[PATCH v4 1/4] lib: Return added message even if it already was in the database

[Michal Sojka]

---
 lib/database.cc |    3 ++-
 lib/notmuch.h   |    3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index e4ac970..9a4f715 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1671,7 +1671,8 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
 
   DONE:
     if (message) {
-	if (ret == NOTMUCH_STATUS_SUCCESS && message_ret)
+	if ((ret == NOTMUCH_STATUS_SUCCESS ||
+	     ret == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) && message_ret)
 	    *message_ret = message;
 	else
 	    notmuch_message_destroy (message);
diff --git a/lib/notmuch.h b/lib/notmuch.h
index bd0880f..61c68d6 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -238,7 +238,8 @@ notmuch_database_get_directory (notmuch_database_t *database,
  * notmuch database will reference the filename, and will not copy the
  * entire contents of the file.
  *
- * If 'message' is not NULL, then, on successful return '*message'
+ * If 'message' is not NULL, then, on successful return
+ * (NOTMUCH_STATUS_SUCCESS or NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) '*message'
  * will be initialized to a message object that can be used for things
  * such as adding tags to the just-added message. The user should call
  * notmuch_message_destroy when done with the message. On any failure
-- 
1.7.1

[PATCH v4 2/4] Maildir synchronization

[Michal Sojka]

This patch allows bi-directional synchronization between maildir
flags and certain tags. The flag-to-tag mapping is defined by flag2tag
array.

The synchronization works this way:

1) Whenever notmuch new is executed, the following happens:
   o New messages are tagged with configured new_tags.
   o For new or renamed messages with maildir info present in the file
     name, the tags defined in flag2tag are either added or removed
     depending on the flags from the file name.

2) Whenever notmuch tag (or notmuch restore) is executed, a new set of
   flags based on the tags is constructed for every message and a new
   file name is prepared based on the old file name but with the new
   flags. If the flags differs and the old message was in 'new'
   directory then this is replaced with 'cur' in the new file name. If
   the new and old file names differ, the file is renamed and notmuch
   database is updated accordingly.

   The rename happens before the database is updated. In case of crash
   between rename and database update, the next run of notmuch new
   brings the database in sync with the mail store again.

There is currently one known issue: Viewing/storing of attachments of
unread messages doesn't work. The reason is that when you view the
message its unread tag is removed which leads to rename of the file,
but Emacs still uses the original name to access the attachment.

Workaround: close the message and open it again.
---
 lib/database.cc       |    7 ++
 lib/message.cc        |  226 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/notmuch-private.h |    4 +
 lib/notmuch.h         |    7 ++
 notmuch-new.c         |    3 +-
 5 files changed, 246 insertions(+), 1 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 9a4f715..9652013 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1643,6 +1643,13 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
 
 	_notmuch_message_add_filename (message, filename);
 
+	/* This is a new message or it has a new filename and as such,
+	 * its tags in database either do not exists or might be out
+	 * of date. We assign the tags later in notmuch new, but until
+	 * then we should not synchronize the tags back to the maildir
+	 * flags (if notmuch is configured to do so). */
+	notmuch_message_set_flag(message, NOTMUCH_MESSAGE_FLAG_TAGS_INVALID, TRUE);
+
 	/* Is this a newly created message object? */
 	if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
 	    _notmuch_message_add_term (message, "type", "mail");
diff --git a/lib/message.cc b/lib/message.cc
index 71f5619..bb96242 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -43,6 +43,24 @@ struct _notmuch_message {
     Xapian::Document doc;
 };
 
+#define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))
+
+struct maildir_flag_tag {
+    char flag;
+    const char *tag;
+    bool inverse;
+};
+
+/* ASCII ordered table of Maildir flags and associated tags */
+struct maildir_flag_tag flag2tag[] = {
+    { 'D', "draft",   false},
+    { 'F', "flagged", false},
+    { 'P', "passed",  false},
+    { 'R', "replied", false},
+    { 'S', "unread",  true },
+    { 'T', "deleted", false},
+};
+
 /* We end up having to call the destructor explicitly because we had
  * to use "placement new" in order to initialize C++ objects within a
  * block that we allocated with talloc. So C++ is making talloc
@@ -595,15 +613,29 @@ _notmuch_message_set_date (notmuch_message_t *message,
 			    Xapian::sortable_serialise (time_value));
 }
 
+static notmuch_private_status_t
+_notmuch_message_tags_to_maildir (notmuch_message_t *message);
+
 /* Synchronize changes made to message->doc out into the database. */
 void
 _notmuch_message_sync (notmuch_message_t *message)
 {
     Xapian::WritableDatabase *db;
+    notmuch_private_status_t status;
 
     if (message->notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY)
 	return;
 
+    if (// todo_sync_enabled &&
+	!notmuch_message_get_flag(message, NOTMUCH_MESSAGE_FLAG_TAGS_INVALID)) {
+ 	status = _notmuch_message_tags_to_maildir (message);
+	if (status != NOTMUCH_PRIVATE_STATUS_SUCCESS) {
+	    fprintf (stderr, "Error: Cannot sync tags to maildir (%s)\n",
+		     notmuch_status_to_string ((notmuch_status_t)status));
+	    /* Exit to avoid unsynchronized mailstore. */
+	    exit(1);
+	}
+    }
     db = static_cast <Xapian::WritableDatabase *> (message->notmuch->xapian_db);
     db->replace_document (message->doc_id, message->doc);
 }
@@ -715,6 +747,44 @@ _notmuch_message_remove_term (notmuch_message_t *message,
     return NOTMUCH_PRIVATE_STATUS_SUCCESS;
 }
 
+/* Change the message filename stored in the database.
+ *
+ * This change will not be reflected in the database until the next
+ * call to _notmuch_message_sync.
+ */
+notmuch_private_status_t
+_notmuch_message_rename (notmuch_message_t *message,
+			 const char *new_filename)
+{
+    void *local = talloc_new (message);
+    char *direntry;
+    Xapian::PostingIterator i, end;
+    Xapian::Document document;
+    notmuch_private_status_t pstatus;
+    notmuch_status_t status;
+    const char *old_filename;
+
+    old_filename = notmuch_message_get_filename(message);
+    old_filename = talloc_reference(local, old_filename);
+    if (unlikely(!old_filename))
+	return NOTMUCH_PRIVATE_STATUS_OUT_OF_MEMORY;
+
+    status = _notmuch_message_add_filename (message, new_filename);
+    if (status)
+	return (notmuch_private_status_t)status;
+
+    status = _notmuch_database_filename_to_direntry (local, message->notmuch,
+						     old_filename, &direntry);
+    if (status)
+	return (notmuch_private_status_t)status;
+
+    pstatus = _notmuch_message_remove_term (message, "file-direntry", direntry);
+
+    talloc_free (local);
+
+    return pstatus;
+}
+
 notmuch_status_t
 notmuch_message_add_tag (notmuch_message_t *message, const char *tag)
 {
@@ -772,6 +842,162 @@ notmuch_message_remove_tag (notmuch_message_t *message, const char *tag)
 }
 
 notmuch_status_t
+notmuch_message_maildir_to_tags (notmuch_message_t *message, const char *filename)
+{
+    const char *flags, *p;
+    char f;
+    bool valid, unread;
+    unsigned i;
+    notmuch_status_t status;
+
+    flags = strstr (filename, ":2,");
+    if (!flags)
+	return NOTMUCH_STATUS_FILE_NOT_EMAIL;
+    flags += 3;
+
+    /*  Check that the letters are valid Maildir flags */
+    f = 0;
+    valid = true;
+    for (p=flags; valid && *p; p++) {
+	switch (*p) {
+	case 'P':
+	case 'R':
+	case 'S':
+	case 'T':
+	case 'D':
+	case 'F':
+	    if (*p > f) f=*p;
+	    else valid = false;
+	break;
+	default:
+	    valid = false;
+	}
+    }
+    if (!valid) {
+	fprintf (stderr, "Warning: Invalid maildir flags in filename %s\n", filename);
+	return NOTMUCH_STATUS_FILE_NOT_EMAIL;
+    }
+
+    status = notmuch_message_freeze (message);
+    if (status)
+	return status;
+    unread = true;
+    for (i = 0; i < ARRAY_SIZE(flag2tag); i++) {
+	if ((strchr (flags, flag2tag[i].flag) != NULL) ^ flag2tag[i].inverse) {
+	    status = notmuch_message_add_tag (message, flag2tag[i].tag);
+	} else {
+	    status = notmuch_message_remove_tag (message, flag2tag[i].tag);
+	}
+	if (status)
+	    return status;
+    }
+    status = notmuch_message_thaw (message);
+
+    /* From now on, we can synchronize the tags from the database to
+     * the mailstore. */
+    notmuch_message_set_flag (message, NOTMUCH_MESSAGE_FLAG_TAGS_INVALID, FALSE);
+    return status;
+}
+
+static void
+maildir_get_new_flags(notmuch_message_t *message, char *flags)
+{
+    notmuch_tags_t *tags;
+    const char *tag;
+    unsigned i;
+    char *p;
+
+    for (i = 0; i < ARRAY_SIZE(flag2tag); i++)
+	flags[i] = flag2tag[i].inverse ? flag2tag[i].flag : '\0';
+
+    for (tags = notmuch_message_get_tags (message);
+	 notmuch_tags_valid (tags);
+	 notmuch_tags_move_to_next (tags))
+    {
+	tag = notmuch_tags_get (tags);
+	for (i = 0; i < ARRAY_SIZE(flag2tag); i++) {
+	    if (strcmp(tag, flag2tag[i].tag) == 0)
+		flags[i] = flag2tag[i].inverse ? '\0' : flag2tag[i].flag;
+	}
+    }
+
+    p = flags;
+    for (i = 0; i < ARRAY_SIZE(flag2tag); i++) {
+	if (flags[i])
+	    *p++ = flags[i];
+    }
+    *p = '\0';
+}
+
+static char *
+maildir_get_subdir (char *filename)
+{
+    char *p, *subdir = NULL;
+
+    p = filename + strlen (filename) - 1;
+    while (p > filename + 3 && *p != '/')
+	p--;
+    if (*p == '/') {
+	subdir = p - 3;
+	if (subdir > filename && *(subdir - 1) != '/')
+	    subdir = NULL;
+    }
+    return subdir;
+}
+
+/* Rename the message file so that maildir flags corresponds to the
+ * tags and, if aplicable, move the message from new/ to cur/. */
+static notmuch_private_status_t
+_notmuch_message_tags_to_maildir (notmuch_message_t *message)
+{
+    char flags[ARRAY_SIZE(flag2tag)+1];
+    const char *filename, *p;
+    char *filename_new, *subdir = NULL;
+    int ret;
+
+    maildir_get_new_flags (message, flags);
+
+    filename = notmuch_message_get_filename (message);
+    /* TODO: Iterate over all file names. */
+    p = strstr(filename, ":2,");
+    if ((p && strcmp (p+3, flags) == 0) ||
+	(!p && flags[0] == '\0')) {
+	// Return if flags are not to be changed - this suppresses
+	// moving the message from new/ to cur/ during initial
+	// tagging.
+	return NOTMUCH_PRIVATE_STATUS_SUCCESS;
+    }
+    if (!p)
+	p = filename + strlen(filename);
+
+    filename_new = (char*)talloc_size(message, (p-filename) + 3 + sizeof(flags));
+    if (unlikely (filename_new == NULL))
+	return NOTMUCH_PRIVATE_STATUS_OUT_OF_MEMORY;
+    memcpy(filename_new, filename, p-filename);
+    filename_new[p-filename] = '\0';
+
+    /* If message is in new/ move it under cur/. */
+    subdir = maildir_get_subdir (filename_new);
+    if (subdir && memcmp (subdir, "new/", 4) == 0)
+	memcpy (subdir, "cur/", 4);
+
+    strcpy (filename_new+(p-filename), ":2,");
+    strcpy (filename_new+(p-filename)+3, flags);
+
+    if (strcmp (filename, filename_new) != 0) {
+	ret = rename (filename, filename_new);
+	if (ret == -1) {
+	    perror (talloc_asprintf (message, "rename of %s to %s failed",
+				     filename, filename_new));
+	    exit (1);
+	}
+	return _notmuch_message_rename (message, filename_new);
+	/* _notmuch_message_sync is our caller. Do not call it here. */
+    }
+    return NOTMUCH_PRIVATE_STATUS_SUCCESS;
+}
+
+notmuch_status_t
 notmuch_message_remove_all_tags (notmuch_message_t *message)
 {
     notmuch_private_status_t private_status;
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 5a0cf92..1dc0a20 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -259,6 +259,10 @@ notmuch_status_t
 _notmuch_message_add_filename (notmuch_message_t *message,
 			       const char *filename);
 
+notmuch_private_status_t
+_notmuch_message_rename (notmuch_message_t *message,
+			 const char *new_filename);
+
 void
 _notmuch_message_ensure_thread_id (notmuch_message_t *message);
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 61c68d6..fe01e73 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -780,6 +780,7 @@ notmuch_message_get_filename (notmuch_message_t *message);
 /* Message flags */
 typedef enum _notmuch_message_flag {
     NOTMUCH_MESSAGE_FLAG_MATCH,
+    NOTMUCH_MESSAGE_FLAG_TAGS_INVALID,
 } notmuch_message_flag_t;
 
 /* Get a value of a flag for the email corresponding to 'message'. */
@@ -896,6 +897,12 @@ notmuch_message_remove_tag (notmuch_message_t *message, const char *tag);
 notmuch_status_t
 notmuch_message_remove_all_tags (notmuch_message_t *message);
 
+/* Add or remove tags based on the maildir flags in the file name.
+ */
+notmuch_status_t
+notmuch_message_maildir_to_tags (notmuch_message_t *message,
+				 const char *filename);
+
 /* Freeze the current state of 'message' within the database.
  *
  * This means that changes to the message state, (via
diff --git a/notmuch-new.c b/notmuch-new.c
index 8818728..ed3f944 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -410,10 +410,11 @@ add_files_recursive (notmuch_database_t *notmuch,
 	    state->added_messages++;
 	    for (tag=state->new_tags; *tag != NULL; tag++)
 	        notmuch_message_add_tag (message, *tag);
+	    notmuch_message_maildir_to_tags (message, next);
 	    break;
 	/* Non-fatal issues (go on to next file) */
 	case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
-	    /* Stay silent on this one. */
+	    notmuch_message_maildir_to_tags (message, next);
 	    break;
 	case NOTMUCH_STATUS_FILE_NOT_EMAIL:
 	    fprintf (stderr, "Note: Ignoring non-mail file: %s\n",
-- 
1.7.1

[PATCH v4 3/4] Make maildir synchronization configurable

[Michal Sojka]

This adds group [maildir] and key 'synchronize_flags' to the
configuration file. Its value enables (true) or diables (false) the
synchronization between notmuch tags and maildir flags. By default,
the synchronization is disabled.
---
 lib/database-private.h |    2 +-
 lib/database.cc        |    9 ++++++++
 lib/message.cc         |    2 +-
 lib/notmuch.h          |    6 +++++
 notmuch-client.h       |    7 ++++++
 notmuch-config.c       |   49 ++++++++++++++++++++++++++++++++++++++++++++++++
 notmuch-new.c          |    8 +++++-
 notmuch-restore.c      |    2 +
 notmuch-setup.c        |   10 +++++++++
 notmuch-tag.c          |    2 +
 10 files changed, 93 insertions(+), 4 deletions(-)

diff --git a/lib/database-private.h b/lib/database-private.h
index bd72f67..dd89c63 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -49,7 +49,7 @@ struct _notmuch_database {
     Xapian::QueryParser *query_parser;
     Xapian::TermGenerator *term_gen;
     Xapian::ValueRangeProcessor *value_range_processor;
-
+    notmuch_bool_t maildir_sync;
 };
 
 /* Convert tags from Xapian internal format to notmuch format.
diff --git a/lib/database.cc b/lib/database.cc
index 9652013..069918a 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -689,6 +689,8 @@ notmuch_database_open (const char *path,
 	notmuch = NULL;
     }
 
+    notmuch_database_set_maildir_sync (notmuch, FALSE);
+
   DONE:
     if (notmuch_path)
 	free (notmuch_path);
@@ -718,6 +720,13 @@ notmuch_database_close (notmuch_database_t *notmuch)
     talloc_free (notmuch);
 }
 
+void
+notmuch_database_set_maildir_sync (notmuch_database_t *database,
+				   notmuch_bool_t maildir_sync)
+{
+    database->maildir_sync = maildir_sync;
+}
+
 const char *
 notmuch_database_get_path (notmuch_database_t *notmuch)
 {
diff --git a/lib/message.cc b/lib/message.cc
index bb96242..80bef8e 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -626,7 +626,7 @@ _notmuch_message_sync (notmuch_message_t *message)
     if (message->notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY)
 	return;
 
-    if (// todo_sync_enabled &&
+    if (message->notmuch->maildir_sync &&
 	!notmuch_message_get_flag(message, NOTMUCH_MESSAGE_FLAG_TAGS_INVALID)) {
  	status = _notmuch_message_tags_to_maildir (message);
 	if (status != NOTMUCH_PRIVATE_STATUS_SUCCESS) {
diff --git a/lib/notmuch.h b/lib/notmuch.h
index fe01e73..41820b5 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -176,6 +176,12 @@ notmuch_database_open (const char *path,
 void
 notmuch_database_close (notmuch_database_t *database);
 
+/* Sets whether maildir flags should be synchronized with notmuch
+ * tags. */
+void
+notmuch_database_set_maildir_sync (notmuch_database_t *database,
+				   notmuch_bool_t maildir_sync);
+
 /* Return the database path of the given database.
  *
  * The return value is a string owned by notmuch so should not be
diff --git a/notmuch-client.h b/notmuch-client.h
index d530578..c27d64f 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -195,6 +195,13 @@ notmuch_config_set_new_tags (notmuch_config_t *config,
 			     size_t length);
 
 notmuch_bool_t
+notmuch_config_get_maildir_sync (notmuch_config_t *config);
+
+void
+notmuch_config_set_maildir_sync (notmuch_config_t *config,
+				 notmuch_bool_t maildir_sync);
+
+notmuch_bool_t
 debugger_is_active (void);
 
 #endif
diff --git a/notmuch-config.c b/notmuch-config.c
index dcdb036..483d5c6 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -61,6 +61,14 @@ static const char user_config_comment[] =
     " recipient list of replies, and will set the From address based on the\n"
     " address to which the original email was addressed.\n";
 
+static const char maildir_config_comment[] =
+    " Maildir compatibility configuration\n"
+    "\n"
+    " Here you can configure whether notmuch will synchronize its tags with\n"
+    " maildir flags."
+    "\n"
+    "\tsynchronize_flags      Valid values are true and false.\n";
+
 struct _notmuch_config {
     char *filename;
     GKeyFile *key_file;
@@ -72,8 +80,11 @@ struct _notmuch_config {
     size_t user_other_email_length;
     const char **new_tags;
     size_t new_tags_length;
+    notmuch_bool_t maildir_sync;
 };
 
+#define MAILDIR_SYNC_UNDEF ((notmuch_bool_t)-1)
+
 static int
 notmuch_config_destructor (notmuch_config_t *config)
 {
@@ -191,6 +202,7 @@ notmuch_config_open (void *ctx,
     int file_had_database_group;
     int file_had_new_group;
     int file_had_user_group;
+    int file_had_maildir_group;
 
     if (is_new_ret)
 	*is_new_ret = 0;
@@ -221,6 +233,7 @@ notmuch_config_open (void *ctx,
     config->user_other_email_length = 0;
     config->new_tags = NULL;
     config->new_tags_length = 0;
+    config->maildir_sync = MAILDIR_SYNC_UNDEF;
 
     if (! g_key_file_load_from_file (config->key_file,
 				     config->filename,
@@ -263,6 +276,7 @@ notmuch_config_open (void *ctx,
 						    "database");
     file_had_new_group = g_key_file_has_group (config->key_file, "new");
     file_had_user_group = g_key_file_has_group (config->key_file, "user");
+    file_had_maildir_group = g_key_file_has_group (config->key_file, "maildir");
 
 
     if (notmuch_config_get_database_path (config) == NULL) {
@@ -313,6 +327,10 @@ notmuch_config_open (void *ctx,
 	notmuch_config_set_new_tags (config, tags, 2);
     }
 
+    if (notmuch_config_get_maildir_sync (config) == MAILDIR_SYNC_UNDEF) {
+	notmuch_config_set_maildir_sync (config, FALSE);
+    }
+
     /* Whenever we know of configuration sections that don't appear in
      * the configuration file, we add some comments to help the user
      * understand what can be done. */
@@ -340,6 +358,12 @@ notmuch_config_open (void *ctx,
 				user_config_comment, NULL);
     }
 
+    if (! file_had_maildir_group)
+    {
+	g_key_file_set_comment (config->key_file, "maildir", NULL,
+				maildir_config_comment, NULL);
+    }
+
     if (is_new_ret)
 	*is_new_ret = is_new;
 
@@ -703,3 +727,28 @@ notmuch_config_command (void *ctx, int argc, char *argv[])
 	     argv[0]);
     return 1;
 }
+
+notmuch_bool_t
+notmuch_config_get_maildir_sync (notmuch_config_t *config)
+{
+    GError *err = NULL;
+    if (config->maildir_sync == MAILDIR_SYNC_UNDEF) {
+	config->maildir_sync =
+	    g_key_file_get_boolean (config->key_file,
+				    "maildir", "synchronize_flags", &err);
+	if (err) {
+	    config->maildir_sync = MAILDIR_SYNC_UNDEF;
+	    g_error_free (err);
+	}
+    }
+    return config->maildir_sync;
+}
+
+void
+notmuch_config_set_maildir_sync (notmuch_config_t *config,
+				 notmuch_bool_t maildir_sync)
+{
+    g_key_file_set_boolean (config->key_file,
+			    "maildir", "synchronize_flags", maildir_sync);
+    config->maildir_sync = maildir_sync;
+}
diff --git a/notmuch-new.c b/notmuch-new.c
index ed3f944..273916e 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -45,6 +45,7 @@ typedef struct {
 
     _filename_list_t *removed_files;
     _filename_list_t *removed_directories;
+    notmuch_bool_t maildir_sync;
 } add_files_state_t;
 
 static volatile sig_atomic_t do_add_files_print_progress = 0;
@@ -410,11 +411,13 @@ add_files_recursive (notmuch_database_t *notmuch,
 	    state->added_messages++;
 	    for (tag=state->new_tags; *tag != NULL; tag++)
 	        notmuch_message_add_tag (message, *tag);
-	    notmuch_message_maildir_to_tags (message, next);
+	    if (state->maildir_sync == TRUE)
+		notmuch_message_maildir_to_tags (message, next);
 	    break;
 	/* Non-fatal issues (go on to next file) */
 	case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
-	    notmuch_message_maildir_to_tags (message, next);
+	    if (state->maildir_sync == TRUE)
+		notmuch_message_maildir_to_tags (message, next);
 	    break;
 	case NOTMUCH_STATUS_FILE_NOT_EMAIL:
 	    fprintf (stderr, "Note: Ignoring non-mail file: %s\n",
@@ -738,6 +741,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 	return 1;
 
     add_files_state.new_tags = notmuch_config_get_new_tags (config, &add_files_state.new_tags_length);
+    add_files_state.maildir_sync = notmuch_config_get_maildir_sync (config);
     db_path = notmuch_config_get_database_path (config);
 
     dot_notmuch_path = talloc_asprintf (ctx, "%s/%s", db_path, ".notmuch");
diff --git a/notmuch-restore.c b/notmuch-restore.c
index b0a4e1c..b5c5c48 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -41,6 +41,8 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
     if (notmuch == NULL)
 	return 1;
 
+    notmuch_database_set_maildir_sync (notmuch,
+				       notmuch_config_get_maildir_sync (config));
     if (argc) {
 	input = fopen (argv[0], "r");
 	if (input == NULL) {
diff --git a/notmuch-setup.c b/notmuch-setup.c
index c3ea937..36ce71f 100644
--- a/notmuch-setup.c
+++ b/notmuch-setup.c
@@ -195,6 +195,16 @@ notmuch_setup_command (unused (void *ctx),
 	g_ptr_array_free (tags, TRUE);
     }
 
+    prompt ("Synchronize maildir flags with notmuch tags? %s: ",
+	    notmuch_config_get_maildir_sync (config) == TRUE ? "[yes]/no" : "[no]/yes");
+    if (strlen (response) > 0) {
+	if (strcasecmp (response, "yes") == 0||
+	    strcasecmp (response, "y") == 0)
+	    notmuch_config_set_maildir_sync (config, TRUE);
+	else
+	    notmuch_config_set_maildir_sync (config, FALSE);
+    }
+
     if (! notmuch_config_save (config)) {
 	if (is_new)
 	  welcome_message_post_setup ();
diff --git a/notmuch-tag.c b/notmuch-tag.c
index fd54bc7..3a489a9 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -100,6 +100,8 @@ notmuch_tag_command (void *ctx, unused (int argc), unused (char *argv[]))
 				     NOTMUCH_DATABASE_MODE_READ_WRITE);
     if (notmuch == NULL)
 	return 1;
+    notmuch_database_set_maildir_sync (notmuch,
+				       notmuch_config_get_maildir_sync (config));
 
     query = notmuch_query_create (notmuch, query_string);
     if (query == NULL) {
-- 
1.7.1

[PATCH v4 4/4] Tests for maildir synchronization

[Michal Sojka]

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 test/maildir-sync |  231 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 test/notmuch-test |    2 +-
 test/test-lib.sh  |   14 +++-
 3 files changed, 244 insertions(+), 3 deletions(-)
 create mode 100755 test/maildir-sync

diff --git a/test/maildir-sync b/test/maildir-sync
new file mode 100755
index 0000000..23d612e
--- /dev/null
+++ b/test/maildir-sync
@@ -0,0 +1,231 @@
+#!/bin/bash
+
+test_description="Test maildir synchronization"
+
+. ./test-lib.sh
+
+filter_show() {
+    sed -e 's/, /,\n/g'|sed -e "s|${MAIL_DIR}/||" -e '/^"tags"/d'
+    echo
+}
+
+cat >> "$NOTMUCH_CONFIG" <<EOF
+[maildir]
+synchronize_flags=true
+EOF
+
+test_begin_subtest "No new messages"
+output=$(NOTMUCH_NEW)
+test_expect_equal "$output" "No new mail."
+
+cat > expected <<EOF
+Added 1 new message to the database.
+EOF
+test_expect_success "Add a message, no flags" '
+generate_message [subject]="\"test message\"" [date]="\"Sat, 01 Jan 2000 12:00:00 -0000\"" [filename]="\"msg-001:2,\"" &&
+NOTMUCH_NEW > actual &&
+test_cmp expected actual
+#emacs --eval "(gdb \"gdb --annotate=3 --args $(which notmuch) new\")"
+'
+cat > expected <<EOF
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message (inbox unread)
+EOF
+test_expect_success 'Search for the message' '
+notmuch search tag:inbox and tag:unread | notmuch_search_sanitize > actual &&
+test_cmp expected actual
+'
+cat > expected <<EOF
+No new mail. Detected 1 file rename.
+EOF
+test_expect_success 'Add seen flag' '
+mv "${gen_msg_filename}" "${gen_msg_filename}S" &&
+increment_mtime "$(dirname "${gen_msg_filename}")" &&
+NOTMUCH_NEW > actual &&
+test_cmp expected actual
+'
+cat > expected <<EOF
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message (inbox)
+EOF
+test_expect_success 'Check that tags were updated' '
+notmuch search tag:inbox and not tag:unread | notmuch_search_sanitize > actual &&
+test_cmp expected actual
+'
+cat > expected <<EOF
+Added 1 new message to the database.
+EOF
+test_expect_success "Add a seen message" '
+generate_message [subject]="\"test message 2\"" [date]="\"Sat, 01 Jan 2000 12:00:00 -0000\"" [filename]="\"msg-002:2,S\"" &&
+NOTMUCH_NEW > actual &&
+test_cmp expected actual
+'
+cat > expected <<EOF
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message (inbox)
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message 2 (inbox)
+EOF
+test_expect_success 'Check that the seen message is not tagged unread' '
+notmuch search tag:inbox and not tag:unread | notmuch_search_sanitize > actual &&
+test_cmp expected actual
+'
+test_expect_success 'Tag the seen messages as replied' '
+notmuch tag +replied -inbox tag:inbox and not tag:unread
+'
+
+cat > expected <<EOF
+msg-001:2,RS
+msg-002:2,RS
+EOF
+test_expect_success 'Check that R flag was added' '
+ls -1 "${MAIL_DIR}" > actual &&
+test_cmp expected actual
+'
+cat <<EOF > show-expected
+[[[{"id": "msg-001@notmuch-test-suite",
+"match": true,
+"filename": "msg-001:2,RS",
+"timestamp": 946728000,
+"date_relative": "2000-01-01",
+"headers": {"Subject": "test message",
+"From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
+"To": "Notmuch Test Suite <test_suite@notmuchmail.org>",
+"Cc": "",
+"Bcc": "",
+"Date": "Sat,
+01 Jan 2000 12:00:00 -0000"},
+"body": [{"id": 1,
+"content-type": "text/plain",
+"content": "This is just a test message (#1)\n"}]},
+[]]]]
+EOF
+
+test_expect_success 'Message renamed due to changed flags can be shown without running notmuch new' '
+notmuch show --format=json id:msg-001@notmuch-test-suite | filter_show > show-actual &&
+test_cmp show-expected show-actual
+'
+
+test_expect_success 'Test that we can reply to the renamed message' '
+notmuch reply id:msg-001@notmuch-test-suite
+'
+
+echo "No new mail." > expected
+test_expect_success 'No rename should be detected by notmuch new' '
+increment_mtime "$(dirname "${gen_msg_filename}")" &&
+notmuch new > actual &&
+test_cmp expected actual
+'
+test_expect_success "Add a message to new/ without info" '
+generate_message [subject]="\"test message 3\"" [date]="\"Sat, 01 Jan 2000 12:00:00 -0000\"" [dir]=new &&
+NOTMUCH_NEW > actual &&
+test_cmp - actual <<EOF
+Added 1 new message to the database.
+EOF
+'
+test_expect_success "Check that the message has inbox and unread tags" '
+notmuch search tag:inbox and tag:unread | notmuch_search_sanitize > actual &&
+test_cmp - actual <<EOF
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message 3 (inbox unread)
+EOF
+'
+test_expect_success "Tag the message with 'tmp' tag" '
+notmuch tag +tmp tag:inbox and tag:unread'
+
+test_expect_success "Check that the message was not moved from new/ to cur/" '
+echo filename:$gen_msg_filename > expected &&
+notmuch show id:$gen_msg_id|grep -o "filename:.*$" > actual &&
+test_cmp expected actual &&
+test -f "$gen_msg_filename"
+'
+test_expect_success "Check that the message was not renamed" '
+ls "${MAIL_DIR}/new" > actual &&
+test_cmp - actual <<EOF
+msg-003
+EOF
+'
+test_expect_success 'Removing of unread tag should fail without cur/' '
+test_must_fail notmuch tag -unread tag:inbox and tag:unread
+'
+test_expect_success "Check that the tags were not changed" '
+notmuch search tag:inbox and tag:unread | notmuch_search_sanitize > actual &&
+test_cmp - actual <<EOF
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message 3 (inbox tmp unread)
+EOF
+'
+
+# notmuch new is not necessary here, but we run it in order to check
+# for 'no rename' later (*).
+test_expect_success 'Create cur/ and let notmuch know about it' '
+mkdir "$MAIL_DIR/cur" &&
+notmuch new
+'
+test_expect_success 'Removing of unread tag should pass with cur/' '
+notmuch tag -unread tag:inbox and tag:unread
+'
+test_expect_success 'Check that the message was moved to cur/' '\
+ls "$MAIL_DIR/cur" > actual &&
+test_cmp - actual <<EOF
+msg-003:2,S
+EOF
+'
+test_expect_success 'No rename should be detected by notmuch new' '
+increment_mtime "$MAIL_DIR/cur" &&
+notmuch new > actual &&
+test_cmp - actual <<EOF
+No new mail.
+EOF
+'
+# (*) If notmuch new was not run we've got "Processed 1 file in almost
+# no time" here. The reason is that removing unread tag in a previous
+# test created directory document in the database but this document
+# was not linked as subdirectory of $MAIL_DIR. Therefore notmuch new
+# could not reach the cur/ directory and its files in it during
+# recurive traversal.
+test_expect_success 'Remove info from file name' '
+mv "$MAIL_DIR/cur/msg-003:2,S" "$MAIL_DIR/cur/msg-003" &&
+increment_mtime "$MAIL_DIR/cur" &&
+NOTMUCH_NEW > actual
+test_cmp - actual <<EOF
+No new mail. Detected 1 file rename.
+EOF
+'
+test_expect_success "Check that removing info did not change tags" '
+notmuch search tag:inbox | notmuch_search_sanitize > actual &&
+test_cmp - actual <<EOF
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message 3 (inbox tmp)
+EOF
+'
+test_expect_success "Add a message to fakenew/ without info" '
+generate_message [subject]="\"test message 4\"" [date]="\"Sat, 01 Jan 2000 12:00:00 -0000\"" [dir]=fakenew &&
+NOTMUCH_NEW > actual &&
+test_cmp - actual <<EOF
+Added 1 new message to the database.
+EOF
+'
+test_expect_success "Check that the message has inbox and unread tags" '
+notmuch search tag:inbox and tag:unread | notmuch_search_sanitize > actual &&
+test_cmp - actual <<EOF
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message 4 (inbox unread)
+EOF
+'
+test_expect_success 'Removing of unread tag should leave the message in fakenew/' '
+notmuch tag -unread tag:inbox and tag:unread &&
+ls "$MAIL_DIR/fakenew" > actual &&
+test_cmp - actual <<EOF
+msg-004:2,S
+EOF
+'
+
+test_expect_success 'Make maildir flags out of sync with the database' '
+ls $MAIL_DIR > expected &&
+mv $MAIL_DIR/msg-001:2,RS $MAIL_DIR/msg-001:2, &&
+mv $MAIL_DIR/msg-002:2,RS $MAIL_DIR/msg-002:2, &&
+increment_mtime $MAIL_DIR
+'
+
+test_expect_success 'Test whether dump/new/restore synchronizes the maildir flags with the database' '
+notmuch dump dump.txt &&
+notmuch new &&
+notmuch restore dump.txt &&
+ls $MAIL_DIR > actual &&
+test_cmp expected actual
+'
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index 60c3ecb..3691f0d 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -16,7 +16,7 @@ fi
 
 cd $(dirname "$0")
 
-TESTS="basic new search json thread-naming reply dump-restore uuencode thread-order author-order from-guessing long-id encoding emacs"
+TESTS="basic new search json thread-naming reply dump-restore uuencode thread-order author-order from-guessing long-id encoding emacs maildir-sync"
 
 # Clean up any results from a previous run
 rm -r test-results >/dev/null 2>/dev/null
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 8f39aa7..43a1314 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -245,6 +245,12 @@ increment_mtime ()
 #	Generate the message in directory 'directory/of/choice' within
 #	the mail store. The directory will be created if necessary.
 #
+#  [filename]=name
+#
+#	Store the message in file 'name'. The default is to store it
+#	in 'msg-<count>', where <count> is three-digit number of the
+#	message.
+#	
 #  [body]=text
 #
 #	Text to use as the body of the email message
@@ -281,10 +287,14 @@ generate_message ()
     local additional_headers
 
     gen_msg_cnt=$((gen_msg_cnt + 1))
-    gen_msg_name=msg-$(printf "%03d" $gen_msg_cnt)
+    if [ -z "${template[filename]}" ]; then
+	gen_msg_name="msg-$(printf "%03d" $gen_msg_cnt)"
+    else
+	gen_msg_name=${template[filename]}
+    fi
 
     if [ -z "${template[id]}" ]; then
-	gen_msg_id="${gen_msg_name}@notmuch-test-suite"
+	gen_msg_id="${gen_msg_name%:2,*}@notmuch-test-suite"
     else
 	gen_msg_id="${template[id]}"
     fi
-- 
1.7.1

Re: [PATCH v4 0/4] Maildir synchronization

[Carl Worth]

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

On Sun, 31 Oct 2010 22:29:14 +0100, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> This is the next iteration of maildir synchronization patches. The
> changes are:
> - Configuration is now simplified. The synchronization can only be
>   full enabled or disabled. By default it is still disabled.
> - Added test for notmuch restore (with enabled synchronization)
> - Rebased to the current master
> - 'D' flag is mapped to 'deleted' tag instead of 'delete' (this has
>   already been in v3)

This is coming along nicely, Michal. I'm delighted to see the testing
here and the improved configuration support.

I think after merging this, I'd be inclined to change two tiny things:

	1. Enable it by default

	2. Augment the documentation in the configuration file to say
	   exactly which tags are synchronized with which flags.

I could easily do those in follow-up commits, so there's no need to
worry about resending the series for those.

Meanwhile, here are some of the things I'm still thinking about in
regards to this patch. First, the commit message describes the
synchronization happening at "notmuch new" and "notmuch tag/notmuch
restore". But the implementation shows that the functionality is within
the library, not the command-line tool above it.

I think having this at the library makes sense, but as you certainly
noticed, the library has historically been entirely unaware of any
configuration, (which I'd like to keep). Obviously, you maintained that
separation in your patch series, but you added a new
notmuch_database_set_maildir_sync function so that the library can be
informed of the desired behavior.

I'm not entirely sure I like a big, global state-changing function like
that in the library. But if we do want to have that, we need to fix the
documentation of all functions that are affected by it to correctly
document their current behavior.

Also, the synchronization is inherently 2-way, but the two directions
are implemented differently in the library. One direction, (from tags to
maildir flags), is implemented implicitly in the library (existing
functions do the new synchronization under the influence of
database_set_maildir_sync). But the other direction, (from maildir flags
to tags), requires an explicit call to a new function
(notmuch_message_maildir_to_flags). I definitely don't like this.

Finally, the documentation for notmuch_message_maildir_to_tags ("Add or
remove tags based on the maildir flags in the file name")
inadequate. This documentation needs to say which tags are added/removed
in what conditions. It should also give guidance on when this function
should be called in order to achieve some particular behavior.

I recognize that in the above I don't give specific guidance on whether
the new functionality should be implicit or explicit in the
library. I'm not certain which is better, and I'm willing to listen to
justification from someone who has spent some time implementing and
testing this stuff. But I don't like the current mixed state.

One other issue, how does this support deal with multiple files that
have the same message ID (and hence a single record in the database)?
Some bad failure modes I can imagine are cycling of tags/filenames with
successive runs of notmuch new, or "leaking"[*] of tags from one filename
to another through the database.

[*] Imagine, for example, someone using an external client that
identifies duplicate messages in the mailstore and adds the maildir flag
corresponding to "deleted" to all but one of each of the
duplicates. There's then the possibility that notmuch could propagate
this "deleted" flag through the "deleted" tag in the database, (perhaps
after a notmuch dump/restore cycle). And that could be a catastrophic
result if all messages that have duplicates get flagged for deletion!

What thoughts do you have on this multiple-file issue?

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 0/4] Maildir synchronization

[Sebastian Spaeth]

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

On Wed, 09 Jun 2010 20:01:15 -0700, Dirk Hohndel wrote:
> I've been using them for a while as well and love the feature, but I
> keep running into situations where notmuch seems to get out of sync
> regarding file names on disk. I haven't done a lot of searching on what
> exactly causes this - but the symptom is that you'll open a message,
> read it and then try to do something on it (like, save an attachment)
> and suddenly are told that the message file on disk can't be found.
> Or that you reply to a message and just as you are trying to send the
> reply things fail for the same reason.

But didn't Michal say that it gets the messages by id rather than
filename now? In that case, this might have gone away...

Sebastian

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH v4 0/4] Maildir synchronization

[Michal Sojka]

On Thu, 04 Nov 2010, Carl Worth wrote:
> Meanwhile, here are some of the things I'm still thinking about in
> regards to this patch. First, the commit message describes the
> synchronization happening at "notmuch new" and "notmuch tag/notmuch
> restore". But the implementation shows that the functionality is within
> the library, not the command-line tool above it.
> 
> I think having this at the library makes sense, but as you certainly
> noticed, the library has historically been entirely unaware of any
> configuration, (which I'd like to keep). Obviously, you maintained that
> separation in your patch series, but you added a new
> notmuch_database_set_maildir_sync function so that the library can be
> informed of the desired behavior.
> 
> I'm not entirely sure I like a big, global state-changing function like
> that in the library. But if we do want to have that, we need to fix the
> documentation of all functions that are affected by it to correctly
> document their current behavior.

I can imagine two other solutions. One would be to add a parameter
(probably called flags) to the following functions:

    notmuch_message_add_tag()
    notmuch_message_remove_tag()
    notmuch_message_thaw()

Since you want to keep ABI this would require to implement second
versions of these functions to keep backward compatibility.

The other option would be to put a flag into notmuch_message_t but this
is probably not very different from having it in notmuch_database_t.

> Also, the synchronization is inherently 2-way, but the two directions
> are implemented differently in the library. One direction, (from tags to
> maildir flags), is implemented implicitly in the library (existing
> functions do the new synchronization under the influence of
> database_set_maildir_sync). But the other direction, (from maildir flags
> to tags), requires an explicit call to a new function
> (notmuch_message_maildir_to_flags). I definitely don't like this.

I think I could make notmuch_message_maildir_to_flags() private and call
it from notmuch_database_add_message() so that both directions will be
implemented in the library.
 
> Finally, the documentation for notmuch_message_maildir_to_tags ("Add or
> remove tags based on the maildir flags in the file name")
> inadequate. This documentation needs to say which tags are added/removed
> in what conditions. It should also give guidance on when this function
> should be called in order to achieve some particular behavior.
> 
> I recognize that in the above I don't give specific guidance on whether
> the new functionality should be implicit or explicit in the
> library. I'm not certain which is better, and I'm willing to listen to
> justification from someone who has spent some time implementing and
> testing this stuff. But I don't like the current mixed state.

I found the following what I'd like the most: Do not export
notmuch_message_maildir_to_tags() from the library and call it
implicitly from notmuch_database_add_message(). Keep
notmuch_database_set_maildir_sync() and update the documentation of the
affected functions. This sounds to me better than adding explicit flags
parameters to enable/disable synchronization to all of the affected
functions. The additional parameters would make the API harder to use.

> One other issue, how does this support deal with multiple files that
> have the same message ID (and hence a single record in the database)?
> Some bad failure modes I can imagine are cycling of tags/filenames with
> successive runs of notmuch new, or "leaking"[*] of tags from one filename
> to another through the database.
> 
> [*] Imagine, for example, someone using an external client that
> identifies duplicate messages in the mailstore and adds the maildir flag
> corresponding to "deleted" to all but one of each of the
> duplicates. There's then the possibility that notmuch could propagate
> this "deleted" flag through the "deleted" tag in the database, (perhaps
> after a notmuch dump/restore cycle). And that could be a catastrophic
> result if all messages that have duplicates get flagged for deletion!
> 
> What thoughts do you have on this multiple-file issue?

The current implementation renames only the file whose name is stored
first in the database. I have a TODO comment there to add a loop through
all file names, but I have never realized that deleted flag could be so
dangerous.

I will need to extend the test suite for tests with multiple messages
having the same id and during that work I'll hopefully find some sane
solution.

It will take me probably a few days until I find time to work on this.
So let me now in that time whether you have some preferences in the
above.

-Michal

Re: [PATCH 0/4] Maildir synchronization

[Michal Sojka]

On Fri, 05 Nov 2010, Sebastian Spaeth wrote:
> On Wed, 09 Jun 2010 20:01:15 -0700, Dirk Hohndel wrote:
> > I've been using them for a while as well and love the feature, but I
> > keep running into situations where notmuch seems to get out of sync
> > regarding file names on disk. I haven't done a lot of searching on what
> > exactly causes this - but the symptom is that you'll open a message,
> > read it and then try to do something on it (like, save an attachment)
> > and suddenly are told that the message file on disk can't be found.
> > Or that you reply to a message and just as you are trying to send the
> > reply things fail for the same reason.
> 
> But didn't Michal say that it gets the messages by id rather than
> filename now? In that case, this might have gone away...

Yes, yesterday Carl merged --format=raw a.k.a. cat and these problems
doesn't occur now.

-Michal

Re: [PATCH v4 0/4] Maildir synchronization

[Carl Worth]

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

On Sun, 07 Nov 2010 02:46:08 +0100, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> On Thu, 04 Nov 2010, Carl Worth wrote:
> > I'm not entirely sure I like a big, global state-changing function like
> > that in the library. But if we do want to have that, we need to fix the
> > documentation of all functions that are affected by it to correctly
> > document their current behavior.
> 
> I can imagine two other solutions. One would be to add a parameter
> (probably called flags) to the following functions:
> 
>     notmuch_message_add_tag()
>     notmuch_message_remove_tag()
>     notmuch_message_thaw()
...
> The other option would be to put a flag into notmuch_message_t but this
> is probably not very different from having it in notmuch_database_t.

Here's yet another approach that I have in mind.

We can implement all of the support by adding two new library functions:

	notmuch_database_add_message_with_maildir_flags
	notmuch_message_sync_with_maildir_flags

These functions would work like the existing
notmuch_database_add_message and notmuch_message_sync except they would
do the maildir-flag synchronization.

This allows a user of the library to do whatever it wants, (no
synchronization, one-way synchronization in either direction, or two-way
synchronization), without having any sort of "configuration" API calls
in the library.

> I think I could make notmuch_message_maildir_to_flags() private and call
> it from notmuch_database_add_message() so that both directions will be
> implemented in the library.

Yes. That's in line with what I propose above.

> The current implementation renames only the file whose name is stored
> first in the database. I have a TODO comment there to add a loop through
> all file names, but I have never realized that deleted flag could be so
> dangerous.

I think what I'd like to do for now is to simply remove "deleted" from
the set of tags being manipulated by this support. This is the similar
to what Sebastian decided to do with notmuchsync when he described in
detail the same scenario I was concerned with:

	id:87eickhor1.fsf@SSpaeth.de

Sebastian's solution was slightly different, ("deleted" was not
synchronized by default but could be explicitly requested). I propose
simply not synchronizing "deleted" until it can be made safe.

> It will take me probably a few days until I find time to work on this.
> So let me now in that time whether you have some preferences in the
> above.

I'd like to get things merged today, so I plan to take your patches and
then add new commits on top to implement the functions I described
above.

I'll be interested in any feedback.

Thanks again,

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH v4 0/4] Maildir synchronization

[Michal Sojka]

On Mon, 08 Nov 2010, Carl Worth wrote:
> On Sun, 07 Nov 2010 02:46:08 +0100, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> > The current implementation renames only the file whose name is stored
> > first in the database. I have a TODO comment there to add a loop through
> > all file names, but I have never realized that deleted flag could be so
> > dangerous.
> 
> I think what I'd like to do for now is to simply remove "deleted" from
> the set of tags being manipulated by this support. 

This sounds good. Still it will be neccessary to synchronize with all
files, not only the first one. Currently, I experience a problem with
messages sent by myself to a list. The S flag is sometimes added to
the message in sent folder insted of to the message in inbox.

> This is the similar to what Sebastian decided to do with notmuchsync
> when he described in detail the same scenario I was concerned with:
> 
> 	id:87eickhor1.fsf@SSpaeth.de
> 
> Sebastian's solution was slightly different, ("deleted" was not
> synchronized by default but could be explicitly requested). I propose
> simply not synchronizing "deleted" until it can be made safe.
> 
> > It will take me probably a few days until I find time to work on this.
> > So let me now in that time whether you have some preferences in the
> > above.
> 
> I'd like to get things merged today, so I plan to take your patches and
> then add new commits on top to implement the functions I described
> above.

Great! Seems that it is still not finished and I do not want to do
duplicite work, so let me know what is the current status and whether
you need some help from me (e.g. tests for multiple messages with same
ID). I should be on IRC today.

-Michal

Re: [PATCH v4 0/4] Maildir synchronization

[Carl Worth]

[-- Attachment #1.1: Type: text/plain, Size: 2693 bytes --]

On Tue, 09 Nov 2010 11:06:30 +0100, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> This sounds good. Still it will be neccessary to synchronize with all
> files, not only the first one.

OK. I'll add that to the list of things I'll fix up.

> > I'd like to get things merged today, so I plan to take your patches and
> > then add new commits on top to implement the functions I described
> > above.
> 
> Great! Seems that it is still not finished and I do not want to do
> duplicite work, so let me know what is the current status and whether
> you need some help from me (e.g. tests for multiple messages with same
> ID). I should be on IRC today.

Yesterday ended up being very busy, but I'm working on this stuff
now. My plan is to merge it and then release notmuch 0.5. (And yes, if I
had a more strict release manager then 0.5 would have been released
yesterday as-is---but it turns out my release manager had an equally
busy day yesterday).

If you wanted to write some more tests, then that would be very useful.

I've been changing the way the tests are written, so you might want to
look at the attached file to see what I'm doing and match it. Notable
differences:

	* Using test_begin_subtest and test_expect_equal rather than the
          list-of-commands to test_expect_success.

	* Not emitting separate line-item results for things that are
          already tested in other scripts, (like "add message" and
          "search for message", etc.)

The updating I've done here only goes as far as just before "Add a
message to new/ without info". And it looks like one change I made
inadvertently broke a later test, so it's expected that "Check that
removing info did not change tags" currently fails. [And this failure
shows what I don't like about test_expect_success---if we were using
test_expect_equal it would be trivial to see what problem I made here.]

Finally, when going through these tests I saw:

	"Removing of unread tag should fail without cur/"

And that's behavior I do not want. Adding and removing tags should be
reliable whether or not the maildir synchronization can succeed. In this
specific case, the right answer is probably to say that a directory
without "new" and "cur" is not a maildir so no synchronization should be
done.

Notmuch does need to be able to support things like mh format still. Do
the current patches break that by doing maildir-style renaming in
non-maildir directories? If so, we'll need to fix that as well. And that
might require an "is_maildir" term to be stored for directory documents
in the database.

Again, that's something I can help with.

Thanks again,

-Carl


[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: maildir-sync --]
[-- Type: text/plain, Size: 6797 bytes --]

#!/bin/bash

test_description="maildir synchronization"

. ./test-lib.sh

# Much easier to examine differences if the "notmuch show
# --format=json" output includes some newlines. Also, need to avoid
# including the local value of MAIL_DIR in the result.
filter_show_json() {
    sed -e 's/, /,\n/g'  | sed -e "s|${MAIL_DIR}/|MAIL_DIR/|"
    echo
}

cat >> "$NOTMUCH_CONFIG" <<EOF
[maildir]
synchronize_flags=true
EOF

test_begin_subtest "Adding 'S' flag to existing file removes 'unread' tag"
add_message [subject]='"test message"' [date]='"Sat, 01 Jan 2000 12:00:00 -0000"' [filename]='"msg-001:2,"'
NOTMUCH_NEW > /dev/null
output=$(notmuch search tag:inbox | notmuch_search_sanitize)
output+="
"
mv "${gen_msg_filename}" "${gen_msg_filename}S"
increment_mtime "$(dirname "${gen_msg_filename}")"
output+=$(NOTMUCH_NEW)
output+="
"
output+=$(notmuch search tag:inbox | notmuch_search_sanitize)
test_expect_equal "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message (inbox unread)
No new mail. Detected 1 file rename.
thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message (inbox)"

test_begin_subtest "Adding message with 'S' flag prevents 'unread' tag"
add_message [subject]='"test message 2"' [date]='"Sat, 01 Jan 2000 12:00:00 -0000"' [filename]='"msg-002:2,S"' &&
output=$(notmuch search tag:inbox | notmuch_search_sanitize)
test_expect_equal "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message (inbox)
thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message 2 (inbox)"

test_begin_subtest "Adding 'replied' flag adds 'R' tag to filename"
notmuch tag +replied tag:inbox
output=$(ls -1 "${MAIL_DIR}")
test_expect_equal "$output" "msg-001:2,RS
msg-002:2,RS"

test_begin_subtest "notmuch show works with renamed file (without notmuch new)"
output=$(notmuch show --format=json id:msg-001@notmuch-test-suite | filter_show_json)
test_expect_equal "$output" '[[[{"id": "msg-001@notmuch-test-suite",
"match": true,
"filename": "MAIL_DIR/msg-001:2,RS",
"timestamp": 946728000,
"date_relative": "2000-01-01",
"tags": ["inbox","replied"],
"headers": {"Subject": "test message",
"From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
"To": "Notmuch Test Suite <test_suite@notmuchmail.org>",
"Cc": "",
"Bcc": "",
"Date": "Sat,
01 Jan 2000 12:00:00 -0000"},
"body": [{"id": 1,
"content-type": "text/plain",
"content": "This is just a test message (#1)\n"}]},
[]]]]'

test_expect_success 'notmuch reply works with renamed file (without notmuch new)' 'notmuch reply id:msg-001@notmuch-test-suite'

test_begin_subtest "notmuch new detects no file rename after tag->flag synchronization"
increment_mtime "$(dirname ${gen_msg_filename})"
output=$(NOTMUCH_NEW)
test_expect_equal "$output" "No new mail."

test_expect_success "Add a message to new/ without info" '
generate_message [subject]="\"test message 3\"" [date]="\"Sat, 01 Jan 2000 12:00:00 -0000\"" [dir]=new &&
NOTMUCH_NEW > actual &&
test_cmp - actual <<EOF
Added 1 new message to the database.
EOF
'
test_expect_success "Check that the message has inbox and unread tags" '
notmuch search tag:inbox and tag:unread | notmuch_search_sanitize > actual &&
test_cmp - actual <<EOF
thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message 3 (inbox unread)
EOF
'
test_expect_success "Tag the message with 'tmp' tag" '
notmuch tag +tmp tag:inbox and tag:unread'

test_expect_success "Check that the message was not moved from new/ to cur/" '
echo filename:$gen_msg_filename > expected &&
notmuch show id:$gen_msg_id|grep -o "filename:.*$" > actual &&
test_cmp expected actual &&
test -f "$gen_msg_filename"
'
test_expect_success "Check that the message was not renamed" '
ls "${MAIL_DIR}/new" > actual &&
test_cmp - actual <<EOF
msg-003
EOF
'
test_expect_success 'Removing of unread tag should fail without cur/' '
test_must_fail notmuch tag -unread tag:inbox and tag:unread
'
test_expect_success "Check that the tags were not changed" '
notmuch search tag:inbox and tag:unread | notmuch_search_sanitize > actual &&
test_cmp - actual <<EOF
thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message 3 (inbox tmp unread)
EOF
'

# notmuch new is not necessary here, but we run it in order to check
# for 'no rename' later (*).
test_expect_success 'Create cur/ and let notmuch know about it' '
mkdir "$MAIL_DIR/cur" &&
notmuch new
'
test_expect_success 'Removing of unread tag should pass with cur/' '
notmuch tag -unread tag:inbox and tag:unread
'
test_expect_success 'Check that the message was moved to cur/' '\
ls "$MAIL_DIR/cur" > actual &&
test_cmp - actual <<EOF
msg-003:2,S
EOF
'
test_expect_success 'No rename should be detected by notmuch new' '
increment_mtime "$MAIL_DIR/cur" &&
notmuch new > actual &&
test_cmp - actual <<EOF
No new mail.
EOF
'
# (*) If notmuch new was not run we've got "Processed 1 file in almost
# no time" here. The reason is that removing unread tag in a previous
# test created directory document in the database but this document
# was not linked as subdirectory of $MAIL_DIR. Therefore notmuch new
# could not reach the cur/ directory and its files in it during
# recurive traversal.
test_expect_success 'Remove info from file name' '
mv "$MAIL_DIR/cur/msg-003:2,S" "$MAIL_DIR/cur/msg-003" &&
increment_mtime "$MAIL_DIR/cur" &&
NOTMUCH_NEW > actual
test_cmp - actual <<EOF
No new mail. Detected 1 file rename.
EOF
'
test_expect_success "Check that removing info did not change tags" '
notmuch search tag:inbox | notmuch_search_sanitize > actual &&
test_cmp - actual <<EOF
thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message 3 (inbox tmp)
EOF
'
test_expect_success "Add a message to fakenew/ without info" '
generate_message [subject]="\"test message 4\"" [date]="\"Sat, 01 Jan 2000 12:00:00 -0000\"" [dir]=fakenew &&
NOTMUCH_NEW > actual &&
test_cmp - actual <<EOF
Added 1 new message to the database.
EOF
'
test_expect_success "Check that the message has inbox and unread tags" '
notmuch search tag:inbox and tag:unread | notmuch_search_sanitize > actual &&
test_cmp - actual <<EOF
thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message 4 (inbox unread)
EOF
'
test_expect_success 'Removing of unread tag should leave the message in fakenew/' '
notmuch tag -unread tag:inbox and tag:unread &&
ls "$MAIL_DIR/fakenew" > actual &&
test_cmp - actual <<EOF
msg-004:2,S
EOF
'

test_expect_success 'Make maildir flags out of sync with the database' '
ls $MAIL_DIR > expected &&
mv $MAIL_DIR/msg-001:2,RS $MAIL_DIR/msg-001:2, &&
mv $MAIL_DIR/msg-002:2,RS $MAIL_DIR/msg-002:2, &&
increment_mtime $MAIL_DIR
'

test_expect_success 'Test whether dump/new/restore synchronizes the maildir flags with the database' '
notmuch dump dump.txt &&
notmuch new &&
notmuch restore dump.txt &&
ls $MAIL_DIR > actual &&
test_cmp expected actual
'

test_done

[-- Attachment #3: Type: text/plain, Size: 28 bytes --]


-- 
carl.d.worth@intel.com

Re: [PATCH v4 0/4] Maildir synchronization

[Michal Sojka]

On Tue, 09 Nov 2010, Carl Worth wrote:
> The updating I've done here only goes as far as just before "Add a
> message to new/ without info". And it looks like one change I made
> inadvertently broke a later test, so it's expected that "Check that
> removing info did not change tags" currently fails. [And this failure
> shows what I don't like about test_expect_success---if we were using
> test_expect_equal it would be trivial to see what problem I made
> here.]

Did you try ./maildyr-sync -v? In fact, this is what I don't like about
test_begin_subtest. test_begin_subtest does not hide debug output that
goes to stdout and should only be shown with -v. I admit that
test_expect_success is not ideal though. Without -v, it should
automatically show stdout when a test fails. I'll send patch for this in
another mail.

> 
> Finally, when going through these tests I saw:
> 
> 	"Removing of unread tag should fail without cur/"
> 
> And that's behavior I do not want. Adding and removing tags should be
> reliable whether or not the maildir synchronization can succeed. In this
> specific case, the right answer is probably to say that a directory
> without "new" and "cur" is not a maildir so no synchronization should be
> done.

This only fails if the message is in */new and there is no */cur.

> Notmuch does need to be able to support things like mh format still. Do
> the current patches break that by doing maildir-style renaming in
> non-maildir directories?

I do not know if MH format has something special or it is just plain
files in plain directories. If the latter, the synchronzation should
work unless one of the directories is named 'new'. See the tests with
fakenew directory [fakenew is probably not the most obvious name for a
simple non-maildir directory].

> If so, we'll need to fix that as well. And that might require an
> "is_maildir" term to be stored for directory documents in the
> database.
> 
> Again, that's something I can help with.

If you think that what we have now is not sufficient, I'd need some help
with this.

-Michal

P.S. Because of my work on test suite, I didn't do any additional tests
for maildir synchronization, so maybe tomorrow :-(

Re: [PATCH v4 0/4] Maildir synchronization

[Carl Worth]

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

On Wed, 10 Nov 2010 00:39:31 +0100, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> Did you try ./maildyr-sync -v?

Ah, no. That's always seemed really noisy. What I want is simply to see
the information related to a failure *when a failure occurs*, and not a
lot of noise, (nor having to re-run with extra options to see the output
I want).

> test_expect_success is not ideal though. Without -v, it should
> automatically show stdout when a test fails. I'll send patch for this in
> another mail.

Cool. I'll look for that.

> This only fails if the message is in */new and there is no */cur.

Right. I think that's a little too severe.

> I do not know if MH format has something special or it is just plain
> files in plain directories. If the latter, the synchronzation should
> work unless one of the directories is named 'new'.

The MH format is plain files in plain directories.

But in this case, I would contend that we don't _want_ the
synchronization to work. The files shouldn't be getting renamed at all
unless we are dealing with maildir.

Neither maildir nor mh give us anything extremely reliable that we can
use to unambiguously distinguish between them. But I think a heuristic
of:

	if "cur" and "new" sub-directories exist:
		then this directory is maildir;

And only when this heuristic passes should we be fiddling with filenames
in maildir-specified ways.

> If you think that what we have now is not sufficient, I'd need some help
> with this.

I'll look.

> P.S. Because of my work on test suite, I didn't do any additional tests
> for maildir synchronization, so maybe tomorrow :-(

No problem. I appreciate your help improving the test-suite
infrastructure!

Meanwhile, I ran into one problem with my proposal. We can't use
notmuch_message_sync_with_maildir_flags since notmuch_message_sync is an
internal interface. The corresponding public interface actually consists
of three or four different functions (notmuch_message_add_tag,
notmuch_message_remove_tag, and notmuch_message_freeze/thaw). I think it
would be quite crazy to add _with_maildir_flags variants of all of
those.

So maybe we will need a new function for the purpose of synchronizing
the current tags of a message to a maildir filename. So that would be,
perhaps, notmuch_message_tags_to_maildir_flags or so?

Finally, I'm also a bit unsettled about the handling of the "S"
flag. For all the other flags it is easy to document that
notmuch_database_add_message_with_maildir_flags simply adds the
corresponding tag to the message. But we can't say that the presence of
the "S" tag prevents this function from adding the "unread" tag, since
this function never does add an "unread" tag.

Instead, the setting of "unread" is taking place at a higher-level,
(inside "notmuch new"). So perhaps the right answer is for the library
function to add a "seen" tag, (making the handling of 'T' consistent
with all other tags and dropping the "inverse" field from the
table). Then, the "notmuch new" program can query the "seen" tag to
decide whether it should add its configured new_tags, ("inbox" and
"unread" by default).

I do want something like that anyway, because I want to make it so that
someone that first starts with notmuch and a large collection of maildir
messages doesn't end up with every message tagged as "inbox" after their
first run of "notmuch new".

So anyway, I'm currently working on implementing what I described
above. And I may change my mind slightly as I work through things.

I'll let you know,

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH v4 0/4] Maildir synchronization

[Michal Sojka]

On Wed, 10 Nov 2010, Carl Worth wrote:
> > This only fails if the message is in */new and there is no */cur.
> 
> Right. I think that's a little too severe.
> 
> > I do not know if MH format has something special or it is just plain
> > files in plain directories. If the latter, the synchronzation should
> > work unless one of the directories is named 'new'.
> 
> The MH format is plain files in plain directories.
> 
> But in this case, I would contend that we don't _want_ the
> synchronization to work. The files shouldn't be getting renamed at all
> unless we are dealing with maildir.

Yes, I think this could be easily implemented.

> Neither maildir nor mh give us anything extremely reliable that we can
> use to unambiguously distinguish between them. But I think a heuristic
> of:
> 
> 	if "cur" and "new" sub-directories exist:
> 		then this directory is maildir;
> 
> And only when this heuristic passes should we be fiddling with filenames
> in maildir-specified ways.

Agreed.

> Meanwhile, I ran into one problem with my proposal. We can't use
> notmuch_message_sync_with_maildir_flags since notmuch_message_sync is an
> internal interface. The corresponding public interface actually consists
> of three or four different functions (notmuch_message_add_tag,
> notmuch_message_remove_tag, and notmuch_message_freeze/thaw). I think it
> would be quite crazy to add _with_maildir_flags variants of all of
> those.
> 
> So maybe we will need a new function for the purpose of synchronizing
> the current tags of a message to a maildir filename. So that would be,
> perhaps, notmuch_message_tags_to_maildir_flags or so?

This sounds good and allows us to get rid of
NOTMUCH_MESSAGE_FLAG_TAGS_INVALID.

> Finally, I'm also a bit unsettled about the handling of the "S"
> flag. For all the other flags it is easy to document that
> notmuch_database_add_message_with_maildir_flags simply adds the
> corresponding tag to the message. But we can't say that the presence of
> the "S" tag prevents this function from adding the "unread" tag, since
> this function never does add an "unread" tag.

But can we say the the function _removes_ the unread tag if 'S' is
present and adds it otherwise?

> Instead, the setting of "unread" is taking place at a higher-level,
> (inside "notmuch new"). So perhaps the right answer is for the library
> function to add a "seen" tag, (making the handling of 'T' consistent
> with all other tags and dropping the "inverse" field from the
> table). Then, the "notmuch new" program can query the "seen" tag to
> decide whether it should add its configured new_tags, ("inbox" and
> "unread" by default).
> 
> I do want something like that anyway, because I want to make it so that
> someone that first starts with notmuch and a large collection of maildir
> messages doesn't end up with every message tagged as "inbox" after their
> first run of "notmuch new".

I understand your point but this change would break how I use notmuch
now. My new_tags contains only "new" tag and if this tag is not added
during notmuch new the message would not be properly tagged by my
initial tagging script. I want to avoid the situation that I accidentaly
view a message in a web-mail client, which adds the 'S' flag and this
will lead to exclusion of the message from initial tagging.

If we leave the things the way they are now, it should be easy for the
user to run

  notmuch tag -inbox not tag:unread

We could also show this hint at the end of notmuch new when it is run
for the first time and all messages are tagged by inbox.

> 
> So anyway, I'm currently working on implementing what I described
> above. And I may change my mind slightly as I work through things.
> 
> I'll let you know,

Great. I've finished the additional tests, which I send as a reply to
this mail. Some test are marked as broken because I do not want to touch
C sources while you are woking on them.

-Michal

[PATCH] test: More maildir synchronization tests

[Michal Sojka]

Add maildir synchronization tests for multiple messages with the same
message-id. As this is not yet implemented in notmuch, some of these
teste are marked as BROKEN.

I use $(< ) operator to avoid fiddling with stripped trailing newlines
from test results which happens when output+=$(command) is used.
---
 test/maildir-sync |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/test/maildir-sync b/test/maildir-sync
index 8e216b3..67d04b9 100755
--- a/test/maildir-sync
+++ b/test/maildir-sync
@@ -188,4 +188,23 @@ ls $MAIL_DIR > actual &&
 test_cmp expected actual
 '
 
+test_begin_subtest 'Duplicated message is tagged according to the duplicate'
+cp "$MAIL_DIR/cur/msg-003" "$MAIL_DIR/cur/msg-003-dup:2,RS" 
+increment_mtime $MAIL_DIR/cur 
+NOTMUCH_NEW > output
+notmuch search id:msg-003@notmuch-test-suite | notmuch_search_sanitize >> output
+test_expect_equal "$(< output)" "No new mail.
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; test message 3 (inbox replied tmp)"
+
+test_begin_subtest 'The original message receives the same flags as the duplicate'
+ls $MAIL_DIR/cur > actual
+test_expect_equal_failure "$(< actual)" "msg-003:2,RS
+msg-003-dup:2,RS"
+
+test_begin_subtest 'Tagging modifies flags of both the original and the duplicate'
+notmuch tag -replied id:msg-003@notmuch-test-suite
+ls $MAIL_DIR/cur > actual
+test_expect_equal_failure "$(< actual)"  "msg-003:2,S
+msg-003-dup:2,S"
+
 test_done
-- 
1.7.2.3

Re: [PATCH v4 0/4] Maildir synchronization

[Carl Worth]

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

On Wed, 10 Nov 2010 11:26:40 +0100, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> > So maybe we will need a new function for the purpose of synchronizing
> > the current tags of a message to a maildir filename. So that would be,
> > perhaps, notmuch_message_tags_to_maildir_flags or so?
> 
> This sounds good and allows us to get rid of
> NOTMUCH_MESSAGE_FLAG_TAGS_INVALID.

Yes. And I'm now pursuing simply two new synchronization functions, (as
opposed to the _with_maildir_flags variant of _add_message I proposed
earlier). So the two new functions are:

	notmuch_message_maildir_flags_to_tags
and	notmuch_message_tags_to_maildir_flags

> But can we say the the function _removes_ the unread tag if 'S' is
> present and adds it otherwise?

I think so. I ended up getting close to this with the code I was writing
yesterday. I eventually had the original (non-maildir-related)
notmuch_database_add_message adding the "unread" tag (which is a new
behavior) and the _add_message_with_maildir_flags removing it if
necessary.

The semantics will be slightly different with the explicit flags_to_tags
function, but I think I'll get this right.

> I understand your point but this change would break how I use notmuch
> now. My new_tags contains only "new" tag and if this tag is not added
> during notmuch new the message would not be properly tagged by my
> initial tagging script.

And of course I found the same problem with my own setup. ;-)

So the tag named configured in [new.tags] will need to be applied
unconditionally to all new messages, (as currently documented). I was
getting concerned when my patches started adding an "unread" tag to new
messages (even without [maildir.synchronize_flags] configuration) and
the existing configuration wouldn't allow a user to prevent that.

But perhaps with the new functions proposed above we can avoid that
problem.

>   notmuch tag -inbox not tag:unread
> 
> We could also show this hint at the end of notmuch new when it is run
> for the first time and all messages are tagged by inbox.

Yes. That might be the right answer.

> Great. I've finished the additional tests, which I send as a reply to
> this mail. Some test are marked as broken because I do not want to touch
> C sources while you are woking on them.

Thanks!

In addition to this test:

       +test_begin_subtest 'Duplicated message is tagged according to the duplicate'
       +cp "$MAIL_DIR/cur/msg-003" "$MAIL_DIR/cur/msg-003-dup:2,RS" 

I'd like to see a similar test that starts with "some-message:2,RS" and
then copies it to "some-message". When we add this message would we
expect the "replied" tag to be removed and the "unread" tag added?

I'm still trying to work out exactly how the
notmuch_message_maildir_flags_to_tags function should be documented,
(whereas notmuch_message_tags_to_maildir_flags seems entirely obvious).

Thanks,

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH v4 0/4] Maildir synchronization

[Carl Worth]

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

On Wed, 10 Nov 2010 08:48:54 -0800, Carl Worth <cworth@cworth.org> wrote:
> > Great. I've finished the additional tests, which I send as a reply to
> > this mail. Some test are marked as broken because I do not want to touch
> > C sources while you are woking on them.
> 
> Thanks!

I've now got these tests merged in locally, (and updated to work with
the rest of my maildir-sync changes---I'm not reusing filenames like
"msg-003" but instead using filenames associated with particular tests
such as "duplicate-file" to avoid unintended dependencies and side
effects between difference tests).

> I'd like to see a similar test that starts with "some-message:2,RS" and
> then copies it to "some-message". When we add this message would we
> expect the "replied" tag to be removed and the "unread" tag added?

I've added a test for this now. The direction my thoughts are headed is
that adding a file like this will not result in tag changes. See below.

> I'm still trying to work out exactly how the
> notmuch_message_maildir_flags_to_tags function should be documented,
> (whereas notmuch_message_tags_to_maildir_flags seems entirely
> obvious).

I'm thinking of documenting/implementing this such that the
_flags_to_tags function merges (as a logical OR) the set of flags from
all filenames for a given email message, and then computes tags from the
final set. This does assume a particular kind of life-cycle for the
flags, (that they are additionally unset, and once set for the logical
message will not want to be cleared again). This lifecycle seems correct
for things like the flags for "seen" or "replied" as documented in the
original maildir specification.

This behavior might be less appropriate if the flag support were
generalized to a user-customizable set of flag<->tag mappings. But we're
not dealing with this now, so I'll avoid getting bogged down by it.

Further suggestions from anyone are welcome.

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH v4 0/4] Maildir synchronization

[Michal Sojka]

On Wed, 10 Nov 2010, Carl Worth wrote:
> I'm thinking of documenting/implementing this such that the
> _flags_to_tags function merges (as a logical OR) the set of flags from
> all filenames for a given email message, and then computes tags from the
> final set. This does assume a particular kind of life-cycle for the
> flags, (that they are additionally unset, and once set for the logical
> message will not want to be cleared again). This lifecycle seems correct
> for things like the flags for "seen" or "replied" as documented in the
> original maildir specification.

Does this mean, that if I want to remove the replied tag (for example) I
can still do it by manipulating flags, i.e. I would remove the R flag
from all messages with the coreposnding Message-ID? Or does it mean that
the only way is to remove the replied tag in notmuch? I'd prefer the
first.

-Michal

Re: [PATCH v4 0/4] Maildir synchronization

[Carl Worth]

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

On Thu, 11 Nov 2010 16:51:28 +0100, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> Does this mean, that if I want to remove the replied tag (for example) I
> can still do it by manipulating flags, i.e. I would remove the R flag
> from all messages with the coreposnding Message-ID? Or does it mean that
> the only way is to remove the replied tag in notmuch? I'd prefer the
> first.

It is the former, yes. See, for example the following test in the test
suite:

	"Removing 'S' flag from existing filename adds 'unread' tag"

Also, here is the latest documentation of
notmuch_message_maildir_flags_to_tags. If this could be made more clear,
please let me know:

/* Add/remove tags according to maildir flags in the message filename(s)
 *
 * This function examines the filenames of 'message' for maildir
 * flags, and adds or removes tags on 'message' as follows when these
 * flags are present:
 *
 *	Flag	Action if present
 *	----	-----------------
 *	'D'	Adds the "draft" tag to the message
 *	'F'	Adds the "flagged" tag to the message
 *	'P'	Adds the "passed" tag to the message
 *	'R'	Adds the "replied" tag to the message
 *	'S'	Removes the "unread" tag from the message
 *
 * For each flag that is not present, the opposite action (add/remove)
 * is performed for the corresponding tags.
 *
 * Flags are identified as trailing components of the filename after a
 * sequence of ":2,".
 *
 * If there are multiple filenames associated with this message, the
 * flag is considered present if it appears in one or more
 * filenames. (That is, the flags from the multiple filenames are
 * combined with the logical OR operator.)
 *
 * 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_message_tags_to_maildir_flags for synchronizing tag changes
 * back to maildir flags.
 */

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]