Re: [PATCH v2] emacs: add compatability functions for emacs 23

[David Bremner]

Tomi Ollila  writes:


> But this compatIbility change is not just emacs 23 -- iirc there were some
> changes required to get emacs 24.1, 24.2, and 24.3 to work. It might be
> easier to keep testing using emacs 23 until we deprecate everything before
> emacs 24.4 (released October 20, 2014) -- or supporting emacs23 gets
> nontrivial while supporting older emacs24 releases is still trivial (*)

This might refer to the defadvice on ido-completing-read in
notmuch-mua.el. In any case that could also go is a seperate
compatability file.

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

Re: [PATCH v2] emacs: add compatability functions for emacs 23

[David Bremner]

Tomi Ollila  writes:


>> 2) It's getting hard to debug emacs23 problems, and developer time is
>>scarce, so at the same time, deprecate emacs23 support, so e.g. after
>>the next major release we can just drop it.
>
> It is not hard at all. Just do not mess w/ mixed environments >;).

I guess a more accurate way of saying it is that I don't want to spend
time on it anymore. So I think I want to deprecate it anyway, with the
understanding that we'll keep supporting as long as we (we being someone
who is not me) can, but no guarantees.

d



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

Re: [PATCH v2] emacs: add compatability functions for emacs 23

[Tomi Ollila]

On Fri, Nov 11 2016, David Bremner  wrote:

> David Bremner  writes:
>
>> The plot thickens. With this patch applied, I can generate an emacs
>> segfault with
>>
>> 1) EMACS=emacs23 ./devel/try-emacs-mua -q
>>
>> 2) M-x notmuch-search 
>>
>> 3) tag:inbox 
>
> OK, I managed to figure out the problem. I had byte compiled the emacs
> code with emacs24, and loading that in emacs23 caused all heck to break
> loose. With the byte compilation done with a matching version of emacs,
> the test suite passes.

I tested this (almost 2 weeks ago :/) using debian wheezy (7.11) docker
container and all relevant (i.e. emacs) tests passed.

>
> So my conclusions from all this fun:
>
> 1) since Mark went to the trouble of doing the compatiblity functions,
>we may as well get emacs23 working again.
>
> 2) It's getting hard to debug emacs23 problems, and developer time is
>scarce, so at the same time, deprecate emacs23 support, so e.g. after
>the next major release we can just drop it.

It is not hard at all. Just do not mess w/ mixed environments >;).

But this compatIbility change is not just emacs 23 -- iirc there were some
changes required to get emacs 24.1, 24.2, and 24.3 to work. It might be
easier to keep testing using emacs 23 until we deprecate everything before
emacs 24.4 (released October 20, 2014) -- or supporting emacs23 gets
nontrivial while supporting older emacs24 releases is still trivial (*)

(*) tried same with emacs22. when it got too hard we just did
ab30a846a49f576e1bfcd73f2ec41def474a6d96

I put the (new, shiny! ;) docker creation script I used to create this
wheezy container to:

https://github.com/domo141/nottoomuch/blob/dogfood/devel/test-in-docker.sh

(with https://github.com/domo141/nottoomuch/blob/dogfood/devel/test-on-macos.sh 
!)

> 3) With the caveat that we plan to drop the compat functions, I leave it
>to mark's judgement whether it's worth putting them in a seperate
>file, or just adding the FSF copyright to the top of the file.

compat functions will eventually be dropped (along with the code we already
have there to support emacs23)...

sepArate >;)

... but that said I'd suggest if there is going to be FSF copyright in the
file, create separate file for that.

and, also append 'g' to s/compatability/compatibility/ so that *all*
strings are replaced with correctly spelled version ;D

Tomi

PS: using google via smtpmail is pain... Now trying 3rd time... 5th...

got text message to go to google.com/blocked -- now 7th try...

8th... 9th...

Ok. It just fails. Sending the ordinary way (w/o google recipients, then).
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2] emacs: add compatability functions for emacs 23

[David Bremner]

David Bremner  writes:

> The plot thickens. With this patch applied, I can generate an emacs
> segfault with
>
> 1) EMACS=emacs23 ./devel/try-emacs-mua -q
>
> 2) M-x notmuch-search 
>
> 3) tag:inbox 

OK, I managed to figure out the problem. I had byte compiled the emacs
code with emacs24, and loading that in emacs23 caused all heck to break
loose. With the byte compilation done with a matching version of emacs,
the test suite passes.

So my conclusions from all this fun:

1) since Mark went to the trouble of doing the compatiblity functions,
   we may as well get emacs23 working again.

2) It's getting hard to debug emacs23 problems, and developer time is
   scarce, so at the same time, deprecate emacs23 support, so e.g. after
   the next major release we can just drop it.

3) With the caveat that we plan to drop the compat functions, I leave it
   to mark's judgement whether it's worth putting them in a seperate
   file, or just adding the FSF copyright to the top of the file.

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

Re: [PATCH v2] emacs: add compatability functions for emacs 23

[David Bremner]

David Bremner  writes:

> Mark Walters  writes:
>
>> Some of the recent changes to the emacs code have used functions
>> introduced in emacs 24. The functions used are read-char-choice and
>> setq-local. This changeset adds compatability functions to
>> notmuch-lib so that it should work on emacs 23.
>> ---
>
>> 1) please could someone with emacs 23 see if the testsuite passes? My
>> system with emacs 23 is so outdated the test suite doesn't run (wrong
>> python versions I think).
>
> I get many failures like
>
> emacsclient.emacs23: connect: Connection refused
> emacsclient.emacs23: error accessing socket 
> "/tmp/emacs1000/notmuch-test-suite-21492"
>
> At a guess, somehow the server is shutting down/dying. I did manage to
> manually run an emacs server on this machine, and the socket is being
> created in the file system.

The plot thickens. With this patch applied, I can generate an emacs
segfault with

1) EMACS=emacs23 ./devel/try-emacs-mua -q

2) M-x notmuch-search 

3) tag:inbox 

without the patch, I get a search view, and the message

Wrong type argument: number-or-marker-p, entry-main

I'm not able to trigger a lisp backtrace.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2] emacs: add compatability functions for emacs 23

[David Bremner]

Mark Walters  writes:

> Some of the recent changes to the emacs code have used functions
> introduced in emacs 24. The functions used are read-char-choice and
> setq-local. This changeset adds compatability functions to
> notmuch-lib so that it should work on emacs 23.
> ---

> 1) please could someone with emacs 23 see if the testsuite passes? My
> system with emacs 23 is so outdated the test suite doesn't run (wrong
> python versions I think).

I get many failures like

emacsclient.emacs23: connect: Connection refused
emacsclient.emacs23: error accessing socket 
"/tmp/emacs1000/notmuch-test-suite-21492"

At a guess, somehow the server is shutting down/dying. I did manage to
manually run an emacs server on this machine, and the socket is being
created in the file system.

> 2) Is the copyright notice I have included above the two functions
> sufficient, and suitably placed?

I think it would be best to modify the copyright at the top of the
file. So to avoid confusing (if for some unforseen reason we wanted to
relicense this file), it might be best to go with a seperate file. But
lets see if someone can figure out the issue with the test suite before
putting much more effort into it.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2] emacs: add compatability functions for emacs 23

[Matt Armstrong]

Mark Walters  writes:

[...]

> Version 1 of this patch (with some discussion) is at
> id:1477191835-17828-1-git-send-email-markwalters1009 at gmail.com
>
> The general consensus is that we should not define functions outside
> our namespace, even when they are just backports of functions from
> later emacs.
>
> rlb on irc gave one additional reason not mentioned earlier in the
> thread -- it could be that some other package choses to test for the
> setq-local say and perhaps draws incorrect conclusions about the
> environment. In particular this would be a case where we were breaking
> otherwise working packages.

Ahh, good point.

Thanks for sticking with the patch.


> I think it makes sense to included the whole of read-char-choice, not
> some cutdown version, as we may use other functionality from it later
> (eg help-forms etc), and it would be confusing if the change only
> worked on emacs 24.
>
> Finally, I have two questions
>
> 1) please could someone with emacs 23 see if the testsuite passes? My
> system with emacs 23 is so outdated the test suite doesn't run (wrong
> python versions I think).

I tried.

FWIW, the patch didn't apply cleanly for me, though I'm not used to
slurping patches around in email.  I used: "git apply -3 mark.patch".
"git apply --reject mark.patch" left a few .rej files that were easily
applied by hand (it looked like some code had moved too far?).

I'm stuck on an Ubuntu trusty release, which is rather old and has a
packaged emacs 23.  I'm not sure how useful my results are, though,
because I show a lot of test errors regardless.  This may be a result of
me compiling my own xapian, which the notmuch build system does not
handle particularly gracefully.  Most failures are of the form "A Xapian
exception occurred performing query", and unrelated to your patch.

Before your patch I got the expected byte compiler warnings:

% XAPIAN_CONFIG=$HOME/opt/xapian-core-1.4.0/bin/xapian-config ./configure
% LD_RUN_PATH=$HOME/opt/xapian-core-1.4.0/lib make test

[...]

notmuch-company.el:95:1:Warning: the function `setq-local' is not known to be
defined.
notmuch-address.el:351:1:Warning: the function `setq-local' is not known to be
defined.
notmuch-maildir-fcc.el:376:1:Warning: the function `read-char-choice' is not
known to be defined.

[...then I see...]

Notmuch test suite complete.
635/783 tests passed.
2 broken tests failed as expected.
43 tests failed.
103 tests skipped.

[...with this patch the byte compiler shuts up, but I get the same test
suite errors...]

Notmuch test suite complete.
635/783 tests passed.
2 broken tests failed as expected.
43 tests failed.
103 tests skipped.

[...but not to worry, I get the same running Emacs 25 too...]

I'll attach full output of the notmuch test run.


> 2) Is the copyright notice I have included above the two functions
> sufficient, and suitably placed?

An alternative is to place these functions in a new file named something
like notmuch-backport.el, with clear FSF copyright.  Since what
constitutes "functions in this section" may drift over time, the rigor
of a separate file might be desirable.


> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -966,6 +966,78 @@ status."
>  (defvar notmuch-show-process-crypto nil)
>  (make-variable-buffer-local 'notmuch-show-process-crypto)
>  
> +;; Compatibility functions for emacs 23.
> +
> +;; The functions in this section are copied from eamcs 24.4 and are

typo: eamcs


-- next part --
Use "make V=1" to print test headings and PASSing results.
INFO: using 2 minute timeout for tests

T050-new: Testing "notmuch new" in several variations
 FAIL   Xapian exception: read only files
--- T050-new.30.expected2016-11-02 23:49:06.740165419 +
+++ T050-new.30.output  2016-11-02 23:49:06.740165419 +
@@ -1 +1,17 @@
-A Xapian exception occurred opening database
+(D) add_files, pass 1
+(D) add_files, pass 1
+(D) add_files, pass 1
+(D) add_files, pass 1
+(D) add_files, pass 1
+(D) add_files, pass 1
+(D) add_files, pass 2
+(D) add_files, pass 2
+(D) add_files, pass 1
+(D) add_files, pass 2
+(D) add_files, pass 1
+(D) add_files, pass 2
+(D) add_files, pass 2
+(D) add_files, pass 2
+(D) add_files, pass 2
+(D) add_files, pass 2
+No new mail.
chmod: cannot access 
'/usr/local/google/home/marmstrong/git/notmuch/test/tmp.T050-new/mail/.notmuch/xapian/*.DB':
 No such file or directory
chmod: cannot access 
'/usr/local/google/home/marmstrong/git/notmuch/test/tmp.T050-new/mail/.notmuch/xapian/*.DB':
 No such file or directory

T060-count: Testing "notmuch count" for messages and threads
 FAIL   error message for database open
--- T060-count.13.expected  2016-11-02 23:49:06.984168633 +
+++ T060-count.13.output2016-11-02 23:49:06.984168633 +
@@ -1 +1 @@

[PATCH v2] emacs: add compatability functions for emacs 23

[Mark Walters]

Some of the recent changes to the emacs code have used functions
introduced in emacs 24. The functions used are read-char-choice and
setq-local. This changeset adds compatability functions to
notmuch-lib so that it should work on emacs 23.
---

Version 1 of this patch (with some discussion) is at
id:1477191835-17828-1-git-send-email-markwalters1...@gmail.com

The general consensus is that we should not define functions outside
our namespace, even when they are just backports of functions from
later emacs.

rlb on irc gave one additional reason not mentioned earlier in the
thread -- it could be that some other package choses to test for the
setq-local say and perhaps draws incorrect conclusions about the
environment. In particular this would be a case where we were breaking
otherwise working packages.

I think it makes sense to included the whole of read-char-choice, not
some cutdown version, as we may use other functionality from it later
(eg help-forms etc), and it would be confusing if the change only
worked on emacs 24.

Finally, I have two questions

1) please could someone with emacs 23 see if the testsuite passes? My
system with emacs 23 is so outdated the test suite doesn't run (wrong
python versions I think).

2) Is the copyright notice I have included above the two functions
sufficient, and suitably placed?

Best wishes

Mark








 emacs/notmuch-address.el |  4 +--
 emacs/notmuch-company.el |  3 +-
 emacs/notmuch-lib.el | 72 
 emacs/notmuch-maildir-fcc.el |  4 +--
 4 files changed, 78 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index b2e1fba..1af3263 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -136,11 +136,11 @@ toggles the setting in this buffer."
   (interactive)
   (if (local-variable-p 'notmuch-address-command)
   (kill-local-variable 'notmuch-address-command)
-(setq-local notmuch-address-command 'internal))
+(notmuch-setq-local notmuch-address-command 'internal))
   (if (boundp 'company-idle-delay)
   (if (local-variable-p 'company-idle-delay)
  (kill-local-variable 'company-idle-delay)
-   (setq-local company-idle-delay nil
+   (notmuch-setq-local company-idle-delay nil
 
 (defun notmuch-address-matching (substring)
   "Returns a list of completion candidates matching SUBSTRING.
diff --git a/emacs/notmuch-company.el b/emacs/notmuch-company.el
index 168315f..5d75c14 100644
--- a/emacs/notmuch-company.el
+++ b/emacs/notmuch-company.el
@@ -28,6 +28,7 @@
 ;;; Code:
 
 (eval-when-compile (require 'cl))
+(require 'notmuch-lib)
 
 (defvar notmuch-company-last-prefix nil)
 (make-variable-buffer-local 'notmuch-company-last-prefix)
@@ -53,7 +54,7 @@
   ;; internal completion) can still be accessed via standard company
   ;; functions, e.g., company-complete.
   (unless (eq notmuch-address-command 'internal)
-(setq-local company-idle-delay nil)))
+(notmuch-setq-local company-idle-delay nil)))
 
 ;;;###autoload
 (defun notmuch-company (command  arg  _ignore)
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 1f0d167..bb53170 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -966,6 +966,78 @@ status."
 (defvar notmuch-show-process-crypto nil)
 (make-variable-buffer-local 'notmuch-show-process-crypto)
 
+;; Compatibility functions for emacs 23.
+
+;; The functions in this section are copied from eamcs 24.4 and are
+;; Copyright (C) 1985-1986, 1992, 1994-1995, 1999-2014 Free Software
+;; Foundation, Inc.
+
+(if (fboundp 'setq-local)
+(defalias 'notmuch-setq-local 'setq-local)
+  (defmacro notmuch-setq-local (var val)
+"Set variable VAR to value VAL in current buffer.
+
+Backport of setq-local for emacs without setq-local (pre 24.3)."
+`(set (make-local-variable ',var) ,val)))
+
+(if (fboundp 'read-char-choice)
+(defalias 'notmuch-read-char-choice 'read-char-choice)
+  (defun notmuch-read-char-choice (prompt chars  
inhibit-keyboard-quit)
+  "Read and return one of CHARS, prompting for PROMPT.
+Any input that is not one of CHARS is ignored.
+
+If optional argument INHIBIT-KEYBOARD-QUIT is non-nil, ignore
+keyboard-quit events while waiting for a valid input.
+
+This is an exact copy of this function from emacs 24 for use on
+emacs 23, except with the one emacs 24 only function it calls
+inlined."
+  (unless (consp chars)
+(error "Called `read-char-choice' without valid char choices"))
+  (let (char done show-help (helpbuf " *Char Help*"))
+(let ((cursor-in-echo-area t)
+  (executing-kbd-macro executing-kbd-macro)
+ (esc-flag nil))
+  (save-window-excursion ; in case we call help-form-show
+   (while (not done)
+ (unless (get-text-property 0 'face prompt)
+   (setq prompt (propertize prompt 'face 'minibuffer-prompt)))
+ (setq char (let ((inhibit-quit inhibit-keyboard-quit))
+  (read-key prompt)))