Hallo!, and hooray!, and some first work, too
Thomas Schwinge
thomas at schwinge.name
Sun Jan 9 11:19:15 PST 2011
Hallo!
After having read about it in ..., in ..., (don't know anymore), and then
again over at LWN, I have had the plan to try (or rather: directly switch
to) notmuch like half a year ago, or longer. I eventually was brave
enough to begin that process in the last days of 2010. I'm still with
you, as you can see. Hooray! :-)
As I said on IRC already: there are, of course, still some rough edges,
but what else should I expect from such a young project.
I'm currently using getmail, then maildrop which invokes notmuch-deliver
for getting my emails into both maildir storage, and making them known to
notmuch. In that process I can apply tags at will (based on List-Id: /
Mailing-List: headers, mostly, but also for possibly-spam, and so on),
and then use a script that operates on these to further massage them.
Thus, I don't really have a need for ``notmuch new'' anymore, and
notmuch-deliver is sort of an atomic / integrated ``put into maildir and
add tags'' tool for me. I will post more details on my setup (or rather:
directly put it into the wiki), as soon as all this settles down some
more.
I have not yet switched all my email channels to notmuch, but will do so
soon, I think.
Likewise, there are some topics I'd like to discuss. I'll also begin to
plow through notmuch's mailing list archive, and will gladly accept
id:xxx pointers that answer / discuss the questions I'll be sending.
I poked at notmuch-deliver's code two weeks ago already (Ali, beware,
there'll be some few further patches coming), and in the last hours, I
finally had a look at notmuch.h and some of the source code. Here is a
diff containing some comments, or to-do items. Not all are fully fledged
(I have neither been using talloc, nor Xapian before), but perhaps
someone nevertheless feels like commenting on them. In general I can
say, that the notmuch code makes a solid impression. :-)
diff --git a/emacs/notmuch-maildir-fcc.el b/emacs/notmuch-maildir-fcc.el
index 3f1c124..0d7017c 100644
--- a/emacs/notmuch-maildir-fcc.el
+++ b/emacs/notmuch-maildir-fcc.el
@@ -16,6 +16,14 @@
;; To use this as the fcc handler for message-mode,
;; customize the notmuch-fcc-dirs variable
+;; TODO. A bunch of maildir code is not specific to fcc handling and should be
+;; factored out.
+
+;; TODO. In fact, apart from using notmuch-database-path (which should
+;; probably be message-directory instead, anyways?), this whole unit is not
+;; specific to notmuch, and should perhaps be integrated into Emacs'
+;; message.el -- or is there even anything in Gnus that can directly be used?
+
(eval-when-compile (require 'cl))
(require 'message)
@@ -73,7 +81,7 @@ yet when sending a mail."
(defun notmuch-fcc-header-setup ()
"Add an Fcc header to the current message buffer.
-Can be added to `message-send-hook' and will set the Fcc header
+Is by default added to `message-header-setup-hook' and will set the Fcc header
based on the values of `notmuch-fcc-dirs'. An existing Fcc header
will NOT be removed or replaced."
@@ -163,7 +171,7 @@ will NOT be removed or replaced."
(make-directory (concat path "/new/") t)
(make-directory (concat path "/tmp/") t))
((file-regular-p path)
- (error "%s is a file. Can't creat maildir." path))
+ (error "%s is a file. Can't create maildir." path))
(t
(error "I don't know how to create a maildir here"))))
@@ -173,6 +181,8 @@ if successful, nil if not."
(let ((msg-id (notmuch-maildir-fcc-make-uniq-maildir-id)))
(while (file-exists-p (concat destdir "/tmp/" msg-id))
(setq msg-id (notmuch-maildir-fcc-make-uniq-maildir-id)))
+;; TODO. Race. Should instead (try to) open (create) the file exclusively,
+;; until that succeeds.
(cond ((notmuch-maildir-fcc-dir-is-maildir-p destdir)
(write-file (concat destdir "/tmp/" msg-id))
msg-id)
diff --git a/lib/database.cc b/lib/database.cc
index 7a00917..b08c429 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -551,7 +551,10 @@ notmuch_database_create (const char *path)
notmuch = notmuch_database_open (path,
NOTMUCH_DATABASE_MODE_READ_WRITE);
+ /* XXX: should check 'notmuch'. */
+
notmuch_database_upgrade (notmuch, NULL, NULL);
+ /* XXX: should check return value. */
DONE:
if (notmuch_path)
@@ -760,18 +763,6 @@ handle_sigalrm (unused (int signal))
do_progress_notify = 1;
}
-/* Upgrade the current database.
- *
- * After opening a database in read-write mode, the client should
- * check if an upgrade is needed (notmuch_database_needs_upgrade) and
- * if so, upgrade with this function before making any modifications.
- *
- * The optional progress_notify callback can be used by the caller to
- * provide progress indication to the user. If non-NULL it will be
- * called periodically with 'count' as the number of messages upgraded
- * so far and 'total' the overall number of messages that will be
- * converted.
- */
notmuch_status_t
notmuch_database_upgrade (notmuch_database_t *notmuch,
void (*progress_notify) (void *closure,
@@ -1012,6 +1003,7 @@ _notmuch_database_get_directory_db_path (const char *path)
* is an empty string, then both directory and basename will be
* returned as NULL.
*/
+/* XXX: isn't there a standard libc function that can be used? */
notmuch_status_t
_notmuch_database_split_path (void *ctx,
const char *path,
@@ -1141,6 +1133,7 @@ _notmuch_database_filename_to_direntry (void *ctx,
return status;
*direntry = talloc_asprintf (ctx, "%u:%s", directory_id, basename);
+ /* XXX: *direntry != NULL? */
return NOTMUCH_STATUS_SUCCESS;
}
@@ -1168,6 +1161,9 @@ _notmuch_database_relative_path (notmuch_database_t *notmuch,
while (*relative == '/' && *(relative+1) == '/')
relative++;
+ /* XXX: canonicalize paths (symlinks, `x/./y, `x/../y') before
+ comparing? */
+
if (strncmp (relative, db_path, db_path_len) == 0)
{
relative += db_path_len;
@@ -1601,6 +1597,8 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
/* If a message ID is too long, substitute its sha1 instead. */
if (message_id && strlen (message_id) > NOTMUCH_MESSAGE_ID_MAX) {
+ /* XXX: How to distinguish this one later on (or don't we have
+ to?) from the message-id (possibly) generated below? */
char *compressed = _message_id_compressed (message_file,
message_id);
talloc_free (message_id);
@@ -1619,6 +1617,8 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
goto DONE;
}
+ /* XXX: How to distinguish this one later on (or don't we have to?)
+ from the message-id (possibly) generated above? */
message_id = talloc_asprintf (message_file,
"notmuch-sha1-%s", sha1);
free (sha1);
@@ -1641,6 +1641,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
}
_notmuch_message_add_filename (message, filename);
+ /* XXX: check return value. */
/* Is this a newly created message object? */
if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
@@ -1655,6 +1656,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
_notmuch_message_set_date (message, date);
_notmuch_message_index_file (message, filename);
+ /* XXX: check return value. */
} else {
ret = NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;
}
@@ -1735,11 +1737,15 @@ notmuch_database_remove_message (notmuch_database_t *notmuch,
status = NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;
}
}
+
+ /* XXX: talloc_free (term); */
} catch (const Xapian::Error &error) {
fprintf (stderr, "Error: A Xapian exception occurred removing message: %s\n",
error.get_msg().c_str());
notmuch->exception_reported = TRUE;
status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
+
+ /* XXX: (conditionally) talloc_free (term); */
}
talloc_free (local);
diff --git a/lib/directory.cc b/lib/directory.cc
index 946be4f..925b1da 100644
--- a/lib/directory.cc
+++ b/lib/directory.cc
@@ -140,6 +140,7 @@ _notmuch_directory_create (notmuch_database_t *notmuch,
char *term = talloc_asprintf (local, "%s%s",
_find_prefix ("directory"), db_path);
directory->doc.add_term (term, 0);
+ /* XXX?: talloc_free (term); */
directory->doc.set_data (path);
@@ -152,6 +153,7 @@ _notmuch_directory_create (notmuch_database_t *notmuch,
_find_prefix ("directory-direntry"),
parent_id, basename);
directory->doc.add_term (term, 0);
+ /* XXX?: talloc_free (term); */
}
directory->doc.add_value (NOTMUCH_VALUE_TIMESTAMP,
diff --git a/lib/message.cc b/lib/message.cc
index adcd07d..cf4e36a 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -187,13 +187,14 @@ _notmuch_message_create (const void *talloc_owner,
* There is already a document with message ID 'message_id' in the
* database. The returned message can be used to query/modify the
* document.
+ *
* NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND:
*
* No document with 'message_id' exists in the database. The
* returned message contains a newly created document (not yet
* added to the database) and a document ID that is known not to
* exist in the database. The caller can modify the message, and a
- * call to _notmuch_message_sync will add * the document to the
+ * call to _notmuch_message_sync will add the document to the
* database.
*
* If an error occurs, this function will return NULL and *status
@@ -449,8 +450,10 @@ _notmuch_message_add_filename (notmuch_message_t *message,
status = _notmuch_database_filename_to_direntry (local,
message->notmuch,
filename, &direntry);
- if (status)
+ if (status) {
+ /* XXX: talloc_free (local); */
return status;
+ }
_notmuch_message_add_term (message, "file-direntry", direntry);
@@ -717,6 +720,7 @@ _notmuch_message_close (notmuch_message_t *message)
*
* This change will not be reflected in the database until the next
* call to _notmuch_message_sync. */
+/* XXX: basically nowehere this function's return value is checked. */
notmuch_private_status_t
_notmuch_message_add_term (notmuch_message_t *message,
const char *prefix_name,
@@ -730,9 +734,12 @@ _notmuch_message_add_term (notmuch_message_t *message,
term = talloc_asprintf (message, "%s%s",
_find_prefix (prefix_name), value);
+ /* XXX: term != NULL? */
- if (strlen (term) > NOTMUCH_TERM_MAX)
+ if (strlen (term) > NOTMUCH_TERM_MAX) {
+ /* XXX: talloc_free (term); */
return NOTMUCH_PRIVATE_STATUS_TERM_TOO_LONG;
+ }
message->doc.add_term (term, 0);
@@ -820,6 +827,7 @@ notmuch_message_add_tag (notmuch_message_t *message, const char *tag)
if (strlen (tag) > NOTMUCH_TAG_MAX)
return NOTMUCH_STATUS_TAG_TOO_LONG;
+ /* XXX: what if tag is already present -- added again? */
private_status = _notmuch_message_add_term (message, "tag", tag);
if (private_status) {
INTERNAL_ERROR ("_notmuch_message_add_term return unexpected value: %d\n",
diff --git a/lib/notmuch.h b/lib/notmuch.h
index e508309..ffc7f8f 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -810,6 +810,7 @@ notmuch_message_set_flag (notmuch_message_t *message,
* For the original textual representation of the Date header from the
* message call notmuch_message_get_header() with a header value of
* "date". */
+/* XXX: what if Date: was missing? */
time_t
notmuch_message_get_date (notmuch_message_t *message);
@@ -866,6 +867,8 @@ notmuch_message_get_tags (notmuch_message_t *message);
/* Add a tag to the given message.
*
+ * XXX: which are valid characters for a tag?
+ *
* Return value:
*
* NOTMUCH_STATUS_SUCCESS: Tag successfully added to message
@@ -1112,7 +1115,7 @@ notmuch_tags_destroy (notmuch_tags_t *tags);
*
* o Read the mtime of a directory from the filesystem
*
- * o Call add_message for all mail files in the directory
+ * o Call notmuch_database_add_message for all mail files in the directory
*
* o Call notmuch_directory_set_mtime with the mtime read from the
* filesystem.
@@ -1157,7 +1160,7 @@ notmuch_directory_get_mtime (notmuch_directory_t *directory);
notmuch_filenames_t *
notmuch_directory_get_child_files (notmuch_directory_t *directory);
-/* Get a notmuch_filenams_t iterator listing all the filenames of
+/* Get a notmuch_filenames_t iterator listing all the filenames of
* sub-directories in the database within the given directory.
*
* The returned filenames will be the basename-entries only (not
diff --git a/notmuch-new.c b/notmuch-new.c
index cdf8513..74a09bf 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -784,6 +784,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
gettimeofday (&add_files_state.tv_start, NULL);
notmuch_database_upgrade (notmuch, upgrade_print_progress,
&add_files_state);
+ /* XXX: should check return value. */
printf ("Your notmuch database has now been upgraded to database format version %u.\n",
notmuch_database_get_version (notmuch));
}
diff --git a/notmuch-restore.c b/notmuch-restore.c
index f095f64..2c607c3 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -115,7 +115,10 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
}
notmuch_message_freeze (message);
+ /* XXX: check return value. */
+
notmuch_message_remove_all_tags (message);
+ /* XXX: check return value. */
next = file_tags;
while (next) {
@@ -133,6 +136,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
}
notmuch_message_thaw (message);
+ /* XXX: check return value. */
if (synchronize_flags)
notmuch_message_tags_to_maildir_flags (message);
diff --git a/notmuch-tag.c b/notmuch-tag.c
index 60e21e0..3b7a00c 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -120,6 +120,7 @@ notmuch_tag_command (void *ctx, unused (int argc), unused (char *argv[]))
message = notmuch_messages_get (messages);
notmuch_message_freeze (message);
+ /* XXX: check return value. */
for (i = 0; i < remove_tags_count; i++)
notmuch_message_remove_tag (message,
@@ -129,6 +130,7 @@ notmuch_tag_command (void *ctx, unused (int argc), unused (char *argv[]))
notmuch_message_add_tag (message, argv[add_tags[i]] + 1);
notmuch_message_thaw (message);
+ /* XXX: check return value. */
if (synchronize_flags)
notmuch_message_tags_to_maildir_flags (message);
Grüße,
Thomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20110109/9d44b2f3/attachment.pgp>
More information about the notmuch
mailing list