unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] emacs: hello: make --batch error gracefully
@ 2013-07-04 22:18 Mark Walters
  2013-07-06  9:06 ` Tomi Ollila
  2013-07-24  2:28 ` David Bremner
  0 siblings, 2 replies; 6+ messages in thread
From: Mark Walters @ 2013-07-04 22:18 UTC (permalink / raw)
  To: notmuch

Recently notmuch-hello was converted to use batch count. However, it
seems that several people run different versions of notmuch-emacs and
notmuch-cli so this batch makes emacs fail with an error message if
--batch is not available in the CLI.
---
There have been two cases on irc of people getting backtraces when
hitting this problem so it might be worth adding an informative error
message.

Best wishes

Mark

 emacs/notmuch-hello.el |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 147c08c..fa46b7a 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -402,8 +402,13 @@ options will be handled as specified for
 					   (plist-get options :filter)))
 	 "\n")))
 
-    (call-process-region (point-min) (point-max) notmuch-command
-			 t t nil "count" "--batch")
+    (unless (= (call-process-region (point-min) (point-max) notmuch-command
+				    t t nil "count" "--batch") 0)
+      (notmuch-logged-error "notmuch CLI version mismatch error (count --batch)
+The most likely cause of this error is that the CLI is too old
+to support count --batch and needs to be upgraded to the same
+version as notmuch-emacs"))
+
     (goto-char (point-min))
 
     (notmuch-remove-if-not
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] emacs: hello: make --batch error gracefully
  2013-07-04 22:18 [PATCH] emacs: hello: make --batch error gracefully Mark Walters
@ 2013-07-06  9:06 ` Tomi Ollila
  2013-07-07 10:21   ` Mark Walters
  2013-07-24  2:28 ` David Bremner
  1 sibling, 1 reply; 6+ messages in thread
From: Tomi Ollila @ 2013-07-06  9:06 UTC (permalink / raw)
  To: Mark Walters, notmuch

On Fri, Jul 05 2013, Mark Walters <markwalters1009@gmail.com> wrote:

> Recently notmuch-hello was converted to use batch count. However, it
> seems that several people run different versions of notmuch-emacs and
> notmuch-cli so this batch makes emacs fail with an error message if
> --batch is not available in the CLI.
> ---
> There have been two cases on irc of people getting backtraces when
> hitting this problem so it might be worth adding an informative error
> message.

This change takes care of the mismatching version problem now -- for
a short while in most cases but in the future we might face with new
incompabilities that would -- again -- need new solution. Some day
we might have a pile of these who everyone is shy to remove from the
code base ;/

Although I am not against applying this patch (if there are supporters
of this) I'd like to concentrate fixing this for example the following
way:

We'll add a global option to notmuch, e.g.

--compatibility-version=x.y

Whenever the caller chooses to use this option, notmuch checks whether
it can comply with the option -- it it can, execution continues, otherwise 
aborts.

The compatibility is determined so that the major 'x' needs to be same
and caller may have lower 'y' that notmuch is capable of handling.

For example. if notmuch compatibility version was 2.5

--compatibility-version=1.9  --  abort
--compatibility-version=2.3  --  continue
--compatibility-version=2.5  --  continue
--compatibility-version=2.8  --  abort
--compatibility-version=3.1  --  abort


I can work on this (or on something similar) if this is generally thought
as a good idea...

>
> Best wishes
>
> Mark

Tomi


>
>  emacs/notmuch-hello.el |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
> index 147c08c..fa46b7a 100644
> --- a/emacs/notmuch-hello.el
> +++ b/emacs/notmuch-hello.el
> @@ -402,8 +402,13 @@ options will be handled as specified for
>  					   (plist-get options :filter)))
>  	 "\n")))
>  
> -    (call-process-region (point-min) (point-max) notmuch-command
> -			 t t nil "count" "--batch")
> +    (unless (= (call-process-region (point-min) (point-max) notmuch-command
> +				    t t nil "count" "--batch") 0)
> +      (notmuch-logged-error "notmuch CLI version mismatch error (count --batch)
> +The most likely cause of this error is that the CLI is too old
> +to support count --batch and needs to be upgraded to the same
> +version as notmuch-emacs"))
> +
>      (goto-char (point-min))
>  
>      (notmuch-remove-if-not
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] emacs: hello: make --batch error gracefully
  2013-07-06  9:06 ` Tomi Ollila
@ 2013-07-07 10:21   ` Mark Walters
  2013-07-13  8:06     ` Tomi Ollila
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Walters @ 2013-07-07 10:21 UTC (permalink / raw)
  To: Tomi Ollila, notmuch


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

> On Fri, Jul 05 2013, Mark Walters <markwalters1009@gmail.com> wrote:
>
>> Recently notmuch-hello was converted to use batch count. However, it
>> seems that several people run different versions of notmuch-emacs and
>> notmuch-cli so this batch makes emacs fail with an error message if
>> --batch is not available in the CLI.
>> ---
>> There have been two cases on irc of people getting backtraces when
>> hitting this problem so it might be worth adding an informative error
>> message.
>
> This change takes care of the mismatching version problem now -- for
> a short while in most cases but in the future we might face with new
> incompabilities that would -- again -- need new solution. Some day
> we might have a pile of these who everyone is shy to remove from the
> code base ;/

I think we want the unless clause anyway: we do not want to try parsing
something and getting stacktrace errors whatever happens. The error
message will need updating so I should probably add a FIXME to the
code. Perhaps the patch roughly as is could be applied to 0.16 and the
fixed error message together with your ideas from below to master?

> Although I am not against applying this patch (if there are supporters
> of this) I'd like to concentrate fixing this for example the following
> way:

This looks good. Some thoughts below.

> We'll add a global option to notmuch, e.g.
>
> --compatibility-version=x.y
>
> Whenever the caller chooses to use this option, notmuch checks whether
> it can comply with the option -- it it can, execution continues, otherwise 
> aborts.

I think the return value could indicate what the problem was (ie too new
or too old): so frontends could decide to work around it (perhaps 22 and
23 to come after the format-version return values). This would mean that
callers would have an easier way of telling if --compatability-version
is supported at all to ease the transition.

I wondered whether it overlapped with Austin's format-version stuff but
I think it is sufficiently different but some clear documentation as to
which means what could be helpful.

Best wishes

Mark

> The compatibility is determined so that the major 'x' needs to be same
> and caller may have lower 'y' that notmuch is capable of handling.
>
> For example. if notmuch compatibility version was 2.5
>
> --compatibility-version=1.9  --  abort
> --compatibility-version=2.3  --  continue
> --compatibility-version=2.5  --  continue
> --compatibility-version=2.8  --  abort
> --compatibility-version=3.1  --  abort
>
>
> I can work on this (or on something similar) if this is generally thought
> as a good idea...
>
>>
>> Best wishes
>>
>> Mark
>
> Tomi
>
>
>>
>>  emacs/notmuch-hello.el |    9 +++++++--
>>  1 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
>> index 147c08c..fa46b7a 100644
>> --- a/emacs/notmuch-hello.el
>> +++ b/emacs/notmuch-hello.el
>> @@ -402,8 +402,13 @@ options will be handled as specified for
>>  					   (plist-get options :filter)))
>>  	 "\n")))
>>  
>> -    (call-process-region (point-min) (point-max) notmuch-command
>> -			 t t nil "count" "--batch")
>> +    (unless (= (call-process-region (point-min) (point-max) notmuch-command
>> +				    t t nil "count" "--batch") 0)
>> +      (notmuch-logged-error "notmuch CLI version mismatch error (count --batch)
>> +The most likely cause of this error is that the CLI is too old
>> +to support count --batch and needs to be upgraded to the same
>> +version as notmuch-emacs"))
>> +
>>      (goto-char (point-min))
>>  
>>      (notmuch-remove-if-not
>> -- 
>> 1.7.9.1
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] emacs: hello: make --batch error gracefully
  2013-07-07 10:21   ` Mark Walters
@ 2013-07-13  8:06     ` Tomi Ollila
  0 siblings, 0 replies; 6+ messages in thread
From: Tomi Ollila @ 2013-07-13  8:06 UTC (permalink / raw)
  To: Mark Walters, notmuch

On Sun, Jul 07 2013, Mark Walters <markwalters1009@gmail.com> wrote:

> Hi
> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>> On Fri, Jul 05 2013, Mark Walters <markwalters1009@gmail.com> wrote:
>>
>>> Recently notmuch-hello was converted to use batch count. However, it
>>> seems that several people run different versions of notmuch-emacs and
>>> notmuch-cli so this batch makes emacs fail with an error message if
>>> --batch is not available in the CLI.
>>> ---
>>> There have been two cases on irc of people getting backtraces when
>>> hitting this problem so it might be worth adding an informative error
>>> message.
>>
>> This change takes care of the mismatching version problem now -- for
>> a short while in most cases but in the future we might face with new
>> incompabilities that would -- again -- need new solution. Some day
>> we might have a pile of these who everyone is shy to remove from the
>> code base ;/
>
> I think we want the unless clause anyway: we do not want to try parsing
> something and getting stacktrace errors whatever happens. The error
> message will need updating so I should probably add a FIXME to the
> code. Perhaps the patch roughly as is could be applied to 0.16 and the
> fixed error message together with your ideas from below to master?

Like said below (even) the current ok by me -- just that whatever is
chosen will be there into foreseeable future... ;)

>> Although I am not against applying this patch (if there are supporters
>> of this) I'd like to concentrate fixing this for example the following
>> way:
>
> This looks good. Some thoughts below.
>
>> We'll add a global option to notmuch, e.g.
>>
>> --compatibility-version=x.y
>>
>> Whenever the caller chooses to use this option, notmuch checks whether
>> it can comply with the option -- it it can, execution continues, otherwise 
>> aborts.
>
> I think the return value could indicate what the problem was (ie too new
> or too old): so frontends could decide to work around it (perhaps 22 and
> 23 to come after the format-version return values). This would mean that
> callers would have an easier way of telling if --compatability-version
> is supported at all to ease the transition.
>
> I wondered whether it overlapped with Austin's format-version stuff but
> I think it is sufficiently different but some clear documentation as to
> which means what could be helpful.

Yes, I had Austin's format-version options into mind when quickly drafting
the suggestion (we'd use the same 21&22 (or whatever) return values too...)

Now that I spend a week with only N9 as a computing device I've also
thought how much of a maintenance burden we want to take with this...
... and thought as an alternative that during byte compilation of emacs
files we pick notmuch cli version to the .elc files -- and just popup
a warning message (on first notmuch-hello invocation) if the versions 
differ...

I'll investigate what kind of emacs macro functionality could be used
to do this (more fun than solving sudoku's that is :D)

> Best wishes
>
> Mark

Tomi


>
>> The compatibility is determined so that the major 'x' needs to be same
>> and caller may have lower 'y' that notmuch is capable of handling.
>>
>> For example. if notmuch compatibility version was 2.5
>>
>> --compatibility-version=1.9  --  abort
>> --compatibility-version=2.3  --  continue
>> --compatibility-version=2.5  --  continue
>> --compatibility-version=2.8  --  abort
>> --compatibility-version=3.1  --  abort
>>
>>
>> I can work on this (or on something similar) if this is generally thought
>> as a good idea...
>>
>>>
>>> Best wishes
>>>
>>> Mark
>>
>> Tomi
>>
>>
>>>
>>>  emacs/notmuch-hello.el |    9 +++++++--
>>>  1 files changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
>>> index 147c08c..fa46b7a 100644
>>> --- a/emacs/notmuch-hello.el
>>> +++ b/emacs/notmuch-hello.el
>>> @@ -402,8 +402,13 @@ options will be handled as specified for
>>>  					   (plist-get options :filter)))
>>>  	 "\n")))
>>>  
>>> -    (call-process-region (point-min) (point-max) notmuch-command
>>> -			 t t nil "count" "--batch")
>>> +    (unless (= (call-process-region (point-min) (point-max) notmuch-command
>>> +				    t t nil "count" "--batch") 0)
>>> +      (notmuch-logged-error "notmuch CLI version mismatch error (count --batch)
>>> +The most likely cause of this error is that the CLI is too old
>>> +to support count --batch and needs to be upgraded to the same
>>> +version as notmuch-emacs"))
>>> +
>>>      (goto-char (point-min))
>>>  
>>>      (notmuch-remove-if-not
>>> -- 
>>> 1.7.9.1
>>>
>>> _______________________________________________
>>> notmuch mailing list
>>> notmuch@notmuchmail.org
>>> http://notmuchmail.org/mailman/listinfo/notmuch
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] emacs: hello: make --batch error gracefully
  2013-07-04 22:18 [PATCH] emacs: hello: make --batch error gracefully Mark Walters
  2013-07-06  9:06 ` Tomi Ollila
@ 2013-07-24  2:28 ` David Bremner
  2013-07-27 21:52   ` David Bremner
  1 sibling, 1 reply; 6+ messages in thread
From: David Bremner @ 2013-07-24  2:28 UTC (permalink / raw)
  To: Mark Walters; +Cc: notmuch mailing list

Mark Walters <markwalters1009@gmail.com> writes:
> -    (call-process-region (point-min) (point-max) notmuch-command
> -			 t t nil "count" "--batch")
> +    (unless (= (call-process-region (point-min) (point-max) notmuch-command
> +				    t t nil "count" "--batch") 0)
> +      (notmuch-logged-error "notmuch CLI version mismatch error (count --batch)
> +The most likely cause of this error is that the CLI is too old
> +to support count --batch and needs to be upgraded to the same
> +version as notmuch-emacs"))
> +

I had a look at this, and I agree the current failure mode is not nice
(it just says something mysterious about nil not being a string). On the
other hand, I think we should use the two argument form of
notmuch-logged-error, and the the first argument should say only what we
know. Something like

(notmuch-logged-error "notmuch count --batch failed"
"notmuch count --batch failed
Please check that the notmuch CLI is new enough to  support `count
--batch'. In general we recommend running matching versions of the CLI
and emacs interface.")

d

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] emacs: hello: make --batch error gracefully
  2013-07-24  2:28 ` David Bremner
@ 2013-07-27 21:52   ` David Bremner
  0 siblings, 0 replies; 6+ messages in thread
From: David Bremner @ 2013-07-27 21:52 UTC (permalink / raw)
  To: Mark Walters; +Cc: notmuch mailing list

David Bremner <david@tethera.net> writes:

> I had a look at this, and I agree the current failure mode is not nice
> (it just says something mysterious about nil not being a string). On the
> other hand, I think we should use the two argument form of
> notmuch-logged-error, and the the first argument should say only what we
> know. Something like
>
> (notmuch-logged-error "notmuch count --batch failed"
> "notmuch count --batch failed
> Please check that the notmuch CLI is new enough to  support `count
> --batch'. In general we recommend running matching versions of the CLI
> and emacs interface.")

I pushed an amended version along these lines; it turns out the first
argument does not need to be repeated.

d

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-07-27 21:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-04 22:18 [PATCH] emacs: hello: make --batch error gracefully Mark Walters
2013-07-06  9:06 ` Tomi Ollila
2013-07-07 10:21   ` Mark Walters
2013-07-13  8:06     ` Tomi Ollila
2013-07-24  2:28 ` David Bremner
2013-07-27 21:52   ` David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).