[PATCH 1/2] fix compilation without emacs

[PATCH 1/2] fix compilation without emacs

[Tomi Valkeinen]

If emacs is not installed, the following error is printed while
compiling:

/bin/sh: 1: emacs: not found

This patch fixes the build.

It might be better to exclude the whole emacs directory from build when
compiling without emacs, but that's a bigger change.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@iki.fi>
---
 emacs/Makefile.local | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/emacs/Makefile.local b/emacs/Makefile.local
index 92467a3..0fcf64c 100644
--- a/emacs/Makefile.local
+++ b/emacs/Makefile.local
@@ -24,6 +24,7 @@ emacs_images := \
 
 emacs_bytecode = $(emacs_sources:.el=.elc)
 
+ifeq ($(WITH_EMACS),1)
 # Because of defmacro's and defsubst's, we have to account for load
 # dependencies between Elisp files when byte compiling.  Otherwise,
 # the byte compiler may load an old .elc file when processing a
@@ -39,7 +40,6 @@ CLEAN+=$(dir)/.eldeps $(dir)/.eldeps.tmp
 %.elc: %.el $(global_deps)
 	$(call quiet,EMACS) --directory emacs -batch -f batch-byte-compile $<
 
-ifeq ($(WITH_EMACS),1)
 ifeq ($(HAVE_EMACS),1)
 all: $(emacs_bytecode)
 endif
-- 
1.8.3.2

[PATCH 2/2] lib: fix error handling

[Tomi Valkeinen]

Currently if a Xapian exception happens in notmuch_message_get_header,
the exception is not caught leading to crash. In
notmuch_message_get_date the exception is caught, but an internal error
is raised, again leading to crash.

This patch fixes the error handling by making both functions catch the
Xapian exceptions, print an error and return NULL or 0.

The 'notmuch->exception_reported' is also set, as is done elsewhere,
even if I don't really get the idea of that field.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@iki.fi>
---
 lib/message.cc | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index 1b46379..c91f3a5 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -412,19 +412,27 @@ _notmuch_message_ensure_message_file (notmuch_message_t *message)
 const char *
 notmuch_message_get_header (notmuch_message_t *message, const char *header)
 {
-    std::string value;
+    try {
+	    std::string value;
 
-    /* Fetch header from the appropriate xapian value field if
-     * available */
-    if (strcasecmp (header, "from") == 0)
-	value = message->doc.get_value (NOTMUCH_VALUE_FROM);
-    else if (strcasecmp (header, "subject") == 0)
-	value = message->doc.get_value (NOTMUCH_VALUE_SUBJECT);
-    else if (strcasecmp (header, "message-id") == 0)
-	value = message->doc.get_value (NOTMUCH_VALUE_MESSAGE_ID);
+	    /* Fetch header from the appropriate xapian value field if
+	     * available */
+	    if (strcasecmp (header, "from") == 0)
+		value = message->doc.get_value (NOTMUCH_VALUE_FROM);
+	    else if (strcasecmp (header, "subject") == 0)
+		value = message->doc.get_value (NOTMUCH_VALUE_SUBJECT);
+	    else if (strcasecmp (header, "message-id") == 0)
+		value = message->doc.get_value (NOTMUCH_VALUE_MESSAGE_ID);
 
-    if (!value.empty())
-	return talloc_strdup (message, value.c_str ());
+	    if (!value.empty())
+		return talloc_strdup (message, value.c_str ());
+
+    } catch (Xapian::Error &error) {
+	fprintf (stderr, "A Xapian exception occurred when reading header: %s\n",
+		 error.get_msg().c_str());
+	message->notmuch->exception_reported = TRUE;
+	return NULL;
+    }
 
     /* Otherwise fall back to parsing the file */
     _notmuch_message_ensure_message_file (message);
@@ -766,7 +774,9 @@ notmuch_message_get_date (notmuch_message_t *message)
     try {
 	value = message->doc.get_value (NOTMUCH_VALUE_TIMESTAMP);
     } catch (Xapian::Error &error) {
-	INTERNAL_ERROR ("Failed to read timestamp value from document.");
+	fprintf (stderr, "A Xapian exception occurred when reading date: %s\n",
+		 error.get_msg().c_str());
+	message->notmuch->exception_reported = TRUE;
 	return 0;
     }
 
-- 
1.8.3.2

Re: [PATCH 2/2] lib: fix error handling

[Tomi Ollila]

On Tue, Nov 19 2013, Tomi Valkeinen <tomi.valkeinen@iki.fi> wrote:

> Currently if a Xapian exception happens in notmuch_message_get_header,
> the exception is not caught leading to crash. In
> notmuch_message_get_date the exception is caught, but an internal error
> is raised, again leading to crash.
>
> This patch fixes the error handling by making both functions catch the
> Xapian exceptions, print an error and return NULL or 0.
>
> The 'notmuch->exception_reported' is also set, as is done elsewhere,
> even if I don't really get the idea of that field.

Althoug this adds the library fprintf() count by one this is important
and consistent fix to the library...

... whenever we get into consensus how to handle library logging it
is not big deal to convert this also to the new call too.

+1

Tomi


>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@iki.fi>
> ---
>  lib/message.cc | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/lib/message.cc b/lib/message.cc
> index 1b46379..c91f3a5 100644
> --- a/lib/message.cc
> +++ b/lib/message.cc
> @@ -412,19 +412,27 @@ _notmuch_message_ensure_message_file (notmuch_message_t *message)
>  const char *
>  notmuch_message_get_header (notmuch_message_t *message, const char *header)
>  {
> -    std::string value;
> +    try {
> +	    std::string value;
>  
> -    /* Fetch header from the appropriate xapian value field if
> -     * available */
> -    if (strcasecmp (header, "from") == 0)
> -	value = message->doc.get_value (NOTMUCH_VALUE_FROM);
> -    else if (strcasecmp (header, "subject") == 0)
> -	value = message->doc.get_value (NOTMUCH_VALUE_SUBJECT);
> -    else if (strcasecmp (header, "message-id") == 0)
> -	value = message->doc.get_value (NOTMUCH_VALUE_MESSAGE_ID);
> +	    /* Fetch header from the appropriate xapian value field if
> +	     * available */
> +	    if (strcasecmp (header, "from") == 0)
> +		value = message->doc.get_value (NOTMUCH_VALUE_FROM);
> +	    else if (strcasecmp (header, "subject") == 0)
> +		value = message->doc.get_value (NOTMUCH_VALUE_SUBJECT);
> +	    else if (strcasecmp (header, "message-id") == 0)
> +		value = message->doc.get_value (NOTMUCH_VALUE_MESSAGE_ID);
>  
> -    if (!value.empty())
> -	return talloc_strdup (message, value.c_str ());
> +	    if (!value.empty())
> +		return talloc_strdup (message, value.c_str ());
> +
> +    } catch (Xapian::Error &error) {
> +	fprintf (stderr, "A Xapian exception occurred when reading header: %s\n",
> +		 error.get_msg().c_str());
> +	message->notmuch->exception_reported = TRUE;
> +	return NULL;
> +    }
>  
>      /* Otherwise fall back to parsing the file */
>      _notmuch_message_ensure_message_file (message);
> @@ -766,7 +774,9 @@ notmuch_message_get_date (notmuch_message_t *message)
>      try {
>  	value = message->doc.get_value (NOTMUCH_VALUE_TIMESTAMP);
>      } catch (Xapian::Error &error) {
> -	INTERNAL_ERROR ("Failed to read timestamp value from document.");
> +	fprintf (stderr, "A Xapian exception occurred when reading date: %s\n",
> +		 error.get_msg().c_str());
> +	message->notmuch->exception_reported = TRUE;
>  	return 0;
>      }
>  
> -- 
> 1.8.3.2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/2] lib: fix error handling

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

>
> Althoug this adds the library fprintf() count by one this is important
> and consistent fix to the library...
>

replacing a fprintf+exit by an fprintf doesn't actually increase the
fprintf count ;).

d

Re: [PATCH 2/2] lib: fix error handling

[David Bremner]

Tomi Valkeinen <tomi.valkeinen@iki.fi> writes:

> Currently if a Xapian exception happens in notmuch_message_get_header,
> the exception is not caught leading to crash. In
> notmuch_message_get_date the exception is caught, but an internal error
> is raised, again leading to crash.
>
> This patch fixes the error handling by making both functions catch the
> Xapian exceptions, print an error and return NULL or 0.

pushed,

thanks

d

Re: [PATCH 1/2] fix compilation without emacs

[Tomi Ollila]

On Tue, Nov 19 2013, Tomi Valkeinen <tomi.valkeinen@iki.fi> wrote:

> If emacs is not installed, the following error is printed while
> compiling:
>
> /bin/sh: 1: emacs: not found
>
> This patch fixes the build.

Tervetuloa notmuch-muutosten ihmeelliseen maailmaan !

> It might be better to exclude the whole emacs directory from build when
> compiling without emacs, but that's a bigger change.

Well, that change would probably break my build: I configure --without-emacs
(to disable emacs build during normal build) but then I build emacs MUA
separately (to just one notmuch.elc file)...

You could look http://article.gmane.org/gmane.mail.notmuch.general/15837
( id:"1377630047-27756-1-git-send-email-tomi.ollila@iki.fi" ) instead.

If that works for you you can gain karma by reviewing it :D

Tomi

>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@iki.fi>
> ---
>  emacs/Makefile.local | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/emacs/Makefile.local b/emacs/Makefile.local
> index 92467a3..0fcf64c 100644
> --- a/emacs/Makefile.local
> +++ b/emacs/Makefile.local
> @@ -24,6 +24,7 @@ emacs_images := \
>  
>  emacs_bytecode = $(emacs_sources:.el=.elc)
>  
> +ifeq ($(WITH_EMACS),1)
>  # Because of defmacro's and defsubst's, we have to account for load
>  # dependencies between Elisp files when byte compiling.  Otherwise,
>  # the byte compiler may load an old .elc file when processing a
> @@ -39,7 +40,6 @@ CLEAN+=$(dir)/.eldeps $(dir)/.eldeps.tmp
>  %.elc: %.el $(global_deps)
>  	$(call quiet,EMACS) --directory emacs -batch -f batch-byte-compile $<
>  
> -ifeq ($(WITH_EMACS),1)
>  ifeq ($(HAVE_EMACS),1)
>  all: $(emacs_bytecode)
>  endif
> -- 
> 1.8.3.2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 1/2] fix compilation without emacs

[Tomi Valkeinen]

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

On 2013-11-19 12:43, Tomi Ollila wrote:
> On Tue, Nov 19 2013, Tomi Valkeinen <tomi.valkeinen@iki.fi> wrote:

>> It might be better to exclude the whole emacs directory from build when
>> compiling without emacs, but that's a bigger change.
> 
> Well, that change would probably break my build: I configure --without-emacs
> (to disable emacs build during normal build) but then I build emacs MUA
> separately (to just one notmuch.elc file)...

Hmm, isn't --without-xxx supposed to mean that I don't care about xxx,
don't configure it, build it, or do anything else with it.

If you do want emacs, but don't want to compile it right now, that can
be done by giving 'make' the components you do want to build (instead of
all).

> You could look http://article.gmane.org/gmane.mail.notmuch.general/15837
> ( id:"1377630047-27756-1-git-send-email-tomi.ollila@iki.fi" ) instead.

Yep, that one also works for me.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]