From: Mark Walters <markwalters1009@gmail.com>
To: Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org
Subject: Re: [PATCH] emacs: hello: make --batch error gracefully
In-Reply-To: <m2mwq0t5v5.fsf@guru.guru-group.fi>
References: <1372976299-30389-1-git-send-email-markwalters1009@gmail.com>
	<m2mwq0t5v5.fsf@guru.guru-group.fi>
User-Agent: Notmuch/0.15.2+192~g8222af3 (http://notmuchmail.org) Emacs/23.4.1
	(i486-pc-linux-gnu)
Date: Sun, 07 Jul 2013 11:21:39 +0100
Message-ID: <87wqp2d618.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list


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