Re: [PATCH v2] emacs: add compatability functions for emacs 23
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
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
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 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 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
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
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
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 &optional arg &rest _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 &optional 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)) +