unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] emacs: Compute build dependencies to fix byte compile issues
@ 2013-05-17 20:13 Austin Clements
  2013-05-19 11:15 ` David Bremner
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Austin Clements @ 2013-05-17 20:13 UTC (permalink / raw)
  To: notmuch

Previously, we simply byte compiled each Elisp source file
independently.  This is actually the wrong thing to do and can lead to
issues with macros and performance issues with substitutions because
1) when the byte compiler encounters a (require 'x) form, it will load
x.elc in preference to x.el, even if x.el is newer, and as a result
may load old macro and substitution definitions and 2) if we update a
macro or substitution definition in one file, we currently won't
re-compile other files that depend on the file containing the
definition.

This patch addresses these problems by computing make dependency rules
from the (require 'x) forms in the Elisp source files, which we inject
into make's dependency database.
---
 emacs/Makefile.local |   12 +++++++++
 emacs/make-deps.el   |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)
 create mode 100644 emacs/make-deps.el

diff --git a/emacs/Makefile.local b/emacs/Makefile.local
index fb82247..456700a 100644
--- a/emacs/Makefile.local
+++ b/emacs/Makefile.local
@@ -22,6 +22,18 @@ emacs_images := \
 
 emacs_bytecode = $(emacs_sources:.el=.elc)
 
+# 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
+# "require" or we may fail to rebuild a .elc that depended on a macro
+# from an updated file.
+$(dir)/.eldeps: $(dir)/Makefile.local $(dir)/make-deps.el $(emacs_sources)
+	$(call quiet,EMACS) --directory emacs -batch -l make-deps.el \
+		-f batch-make-deps $(emacs_sources) > $@.tmp && \
+		(cmp -s $@.tmp $@ || mv $@.tmp $@)
+-include $(dir)/.eldeps
+CLEAN+=$(dir)/.eldeps $(dir)/.eldeps.tmp
+
 %.elc: %.el $(global_deps)
 	$(call quiet,EMACS) --directory emacs -batch -f batch-byte-compile $<
 
diff --git a/emacs/make-deps.el b/emacs/make-deps.el
new file mode 100644
index 0000000..a1cd731
--- /dev/null
+++ b/emacs/make-deps.el
@@ -0,0 +1,66 @@
+;; make-deps.el --- compute make dependencies for Elisp sources
+;;
+;; Copyright © Austin Clements
+;;
+;; This file is part of Notmuch.
+;;
+;; Notmuch is free software: you can redistribute it and/or modify it
+;; under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+;;
+;; Notmuch is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with Notmuch.  If not, see <http://www.gnu.org/licenses/>.
+;;
+;; Authors: Austin Clements <aclements@csail.mit.edu>
+
+(defun batch-make-deps ()
+  "Invoke `make-deps' for each file on the command line."
+
+  (setq debug-on-error t)
+  (dolist (file command-line-args-left)
+    (let ((default-directory command-line-default-directory))
+      (find-file-literally file))
+    (make-deps command-line-default-directory))
+  (kill-emacs))
+
+(defun make-deps (&optional dir)
+  "Print make dependencies for the current buffer.
+
+This prints make dependencies to `standard-output' based on the
+top-level `require' expressions in the current buffer.  Paths in
+rules will be given relative to DIR, or `default-directory'."
+
+  (setq dir (or dir default-directory))
+  (save-excursion
+    (goto-char (point-min))
+    (condition-case nil
+	(while t
+	  (let ((form (read (current-buffer))))
+	    ;; Is it a (require 'x) form?
+	    (when (and (listp form) (= (length form) 2)
+		       (eq (car form) 'require)
+		       (listp (cadr form)) (= (length (cadr form)) 2)
+		       (eq (car (cadr form)) 'quote)
+		       (symbolp (cadr (cadr form))))
+	      ;; Find the required library
+	      (let* ((name (cadr (cadr form)))
+		     (fname (locate-library (symbol-name name))))
+		;; Is this file and the library in the same directory?
+		;; If not, assume it's a system library and don't
+		;; bother depending on it.
+		(when (and fname
+			   (string= (file-name-directory (buffer-file-name))
+				    (file-name-directory fname)))
+		  ;; Print the dependency
+		  (princ (format "%s.elc: %s.elc\n"
+				 (file-name-sans-extension
+				  (file-relative-name (buffer-file-name) dir))
+				 (file-name-sans-extension
+				  (file-relative-name fname dir)))))))))
+      (end-of-file nil))))
-- 
1.7.10.4

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

* Re: [PATCH] emacs: Compute build dependencies to fix byte compile issues
  2013-05-17 20:13 [PATCH] emacs: Compute build dependencies to fix byte compile issues Austin Clements
@ 2013-05-19 11:15 ` David Bremner
  2013-05-19 12:14   ` Austin Clements
  2013-05-19 14:33 ` Tomi Ollila
  2013-05-23 11:46 ` David Bremner
  2 siblings, 1 reply; 6+ messages in thread
From: David Bremner @ 2013-05-19 11:15 UTC (permalink / raw)
  To: Austin Clements, notmuch

Austin Clements <amdragon@MIT.EDU> writes:
>
> This patch addresses these problems by computing make dependency rules
> from the (require 'x) forms in the Elisp source files, which we inject
> into make's dependency database.

this seems to work as advertised.

> +;;
> +;; Copyright © Austin Clements

I guess you need a copyright year?

> +	    ;; Is it a (require 'x) form?
> +	    (when (and (listp form) (= (length form) 2)
> +		       (eq (car form) 'require)
> +		       (listp (cadr form)) (= (length (cadr form)) 2)
> +		       (eq (car (cadr form)) 'quote)
> +		       (symbolp (cadr (cadr form))))

This might be a corner case, but formally can't the argument to require
be a variable or even a function call? Maybe this never happens in
practice.

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

* Re: [PATCH] emacs: Compute build dependencies to fix byte compile issues
  2013-05-19 11:15 ` David Bremner
@ 2013-05-19 12:14   ` Austin Clements
  2013-05-20 19:48     ` David Bremner
  0 siblings, 1 reply; 6+ messages in thread
From: Austin Clements @ 2013-05-19 12:14 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch

Quoth David Bremner on May 19 at  8:15 am:
> Austin Clements <amdragon@MIT.EDU> writes:
> >
> > This patch addresses these problems by computing make dependency rules
> > from the (require 'x) forms in the Elisp source files, which we inject
> > into make's dependency database.
> 
> this seems to work as advertised.
> 
> > +;;
> > +;; Copyright © Austin Clements
> 
> I guess you need a copyright year?

Strangely, none of the Elisp files have copyright years.  But maybe
they should?

> > +	    ;; Is it a (require 'x) form?
> > +	    (when (and (listp form) (= (length form) 2)
> > +		       (eq (car form) 'require)
> > +		       (listp (cadr form)) (= (length (cadr form)) 2)
> > +		       (eq (car (cadr form)) 'quote)
> > +		       (symbolp (cadr (cadr form))))
> 
> This might be a corner case, but formally can't the argument to require
> be a variable or even a function call? Maybe this never happens in
> practice.

That is technically true (really, it can be an arbitrary expression),
but I don't have an environment in which to evaluate that expression,
so I opted for only supporting the obvious static case, which also
happens to be the only case that matters for the notmuch code.

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

* Re: [PATCH] emacs: Compute build dependencies to fix byte compile issues
  2013-05-17 20:13 [PATCH] emacs: Compute build dependencies to fix byte compile issues Austin Clements
  2013-05-19 11:15 ` David Bremner
@ 2013-05-19 14:33 ` Tomi Ollila
  2013-05-23 11:46 ` David Bremner
  2 siblings, 0 replies; 6+ messages in thread
From: Tomi Ollila @ 2013-05-19 14:33 UTC (permalink / raw)
  To: Austin Clements, notmuch

On Fri, May 17 2013, Austin Clements <amdragon@MIT.EDU> wrote:

> Previously, we simply byte compiled each Elisp source file
> independently.  This is actually the wrong thing to do and can lead to
> issues with macros and performance issues with substitutions because
> 1) when the byte compiler encounters a (require 'x) form, it will load
> x.elc in preference to x.el, even if x.el is newer, and as a result
> may load old macro and substitution definitions and 2) if we update a
> macro or substitution definition in one file, we currently won't
> re-compile other files that depend on the file containing the
> definition.
>
> This patch addresses these problems by computing make dependency rules
> from the (require 'x) forms in the Elisp source files, which we inject
> into make's dependency database.
> ---
>  emacs/Makefile.local |   12 +++++++++
>  emacs/make-deps.el   |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 78 insertions(+)
>  create mode 100644 emacs/make-deps.el
>
> diff --git a/emacs/Makefile.local b/emacs/Makefile.local
> index fb82247..456700a 100644
> --- a/emacs/Makefile.local
> +++ b/emacs/Makefile.local
> @@ -22,6 +22,18 @@ emacs_images := \
>  
>  emacs_bytecode = $(emacs_sources:.el=.elc)
>  
> +# 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
> +# "require" or we may fail to rebuild a .elc that depended on a macro
> +# from an updated file.
> +$(dir)/.eldeps: $(dir)/Makefile.local $(dir)/make-deps.el $(emacs_sources)
> +	$(call quiet,EMACS) --directory emacs -batch -l make-deps.el \
> +		-f batch-make-deps $(emacs_sources) > $@.tmp && \
> +		(cmp -s $@.tmp $@ || mv $@.tmp $@)

The last line above could be in format

		{ cmp -s $@.tmp $@ || mv $@.tmp $@; }

so that the expression is evaluated in current shell instead of a subshell.

You may consider changing this if you add year to copyright line.

Tomi


> +-include $(dir)/.eldeps
> +CLEAN+=$(dir)/.eldeps $(dir)/.eldeps.tmp
> +
>  %.elc: %.el $(global_deps)
>  	$(call quiet,EMACS) --directory emacs -batch -f batch-byte-compile $<
>  

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

* Re: [PATCH] emacs: Compute build dependencies to fix byte compile issues
  2013-05-19 12:14   ` Austin Clements
@ 2013-05-20 19:48     ` David Bremner
  0 siblings, 0 replies; 6+ messages in thread
From: David Bremner @ 2013-05-20 19:48 UTC (permalink / raw)
  To: Austin Clements; +Cc: notmuch

Austin Clements <amdragon@MIT.EDU> writes:

> Quoth David Bremner on May 19 at  8:15 am:
>> 
>> I guess you need a copyright year?
>
> Strangely, none of the Elisp files have copyright years.  But maybe
> they should?

OK, I guess no point complaining about this one file.

It seems like post Berne convention the exact wording of copyright
notices is less important. At some point we should probably make the
files in ./emacs consistent with the other files in the source code, if
only so we don't have to discuss this again later.

d

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

* Re: [PATCH] emacs: Compute build dependencies to fix byte compile issues
  2013-05-17 20:13 [PATCH] emacs: Compute build dependencies to fix byte compile issues Austin Clements
  2013-05-19 11:15 ` David Bremner
  2013-05-19 14:33 ` Tomi Ollila
@ 2013-05-23 11:46 ` David Bremner
  2 siblings, 0 replies; 6+ messages in thread
From: David Bremner @ 2013-05-23 11:46 UTC (permalink / raw)
  To: Austin Clements, notmuch

Austin Clements <amdragon@MIT.EDU> writes:

> Previously, we simply byte compiled each Elisp source file
> independently.  This is actually the wrong thing to do and can lead to
> issues with macros and performance issues with substitutions because
> 1) when the byte compiler encounters a (require 'x) form, it will load
> x.elc in preference to x.el, even if x.el is newer, and as a result
> may load old macro and substitution definitions and 2) if we update a
> macro or substitution definition in one file, we currently won't
> re-compile other files that depend on the file containing the
> definition.

Pushed,

d

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

end of thread, other threads:[~2013-05-23 11:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-17 20:13 [PATCH] emacs: Compute build dependencies to fix byte compile issues Austin Clements
2013-05-19 11:15 ` David Bremner
2013-05-19 12:14   ` Austin Clements
2013-05-20 19:48     ` David Bremner
2013-05-19 14:33 ` Tomi Ollila
2013-05-23 11:46 ` 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).