date:20120507

[PATCH v2 2/2] new: Centralize file type stat-ing logic

2012-05-07 Thread Austin Clements

This moves our logic to get a file's type into one function.  This has
several benefits: we can support OSes and file systems that do not
provide dirent.d_type or always return DT_UNKNOWN, complex
symlink-handling logic has been replaced by a simple stat fall-through
in one place, and the error message for un-stat-able file is more
accurate (previously, the error always mentioned directories, even
though a broken symlink is not a directory).
---
 notmuch-new.c |  103 +++--
 test/new  |2 +-
 2 files changed, 64 insertions(+), 41 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index cb720cc..8955677 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -154,6 +154,48 @@ dirent_sort_strcmp_name (const struct dirent **a, const 
struct dirent **b)
 return strcmp ((*a)->d_name, (*b)->d_name);
 }

+/* Return the type of a directory entry relative to path as a stat(2)
+ * mode.  Like stat, this follows symlinks.  Returns -1 and sets errno
+ * if the file's type cannot be determined (which includes dangling
+ * symlinks).
+ */
+static int
+dirent_type (const char *path, const struct dirent *entry)
+{
+struct stat statbuf;
+char *abspath;
+int err, saved_errno;
+
+#ifdef _DIRENT_HAVE_D_TYPE
+/* Mapping from d_type to stat mode_t.  We omit DT_LNK so that
+ * we'll fall through to stat and get the real file type. */
+static const mode_t modes[] = {
+   [DT_BLK]  = S_IFBLK,
+   [DT_CHR]  = S_IFCHR,
+   [DT_DIR]  = S_IFDIR,
+   [DT_FIFO] = S_IFIFO,
+   [DT_REG]  = S_IFREG,
+   [DT_SOCK] = S_IFSOCK
+};
+if (entry->d_type < ARRAY_SIZE(modes) && modes[entry->d_type])
+   return modes[entry->d_type];
+#endif
+
+abspath = talloc_asprintf (NULL, "%s/%s", path, entry->d_name);
+if (!abspath) {
+   errno = ENOMEM;
+   return -1;
+}
+err = stat(abspath, );
+saved_errno = errno;
+talloc_free (abspath);
+if (err < 0) {
+   errno = saved_errno;
+   return -1;
+}
+return statbuf.st_mode & S_IFMT;
+}
+
 /* Test if the directory looks like a Maildir directory.
  *
  * Search through the array of directory entries to see if we can find all
@@ -162,12 +204,12 @@ dirent_sort_strcmp_name (const struct dirent **a, const 
struct dirent **b)
  * Return 1 if the directory looks like a Maildir and 0 otherwise.
  */
 static int
-_entries_resemble_maildir (struct dirent **entries, int count)
+_entries_resemble_maildir (const char *path, struct dirent **entries, int 
count)
 {
 int i, found = 0;

 for (i = 0; i < count; i++) {
-   if (entries[i]->d_type != DT_DIR && entries[i]->d_type != DT_UNKNOWN)
+   if (dirent_type (path, entries[i]) != S_IFDIR)
continue;

if (strcmp(entries[i]->d_name, "new") == 0 ||
@@ -250,7 +292,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
 notmuch_message_t *message = NULL;
 struct dirent **fs_entries = NULL;
-int i, num_fs_entries;
+int i, num_fs_entries, entry_type;
 notmuch_directory_t *directory;
 notmuch_filenames_t *db_files = NULL;
 notmuch_filenames_t *db_subdirs = NULL;
@@ -317,7 +359,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 }

 /* Pass 1: Recurse into all sub-directories. */
-is_maildir = _entries_resemble_maildir (fs_entries, num_fs_entries);
+is_maildir = _entries_resemble_maildir (path, fs_entries, num_fs_entries);

 for (i = 0; i < num_fs_entries; i++) {
if (interrupted)
@@ -325,17 +367,16 @@ add_files_recursive (notmuch_database_t *notmuch,

entry = fs_entries[i];

-   /* We only want to descend into directories.
-* But symlinks can be to directories too, of course.
-*
-* And if the filesystem doesn't tell us the file type in the
-* scandir results, then it might be a directory (and if not,
-* then we'll stat and return immediately in the next level of
-* recursion). */
-   if (entry->d_type != DT_DIR &&
-   entry->d_type != DT_LNK &&
-   entry->d_type != DT_UNKNOWN)
-   {
+   /* We only want to descend into directories (and symlinks to
+* directories). */
+   entry_type = dirent_type (path, entry);
+   if (entry_type == -1) {
+   /* Be pessimistic, e.g. so we don't lose lots of mail just
+* because a user broke a symlink. */
+   fprintf (stderr, "Error reading file %s/%s: %s\n",
+path, entry->d_name, strerror (errno));
+   return NOTMUCH_STATUS_FILE_ERROR;
+   } else if (entry_type != S_IFDIR) {
continue;
}

@@ -425,31 +466,13 @@ add_files_recursive (notmuch_database_t *notmuch,
notmuch_filenames_move_to_next (db_subdirs);
}

-   /* If we're looking at a symlink, we only want to add it if it
-* links to a regular file, (and not to a

[PATCH v2 1/2] test: Test notmuch new with a broken symlink

2012-05-07 Thread Austin Clements

---
 test/new |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/test/new b/test/new
index 99f9913..26253db 100755
--- a/test/new
+++ b/test/new
@@ -136,6 +136,16 @@ output=$(NOTMUCH_NEW)
 test_expect_equal "$output" "Added 1 new message to the database."


+test_begin_subtest "Broken symlink aborts"
+ln -s does-not-exist "${MAIL_DIR}/broken"
+output=$(NOTMUCH_NEW 2>&1)
+test_expect_equal "$output" \
+"Error reading directory /run/shm/nm/tmp.new/mail/broken: No such file or 
directory
+Note: A fatal error was encountered: Something went wrong trying to read or 
write a file
+No new mail."
+rm "${MAIL_DIR}/broken"
+
+
 test_begin_subtest "New two-level directory"

 generate_message [dir]=two/levels
-- 
1.7.10

[PATCH 2/2] new: Centralize file type stat-ing logic

2012-05-07 Thread Austin Clements

Quoth Jani Nikula on May 08 at 12:08 am:
>On May 7, 2012 9:10 PM, "Austin Clements" <[1]amdragon at mit.edu> wrote:
>>
>> This moves our logic to get a file's type into one function. ?This has
>> several benefits: we can support OSes and file systems that do not
>> provide dirent.d_type or always return DT_UNKNOWN, complex
>> symlink-handling logic has been replaced by a simple stat fall-through
>> in one place, and the error message for un-stat-able file is more
>> accurate (previously, the error always mentioned directories, even
>> though a broken symlink is not a directory).
> 
>Please find some quick drive-by-review below.

Thanks for the review!  New version on its way...

>J.
> 
>> ---
>> ?notmuch-new.c | ? 99
>++---
>> ?test/new ? ? ?| ? ?2 +-
>> ?2 files changed, 60 insertions(+), 41 deletions(-)
>>
>> diff --git a/notmuch-new.c b/notmuch-new.c
>> index cb720cc..cf2580e 100644
>> --- a/notmuch-new.c
>> +++ b/notmuch-new.c
>> @@ -154,6 +154,44 @@ dirent_sort_strcmp_name (const struct dirent **a,
>const struct dirent **b)
>> ? ? return strcmp ((*a)->d_name, (*b)->d_name);
>> ?}
>>
>> +/* Return the type of a directory entry relative to path as a stat(2)
>> + * mode. ?Like stat, this follows symlinks. ?Returns -1 and sets errno
>> + * if the file's type cannot be determined (which includes dangling
>> + * symlinks).
>> + */
>> +static int
>> +dirent_type (const char *path, const struct dirent *entry)
>> +{
>> + ? ?struct stat statbuf;
>> + ? ?char *abspath;
>> + ? ?int err;
>> +
>> +#ifdef _DIRENT_HAVE_D_TYPE
>> + ? ?/* Mapping from d_type to stat mode_t. ?We omit DT_LNK so that
>> + ? ? * we'll fall through to stat and get the real file type. */
>> + ? ?static const mode_t modes[] = {
>> + ? ? ? [DT_BLK] ?= S_IFBLK,
>> + ? ? ? [DT_CHR] ?= S_IFCHR,
>> + ? ? ? [DT_DIR] ?= S_IFDIR,
>> + ? ? ? [DT_FIFO] = S_IFIFO,
>> + ? ? ? [DT_REG] ?= S_IFREG,
>> + ? ? ? [DT_SOCK] = S_IFSOCK
>> + ? ?};
>> + ? ?if (entry->d_type < sizeof(modes)/sizeof(modes[0]) &&
> 
>ARRAY_SIZE()

Huh.  I went looking for that.  I wonder how I missed it.

>> + ? ? ? modes[entry->d_type])
>> + ? ? ? return modes[entry->d_type];
>> +#endif
>> +
>> + ? ?abspath = talloc_asprintf (NULL, "%s/%s", path, entry->d_name);
>> + ? ?if (!abspath)
>> + ? ? ? return -1;
> 
>Does talloc set errno in this case? I suspect not.
> 
>> + ? ?err = stat(abspath, );
>> + ? ?talloc_free (abspath);
> 
>This likely breaks your promise about errno. You can't trust talloc_free()
>not calling some function that sets errno.

Setting errno was a late and apparently not well thought-out addition.
The new patch should set it on all of the error paths and save it
around things that may modify it.

>> + ? ?if (err < 0)
>> + ? ? ? return -1;
>> + ? ?return statbuf.st_mode & S_IFMT;
>> +}
>> +
>> ?/* Test if the directory looks like a Maildir directory.
>> ?*
>> ?* Search through the array of directory entries to see if we can find
>all
>> @@ -162,12 +200,12 @@ dirent_sort_strcmp_name (const struct dirent **a,
>const struct dirent **b)
>> ?* Return 1 if the directory looks like a Maildir and 0 otherwise.
>> ?*/
>> ?static int
>> -_entries_resemble_maildir (struct dirent **entries, int count)
>> +_entries_resemble_maildir (const char *path, struct dirent **entries,
>int count)
>> ?{
>> ? ? int i, found = 0;
>>
>> ? ? for (i = 0; i < count; i++) {
>> - ? ? ? if (entries[i]->d_type != DT_DIR && entries[i]->d_type !=
>DT_UNKNOWN)
>> + ? ? ? if (dirent_type (path, entries[i]) != S_IFDIR)
>> ? ? ? ? ? ?continue;
>>
>> ? ? ? ?if (strcmp(entries[i]->d_name, "new") == 0 ||
>> @@ -250,7 +288,7 @@ add_files_recursive (notmuch_database_t *notmuch,
>> ? ? notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
>> ? ? notmuch_message_t *message = NULL;
>> ? ? struct dirent **fs_entries = NULL;
>> - ? ?int i, num_fs_entries;
>> + ? ?int i, num_fs_entries, entry_type;
>> ? ? notmuch_directory_t *directory;
>> ? ? notmuch_filenames_t *db_files = NULL;
>> ? ? notmuch_filenames_t *db_subdirs = NULL;
>> @@ -317,7 +355,7 @@ add_files_recursive (notmuch_database_t *notmuch,
>> ? ? }
>>
>> ? ? /* Pass 1: Recurse into all sub-directories. */
>> - ? ?is_maildir = _entries_resemble_maildir (fs_entries,
>num_fs_entries);
>> + ? ?is_maildir = _entries_resemble_maildir (path, fs_entries,
>num_fs_entries);
>>
>> ? ? for (i = 0; i < num_fs_entries; i++) {
>> ? ? ? ?if (interrupted)
>> @@ -325,17 +363,16 @@ add_files_recursive (notmuch_database_t *notmuch,
>>
>> ? ? ? ?entry = fs_entries[i];
>>
>> -

[PATCH 4/4] ruby: Add wrapper for notmuch_query_set_omit_excluded()

2012-05-07 Thread Ali Polatel

---
 bindings/ruby/defs.h  |3 +++
 bindings/ruby/init.c  |7 +++
 bindings/ruby/query.c |   18 ++
 3 files changed, 28 insertions(+)

diff --git a/bindings/ruby/defs.h b/bindings/ruby/defs.h
index 6fe5787..85d8205 100644
--- a/bindings/ruby/defs.h
+++ b/bindings/ruby/defs.h
@@ -220,6 +220,9 @@ VALUE
 notmuch_rb_query_add_tag_exclude (VALUE self, VALUE tagv);

 VALUE
+notmuch_rb_query_set_omit_excluded (VALUE self, VALUE omitv);
+
+VALUE
 notmuch_rb_query_search_threads (VALUE self);

 VALUE
diff --git a/bindings/ruby/init.c b/bindings/ruby/init.c
index edcf101..3fe60fb 100644
--- a/bindings/ruby/init.c
+++ b/bindings/ruby/init.c
@@ -96,6 +96,12 @@ Init_notmuch (void)
  */
 rb_define_const (mod, "MESSAGE_FLAG_MATCH", INT2FIX 
(NOTMUCH_MESSAGE_FLAG_MATCH));
 /*
+ * Document-const: Notmuch::MESSAGE_FLAG_EXCLUDED
+ *
+ * Message flag "excluded"
+ */
+rb_define_const (mod, "MESSAGE_FLAG_EXCLUDED", INT2FIX 
(NOTMUCH_MESSAGE_FLAG_EXCLUDED));
+/*
  * Document-const: Notmuch::TAG_MAX
  *
  * Maximum allowed length of a tag
@@ -235,6 +241,7 @@ Init_notmuch (void)
 rb_define_method (notmuch_rb_cQuery, "sort=", notmuch_rb_query_set_sort, 
1); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, "to_s", notmuch_rb_query_get_string, 
0); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, "add_tag_exclude", 
notmuch_rb_query_add_tag_exclude, 1); /* in query.c */
+rb_define_method (notmuch_rb_cQuery, "omit_excluded=", 
notmuch_rb_query_set_omit_excluded, 1); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, "search_threads", 
notmuch_rb_query_search_threads, 0); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, "search_messages", 
notmuch_rb_query_search_messages, 0); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, "count_messages", 
notmuch_rb_query_count_messages, 0); /* in query.c */
diff --git a/bindings/ruby/query.c b/bindings/ruby/query.c
index 2a80008..e5ba1b7 100644
--- a/bindings/ruby/query.c
+++ b/bindings/ruby/query.c
@@ -107,6 +107,24 @@ notmuch_rb_query_add_tag_exclude (VALUE self, VALUE tagv)
 }

 /*
+ * call-seq: QUERY.omit_excluded=(boolean) => nil
+ *
+ * Specify whether to omit excluded results or simply flag them.
+ * By default, this is set to +true+.
+ */
+VALUE
+notmuch_rb_query_set_omit_excluded (VALUE self, VALUE omitv)
+{
+notmuch_query_t *query;
+
+Data_Get_Notmuch_Query (self, query);
+
+notmuch_query_set_omit_excluded (query, RTEST (omitv));
+
+return Qnil;
+}
+
+/*
  * call-seq: QUERY.search_threads => THREADS
  *
  * Search for threads
-- 
1.7.10.1

[PATCH 3/4] ruby: Add workarounds to use in-tree build not the installed one

2012-05-07 Thread Ali Polatel

- Make mkmf use the notmuch.h under ../../lib
- Use libnotmuch.a instead of linking to the installed libnotmuch.so
---
 bindings/ruby/defs.h |4 ++--
 bindings/ruby/extconf.rb |   26 ++
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/bindings/ruby/defs.h b/bindings/ruby/defs.h
index a41cf10..6fe5787 100644
--- a/bindings/ruby/defs.h
+++ b/bindings/ruby/defs.h
@@ -1,6 +1,6 @@
 /* The Ruby interface to the notmuch mail library
  *
- * Copyright ? 2010, 2011 Ali Polatel
+ * Copyright ? 2010, 2011, 2012 Ali Polatel
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -21,8 +21,8 @@
 #ifndef DEFS_H
 #define DEFS_H

-#include 
 #include 
+#include "notmuch.h"

 VALUE notmuch_rb_cDatabase;
 VALUE notmuch_rb_cDirectory;
diff --git a/bindings/ruby/extconf.rb b/bindings/ruby/extconf.rb
index ccac609..933f34a 100644
--- a/bindings/ruby/extconf.rb
+++ b/bindings/ruby/extconf.rb
@@ -1,13 +1,31 @@
 #!/usr/bin/env ruby
 # coding: utf-8
-# Copyright 2010, 2011 Ali Polatel 
+# Copyright 2010, 2011, 2012 Ali Polatel 
 # Distributed under the terms of the GNU General Public License v3

 require 'mkmf'

-# Notmuch Library
-find_header('notmuch.h', '../../lib')
-find_library('notmuch', 'notmuch_database_create', '../../lib')
+NOTDIR = File.expand_path(File.join(File.dirname(__FILE__), '..', '..', 'lib'))
+NOTHDR = File.join(NOTDIR, 'notmuch.h')
+NOTLIB = File.join(NOTDIR, 'libnotmuch.a')
+
+unless File.exists? NOTHDR
+  $stderr.puts "notmuch.h is missing under #{NOTDIR}"
+  exit 1
+end
+
+unless File.exists? NOTLIB
+  $stderr.puts "libnotmuch.a is missing under #{NOTDIR}"
+  exit 1
+end
+
+# Small hack to build with in-tree version not the installed one.
+# find_header() and friends use standard include/library paths first.
+$stderr.puts "Added -I#{NOTDIR} to $INCFLAGS"
+$INCFLAGS = "-I#{NOTDIR}".quote + " " + $INCFLAGS
+find_header('notmuch.h', NOTDIR)
+
+$LOCAL_LIBS += NOTLIB

 # Create Makefile
 dir_config('notmuch')
-- 
1.7.10.1

[PATCH 2/4] ruby: Add wrapper for notmuch_query_add_tag_exclude

2012-05-07 Thread Ali Polatel

---
 bindings/ruby/defs.h  |3 +++
 bindings/ruby/init.c  |1 +
 bindings/ruby/query.c |   18 ++
 3 files changed, 22 insertions(+)

diff --git a/bindings/ruby/defs.h b/bindings/ruby/defs.h
index 25222a6..a41cf10 100644
--- a/bindings/ruby/defs.h
+++ b/bindings/ruby/defs.h
@@ -217,6 +217,9 @@ VALUE
 notmuch_rb_query_get_string (VALUE self);

 VALUE
+notmuch_rb_query_add_tag_exclude (VALUE self, VALUE tagv);
+
+VALUE
 notmuch_rb_query_search_threads (VALUE self);

 VALUE
diff --git a/bindings/ruby/init.c b/bindings/ruby/init.c
index 7ad0ecf..edcf101 100644
--- a/bindings/ruby/init.c
+++ b/bindings/ruby/init.c
@@ -234,6 +234,7 @@ Init_notmuch (void)
 rb_define_method (notmuch_rb_cQuery, "sort", notmuch_rb_query_get_sort, 
0); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, "sort=", notmuch_rb_query_set_sort, 
1); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, "to_s", notmuch_rb_query_get_string, 
0); /* in query.c */
+rb_define_method (notmuch_rb_cQuery, "add_tag_exclude", 
notmuch_rb_query_add_tag_exclude, 1); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, "search_threads", 
notmuch_rb_query_search_threads, 0); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, "search_messages", 
notmuch_rb_query_search_messages, 0); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, "count_messages", 
notmuch_rb_query_count_messages, 0); /* in query.c */
diff --git a/bindings/ruby/query.c b/bindings/ruby/query.c
index 02b7819..2a80008 100644
--- a/bindings/ruby/query.c
+++ b/bindings/ruby/query.c
@@ -89,6 +89,24 @@ notmuch_rb_query_get_string (VALUE self)
 }

 /*
+ * call-seq: QUERY.add_tag_exclude(tag) => nil
+ *
+ * Add a tag that will be excluded from the query results by default.
+ */
+VALUE
+notmuch_rb_query_add_tag_exclude (VALUE self, VALUE tagv)
+{
+notmuch_query_t *query;
+const char *tag;
+
+Data_Get_Notmuch_Query (self, query);
+tag = RSTRING_PTR(tagv);
+
+notmuch_query_add_tag_exclude(query, tag);
+return Qnil;
+}
+
+/*
  * call-seq: QUERY.search_threads => THREADS
  *
  * Search for threads
-- 
1.7.10.1

[PATCH 1/4] ruby: Add wrapper for notmuch_query_count_messages

2012-05-07 Thread Ali Polatel

---
 bindings/ruby/defs.h  |3 +++
 bindings/ruby/init.c  |3 ++-
 bindings/ruby/query.c |   21 -
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/bindings/ruby/defs.h b/bindings/ruby/defs.h
index 81f652f..25222a6 100644
--- a/bindings/ruby/defs.h
+++ b/bindings/ruby/defs.h
@@ -222,6 +222,9 @@ notmuch_rb_query_search_threads (VALUE self);
 VALUE
 notmuch_rb_query_search_messages (VALUE self);

+VALUE
+notmuch_rb_query_count_messages (VALUE self);
+
 /* threads.c */
 VALUE
 notmuch_rb_threads_destroy (VALUE self);
diff --git a/bindings/ruby/init.c b/bindings/ruby/init.c
index 4405f19..7ad0ecf 100644
--- a/bindings/ruby/init.c
+++ b/bindings/ruby/init.c
@@ -1,6 +1,6 @@
 /* The Ruby interface to the notmuch mail library
  *
- * Copyright ? 2010, 2011 Ali Polatel
+ * Copyright ? 2010, 2011, 2012 Ali Polatel
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -236,6 +236,7 @@ Init_notmuch (void)
 rb_define_method (notmuch_rb_cQuery, "to_s", notmuch_rb_query_get_string, 
0); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, "search_threads", 
notmuch_rb_query_search_threads, 0); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, "search_messages", 
notmuch_rb_query_search_messages, 0); /* in query.c */
+rb_define_method (notmuch_rb_cQuery, "count_messages", 
notmuch_rb_query_count_messages, 0); /* in query.c */

 /*
  * Document-class: Notmuch::Threads
diff --git a/bindings/ruby/query.c b/bindings/ruby/query.c
index 74fd585..02b7819 100644
--- a/bindings/ruby/query.c
+++ b/bindings/ruby/query.c
@@ -1,6 +1,6 @@
 /* The Ruby interface to the notmuch mail library
  *
- * Copyright ? 2010, 2011 Ali Polatel
+ * Copyright ? 2010, 2011, 2012 Ali Polatel
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -127,3 +127,22 @@ notmuch_rb_query_search_messages (VALUE self)

 return Data_Wrap_Struct (notmuch_rb_cMessages, NULL, NULL, messages);
 }
+
+/*
+ * call-seq: QUERY.count_messages => Fixnum
+ *
+ * Return an estimate of the number of messages matching a search
+ */
+VALUE
+notmuch_rb_query_count_messages (VALUE self)
+{
+notmuch_query_t *query;
+
+Data_Get_Notmuch_Query (self, query);
+
+/* Xapian exceptions are not handled properly.
+ * (function may return 0 after printing a message)
+ * Thus there is nothing we can do here...
+ */
+return UINT2FIX(notmuch_query_count_messages(query));
+}
-- 
1.7.10.1

[PATCH 0/4] ruby: quick update before the freeze!

2012-05-07 Thread Ali Polatel

Hey David,

I've polished the small patch series I had lying around.
I believe I've covered all the lacking API changes.

In addition, the third patch fixes a long-standing bug about the build.

I've decided to send these to you and the list instead of committing right away
in case someone notices any quick errors. Otherwise, please commit these before
the freeze and make rubyists happy.

Ali Polatel (4):
  ruby: Add wrapper for notmuch_query_count_messages
  ruby: Add wrapper for notmuch_query_add_tag_exclude
  ruby: Add workarounds to use in-tree build not the installed one
  ruby: Add wrapper for notmuch_query_set_omit_excluded()

 bindings/ruby/defs.h |   13 +--
 bindings/ruby/extconf.rb |   26 +
 bindings/ruby/init.c |   11 -
 bindings/ruby/query.c|   57 +-
 4 files changed, 99 insertions(+), 8 deletions(-)

-- 
1.7.10.1

[Patch v2] emacs: add pipe attachment command

2012-05-07 Thread Mark Walters

On Mon, 07 May 2012, Jameson Graef Rollins  
wrote:
> On Mon, May 07 2012, Mark Walters  wrote:
>> Here is a rebased to current master version of the previous patch: no
>> other changes.
>
> Actually, what I sent was a rebase.  

Sorry somehow I missed that you had actually sent the rebase too: and
then I messed up my rebase. How embarrassing! 

Anyway many thanks for the correct rebase, and for spotting my stupid error.

Best wishes

Mark

[PATCH 2/2] new: Centralize file type stat-ing logic

2012-05-07 Thread Austin Clements

This moves our logic to get a file's type into one function.  This has
several benefits: we can support OSes and file systems that do not
provide dirent.d_type or always return DT_UNKNOWN, complex
symlink-handling logic has been replaced by a simple stat fall-through
in one place, and the error message for un-stat-able file is more
accurate (previously, the error always mentioned directories, even
though a broken symlink is not a directory).
---
 notmuch-new.c |   99 ++---
 test/new  |2 +-
 2 files changed, 60 insertions(+), 41 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index cb720cc..cf2580e 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -154,6 +154,44 @@ dirent_sort_strcmp_name (const struct dirent **a, const 
struct dirent **b)
 return strcmp ((*a)->d_name, (*b)->d_name);
 }

+/* Return the type of a directory entry relative to path as a stat(2)
+ * mode.  Like stat, this follows symlinks.  Returns -1 and sets errno
+ * if the file's type cannot be determined (which includes dangling
+ * symlinks).
+ */
+static int
+dirent_type (const char *path, const struct dirent *entry)
+{
+struct stat statbuf;
+char *abspath;
+int err;
+
+#ifdef _DIRENT_HAVE_D_TYPE
+/* Mapping from d_type to stat mode_t.  We omit DT_LNK so that
+ * we'll fall through to stat and get the real file type. */
+static const mode_t modes[] = {
+   [DT_BLK]  = S_IFBLK,
+   [DT_CHR]  = S_IFCHR,
+   [DT_DIR]  = S_IFDIR,
+   [DT_FIFO] = S_IFIFO,
+   [DT_REG]  = S_IFREG,
+   [DT_SOCK] = S_IFSOCK
+};
+if (entry->d_type < sizeof(modes)/sizeof(modes[0]) &&
+   modes[entry->d_type])
+   return modes[entry->d_type];
+#endif
+
+abspath = talloc_asprintf (NULL, "%s/%s", path, entry->d_name);
+if (!abspath)
+   return -1;
+err = stat(abspath, );
+talloc_free (abspath);
+if (err < 0)
+   return -1;
+return statbuf.st_mode & S_IFMT;
+}
+
 /* Test if the directory looks like a Maildir directory.
  *
  * Search through the array of directory entries to see if we can find all
@@ -162,12 +200,12 @@ dirent_sort_strcmp_name (const struct dirent **a, const 
struct dirent **b)
  * Return 1 if the directory looks like a Maildir and 0 otherwise.
  */
 static int
-_entries_resemble_maildir (struct dirent **entries, int count)
+_entries_resemble_maildir (const char *path, struct dirent **entries, int 
count)
 {
 int i, found = 0;

 for (i = 0; i < count; i++) {
-   if (entries[i]->d_type != DT_DIR && entries[i]->d_type != DT_UNKNOWN)
+   if (dirent_type (path, entries[i]) != S_IFDIR)
continue;

if (strcmp(entries[i]->d_name, "new") == 0 ||
@@ -250,7 +288,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
 notmuch_message_t *message = NULL;
 struct dirent **fs_entries = NULL;
-int i, num_fs_entries;
+int i, num_fs_entries, entry_type;
 notmuch_directory_t *directory;
 notmuch_filenames_t *db_files = NULL;
 notmuch_filenames_t *db_subdirs = NULL;
@@ -317,7 +355,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 }

 /* Pass 1: Recurse into all sub-directories. */
-is_maildir = _entries_resemble_maildir (fs_entries, num_fs_entries);
+is_maildir = _entries_resemble_maildir (path, fs_entries, num_fs_entries);

 for (i = 0; i < num_fs_entries; i++) {
if (interrupted)
@@ -325,17 +363,16 @@ add_files_recursive (notmuch_database_t *notmuch,

entry = fs_entries[i];

-   /* We only want to descend into directories.
-* But symlinks can be to directories too, of course.
-*
-* And if the filesystem doesn't tell us the file type in the
-* scandir results, then it might be a directory (and if not,
-* then we'll stat and return immediately in the next level of
-* recursion). */
-   if (entry->d_type != DT_DIR &&
-   entry->d_type != DT_LNK &&
-   entry->d_type != DT_UNKNOWN)
-   {
+   /* We only want to descend into directories (and symlinks to
+* directories). */
+   entry_type = dirent_type (path, entry);
+   if (entry_type == -1) {
+   /* Be pessimistic, e.g. so we don't loose lots of mail
+* just because a user broke a symlink. */
+   fprintf (stderr, "Error reading file %s/%s: %s\n",
+path, entry->d_name, strerror (errno));
+   return NOTMUCH_STATUS_FILE_ERROR;
+   } else if (entry_type != S_IFDIR) {
continue;
}

@@ -425,31 +462,13 @@ add_files_recursive (notmuch_database_t *notmuch,
notmuch_filenames_move_to_next (db_subdirs);
}

-   /* If we're looking at a symlink, we only want to add it if it
-* links to a regular file, (and not to a directory, say).
-*
-* Similarly, if the file is of unknown type (due to

[PATCH 1/2] test: Test notmuch new with a broken symlink

2012-05-07 Thread Austin Clements

---
 test/new |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/test/new b/test/new
index 99f9913..26253db 100755
--- a/test/new
+++ b/test/new
@@ -136,6 +136,16 @@ output=$(NOTMUCH_NEW)
 test_expect_equal "$output" "Added 1 new message to the database."


+test_begin_subtest "Broken symlink aborts"
+ln -s does-not-exist "${MAIL_DIR}/broken"
+output=$(NOTMUCH_NEW 2>&1)
+test_expect_equal "$output" \
+"Error reading directory /run/shm/nm/tmp.new/mail/broken: No such file or 
directory
+Note: A fatal error was encountered: Something went wrong trying to read or 
write a file
+No new mail."
+rm "${MAIL_DIR}/broken"
+
+
 test_begin_subtest "New two-level directory"

 generate_message [dir]=two/levels
-- 
1.7.10

[PATCH 0/2] Better, more portable stat-ing logic for notmuch new

2012-05-07 Thread Austin Clements

This fully implements my suggestion from
id:"20120411193609.GC13549 at mit.edu".  Originally this was just meant
to make the code more portable (since d_type is  not portable), but it
wound up also simplifying some of the notmuch new logic.

android 3.0 tablet and notmuch?

2012-05-07 Thread Michal Sojka

David Belohrad  writes:

> Dear All,
>
> i experience very weird problem: have samsung galaxy tab 10.1 without
> any firmware tweaks. now, when I take a picture in the tablet and send
> it via email to me, then I open this email in emacs, the picture is not
> there. Only text part of the email is present.
>
> It seems like the emacs does not recognize an image. When I open the
> email in the roundcube via web interface, the image is there. As well
> when I open the message in the tablet itself, the image is there as
> well.
>
> Any idea what this might be? Anyone experiences the same problem?

Yes, it happens to me with some emails sent from MS Outlook. JSON output
contains the image part, but emacs doesn't show anything. I have no clue
why.

-Michal

[PATCH] test: Force reply to use html2text for consistency

2012-05-07 Thread Tomi Ollila

On Mon, May 07 2012, David Bremner  wrote:

> Adam Wolfe Gordon  writes:
>
>> The output of the HTML reply test in the emacs suite can vary
>> depending on which HTML renderers are installed on the machine running
>> the tests. The renderer that is always available is emacs's builtin
>> html2text function. In order to get consistency, force the test to use
>> html2text even if other renderers are available.
>
> pushed, and it seems to replicate the test results on the buildbot. If
> html2text ever gets less crappy I guess we'll have to redo the test
> data.

If html2text ever gets less crappy then we face the same problem -- users
with different emacs get different rendering output making tests fail
for some users.

Then we can do "(let ... (mm-text-html-renderer 'mm-insert-part))

to get html inserted unmodified; excerpt from test output using above:

 -> Hi,This is an HTML test message.OK?
 +> Hi,This is an HTML test message.OK?

>
> d

Tomi

[Patch v2] emacs: add pipe attachment command

2012-05-07 Thread Jameson Graef Rollins

On Mon, May 07 2012, Mark Walters  wrote:
> Sorry somehow I missed that you had actually sent the rebase too: and
> then I messed up my rebase. How embarrassing! 
>
> Anyway many thanks for the correct rebase, and for spotting my stupid error.

np ;) I've done plenty of things much more embarrassing than that!

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120507/cf43f036/attachment.pgp>

[Patch v2] emacs: add pipe attachment command

2012-05-07 Thread Mark Walters

Allow the user to pipe the attachment somewhere. Bound to '|' on the
attachment button.
---

Thanks for the review!

Here is a rebased to current master version of the previous patch: no
other changes.

Best wishes

Mark

 emacs/notmuch-show.el |   27 +++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index d318430..b6a1980 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -453,6 +453,7 @@ message at DEPTH in the current thread."
 (define-key map "s" 'notmuch-show-part-button-save)
 (define-key map "v" 'notmuch-show-part-button-view)
 (define-key map "o" 'notmuch-show-part-button-interactively-view)
+(define-key map "|" 'notmuch-show-part-button-pipe)
 map)
   "Submap for button commands")
 (fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
@@ -524,6 +525,28 @@ message at DEPTH in the current thread."
 (let ((handle (mm-make-handle (current-buffer) (list content-type
   (mm-interactively-view-part handle

+(defun notmuch-show-pipe-part (message-id nth  filename content-type)
+  (notmuch-with-temp-part-buffer message-id nth
+(let ((handle (mm-make-handle (current-buffer) (list content-type
+  (mm-pipe-part handle
+
+(defun notmuch-show-mm-display-part-inline (msg part nth content-type)
+  "Use the mm-decode/mm-view functions to display a part in the
+current buffer, if possible."
+  (let ((display-buffer (current-buffer)))
+(with-temp-buffer
+  (let* ((charset (plist-get part :content-charset))
+(handle (mm-make-handle (current-buffer) `(,content-type (charset 
. ,charset)
+   ;; If the user wants the part inlined, insert the content and
+   ;; test whether we are able to inline it (which includes both
+   ;; capability and suitability tests).
+   (when (mm-inlined-p handle)
+ (insert (notmuch-get-bodypart-content msg part nth 
notmuch-show-process-crypto))
+ (when (mm-inlinable-p handle)
+   (set-buffer display-buffer)
+   (mm-display-part handle)
+   t))
+
 (defun notmuch-show-multipart/*-to-list (part)
   (mapcar (lambda (inner-part) (plist-get inner-part :content-type))
  (plist-get part :content)))
@@ -1874,6 +1897,10 @@ the user (see 
`notmuch-show-stash-mlarchive-link-alist')."
   (interactive)
   (notmuch-show-part-button-internal button 
#'notmuch-show-interactively-view-part))

+(defun notmuch-show-part-button-pipe ( button)
+  (interactive)
+  (notmuch-show-part-button-internal button #'notmuch-show-pipe-part))
+
 (defun notmuch-show-part-button-internal (button handler)
   (let ((button (or button (button-at (point)
 (if button
-- 
1.7.9.1

[Patch v2] emacs: add pipe attachment command

2012-05-07 Thread Jameson Graef Rollins

On Mon, May 07 2012, Mark Walters  wrote:
> Here is a rebased to current master version of the previous patch: no
> other changes.

Actually, what I sent was a rebase.  And this rebase doesn't look
correct.  The notmuch-show-mm-display-part-inline function moved to
notmuch-lib.el.  It certainly has nothing to do with this patch, so I
don't think adding it here is correct.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120507/d0040d05/attachment.pgp>

[PATCH] emacs: add pipe attachment command

2012-05-07 Thread Jameson Graef Rollins

From: Mark Walters 

Allow the user to pipe the attachment somewhere. Bound to '|' on the
attachment button.

Signed-off-by: Jameson Graef Rollins 
---
Nice missing feature.  Tested and reviewed.  It just needed to be
rebased against the current master.

 emacs/notmuch-show.el |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index d318430..26bff8f 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -453,6 +453,7 @@ message at DEPTH in the current thread."
 (define-key map "s" 'notmuch-show-part-button-save)
 (define-key map "v" 'notmuch-show-part-button-view)
 (define-key map "o" 'notmuch-show-part-button-interactively-view)
+(define-key map "|" 'notmuch-show-part-button-pipe)
 map)
   "Submap for button commands")
 (fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
@@ -524,6 +525,11 @@ message at DEPTH in the current thread."
 (let ((handle (mm-make-handle (current-buffer) (list content-type
   (mm-interactively-view-part handle

+(defun notmuch-show-pipe-part (message-id nth  filename content-type)
+  (notmuch-with-temp-part-buffer message-id nth
+(let ((handle (mm-make-handle (current-buffer) (list content-type
+  (mm-pipe-part handle
+
 (defun notmuch-show-multipart/*-to-list (part)
   (mapcar (lambda (inner-part) (plist-get inner-part :content-type))
  (plist-get part :content)))
@@ -1874,6 +1880,10 @@ the user (see 
`notmuch-show-stash-mlarchive-link-alist')."
   (interactive)
   (notmuch-show-part-button-internal button 
#'notmuch-show-interactively-view-part))

+(defun notmuch-show-part-button-pipe ( button)
+  (interactive)
+  (notmuch-show-part-button-internal button #'notmuch-show-pipe-part))
+
 (defun notmuch-show-part-button-internal (button handler)
   (let ((button (or button (button-at (point)
 (if button
-- 
1.7.10

Re: [PATCH] test: Force reply to use html2text for consistency

2012-05-07 Thread Tomi Ollila

On Mon, May 07 2012, David Bremner da...@tethera.net wrote:

 Adam Wolfe Gordon awg+notm...@xvx.ca writes:

 The output of the HTML reply test in the emacs suite can vary
 depending on which HTML renderers are installed on the machine running
 the tests. The renderer that is always available is emacs's builtin
 html2text function. In order to get consistency, force the test to use
 html2text even if other renderers are available.

 pushed, and it seems to replicate the test results on the buildbot. If
 html2text ever gets less crappy I guess we'll have to redo the test
 data.

If html2text ever gets less crappy then we face the same problem -- users
with different emacs get different rendering output making tests fail
for some users.

Then we can do (let ... (mm-text-html-renderer 'mm-insert-part))

to get html inserted unmodified; excerpt from test output using above:

 - Hi,This is an HTML test message.OK?
 + Hi,br /This is an bHTML/b test message.br /br /OK?


 d

Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] emacs: add pipe attachment command

2012-05-07 Thread Jameson Graef Rollins

From: Mark Walters markwalters1...@gmail.com

Allow the user to pipe the attachment somewhere. Bound to '|' on the
attachment button.

Signed-off-by: Jameson Graef Rollins jroll...@finestructure.net
---
Nice missing feature.  Tested and reviewed.  It just needed to be
rebased against the current master.

 emacs/notmuch-show.el |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index d318430..26bff8f 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -453,6 +453,7 @@ message at DEPTH in the current thread.
 (define-key map s 'notmuch-show-part-button-save)
 (define-key map v 'notmuch-show-part-button-view)
 (define-key map o 'notmuch-show-part-button-interactively-view)
+(define-key map | 'notmuch-show-part-button-pipe)
 map)
   Submap for button commands)
 (fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
@@ -524,6 +525,11 @@ message at DEPTH in the current thread.
 (let ((handle (mm-make-handle (current-buffer) (list content-type
   (mm-interactively-view-part handle
 
+(defun notmuch-show-pipe-part (message-id nth optional filename content-type)
+  (notmuch-with-temp-part-buffer message-id nth
+(let ((handle (mm-make-handle (current-buffer) (list content-type
+  (mm-pipe-part handle
+
 (defun notmuch-show-multipart/*-to-list (part)
   (mapcar (lambda (inner-part) (plist-get inner-part :content-type))
  (plist-get part :content)))
@@ -1874,6 +1880,10 @@ the user (see 
`notmuch-show-stash-mlarchive-link-alist').
   (interactive)
   (notmuch-show-part-button-internal button 
#'notmuch-show-interactively-view-part))
 
+(defun notmuch-show-part-button-pipe (optional button)
+  (interactive)
+  (notmuch-show-part-button-internal button #'notmuch-show-pipe-part))
+
 (defun notmuch-show-part-button-internal (button handler)
   (let ((button (or button (button-at (point)
 (if button
-- 
1.7.10

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[Patch v2] emacs: add pipe attachment command

2012-05-07 Thread Mark Walters

Allow the user to pipe the attachment somewhere. Bound to '|' on the
attachment button.
---

Thanks for the review!

Here is a rebased to current master version of the previous patch: no
other changes.

Best wishes

Mark

 emacs/notmuch-show.el |   27 +++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index d318430..b6a1980 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -453,6 +453,7 @@ message at DEPTH in the current thread.
 (define-key map s 'notmuch-show-part-button-save)
 (define-key map v 'notmuch-show-part-button-view)
 (define-key map o 'notmuch-show-part-button-interactively-view)
+(define-key map | 'notmuch-show-part-button-pipe)
 map)
   Submap for button commands)
 (fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
@@ -524,6 +525,28 @@ message at DEPTH in the current thread.
 (let ((handle (mm-make-handle (current-buffer) (list content-type
   (mm-interactively-view-part handle
 
+(defun notmuch-show-pipe-part (message-id nth optional filename content-type)
+  (notmuch-with-temp-part-buffer message-id nth
+(let ((handle (mm-make-handle (current-buffer) (list content-type
+  (mm-pipe-part handle
+
+(defun notmuch-show-mm-display-part-inline (msg part nth content-type)
+  Use the mm-decode/mm-view functions to display a part in the
+current buffer, if possible.
+  (let ((display-buffer (current-buffer)))
+(with-temp-buffer
+  (let* ((charset (plist-get part :content-charset))
+(handle (mm-make-handle (current-buffer) `(,content-type (charset 
. ,charset)
+   ;; If the user wants the part inlined, insert the content and
+   ;; test whether we are able to inline it (which includes both
+   ;; capability and suitability tests).
+   (when (mm-inlined-p handle)
+ (insert (notmuch-get-bodypart-content msg part nth 
notmuch-show-process-crypto))
+ (when (mm-inlinable-p handle)
+   (set-buffer display-buffer)
+   (mm-display-part handle)
+   t))
+
 (defun notmuch-show-multipart/*-to-list (part)
   (mapcar (lambda (inner-part) (plist-get inner-part :content-type))
  (plist-get part :content)))
@@ -1874,6 +1897,10 @@ the user (see 
`notmuch-show-stash-mlarchive-link-alist').
   (interactive)
   (notmuch-show-part-button-internal button 
#'notmuch-show-interactively-view-part))
 
+(defun notmuch-show-part-button-pipe (optional button)
+  (interactive)
+  (notmuch-show-part-button-internal button #'notmuch-show-pipe-part))
+
 (defun notmuch-show-part-button-internal (button handler)
   (let ((button (or button (button-at (point)
 (if button
-- 
1.7.9.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: android 3.0 tablet and notmuch?

2012-05-07 Thread Michal Sojka

David Belohrad da...@belohrad.ch writes:

 Dear All,

 i experience very weird problem: have samsung galaxy tab 10.1 without
 any firmware tweaks. now, when I take a picture in the tablet and send
 it via email to me, then I open this email in emacs, the picture is not
 there. Only text part of the email is present.

 It seems like the emacs does not recognize an image. When I open the
 email in the roundcube via web interface, the image is there. As well
 when I open the message in the tablet itself, the image is there as
 well.

 Any idea what this might be? Anyone experiences the same problem?

Yes, it happens to me with some emails sent from MS Outlook. JSON output
contains the image part, but emacs doesn't show anything. I have no clue
why.

-Michal
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 0/4] ruby: quick update before the freeze!

2012-05-07 Thread Ali Polatel

Hey David,

I've polished the small patch series I had lying around.
I believe I've covered all the lacking API changes.

In addition, the third patch fixes a long-standing bug about the build.

I've decided to send these to you and the list instead of committing right away
in case someone notices any quick errors. Otherwise, please commit these before
the freeze and make rubyists happy.

Ali Polatel (4):
  ruby: Add wrapper for notmuch_query_count_messages
  ruby: Add wrapper for notmuch_query_add_tag_exclude
  ruby: Add workarounds to use in-tree build not the installed one
  ruby: Add wrapper for notmuch_query_set_omit_excluded()

 bindings/ruby/defs.h |   13 +--
 bindings/ruby/extconf.rb |   26 +
 bindings/ruby/init.c |   11 -
 bindings/ruby/query.c|   57 +-
 4 files changed, 99 insertions(+), 8 deletions(-)

-- 
1.7.10.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 1/4] ruby: Add wrapper for notmuch_query_count_messages

2012-05-07 Thread Ali Polatel

---
 bindings/ruby/defs.h  |3 +++
 bindings/ruby/init.c  |3 ++-
 bindings/ruby/query.c |   21 -
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/bindings/ruby/defs.h b/bindings/ruby/defs.h
index 81f652f..25222a6 100644
--- a/bindings/ruby/defs.h
+++ b/bindings/ruby/defs.h
@@ -222,6 +222,9 @@ notmuch_rb_query_search_threads (VALUE self);
 VALUE
 notmuch_rb_query_search_messages (VALUE self);
 
+VALUE
+notmuch_rb_query_count_messages (VALUE self);
+
 /* threads.c */
 VALUE
 notmuch_rb_threads_destroy (VALUE self);
diff --git a/bindings/ruby/init.c b/bindings/ruby/init.c
index 4405f19..7ad0ecf 100644
--- a/bindings/ruby/init.c
+++ b/bindings/ruby/init.c
@@ -1,6 +1,6 @@
 /* The Ruby interface to the notmuch mail library
  *
- * Copyright © 2010, 2011 Ali Polatel
+ * Copyright © 2010, 2011, 2012 Ali Polatel
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -236,6 +236,7 @@ Init_notmuch (void)
 rb_define_method (notmuch_rb_cQuery, to_s, notmuch_rb_query_get_string, 
0); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, search_threads, 
notmuch_rb_query_search_threads, 0); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, search_messages, 
notmuch_rb_query_search_messages, 0); /* in query.c */
+rb_define_method (notmuch_rb_cQuery, count_messages, 
notmuch_rb_query_count_messages, 0); /* in query.c */
 
 /*
  * Document-class: Notmuch::Threads
diff --git a/bindings/ruby/query.c b/bindings/ruby/query.c
index 74fd585..02b7819 100644
--- a/bindings/ruby/query.c
+++ b/bindings/ruby/query.c
@@ -1,6 +1,6 @@
 /* The Ruby interface to the notmuch mail library
  *
- * Copyright © 2010, 2011 Ali Polatel
+ * Copyright © 2010, 2011, 2012 Ali Polatel
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -127,3 +127,22 @@ notmuch_rb_query_search_messages (VALUE self)
 
 return Data_Wrap_Struct (notmuch_rb_cMessages, NULL, NULL, messages);
 }
+
+/*
+ * call-seq: QUERY.count_messages = Fixnum
+ *
+ * Return an estimate of the number of messages matching a search
+ */
+VALUE
+notmuch_rb_query_count_messages (VALUE self)
+{
+notmuch_query_t *query;
+
+Data_Get_Notmuch_Query (self, query);
+
+/* Xapian exceptions are not handled properly.
+ * (function may return 0 after printing a message)
+ * Thus there is nothing we can do here...
+ */
+return UINT2FIX(notmuch_query_count_messages(query));
+}
-- 
1.7.10.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 2/4] ruby: Add wrapper for notmuch_query_add_tag_exclude

2012-05-07 Thread Ali Polatel

---
 bindings/ruby/defs.h  |3 +++
 bindings/ruby/init.c  |1 +
 bindings/ruby/query.c |   18 ++
 3 files changed, 22 insertions(+)

diff --git a/bindings/ruby/defs.h b/bindings/ruby/defs.h
index 25222a6..a41cf10 100644
--- a/bindings/ruby/defs.h
+++ b/bindings/ruby/defs.h
@@ -217,6 +217,9 @@ VALUE
 notmuch_rb_query_get_string (VALUE self);
 
 VALUE
+notmuch_rb_query_add_tag_exclude (VALUE self, VALUE tagv);
+
+VALUE
 notmuch_rb_query_search_threads (VALUE self);
 
 VALUE
diff --git a/bindings/ruby/init.c b/bindings/ruby/init.c
index 7ad0ecf..edcf101 100644
--- a/bindings/ruby/init.c
+++ b/bindings/ruby/init.c
@@ -234,6 +234,7 @@ Init_notmuch (void)
 rb_define_method (notmuch_rb_cQuery, sort, notmuch_rb_query_get_sort, 
0); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, sort=, notmuch_rb_query_set_sort, 
1); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, to_s, notmuch_rb_query_get_string, 
0); /* in query.c */
+rb_define_method (notmuch_rb_cQuery, add_tag_exclude, 
notmuch_rb_query_add_tag_exclude, 1); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, search_threads, 
notmuch_rb_query_search_threads, 0); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, search_messages, 
notmuch_rb_query_search_messages, 0); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, count_messages, 
notmuch_rb_query_count_messages, 0); /* in query.c */
diff --git a/bindings/ruby/query.c b/bindings/ruby/query.c
index 02b7819..2a80008 100644
--- a/bindings/ruby/query.c
+++ b/bindings/ruby/query.c
@@ -89,6 +89,24 @@ notmuch_rb_query_get_string (VALUE self)
 }
 
 /*
+ * call-seq: QUERY.add_tag_exclude(tag) = nil
+ *
+ * Add a tag that will be excluded from the query results by default.
+ */
+VALUE
+notmuch_rb_query_add_tag_exclude (VALUE self, VALUE tagv)
+{
+notmuch_query_t *query;
+const char *tag;
+
+Data_Get_Notmuch_Query (self, query);
+tag = RSTRING_PTR(tagv);
+
+notmuch_query_add_tag_exclude(query, tag);
+return Qnil;
+}
+
+/*
  * call-seq: QUERY.search_threads = THREADS
  *
  * Search for threads
-- 
1.7.10.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 3/4] ruby: Add workarounds to use in-tree build not the installed one

2012-05-07 Thread Ali Polatel

- Make mkmf use the notmuch.h under ../../lib
- Use libnotmuch.a instead of linking to the installed libnotmuch.so
---
 bindings/ruby/defs.h |4 ++--
 bindings/ruby/extconf.rb |   26 ++
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/bindings/ruby/defs.h b/bindings/ruby/defs.h
index a41cf10..6fe5787 100644
--- a/bindings/ruby/defs.h
+++ b/bindings/ruby/defs.h
@@ -1,6 +1,6 @@
 /* The Ruby interface to the notmuch mail library
  *
- * Copyright © 2010, 2011 Ali Polatel
+ * Copyright © 2010, 2011, 2012 Ali Polatel
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -21,8 +21,8 @@
 #ifndef DEFS_H
 #define DEFS_H
 
-#include notmuch.h
 #include ruby.h
+#include notmuch.h
 
 VALUE notmuch_rb_cDatabase;
 VALUE notmuch_rb_cDirectory;
diff --git a/bindings/ruby/extconf.rb b/bindings/ruby/extconf.rb
index ccac609..933f34a 100644
--- a/bindings/ruby/extconf.rb
+++ b/bindings/ruby/extconf.rb
@@ -1,13 +1,31 @@
 #!/usr/bin/env ruby
 # coding: utf-8
-# Copyright 2010, 2011 Ali Polatel a...@exherbo.org
+# Copyright 2010, 2011, 2012 Ali Polatel a...@exherbo.org
 # Distributed under the terms of the GNU General Public License v3
 
 require 'mkmf'
 
-# Notmuch Library
-find_header('notmuch.h', '../../lib')
-find_library('notmuch', 'notmuch_database_create', '../../lib')
+NOTDIR = File.expand_path(File.join(File.dirname(__FILE__), '..', '..', 'lib'))
+NOTHDR = File.join(NOTDIR, 'notmuch.h')
+NOTLIB = File.join(NOTDIR, 'libnotmuch.a')
+
+unless File.exists? NOTHDR
+  $stderr.puts notmuch.h is missing under #{NOTDIR}
+  exit 1
+end
+
+unless File.exists? NOTLIB
+  $stderr.puts libnotmuch.a is missing under #{NOTDIR}
+  exit 1
+end
+
+# Small hack to build with in-tree version not the installed one.
+# find_header() and friends use standard include/library paths first.
+$stderr.puts Added -I#{NOTDIR} to $INCFLAGS
+$INCFLAGS = -I#{NOTDIR}.quote +   + $INCFLAGS
+find_header('notmuch.h', NOTDIR)
+
+$LOCAL_LIBS += NOTLIB
 
 # Create Makefile
 dir_config('notmuch')
-- 
1.7.10.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 4/4] ruby: Add wrapper for notmuch_query_set_omit_excluded()

2012-05-07 Thread Ali Polatel

---
 bindings/ruby/defs.h  |3 +++
 bindings/ruby/init.c  |7 +++
 bindings/ruby/query.c |   18 ++
 3 files changed, 28 insertions(+)

diff --git a/bindings/ruby/defs.h b/bindings/ruby/defs.h
index 6fe5787..85d8205 100644
--- a/bindings/ruby/defs.h
+++ b/bindings/ruby/defs.h
@@ -220,6 +220,9 @@ VALUE
 notmuch_rb_query_add_tag_exclude (VALUE self, VALUE tagv);
 
 VALUE
+notmuch_rb_query_set_omit_excluded (VALUE self, VALUE omitv);
+
+VALUE
 notmuch_rb_query_search_threads (VALUE self);
 
 VALUE
diff --git a/bindings/ruby/init.c b/bindings/ruby/init.c
index edcf101..3fe60fb 100644
--- a/bindings/ruby/init.c
+++ b/bindings/ruby/init.c
@@ -96,6 +96,12 @@ Init_notmuch (void)
  */
 rb_define_const (mod, MESSAGE_FLAG_MATCH, INT2FIX 
(NOTMUCH_MESSAGE_FLAG_MATCH));
 /*
+ * Document-const: Notmuch::MESSAGE_FLAG_EXCLUDED
+ *
+ * Message flag excluded
+ */
+rb_define_const (mod, MESSAGE_FLAG_EXCLUDED, INT2FIX 
(NOTMUCH_MESSAGE_FLAG_EXCLUDED));
+/*
  * Document-const: Notmuch::TAG_MAX
  *
  * Maximum allowed length of a tag
@@ -235,6 +241,7 @@ Init_notmuch (void)
 rb_define_method (notmuch_rb_cQuery, sort=, notmuch_rb_query_set_sort, 
1); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, to_s, notmuch_rb_query_get_string, 
0); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, add_tag_exclude, 
notmuch_rb_query_add_tag_exclude, 1); /* in query.c */
+rb_define_method (notmuch_rb_cQuery, omit_excluded=, 
notmuch_rb_query_set_omit_excluded, 1); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, search_threads, 
notmuch_rb_query_search_threads, 0); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, search_messages, 
notmuch_rb_query_search_messages, 0); /* in query.c */
 rb_define_method (notmuch_rb_cQuery, count_messages, 
notmuch_rb_query_count_messages, 0); /* in query.c */
diff --git a/bindings/ruby/query.c b/bindings/ruby/query.c
index 2a80008..e5ba1b7 100644
--- a/bindings/ruby/query.c
+++ b/bindings/ruby/query.c
@@ -107,6 +107,24 @@ notmuch_rb_query_add_tag_exclude (VALUE self, VALUE tagv)
 }
 
 /*
+ * call-seq: QUERY.omit_excluded=(boolean) = nil
+ *
+ * Specify whether to omit excluded results or simply flag them.
+ * By default, this is set to +true+.
+ */
+VALUE
+notmuch_rb_query_set_omit_excluded (VALUE self, VALUE omitv)
+{
+notmuch_query_t *query;
+
+Data_Get_Notmuch_Query (self, query);
+
+notmuch_query_set_omit_excluded (query, RTEST (omitv));
+
+return Qnil;
+}
+
+/*
  * call-seq: QUERY.search_threads = THREADS
  *
  * Search for threads
-- 
1.7.10.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [Patch v2] emacs: add pipe attachment command

2012-05-07 Thread Jameson Graef Rollins

On Mon, May 07 2012, Mark Walters markwalters1...@gmail.com wrote:
 Here is a rebased to current master version of the previous patch: no
 other changes.

Actually, what I sent was a rebase.  And this rebase doesn't look
correct.  The notmuch-show-mm-display-part-inline function moved to
notmuch-lib.el.  It certainly has nothing to do with this patch, so I
don't think adding it here is correct.

jamie.


pgpRKaRxTYjat.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [Patch v2] emacs: add pipe attachment command

2012-05-07 Thread Mark Walters


On Mon, 07 May 2012, Jameson Graef Rollins jroll...@finestructure.net wrote:
 On Mon, May 07 2012, Mark Walters markwalters1...@gmail.com wrote:
 Here is a rebased to current master version of the previous patch: no
 other changes.

 Actually, what I sent was a rebase.  

Sorry somehow I missed that you had actually sent the rebase too: and
then I messed up my rebase. How embarrassing! 

Anyway many thanks for the correct rebase, and for spotting my stupid error.

Best wishes

Mark
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [Patch v2] emacs: add pipe attachment command

2012-05-07 Thread Jameson Graef Rollins

On Mon, May 07 2012, Mark Walters markwalters1...@gmail.com wrote:
 Sorry somehow I missed that you had actually sent the rebase too: and
 then I messed up my rebase. How embarrassing! 

 Anyway many thanks for the correct rebase, and for spotting my stupid error.

np ;) I've done plenty of things much more embarrassing than that!

jamie.


pgpaxK3rmUbFa.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 0/2] Better, more portable stat-ing logic for notmuch new

2012-05-07 Thread Austin Clements

This fully implements my suggestion from
id:20120411193609.gc13...@mit.edu.  Originally this was just meant
to make the code more portable (since d_type is  not portable), but it
wound up also simplifying some of the notmuch new logic.

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 1/2] test: Test notmuch new with a broken symlink

2012-05-07 Thread Austin Clements

---
 test/new |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/test/new b/test/new
index 99f9913..26253db 100755
--- a/test/new
+++ b/test/new
@@ -136,6 +136,16 @@ output=$(NOTMUCH_NEW)
 test_expect_equal $output Added 1 new message to the database.
 
 
+test_begin_subtest Broken symlink aborts
+ln -s does-not-exist ${MAIL_DIR}/broken
+output=$(NOTMUCH_NEW 21)
+test_expect_equal $output \
+Error reading directory /run/shm/nm/tmp.new/mail/broken: No such file or 
directory
+Note: A fatal error was encountered: Something went wrong trying to read or 
write a file
+No new mail.
+rm ${MAIL_DIR}/broken
+
+
 test_begin_subtest New two-level directory
 
 generate_message [dir]=two/levels
-- 
1.7.10

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 2/2] new: Centralize file type stat-ing logic

2012-05-07 Thread Austin Clements

This moves our logic to get a file's type into one function.  This has
several benefits: we can support OSes and file systems that do not
provide dirent.d_type or always return DT_UNKNOWN, complex
symlink-handling logic has been replaced by a simple stat fall-through
in one place, and the error message for un-stat-able file is more
accurate (previously, the error always mentioned directories, even
though a broken symlink is not a directory).
---
 notmuch-new.c |   99 ++---
 test/new  |2 +-
 2 files changed, 60 insertions(+), 41 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index cb720cc..cf2580e 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -154,6 +154,44 @@ dirent_sort_strcmp_name (const struct dirent **a, const 
struct dirent **b)
 return strcmp ((*a)-d_name, (*b)-d_name);
 }
 
+/* Return the type of a directory entry relative to path as a stat(2)
+ * mode.  Like stat, this follows symlinks.  Returns -1 and sets errno
+ * if the file's type cannot be determined (which includes dangling
+ * symlinks).
+ */
+static int
+dirent_type (const char *path, const struct dirent *entry)
+{
+struct stat statbuf;
+char *abspath;
+int err;
+
+#ifdef _DIRENT_HAVE_D_TYPE
+/* Mapping from d_type to stat mode_t.  We omit DT_LNK so that
+ * we'll fall through to stat and get the real file type. */
+static const mode_t modes[] = {
+   [DT_BLK]  = S_IFBLK,
+   [DT_CHR]  = S_IFCHR,
+   [DT_DIR]  = S_IFDIR,
+   [DT_FIFO] = S_IFIFO,
+   [DT_REG]  = S_IFREG,
+   [DT_SOCK] = S_IFSOCK
+};
+if (entry-d_type  sizeof(modes)/sizeof(modes[0]) 
+   modes[entry-d_type])
+   return modes[entry-d_type];
+#endif
+
+abspath = talloc_asprintf (NULL, %s/%s, path, entry-d_name);
+if (!abspath)
+   return -1;
+err = stat(abspath, statbuf);
+talloc_free (abspath);
+if (err  0)
+   return -1;
+return statbuf.st_mode  S_IFMT;
+}
+
 /* Test if the directory looks like a Maildir directory.
  *
  * Search through the array of directory entries to see if we can find all
@@ -162,12 +200,12 @@ dirent_sort_strcmp_name (const struct dirent **a, const 
struct dirent **b)
  * Return 1 if the directory looks like a Maildir and 0 otherwise.
  */
 static int
-_entries_resemble_maildir (struct dirent **entries, int count)
+_entries_resemble_maildir (const char *path, struct dirent **entries, int 
count)
 {
 int i, found = 0;
 
 for (i = 0; i  count; i++) {
-   if (entries[i]-d_type != DT_DIR  entries[i]-d_type != DT_UNKNOWN)
+   if (dirent_type (path, entries[i]) != S_IFDIR)
continue;
 
if (strcmp(entries[i]-d_name, new) == 0 ||
@@ -250,7 +288,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
 notmuch_message_t *message = NULL;
 struct dirent **fs_entries = NULL;
-int i, num_fs_entries;
+int i, num_fs_entries, entry_type;
 notmuch_directory_t *directory;
 notmuch_filenames_t *db_files = NULL;
 notmuch_filenames_t *db_subdirs = NULL;
@@ -317,7 +355,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 }
 
 /* Pass 1: Recurse into all sub-directories. */
-is_maildir = _entries_resemble_maildir (fs_entries, num_fs_entries);
+is_maildir = _entries_resemble_maildir (path, fs_entries, num_fs_entries);
 
 for (i = 0; i  num_fs_entries; i++) {
if (interrupted)
@@ -325,17 +363,16 @@ add_files_recursive (notmuch_database_t *notmuch,
 
entry = fs_entries[i];
 
-   /* We only want to descend into directories.
-* But symlinks can be to directories too, of course.
-*
-* And if the filesystem doesn't tell us the file type in the
-* scandir results, then it might be a directory (and if not,
-* then we'll stat and return immediately in the next level of
-* recursion). */
-   if (entry-d_type != DT_DIR 
-   entry-d_type != DT_LNK 
-   entry-d_type != DT_UNKNOWN)
-   {
+   /* We only want to descend into directories (and symlinks to
+* directories). */
+   entry_type = dirent_type (path, entry);
+   if (entry_type == -1) {
+   /* Be pessimistic, e.g. so we don't loose lots of mail
+* just because a user broke a symlink. */
+   fprintf (stderr, Error reading file %s/%s: %s\n,
+path, entry-d_name, strerror (errno));
+   return NOTMUCH_STATUS_FILE_ERROR;
+   } else if (entry_type != S_IFDIR) {
continue;
}
 
@@ -425,31 +462,13 @@ add_files_recursive (notmuch_database_t *notmuch,
notmuch_filenames_move_to_next (db_subdirs);
}
 
-   /* If we're looking at a symlink, we only want to add it if it
-* links to a regular file, (and not to a directory, say).
-*
-* Similarly, if the file is of unknown type (due to filesystem
-*

Re: [PATCH 2/2] new: Centralize file type stat-ing logic

2012-05-07 Thread Jani Nikula

On May 7, 2012 9:10 PM, Austin Clements amdra...@mit.edu wrote:

 This moves our logic to get a file's type into one function.  This has
 several benefits: we can support OSes and file systems that do not
 provide dirent.d_type or always return DT_UNKNOWN, complex
 symlink-handling logic has been replaced by a simple stat fall-through
 in one place, and the error message for un-stat-able file is more
 accurate (previously, the error always mentioned directories, even
 though a broken symlink is not a directory).

Please find some quick drive-by-review below.

J.

 ---
  notmuch-new.c |   99
++---
  test/new  |2 +-
  2 files changed, 60 insertions(+), 41 deletions(-)

 diff --git a/notmuch-new.c b/notmuch-new.c
 index cb720cc..cf2580e 100644
 --- a/notmuch-new.c
 +++ b/notmuch-new.c
 @@ -154,6 +154,44 @@ dirent_sort_strcmp_name (const struct dirent **a,
const struct dirent **b)
 return strcmp ((*a)-d_name, (*b)-d_name);
  }

 +/* Return the type of a directory entry relative to path as a stat(2)
 + * mode.  Like stat, this follows symlinks.  Returns -1 and sets errno
 + * if the file's type cannot be determined (which includes dangling
 + * symlinks).
 + */
 +static int
 +dirent_type (const char *path, const struct dirent *entry)
 +{
 +struct stat statbuf;
 +char *abspath;
 +int err;
 +
 +#ifdef _DIRENT_HAVE_D_TYPE
 +/* Mapping from d_type to stat mode_t.  We omit DT_LNK so that
 + * we'll fall through to stat and get the real file type. */
 +static const mode_t modes[] = {
 +   [DT_BLK]  = S_IFBLK,
 +   [DT_CHR]  = S_IFCHR,
 +   [DT_DIR]  = S_IFDIR,
 +   [DT_FIFO] = S_IFIFO,
 +   [DT_REG]  = S_IFREG,
 +   [DT_SOCK] = S_IFSOCK
 +};
 +if (entry-d_type  sizeof(modes)/sizeof(modes[0]) 

ARRAY_SIZE()

 +   modes[entry-d_type])
 +   return modes[entry-d_type];
 +#endif
 +
 +abspath = talloc_asprintf (NULL, %s/%s, path, entry-d_name);
 +if (!abspath)
 +   return -1;

Does talloc set errno in this case? I suspect not.

 +err = stat(abspath, statbuf);
 +talloc_free (abspath);

This likely breaks your promise about errno. You can't trust talloc_free()
not calling some function that sets errno.

 +if (err  0)
 +   return -1;
 +return statbuf.st_mode  S_IFMT;
 +}
 +
  /* Test if the directory looks like a Maildir directory.
  *
  * Search through the array of directory entries to see if we can find all
 @@ -162,12 +200,12 @@ dirent_sort_strcmp_name (const struct dirent **a,
const struct dirent **b)
  * Return 1 if the directory looks like a Maildir and 0 otherwise.
  */
  static int
 -_entries_resemble_maildir (struct dirent **entries, int count)
 +_entries_resemble_maildir (const char *path, struct dirent **entries,
int count)
  {
 int i, found = 0;

 for (i = 0; i  count; i++) {
 -   if (entries[i]-d_type != DT_DIR  entries[i]-d_type !=
DT_UNKNOWN)
 +   if (dirent_type (path, entries[i]) != S_IFDIR)
continue;

if (strcmp(entries[i]-d_name, new) == 0 ||
 @@ -250,7 +288,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
 notmuch_message_t *message = NULL;
 struct dirent **fs_entries = NULL;
 -int i, num_fs_entries;
 +int i, num_fs_entries, entry_type;
 notmuch_directory_t *directory;
 notmuch_filenames_t *db_files = NULL;
 notmuch_filenames_t *db_subdirs = NULL;
 @@ -317,7 +355,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 }

 /* Pass 1: Recurse into all sub-directories. */
 -is_maildir = _entries_resemble_maildir (fs_entries, num_fs_entries);
 +is_maildir = _entries_resemble_maildir (path, fs_entries,
num_fs_entries);

 for (i = 0; i  num_fs_entries; i++) {
if (interrupted)
 @@ -325,17 +363,16 @@ add_files_recursive (notmuch_database_t *notmuch,

entry = fs_entries[i];

 -   /* We only want to descend into directories.
 -* But symlinks can be to directories too, of course.
 -*
 -* And if the filesystem doesn't tell us the file type in the
 -* scandir results, then it might be a directory (and if not,
 -* then we'll stat and return immediately in the next level of
 -* recursion). */
 -   if (entry-d_type != DT_DIR 
 -   entry-d_type != DT_LNK 
 -   entry-d_type != DT_UNKNOWN)
 -   {
 +   /* We only want to descend into directories (and symlinks to
 +* directories). */
 +   entry_type = dirent_type (path, entry);
 +   if (entry_type == -1) {
 +   /* Be pessimistic, e.g. so we don't loose lots of mail

s/loose/lose/ ?

 +* just because a user broke a symlink. */
 +   fprintf (stderr, Error reading file %s/%s: %s\n,
 +path, entry-d_name, strerror (errno));

You can't trust errno here, as explained above.

 +   return NOTMUCH_STATUS_FILE_ERROR;

Re: [PATCH 2/2] new: Centralize file type stat-ing logic

2012-05-07 Thread Jani Nikula

On May 8, 2012 12:08 AM, Jani Nikula j...@nikula.org wrote:


 On May 7, 2012 9:10 PM, Austin Clements amdra...@mit.edu wrote:
 
  This moves our logic to get a file's type into one function.  This has
  several benefits: we can support OSes and file systems that do not
  provide dirent.d_type or always return DT_UNKNOWN, complex
  symlink-handling logic has been replaced by a simple stat fall-through
  in one place, and the error message for un-stat-able file is more
  accurate (previously, the error always mentioned directories, even
  though a broken symlink is not a directory).

 Please find some quick drive-by-review below.

Forgot to add that the general approach seems sensible to me.


 J.

  ---
   notmuch-new.c |   99
++---
   test/new  |2 +-
   2 files changed, 60 insertions(+), 41 deletions(-)
 
  diff --git a/notmuch-new.c b/notmuch-new.c
  index cb720cc..cf2580e 100644
  --- a/notmuch-new.c
  +++ b/notmuch-new.c
  @@ -154,6 +154,44 @@ dirent_sort_strcmp_name (const struct dirent **a,
const struct dirent **b)
  return strcmp ((*a)-d_name, (*b)-d_name);
   }
 
  +/* Return the type of a directory entry relative to path as a stat(2)
  + * mode.  Like stat, this follows symlinks.  Returns -1 and sets errno
  + * if the file's type cannot be determined (which includes dangling
  + * symlinks).
  + */
  +static int
  +dirent_type (const char *path, const struct dirent *entry)
  +{
  +struct stat statbuf;
  +char *abspath;
  +int err;
  +
  +#ifdef _DIRENT_HAVE_D_TYPE
  +/* Mapping from d_type to stat mode_t.  We omit DT_LNK so that
  + * we'll fall through to stat and get the real file type. */
  +static const mode_t modes[] = {
  +   [DT_BLK]  = S_IFBLK,
  +   [DT_CHR]  = S_IFCHR,
  +   [DT_DIR]  = S_IFDIR,
  +   [DT_FIFO] = S_IFIFO,
  +   [DT_REG]  = S_IFREG,
  +   [DT_SOCK] = S_IFSOCK
  +};
  +if (entry-d_type  sizeof(modes)/sizeof(modes[0]) 

 ARRAY_SIZE()

  +   modes[entry-d_type])
  +   return modes[entry-d_type];
  +#endif
  +
  +abspath = talloc_asprintf (NULL, %s/%s, path, entry-d_name);
  +if (!abspath)
  +   return -1;

 Does talloc set errno in this case? I suspect not.

  +err = stat(abspath, statbuf);
  +talloc_free (abspath);

 This likely breaks your promise about errno. You can't trust
talloc_free() not calling some function that sets errno.

  +if (err  0)
  +   return -1;
  +return statbuf.st_mode  S_IFMT;
  +}
  +
   /* Test if the directory looks like a Maildir directory.
   *
   * Search through the array of directory entries to see if we can find
all
  @@ -162,12 +200,12 @@ dirent_sort_strcmp_name (const struct dirent **a,
const struct dirent **b)
   * Return 1 if the directory looks like a Maildir and 0 otherwise.
   */
   static int
  -_entries_resemble_maildir (struct dirent **entries, int count)
  +_entries_resemble_maildir (const char *path, struct dirent **entries,
int count)
   {
  int i, found = 0;
 
  for (i = 0; i  count; i++) {
  -   if (entries[i]-d_type != DT_DIR  entries[i]-d_type !=
DT_UNKNOWN)
  +   if (dirent_type (path, entries[i]) != S_IFDIR)
 continue;
 
 if (strcmp(entries[i]-d_name, new) == 0 ||
  @@ -250,7 +288,7 @@ add_files_recursive (notmuch_database_t *notmuch,
  notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
  notmuch_message_t *message = NULL;
  struct dirent **fs_entries = NULL;
  -int i, num_fs_entries;
  +int i, num_fs_entries, entry_type;
  notmuch_directory_t *directory;
  notmuch_filenames_t *db_files = NULL;
  notmuch_filenames_t *db_subdirs = NULL;
  @@ -317,7 +355,7 @@ add_files_recursive (notmuch_database_t *notmuch,
  }
 
  /* Pass 1: Recurse into all sub-directories. */
  -is_maildir = _entries_resemble_maildir (fs_entries,
num_fs_entries);
  +is_maildir = _entries_resemble_maildir (path, fs_entries,
num_fs_entries);
 
  for (i = 0; i  num_fs_entries; i++) {
 if (interrupted)
  @@ -325,17 +363,16 @@ add_files_recursive (notmuch_database_t *notmuch,
 
 entry = fs_entries[i];
 
  -   /* We only want to descend into directories.
  -* But symlinks can be to directories too, of course.
  -*
  -* And if the filesystem doesn't tell us the file type in the
  -* scandir results, then it might be a directory (and if not,
  -* then we'll stat and return immediately in the next level of
  -* recursion). */
  -   if (entry-d_type != DT_DIR 
  -   entry-d_type != DT_LNK 
  -   entry-d_type != DT_UNKNOWN)
  -   {
  +   /* We only want to descend into directories (and symlinks to
  +* directories). */
  +   entry_type = dirent_type (path, entry);
  +   if (entry_type == -1) {
  +   /* Be pessimistic, e.g. so we don't loose lots of mail

 s/loose/lose/ ?

  +* just

Re: [PATCH 2/2] new: Centralize file type stat-ing logic

2012-05-07 Thread Austin Clements

Quoth Jani Nikula on May 08 at 12:08 am:
On May 7, 2012 9:10 PM, Austin Clements [1]amdra...@mit.edu wrote:

 This moves our logic to get a file's type into one function.  This has
 several benefits: we can support OSes and file systems that do not
 provide dirent.d_type or always return DT_UNKNOWN, complex
 symlink-handling logic has been replaced by a simple stat fall-through
 in one place, and the error message for un-stat-able file is more
 accurate (previously, the error always mentioned directories, even
 though a broken symlink is not a directory).
 
Please find some quick drive-by-review below.

Thanks for the review!  New version on its way...

J.
 
 ---
  notmuch-new.c |   99
++---
  test/new      |    2 +-
  2 files changed, 60 insertions(+), 41 deletions(-)

 diff --git a/notmuch-new.c b/notmuch-new.c
 index cb720cc..cf2580e 100644
 --- a/notmuch-new.c
 +++ b/notmuch-new.c
 @@ -154,6 +154,44 @@ dirent_sort_strcmp_name (const struct dirent **a,
const struct dirent **b)
     return strcmp ((*a)-d_name, (*b)-d_name);
  }

 +/* Return the type of a directory entry relative to path as a stat(2)
 + * mode.  Like stat, this follows symlinks.  Returns -1 and sets errno
 + * if the file's type cannot be determined (which includes dangling
 + * symlinks).
 + */
 +static int
 +dirent_type (const char *path, const struct dirent *entry)
 +{
 +    struct stat statbuf;
 +    char *abspath;
 +    int err;
 +
 +#ifdef _DIRENT_HAVE_D_TYPE
 +    /* Mapping from d_type to stat mode_t.  We omit DT_LNK so that
 +     * we'll fall through to stat and get the real file type. */
 +    static const mode_t modes[] = {
 +       [DT_BLK]  = S_IFBLK,
 +       [DT_CHR]  = S_IFCHR,
 +       [DT_DIR]  = S_IFDIR,
 +       [DT_FIFO] = S_IFIFO,
 +       [DT_REG]  = S_IFREG,
 +       [DT_SOCK] = S_IFSOCK
 +    };
 +    if (entry-d_type  sizeof(modes)/sizeof(modes[0]) 
 
ARRAY_SIZE()

Huh.  I went looking for that.  I wonder how I missed it.

 +       modes[entry-d_type])
 +       return modes[entry-d_type];
 +#endif
 +
 +    abspath = talloc_asprintf (NULL, %s/%s, path, entry-d_name);
 +    if (!abspath)
 +       return -1;
 
Does talloc set errno in this case? I suspect not.
 
 +    err = stat(abspath, statbuf);
 +    talloc_free (abspath);
 
This likely breaks your promise about errno. You can't trust talloc_free()
not calling some function that sets errno.

Setting errno was a late and apparently not well thought-out addition.
The new patch should set it on all of the error paths and save it
around things that may modify it.

 +    if (err  0)
 +       return -1;
 +    return statbuf.st_mode  S_IFMT;
 +}
 +
  /* Test if the directory looks like a Maildir directory.
  *
  * Search through the array of directory entries to see if we can find
all
 @@ -162,12 +200,12 @@ dirent_sort_strcmp_name (const struct dirent **a,
const struct dirent **b)
  * Return 1 if the directory looks like a Maildir and 0 otherwise.
  */
  static int
 -_entries_resemble_maildir (struct dirent **entries, int count)
 +_entries_resemble_maildir (const char *path, struct dirent **entries,
int count)
  {
     int i, found = 0;

     for (i = 0; i  count; i++) {
 -       if (entries[i]-d_type != DT_DIR  entries[i]-d_type !=
DT_UNKNOWN)
 +       if (dirent_type (path, entries[i]) != S_IFDIR)
            continue;

        if (strcmp(entries[i]-d_name, new) == 0 ||
 @@ -250,7 +288,7 @@ add_files_recursive (notmuch_database_t *notmuch,
     notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
     notmuch_message_t *message = NULL;
     struct dirent **fs_entries = NULL;
 -    int i, num_fs_entries;
 +    int i, num_fs_entries, entry_type;
     notmuch_directory_t *directory;
     notmuch_filenames_t *db_files = NULL;
     notmuch_filenames_t *db_subdirs = NULL;
 @@ -317,7 +355,7 @@ add_files_recursive (notmuch_database_t *notmuch,
     }

     /* Pass 1: Recurse into all sub-directories. */
 -    is_maildir = _entries_resemble_maildir (fs_entries,
num_fs_entries);
 +    is_maildir = _entries_resemble_maildir (path, fs_entries,
num_fs_entries);

     for (i = 0; i  num_fs_entries; i++) {
        if (interrupted)
 @@ -325,17 +363,16 @@ add_files_recursive (notmuch_database_t *notmuch,

        entry = fs_entries[i];

 -       /* We only want to descend into directories.
 -        * But symlinks can be to directories too, of course.
 -        *
 -        * And if the filesystem doesn't tell us the file type in the
 -        * scandir results,

[PATCH v2 2/2] new: Centralize file type stat-ing logic

2012-05-07 Thread Austin Clements

This moves our logic to get a file's type into one function.  This has
several benefits: we can support OSes and file systems that do not
provide dirent.d_type or always return DT_UNKNOWN, complex
symlink-handling logic has been replaced by a simple stat fall-through
in one place, and the error message for un-stat-able file is more
accurate (previously, the error always mentioned directories, even
though a broken symlink is not a directory).
---
 notmuch-new.c |  103 +++--
 test/new  |2 +-
 2 files changed, 64 insertions(+), 41 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index cb720cc..8955677 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -154,6 +154,48 @@ dirent_sort_strcmp_name (const struct dirent **a, const 
struct dirent **b)
 return strcmp ((*a)-d_name, (*b)-d_name);
 }
 
+/* Return the type of a directory entry relative to path as a stat(2)
+ * mode.  Like stat, this follows symlinks.  Returns -1 and sets errno
+ * if the file's type cannot be determined (which includes dangling
+ * symlinks).
+ */
+static int
+dirent_type (const char *path, const struct dirent *entry)
+{
+struct stat statbuf;
+char *abspath;
+int err, saved_errno;
+
+#ifdef _DIRENT_HAVE_D_TYPE
+/* Mapping from d_type to stat mode_t.  We omit DT_LNK so that
+ * we'll fall through to stat and get the real file type. */
+static const mode_t modes[] = {
+   [DT_BLK]  = S_IFBLK,
+   [DT_CHR]  = S_IFCHR,
+   [DT_DIR]  = S_IFDIR,
+   [DT_FIFO] = S_IFIFO,
+   [DT_REG]  = S_IFREG,
+   [DT_SOCK] = S_IFSOCK
+};
+if (entry-d_type  ARRAY_SIZE(modes)  modes[entry-d_type])
+   return modes[entry-d_type];
+#endif
+
+abspath = talloc_asprintf (NULL, %s/%s, path, entry-d_name);
+if (!abspath) {
+   errno = ENOMEM;
+   return -1;
+}
+err = stat(abspath, statbuf);
+saved_errno = errno;
+talloc_free (abspath);
+if (err  0) {
+   errno = saved_errno;
+   return -1;
+}
+return statbuf.st_mode  S_IFMT;
+}
+
 /* Test if the directory looks like a Maildir directory.
  *
  * Search through the array of directory entries to see if we can find all
@@ -162,12 +204,12 @@ dirent_sort_strcmp_name (const struct dirent **a, const 
struct dirent **b)
  * Return 1 if the directory looks like a Maildir and 0 otherwise.
  */
 static int
-_entries_resemble_maildir (struct dirent **entries, int count)
+_entries_resemble_maildir (const char *path, struct dirent **entries, int 
count)
 {
 int i, found = 0;
 
 for (i = 0; i  count; i++) {
-   if (entries[i]-d_type != DT_DIR  entries[i]-d_type != DT_UNKNOWN)
+   if (dirent_type (path, entries[i]) != S_IFDIR)
continue;
 
if (strcmp(entries[i]-d_name, new) == 0 ||
@@ -250,7 +292,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
 notmuch_message_t *message = NULL;
 struct dirent **fs_entries = NULL;
-int i, num_fs_entries;
+int i, num_fs_entries, entry_type;
 notmuch_directory_t *directory;
 notmuch_filenames_t *db_files = NULL;
 notmuch_filenames_t *db_subdirs = NULL;
@@ -317,7 +359,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 }
 
 /* Pass 1: Recurse into all sub-directories. */
-is_maildir = _entries_resemble_maildir (fs_entries, num_fs_entries);
+is_maildir = _entries_resemble_maildir (path, fs_entries, num_fs_entries);
 
 for (i = 0; i  num_fs_entries; i++) {
if (interrupted)
@@ -325,17 +367,16 @@ add_files_recursive (notmuch_database_t *notmuch,
 
entry = fs_entries[i];
 
-   /* We only want to descend into directories.
-* But symlinks can be to directories too, of course.
-*
-* And if the filesystem doesn't tell us the file type in the
-* scandir results, then it might be a directory (and if not,
-* then we'll stat and return immediately in the next level of
-* recursion). */
-   if (entry-d_type != DT_DIR 
-   entry-d_type != DT_LNK 
-   entry-d_type != DT_UNKNOWN)
-   {
+   /* We only want to descend into directories (and symlinks to
+* directories). */
+   entry_type = dirent_type (path, entry);
+   if (entry_type == -1) {
+   /* Be pessimistic, e.g. so we don't lose lots of mail just
+* because a user broke a symlink. */
+   fprintf (stderr, Error reading file %s/%s: %s\n,
+path, entry-d_name, strerror (errno));
+   return NOTMUCH_STATUS_FILE_ERROR;
+   } else if (entry_type != S_IFDIR) {
continue;
}
 
@@ -425,31 +466,13 @@ add_files_recursive (notmuch_database_t *notmuch,
notmuch_filenames_move_to_next (db_subdirs);
}
 
-   /* If we're looking at a symlink, we only want to add it if it
-* links to a regular file, (and not to a directory, say).
-

[PATCH v2 1/2] test: Test notmuch new with a broken symlink

2012-05-07 Thread Austin Clements

---
 test/new |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/test/new b/test/new
index 99f9913..26253db 100755
--- a/test/new
+++ b/test/new
@@ -136,6 +136,16 @@ output=$(NOTMUCH_NEW)
 test_expect_equal $output Added 1 new message to the database.
 
 
+test_begin_subtest Broken symlink aborts
+ln -s does-not-exist ${MAIL_DIR}/broken
+output=$(NOTMUCH_NEW 21)
+test_expect_equal $output \
+Error reading directory /run/shm/nm/tmp.new/mail/broken: No such file or 
directory
+Note: A fatal error was encountered: Something went wrong trying to read or 
write a file
+No new mail.
+rm ${MAIL_DIR}/broken
+
+
 test_begin_subtest New two-level directory
 
 generate_message [dir]=two/levels
-- 
1.7.10

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 0/2] Better, more portable stat-ing logic for notmuch new

2012-05-07 Thread Austin Clements

This version uses ARRAY_SIZE, fixes errno handling bugs, and corrects
a spelling error.

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

39 matches

Mail list logo