notmuch new: Defer maildir_flags synchronization until after removals

When a file in the mailstore is renamed, this appears to "notmuch new"
as both an added file and a removed file (for the same message). We
want the synchronization of the maildir_flags to reflect the final
state, (after the rename is complete). Therefore, it's incorrect to
perform the synchronization immediately after adding a new
file. Instead we queue up these synchronizations (by message ID[*])
and perform them after the removals are complete.

With this change, the "dump/restore" case of the maildir-sync tests,
as well as the recent "remove 'S'" case both now pass where they were
failing before.

Interestingly, the "remove info" test was passing before, but now
fails. This is actually due to a separate bug, (and the bug just fixed
was masking it, by preventing the test from performing as desired).

[*] It's important to queue by message ID---queueing actual message
objects does not work since the message objects will retain stale data
such as the old filenames.

diff --git a/notmuch-new.c b/notmuch-new.c
index 3d628c23f2c8bb20ba6aa3cc49b91407a117afe6..4f3e52fb455832398a7940d0dc4ec80448155334 100644 (file)
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -45,7 +45,9 @@ typedef struct {
 
     _filename_list_t *removed_files;
     _filename_list_t *removed_directories;
+
     notmuch_bool_t synchronize_flags;
+    _filename_list_t *message_ids_to_sync;
 } add_files_state_t;
 
 static volatile sig_atomic_t do_add_files_print_progress = 0;
@@ -411,13 +413,19 @@ add_files_recursive (notmuch_database_t *notmuch,
            state->added_messages++;
            for (tag=state->new_tags; *tag != NULL; tag++)
                notmuch_message_add_tag (message, *tag);
+           /* Defer sync of maildir flags until after old filenames
+            * are removed in the case of a rename. */
            if (state->synchronize_flags == TRUE)
-               notmuch_message_maildir_flags_to_tags (message);
+               _filename_list_add (state->message_ids_to_sync,
+                                   notmuch_message_get_message_id (message));
            break;
        /* Non-fatal issues (go on to next file) */
        case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
+           /* Defer sync of maildir flags until after old filenames
+            * are removed in the case of a rename. */
            if (state->synchronize_flags == TRUE)
-               notmuch_message_maildir_flags_to_tags (message);
+               _filename_list_add (state->message_ids_to_sync,
+                                   notmuch_message_get_message_id (message));
            break;
        case NOTMUCH_STATUS_FILE_NOT_EMAIL:
            fprintf (stderr, "Note: Ignoring non-mail file: %s\n",
@@ -742,6 +750,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 
     add_files_state.new_tags = notmuch_config_get_new_tags (config, &add_files_state.new_tags_length);
     add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
+    add_files_state.message_ids_to_sync = _filename_list_create (ctx);
     db_path = notmuch_config_get_database_path (config);
 
     dot_notmuch_path = talloc_asprintf (ctx, "%s/%s", db_path, ".notmuch");
@@ -817,6 +826,25 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     talloc_free (add_files_state.removed_files);
     talloc_free (add_files_state.removed_directories);
 
+    /* Now that removals are done (hence the database is aware of all
+     * renames), we can synchronize maildir_flags to tags for all
+     * messages that had new filenames appear on this run. */
+    if (add_files_state.synchronize_flags) {
+       _filename_node_t *node;
+       notmuch_message_t *message;
+       for (node = add_files_state.message_ids_to_sync->head;
+            node;
+            node = node->next)
+       {
+           message = notmuch_database_find_message (notmuch, node->filename);
+           notmuch_message_maildir_flags_to_tags (message);
+           notmuch_message_destroy (message);
+       }
+    }
+
+    talloc_free (add_files_state.message_ids_to_sync);
+    add_files_state.message_ids_to_sync = NULL;
+
     gettimeofday (&tv_now, NULL);
     elapsed = notmuch_time_elapsed (add_files_state.tv_start,
                                    tv_now);