From: Tomi Ollila <tomi.ollila@iki.fi>
To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Subject: Re: [PATCH] emacs: Avoid rebuilding .eldeps even when there's nothing
	to do
In-Reply-To: <1392870264-11156-1-git-send-email-amdragon@mit.edu>
References: <m2iosade2y.fsf@guru.guru-group.fi>
	<1392870264-11156-1-git-send-email-amdragon@mit.edu>
User-Agent: Notmuch/0.17+69~g761b031 (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-unknown-linux-gnu)
Date: Thu, 20 Feb 2014 19:00:39 +0200
Message-ID: <m2ppmhwwg8.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Thu, Feb 20 2014, Austin Clements <amdragon@MIT.EDU> wrote:

> Previously, we updated .eldeps only if the file contents actually
> needed to change.  This was done to avoid unnecessary make restarts
> (if the .eldeps rule changes the mtime of .eldeps, make has to restart
> to collect the new dependencies).  However, this meant that, after a
> modification to any .el file that did not change dependencies, .eldeps
> would always be out of date, so every make invocation would run the
> .eldeps rule, which is both expensive because it starts up Emacs and
> noisy.  This was true even when there was nothing to do.  E.g.,
>
> $ make clean && make
> ...
> $ touch emacs/notmuch-lib.el && make
> ...
> $ make
> Use "make V=1" to see the verbose compile lines.
> EMACS emacs/.eldeps
> make: Nothing to be done for `all'.
> $ make
> Use "make V=1" to see the verbose compile lines.
> EMACS emacs/.eldeps
> make: Nothing to be done for `all'.
>
> Fix this by replacing .eldeps with two files with identical content.
> One tracks the mtime of the dependency information and triggers the
> Emacs call to rebuild dependencies only when it may be necessary.  The
> other tracks the content only; this rule over-triggers in the same way
> the old rule did, but this rule is cheap and quiet.
> ---

LGTM. Works as expected.

Tomi

>  emacs/Makefile.local | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/emacs/Makefile.local b/emacs/Makefile.local
> index 42bfbd9..6a39b32 100644
> --- a/emacs/Makefile.local
> +++ b/emacs/Makefile.local
> @@ -33,10 +33,19 @@ ifeq ($(HAVE_EMACS),1)
>  $(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
> +		mv $@.tmp $@
> +# We could include .eldeps directly, but that would cause a make
> +# restart whenever any .el file was modified, even if dependencies
> +# didn't change, because the mtime of .eldeps will change.  Instead,
> +# we include a second file, .eldeps.x, which we ensure always has the
> +# same content as .eldeps, but its mtime only changes when dependency
> +# information changes, in which case a make restart is necessary
> +# anyway.
> +$(dir)/.eldeps.x: $(dir)/.eldeps
> +	@cmp -s $^ $@ || cp $^ $@
> +-include $(dir)/.eldeps.x
>  endif
> -CLEAN+=$(dir)/.eldeps $(dir)/.eldeps.tmp
> +CLEAN+=$(dir)/.eldeps $(dir)/.eldeps.tmp $(dir)/.eldeps.x
>  
>  ifeq ($(HAVE_EMACS),1)
>  %.elc: %.el $(global_deps)
> -- 
> 1.8.4.rc3
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch