reftex.el autoloads

reftex.el autoloads

[Phillip Lord]

I'm trying to run git bisect on emacs, but I have a problem. After
building and testing emacs, routinely, git cannot start the next bisect
because reftex.el seems to have been changed locally.

Of course, I can fix this with git checkout lisp/textmodes/reftex.el,
but I am trying to do an unattended bisect.

The diff looks like this....

git diff
diff --git a/lisp/textmodes/reftex.el b/lisp/textmodes/reftex.el
index 84efa7a..4ee3658 100644
--- a/lisp/textmodes/reftex.el
+++ b/lisp/textmodes/reftex.el
@@ -2397,7 +2397,7 @@ reftex-report-bug
 ^L
 ;;; Start of automatically extracted autoloads.
 ^L
-;;;### (autoloads nil "reftex-auc" "reftex-auc.el" "cf606f7918831321cb46f254436dc66e")
+;;;### (autoloads nil "reftex-auc" "reftex-auc.el" "7c0e0b46919f4ceefe1026e31e73ebcd")
 ;;; Generated autoloads from reftex-auc.el
 
 (autoload 'reftex-arg-label "reftex-auc" "\


So, it appears that reftex.el is actually partially real source and
partially generated during the normal build, which doesn't seem a good
thing to me.

Am I missing something? And is there a good way to fix it?

Phil

Re: reftex.el autoloads

[Artur Malabarba]

[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]

On 10 Dec 2015 11:03 am, "Phillip Lord" <phillip.lord@russet.org.uk> wrote:
>
>
> I'm trying to run git bisect on emacs, but I have a problem. After
> building and testing emacs, routinely, git cannot start the next bisect
> because reftex.el seems to have been changed locally.
>
> Of course, I can fix this with git checkout lisp/textmodes/reftex.el,
> but I am trying to do an unattended bisect.
>
> The diff looks like this....
>
> git diff
> diff --git a/lisp/textmodes/reftex.el b/lisp/textmodes/reftex.el
> index 84efa7a..4ee3658 100644
> --- a/lisp/textmodes/reftex.el
> +++ b/lisp/textmodes/reftex.el
> @@ -2397,7 +2397,7 @@ reftex-report-bug
>  ^L
>  ;;; Start of automatically extracted autoloads.
>  ^L
> -;;;### (autoloads nil "reftex-auc" "reftex-auc.el"
"cf606f7918831321cb46f254436dc66e")
> +;;;### (autoloads nil "reftex-auc" "reftex-auc.el"
"7c0e0b46919f4ceefe1026e31e73ebcd")
>  ;;; Generated autoloads from reftex-auc.el
>
>  (autoload 'reftex-arg-label "reftex-auc" "\
>
>
> So, it appears that reftex.el is actually partially real source and
> partially generated during the normal build, which doesn't seem a good
> thing to me.
>
> Am I missing something? And is there a good way to fix it?
>
> Phil

FWIW,
I've noticed several other autoloads files presenting the same behaviour
whenever I build emacs.

[-- Attachment #2: Type: text/html, Size: 1808 bytes --]

Re: reftex.el autoloads

[Eli Zaretskii]

> From: phillip.lord@russet.org.uk (Phillip Lord)
> Date: Thu, 10 Dec 2015 11:03:30 +0000
> 
> --- a/lisp/textmodes/reftex.el
> +++ b/lisp/textmodes/reftex.el
> @@ -2397,7 +2397,7 @@ reftex-report-bug
>  ^L
>  ;;; Start of automatically extracted autoloads.
>  ^L
> -;;;### (autoloads nil "reftex-auc" "reftex-auc.el" "cf606f7918831321cb46f254436dc66e")
> +;;;### (autoloads nil "reftex-auc" "reftex-auc.el" "7c0e0b46919f4ceefe1026e31e73ebcd")
>  ;;; Generated autoloads from reftex-auc.el
>  
>  (autoload 'reftex-arg-label "reftex-auc" "\
> 
> 
> So, it appears that reftex.el is actually partially real source and
> partially generated during the normal build, which doesn't seem a good
> thing to me.

These are autoloads.  Several files, reftex.el among them, record
autoloads for their parent packages.  Other examples include dired.el,
esh-groups.el, and rmail.el.  Grep the Lisp files for
'generated-autoload-file:' (without the quotes), and you will see them
all.

> Am I missing something?

Not sure, but this is normal.

> And is there a good way to fix it?

You mean, for the bisect script?  Just "git checkout" the offending
file after the build, that's all.

Re: reftex.el autoloads

[Paul Eggert]

On 12/10/2015 08:28 AM, Eli Zaretskii wrote:
> You mean, for the bisect script?  Just "git checkout" the offending
> file after the build, that's all.

It would be nice if the build procedure didn't mangle source files in 
this way, of course. (Not that I'm volunteering right now to fix this....).

Re: reftex.el autoloads

[Phillip Lord]

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 12/10/2015 08:28 AM, Eli Zaretskii wrote:
>> You mean, for the bisect script?  Just "git checkout" the offending
>> file after the build, that's all.
>
> It would be nice if the build procedure didn't mangle source files in this
> way, of course. (Not that I'm volunteering right now to fix this....).

As a quick guess, I would assume that changing all occurences of:

generated-autoload-file: "reftex.el"

to

generated-autoload-file: "reftex-loaddefs.el"

then adding

(require 'reftex-loaddefs)

to the end of reftex should do the job. Likewise, for the other
instances.

Phil

Re: reftex.el autoloads

[Eli Zaretskii]

> From: phillip.lord@russet.org.uk (Phillip Lord)
> Cc: Eli Zaretskii <eliz@gnu.org>,  <emacs-devel@gnu.org>
> Date: Thu, 10 Dec 2015 22:07:47 +0000
> 
> As a quick guess, I would assume that changing all occurences of:
> 
> generated-autoload-file: "reftex.el"
> 
> to
> 
> generated-autoload-file: "reftex-loaddefs.el"
> 
> then adding
> 
> (require 'reftex-loaddefs)
> 
> to the end of reftex should do the job. Likewise, for the other
> instances.

You'd need at least to add rules to lips/Makefile.in to generate
those.

If you decide to go with this (and I'm not expert enough to know
whether there are hidden issues), please do that on master, not on
emacs-25.

Thanks.

Re: reftex.el autoloads

[Phillip Lord]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: phillip.lord@russet.org.uk (Phillip Lord)
>> Cc: Eli Zaretskii <eliz@gnu.org>,  <emacs-devel@gnu.org>
>> Date: Thu, 10 Dec 2015 22:07:47 +0000
>> 
>> As a quick guess, I would assume that changing all occurences of:
>> 
>> generated-autoload-file: "reftex.el"
>> 
>> to
>> 
>> generated-autoload-file: "reftex-loaddefs.el"
>> 
>> then adding
>> 
>> (require 'reftex-loaddefs)
>> 
>> to the end of reftex should do the job. Likewise, for the other
>> instances.
>
> You'd need at least to add rules to lips/Makefile.in to generate
> those.
>
> If you decide to go with this (and I'm not expert enough to know
> whether there are hidden issues), please do that on master, not on
> emacs-25.


I think it is worth trying, and yes, of course, it would be on master.

Phil

Re: reftex.el autoloads

[Phillip Lord]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: phillip.lord@russet.org.uk (Phillip Lord)
>> Date: Thu, 10 Dec 2015 11:03:30 +0000
>> 
>> --- a/lisp/textmodes/reftex.el
>> +++ b/lisp/textmodes/reftex.el
>> @@ -2397,7 +2397,7 @@ reftex-report-bug
>>  ^L
>>  ;;; Start of automatically extracted autoloads.
>>  ^L
>> -;;;### (autoloads nil "reftex-auc" "reftex-auc.el"
>> "cf606f7918831321cb46f254436dc66e")
>> +;;;### (autoloads nil "reftex-auc" "reftex-auc.el"
>> "7c0e0b46919f4ceefe1026e31e73ebcd")
>>  ;;; Generated autoloads from reftex-auc.el
>>  
>>  (autoload 'reftex-arg-label "reftex-auc" "\
>> 
>> 
>> So, it appears that reftex.el is actually partially real source and
>> partially generated during the normal build, which doesn't seem a good
>> thing to me.
>
> These are autoloads.

Yes, I know!

> Several files, reftex.el among them, record autoloads for their parent
> packages. Other examples include dired.el, esh-groups.el, and
> rmail.el. Grep the Lisp files for 'generated-autoload-file:' (without
> the quotes), and you will see them all.

So, quite a lot of files seem to set generated-autoload-file to
loaddefs.el which seems a bit odd (since it's the default). Of the rest,
it looks like dired, rmail, reftex and eieio do this (esh-groups.el
is pure autoload -- is not versioned).

I am not sure why only reftex.el causes me problems -- why not the
others, I wonder?

Also, the changed line is actually this...

>> +;;;### (autoloads nil "reftex-auc" "reftex-auc.el" "7c0e0b46919f4ceefe1026e31e73ebcd")

which is a comment. I don't know what this line does in the autoload
process.



>> And is there a good way to fix it?
>
> You mean, for the bisect script?  Just "git checkout" the offending
> file after the build, that's all.

I was thinking more generally. Having files which are generated in
versioning is never great (c.f. issues with ChangeLog).

Phil

Re: reftex.el autoloads

[Eli Zaretskii]

> From: phillip.lord@russet.org.uk (Phillip Lord)
> Cc: <emacs-devel@gnu.org>
> Date: Thu, 10 Dec 2015 22:03:32 +0000
> 
> So, quite a lot of files seem to set generated-autoload-file to
> loaddefs.el which seems a bit odd (since it's the default).

Only in lisp/cedet/, I think.  I'm guessing this is the result of
integrating CEDET.  If you want the details, maybe the CEDET people
and/or Chong (which did most of the work) could tell you the reasons
for that.  I suggest to look at Git logs and maybe ask them Or maybe
Glenn (CC'ed) remembers.

> I am not sure why only reftex.el causes me problems -- why not the
> others, I wonder?

Probably because the range of versions you tried to bisect didn't
touch the others.  It obviously depends on which files whose autoloads
are there are modified by the commits you are bisecting.

> Also, the changed line is actually this...
> 
> >> +;;;### (autoloads nil "reftex-auc" "reftex-auc.el" "7c0e0b46919f4ceefe1026e31e73ebcd")
> 
> which is a comment. I don't know what this line does in the autoload
> process.

It's documented in the ELisp manual.  Which parts are unclear?

Re: reftex.el autoloads

[Phillip Lord]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: phillip.lord@russet.org.uk (Phillip Lord)
>> Cc: <emacs-devel@gnu.org>
>> Date: Thu, 10 Dec 2015 22:03:32 +0000
>> 
>> So, quite a lot of files seem to set generated-autoload-file to
>> loaddefs.el which seems a bit odd (since it's the default).
>
> Only in lisp/cedet/, I think.  I'm guessing this is the result of
> integrating CEDET.  If you want the details, maybe the CEDET people
> and/or Chong (which did most of the work) could tell you the reasons
> for that.  I suggest to look at Git logs and maybe ask them Or maybe
> Glenn (CC'ed) remembers.

It's not a disaster either way.


>> I am not sure why only reftex.el causes me problems -- why not the
>> others, I wonder?
>
> Probably because the range of versions you tried to bisect didn't
> touch the others.  It obviously depends on which files whose autoloads
> are there are modified by the commits you are bisecting.

Yes, I guess that is true.

>> Also, the changed line is actually this...
>> 
>> >> +;;;### (autoloads nil "reftex-auc" "reftex-auc.el" "7c0e0b46919f4ceefe1026e31e73ebcd")
>> 
>> which is a comment. I don't know what this line does in the autoload
>> process.
>
> It's documented in the ELisp manual.  Which parts are unclear?

I did re-read the elisp manual, of course, and I can't find anything
which documents this; it describes comments in files to be autoloaded,
but not the autoloads. I tracked the source code, and it's inserted by
`autoload-insert-section-header'.

Deep in the bowels of autoload.el, there is a call to this which is
documented like this:

;; MD5 checksums are much better because they do not
;; change unless the file changes (so they'll be
;; equal on two different systems and will change
;; less often than time-stamps, thus leading to fewer
;; unneeded changes causing spurious conflicts), but
;; using time-stamps is a very useful optimization,
;; so we use time-stamps for the main autoloads file
;; (loaddefs.el) where we have special ways to
;; circumvent the "random change problem", and MD5
;; checksum in secondary autoload files where we do
;; not need the time-stamp optimization because it is
;; already provided by the primary autoloads file.

So, it's a checksum (not a git commit which was my first thought) of the
file contents (except in loaddefs.el when it's a timestamp). So it will
update whenever reftex-auc changes whether the autoloads need to change
or not.

Phil

Re: reftex.el autoloads

[Eli Zaretskii]

> From: phillip.lord@russet.org.uk (Phillip Lord)
> Cc: Glenn Morris <rgm@gnu.org>,  <emacs-devel@gnu.org>
> Date: Fri, 11 Dec 2015 15:35:09 +0000
> 
> ;; MD5 checksums are much better because they do not
> ;; change unless the file changes (so they'll be
> ;; equal on two different systems and will change
> ;; less often than time-stamps, thus leading to fewer
> ;; unneeded changes causing spurious conflicts), but
> ;; using time-stamps is a very useful optimization,
> ;; so we use time-stamps for the main autoloads file
> ;; (loaddefs.el) where we have special ways to
> ;; circumvent the "random change problem", and MD5
> ;; checksum in secondary autoload files where we do
> ;; not need the time-stamp optimization because it is
> ;; already provided by the primary autoloads file.
> 
> So, it's a checksum (not a git commit which was my first thought) of the
> file contents (except in loaddefs.el when it's a timestamp). So it will
> update whenever reftex-auc changes whether the autoloads need to change
> or not.

Yes.

Re: reftex.el autoloads

[Phillip Lord]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: phillip.lord@russet.org.uk (Phillip Lord)
>> Cc: Glenn Morris <rgm@gnu.org>,  <emacs-devel@gnu.org>
>> Date: Fri, 11 Dec 2015 15:35:09 +0000
>> 
>> ;; MD5 checksums are much better because they do not
>> ;; change unless the file changes (so they'll be
>> ;; equal on two different systems and will change
>> ;; less often than time-stamps, thus leading to fewer
>> ;; unneeded changes causing spurious conflicts), but
>> ;; using time-stamps is a very useful optimization,
>> ;; so we use time-stamps for the main autoloads file
>> ;; (loaddefs.el) where we have special ways to
>> ;; circumvent the "random change problem", and MD5
>> ;; checksum in secondary autoload files where we do
>> ;; not need the time-stamp optimization because it is
>> ;; already provided by the primary autoloads file.
>> 
>> So, it's a checksum (not a git commit which was my first thought) of the
>> file contents (except in loaddefs.el when it's a timestamp). So it will
>> update whenever reftex-auc changes whether the autoloads need to change
>> or not.
>
> Yes.

I'll chase this up and see if I can fix the issue for reftex. If that
works, I see whether I can remove the others also.

Thanks for the advise.

Phil

Re: reftex.el autoloads

[Glenn Morris]

Personally I don't believe there is anything to fix.
Stefan came up with this system specifically so that autoloads could be
stored in those files.
AFAICS the only "fix" that exists is to put the autoloads back into separate
non-versioned files, which obviously could have been done at any time.
(Note that we have admin/update_autogen which automatically update the
md5sums once a day if needed. It's been working fine for years.)

Anyway, pretty sure you will find this was already discussed in the
emacs-devel archives a long time ago. Eg

http://lists.gnu.org/archive/html/emacs-devel/2007-07/msg00336.html

Nothing's changed, so I don't see a need to change something that works.

Re: reftex.el autoloads

[Phillip Lord]

Glenn Morris <rgm@gnu.org> writes:

> Personally I don't believe there is anything to fix.
> Stefan came up with this system specifically so that autoloads could be
> stored in those files.
> AFAICS the only "fix" that exists is to put the autoloads back into separate
> non-versioned files, which obviously could have been done at any time.
> (Note that we have admin/update_autogen which automatically update the
> md5sums once a day if needed. It's been working fine for years.)
>
> Anyway, pretty sure you will find this was already discussed in the
> emacs-devel archives a long time ago. Eg
>
> http://lists.gnu.org/archive/html/emacs-devel/2007-07/msg00336.html

My reading of that thread (and the comment in autoload.el) is that
Stefan came up with the system specifically to minimize the problems
with files being partially versioned. Consider from Eli:

>> It also breaks the principle that files that are rewritten locally as
>> part of the build process are not kept in CVS.

>> So I think this change in its current incarnation is for the worse.
>> Maybe if the autoloads were written into files that are not in CVS
>> I'd be happier.

And from Stefan:

>> Indeed, there's a problem with the current situation. I have an idea
>> to solve this problem, I'll install a patch shortly,

> Nothing's changed, so I don't see a need to change something that works.

We've changed VC from CVS. Stefan's patch solves the problem for linear
development. But it doesn't solve the problem when you checkout
backwards in time (i.e. git bisect) or sideways (i.e. switching branch).

I still do not understand why having generated statements in a source
file is desirable.

Phil

Re: reftex.el autoloads

[Paul Eggert]

On 12/11/2015 09:46 AM, Glenn Morris wrote:
> I don't see a need to change something that works.
>

But it doesn't work. At least, not for me. Too often I deal with 
spurious autoloads-related diffs that get in the way of real work. I 
often see them in 'git diff' output and have to remember that they're 
not real diffs. Once or twice I even committed them by accident to my 
own repository.

This problem has been a nagging low-level build hassle for some time. 
It's not urgent to fix. But if someone has the time, it'd be nice to fix it.

Re: reftex.el autoloads

[Phillip Lord]

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 12/11/2015 09:46 AM, Glenn Morris wrote:
>> I don't see a need to change something that works.
>>
>
> But it doesn't work. At least, not for me. Too often I deal with spurious
> autoloads-related diffs that get in the way of real work. I often see them in
> 'git diff' output and have to remember that they're not real diffs. Once or
> twice I even committed them by accident to my own repository.
>
> This problem has been a nagging low-level build hassle for some time. It's not
> urgent to fix. But if someone has the time, it'd be nice to fix it.

I am reticent to change things which I don't fully understand, so here
is my understanding of the world at the moment.

AFAICT, the use of autoloads in reftex.el started with 41116c5a in
2014-03, which was Glenn's commit. The idea is, I think, that reftex
defines a number of "global" autoloads, as well as a set of internal
autoloads. Loading reftex runs the internal autoloads which then load
the rest of reftex lazily.

If this is the case, I can see no reason why the autoloads cannot be
redirected to reftex-loaddefs.el which is not versioned, and have
reftex.el require this (I have tried this and it seems to work).

I'm also a little uncertain why the "internal" and "global" autoloads is
necessary. Is is just for performance? On my machine, reftex and
associated files load in somewhat less than a tenth of a second. Why not
put all autoloads into loaddefs.el?

Phil

Re: reftex.el autoloads

[Eli Zaretskii]

> From: phillip.lord@russet.org.uk (Phillip Lord)
> Date: Sat, 12 Dec 2015 18:24:24 +0000
> Cc: Glenn Morris <rgm@gnu.org>, Eli Zaretskii <eliz@gnu.org>,
> 	emacs-devel@gnu.org
> 
> I am reticent to change things which I don't fully understand, so here
> is my understanding of the world at the moment.
> 
> AFAICT, the use of autoloads in reftex.el started with 41116c5a in
> 2014-03, which was Glenn's commit.

I don't understand why you single out reftex.el.  If this issue should
be resolved, it should either be resolved the same way for all of the
files that similarly store autoloads, or for none at all.  It makes
very little sense to solve it only for reftex.el.

Re: reftex.el autoloads

[Phillip Lord]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: phillip.lord@russet.org.uk (Phillip Lord)
>> Date: Sat, 12 Dec 2015 18:24:24 +0000
>> Cc: Glenn Morris <rgm@gnu.org>, Eli Zaretskii <eliz@gnu.org>,
>> 	emacs-devel@gnu.org
>> 
>> I am reticent to change things which I don't fully understand, so here
>> is my understanding of the world at the moment.
>> 
>> AFAICT, the use of autoloads in reftex.el started with 41116c5a in
>> 2014-03, which was Glenn's commit.
>
> I don't understand why you single out reftex.el.  If this issue should
> be resolved, it should either be resolved the same way for all of the
> files that similarly store autoloads, or for none at all.  It makes
> very little sense to solve it only for reftex.el.

Oh, just because it was the one that broke my bisect. Of course, you are
right that it should be resolved for all files, not just reftex.el. But
I need to understand why one file is the way that it is first.

I think that I have found all the other incidences now. These are

dired.el
htmlfontity
ibuffer.el
rmail.el
ps-print.el

Of these, ps-print.el was Stefan's commit in 2007. All the others were
Glenn, over a spread of years (unto 2014 for reftex).

I'm guessing the motivation for all of these was the same -- faster
loading of loaddefs.el, and lazy loading of other parts of subsidiary
files. If this is the motivation I wonder whether it holds as true in
2015 as it did in 2007.

Phil

Re: reftex.el autoloads

[Eli Zaretskii]

> From: phillip.lord@russet.org.uk (Phillip Lord)
> Cc: <rgm@gnu.org>,  <eggert@cs.ucla.edu>,  <emacs-devel@gnu.org>
> Date: Sat, 12 Dec 2015 18:56:31 +0000
> 
> I think that I have found all the other incidences now. These are
> 
> dired.el
> htmlfontity
> ibuffer.el
> rmail.el
> ps-print.el
> 
> Of these, ps-print.el was Stefan's commit in 2007. All the others were
> Glenn, over a spread of years (unto 2014 for reftex).
> 
> I'm guessing the motivation for all of these was the same -- faster
> loading of loaddefs.el, and lazy loading of other parts of subsidiary
> files. If this is the motivation I wonder whether it holds as true in
> 2015 as it did in 2007.

No, the motivation was to replace the hand-written autoloads that were
there before.  See this message, where Stefan posted a short
explanation:

  http://lists.gnu.org/archive/html/emacs-devel/2007-07/msg00263.html

(Originally, these autoloads recorded the time of the update, which
was a hassle discussed in that thread; that was replaced with a
checksum to avoid changing the file too often when the autoloads
didn't really change.)

Re: reftex.el autoloads

[Phillip Lord]

Eli Zaretskii <eliz@gnu.org> writes:
>> I'm guessing the motivation for all of these was the same -- faster
>> loading of loaddefs.el, and lazy loading of other parts of subsidiary
>> files. If this is the motivation I wonder whether it holds as true in
>> 2015 as it did in 2007.
>
> No, the motivation was to replace the hand-written autoloads that were
> there before.  See this message, where Stefan posted a short
> explanation:
>
>   http://lists.gnu.org/archive/html/emacs-devel/2007-07/msg00263.html
>
> (Originally, these autoloads recorded the time of the update, which
> was a hassle discussed in that thread; that was replaced with a
> checksum to avoid changing the file too often when the autoloads
> didn't really change.)


Sure, I can see that, but I am wondering why the hand-written autoloads
were just not replaced with generated autoloads in loaddefs.el? That is
the simplest solution. Simple is good (so long as it's not simplistic).

Or, as an alternative, implied by this post:

http://lists.gnu.org/archive/html/emacs-devel/2007-07/msg00320.html

> So I think this change in its current incarnation is for the worse.
> Maybe if the autoloads were written into files that are not in CVS I'd
> be happier.

Which was your comment at the time. In this scenario, the reftex-* files
would generate autoloads to reftex-loaddefs.el which reftex.el would
require. This preserves the current behaviour, but avoids files which
are both source and generated.

One response to this was:

> cvs already contains one generated file (configure script) and
> possibly others.

But configure isn't under versioning any more. And Stefan's MD5 patch
was the other. It's certainly true that Stefan's solution massively
reduces the problem, but it doesn't fix it, since a *build* can still
produce changes in versioned files.

Phil

Re: reftex.el autoloads

[Eli Zaretskii]

> From: phillip.lord@russet.org.uk (Phillip Lord)
> Cc: <rgm@gnu.org>,  <eggert@cs.ucla.edu>,  <emacs-devel@gnu.org>
> Date: Sat, 12 Dec 2015 21:15:46 +0000
> 
> Or, as an alternative, implied by this post:
> 
> http://lists.gnu.org/archive/html/emacs-devel/2007-07/msg00320.html
> 
> > So I think this change in its current incarnation is for the worse.
> > Maybe if the autoloads were written into files that are not in CVS I'd
> > be happier.
> 
> Which was your comment at the time.

Switching to checksums made the problem much more rare, so it went
under the radar.  See my comments about _that_ later in the thread.

Re: reftex.el autoloads

[Phillip Lord]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: phillip.lord@russet.org.uk (Phillip Lord)
>> Cc: <rgm@gnu.org>,  <eggert@cs.ucla.edu>,  <emacs-devel@gnu.org>
>> Date: Sat, 12 Dec 2015 21:15:46 +0000
>> 
>> Or, as an alternative, implied by this post:
>> 
>> http://lists.gnu.org/archive/html/emacs-devel/2007-07/msg00320.html
>> 
>> > So I think this change in its current incarnation is for the worse.
>> > Maybe if the autoloads were written into files that are not in CVS I'd
>> > be happier.
>> 
>> Which was your comment at the time.
>
> Switching to checksums made the problem much more rare, so it went
> under the radar.  See my comments about _that_ later in the thread.


Where you say "that fixes it"? Checksums do reduce the problem, but then
if you use the same trick for many other files, it increases the problem
again.

I am still confused though. Is there any problem with either:

1) Putting autoloads into another file, and requiring that.
2) Just putting the autoloads into loaddefs.el.

The latter is the minimal change from where we are now. But 2) seems
better because it is much simpler.

Solution 2) also fixes a minor issue with the build. In a built emacs
try

rm lisp/eshell/eshell-groups.el
make check

This *should* rebuild eshell-groups.el but does not, while

rm lisp/loaddefs.el
make check

works fine. My knowledge of make is not that good, but some quick
searching suggests that there is no simple solution to this in the
makefile.

Phil

Re: reftex.el autoloads

[Glenn Morris]

Phillip Lord wrote:

> 2) Just putting the autoloads into loaddefs.el.

That's just plain wrong. Please don't do that.
The only options that make any sense are:

1) do nothing (which is what I would do)

2) forbid generated-autoload-file to ever point to a versioned file.
That means changing all these files:

find lisp -name '*.el' -exec grep '^;.*generated-autoload-file:' {} + | \
  sed -e '/loaddefs\|esh-groups/d' -e 's|/[^/]*: "|/|' -e 's/"//g' | sort -u

lisp/dired.el
lisp/emacs-lisp/eieio-core.el
lisp/emacs-lisp/eieio.el
lisp/htmlfontify.el
lisp/ibuffer.el
lisp/mail/rmail.el
lisp/obsolete/tpu-edt.el
lisp/ps-print.el
lisp/textmodes/reftex.el

to load foo-loadefs.el (which in many cases will be tiny files).

Re: reftex.el autoloads

[Phillip Lord]

Glenn Morris <rgm@gnu.org> writes:

> Phillip Lord wrote:
>
>> 2) Just putting the autoloads into loaddefs.el.
>
> That's just plain wrong. Please don't do that.

It may be wrong, but it's not plain. I assumed that there was a reason
for not doing this. But I still do not understand what that reason is.
Can you please tell me what is wrong with it, because I cannot figure it
out?


> The only options that make any sense are:
>
> 1) do nothing (which is what I would do)
>
> 2) forbid generated-autoload-file to ever point to a versioned file.
> That means changing all these files:
>
> find lisp -name '*.el' -exec grep '^;.*generated-autoload-file:' {} + | \
>   sed -e '/loaddefs\|esh-groups/d' -e 's|/[^/]*: "|/|' -e 's/"//g' | sort -u
>
> lisp/dired.el
> lisp/emacs-lisp/eieio-core.el
> lisp/emacs-lisp/eieio.el
> lisp/htmlfontify.el
> lisp/ibuffer.el
> lisp/mail/rmail.el
> lisp/obsolete/tpu-edt.el
> lisp/ps-print.el
> lisp/textmodes/reftex.el
>
> to load foo-loadefs.el (which in many cases will be tiny files).

Yes, some of them are pretty small.

Phil

Re: reftex.el autoloads

[Glenn Morris]

Phillip Lord wrote:

>>> 2) Just putting the autoloads into loaddefs.el.
>>
>> That's just plain wrong. Please don't do that.
>
> It may be wrong, but it's not plain. I assumed that there was a reason
> for not doing this. But I still do not understand what that reason is.
> Can you please tell me what is wrong with it, because I cannot figure it
> out?

I don't know if I can explain it well.
Basically, code should be loaded where it needs to be loaded, not at the
global level to work around some perceived problem with the toolchain.

>> to load foo-loadefs.el (which in many cases will be tiny files).
>
> Yes, some of them are pretty small.

That was the traditional deciding criterion.

http://lists.gnu.org/archive/html/emacs-devel/2009-09/msg00264.html

The reftex example is surely on the wrong side of that, but as the
commit message says, it was replacing prevous hand-written autoloads
that were in reftex.el.

Re: reftex.el autoloads

[Phillip Lord]

Glenn Morris <rgm@gnu.org> writes:

> Phillip Lord wrote:
>
>>>> 2) Just putting the autoloads into loaddefs.el.
>>>
>>> That's just plain wrong. Please don't do that.
>>
>> It may be wrong, but it's not plain. I assumed that there was a reason
>> for not doing this. But I still do not understand what that reason is.
>> Can you please tell me what is wrong with it, because I cannot figure it
>> out?
>
> I don't know if I can explain it well.
> Basically, code should be loaded where it needs to be loaded, not at the
> global level to work around some perceived problem with the toolchain.

Yep, well that is the point of autoloads, and that would be true where
ever the autoloads are.

In this case, you mean "autoloads should be loaded only when the
functions that they load might need to be loaded, as opposed to a global
when they might need to be loaded, but less might".

Still, it's not worth arguing about that much. I can maintain the status
quo in this respect, so I will do, even if I think it should be
simplified further.

>
>>> to load foo-loadefs.el (which in many cases will be tiny files).
>>
>> Yes, some of them are pretty small.
>
> That was the traditional deciding criterion.
>
> http://lists.gnu.org/archive/html/emacs-devel/2009-09/msg00264.html
>
> The reftex example is surely on the wrong side of that, but as the
> commit message says, it was replacing prevous hand-written autoloads
> that were in reftex.el.


There's actually a bionomial distribution -- about half the files are
under 10, and the other half average around 50 (starting at 20). But I
see no point in moving half, and will just do them all.

Phil

Re: reftex.el autoloads

[Glenn Morris]

Phillip Lord wrote:

> But I see no point in moving half, and will just do them all.

So is everyone ok with this decision to ban versioned autoloads from
Emacs? They've been there for years, causing no problem IMO.

Re: reftex.el autoloads

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Date: Wed, 16 Dec 2015 21:15:28 -0500
> Cc: Eli Zaretskii <eliz@gnu.org>, eggert@cs.ucla.edu, emacs-devel@gnu.org
> 
> Phillip Lord wrote:
> 
> > But I see no point in moving half, and will just do them all.
> 
> So is everyone ok with this decision to ban versioned autoloads from
> Emacs? They've been there for years, causing no problem IMO.

They aren't a problem for me, but I don't have any strong opinions
about this.

Re: reftex.el autoloads

[John Wiegley]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

>> So is everyone ok with this decision to ban versioned autoloads from
>> Emacs? They've been there for years, causing no problem IMO.

> They aren't a problem for me, but I don't have any strong opinions about
> this.

I can't say I have a strong opinion either. They seem like an unnecessary
complexity, but perhaps I'm missing something.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: reftex.el autoloads

[Phillip Lord]

John Wiegley <jwiegley@gmail.com> writes:

>>>>>> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> So is everyone ok with this decision to ban versioned autoloads from
>>> Emacs? They've been there for years, causing no problem IMO.
>
>> They aren't a problem for me, but I don't have any strong opinions about
>> this.
>
> I can't say I have a strong opinion either. They seem like an unnecessary
> complexity, but perhaps I'm missing something.

Assuming I have done it correctly, they are now gone on master. My own
feeling is that some of these (esp htmlfontify and ps-print) do not
really merit an independent loaddefs file and they should just be moved
to the main autoloads file. But the main issue -- versioned autogen
files -- has been fixed, at the cost of slightly longer makefile.

Phil

Re: reftex.el autoloads

[Glenn Morris]

Somewhat disappointed, but obviously I'm in a minority.
Anyway, it doesn't build. Did you test it?

http://hydra.nixos.org/build/29118180

Re: reftex.el autoloads

[Paul Eggert]

On 12/18/2015 08:39 AM, Glenn Morris wrote:
> Somewhat disappointed, but obviously I'm in a minority.
> Anyway, it doesn't build. Did you test it?

Yes, I suggest reverting the change from master until it's tested and 
known to work from a fresh checkout.

Here are a couple of other comments. That's not the right way to declare 
autoloads-force to be phony. Also, there shouldn't need to be an 
autoloads-force at all: if some other files need to be built they should 
be listed as dependencies of autoload.

Re: reftex.el autoloads

[Phillip Lord]

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 12/18/2015 08:39 AM, Glenn Morris wrote:
>> Somewhat disappointed, but obviously I'm in a minority.
>> Anyway, it doesn't build. Did you test it?
>
> Yes, I suggest reverting the change from master until it's tested and known to
> work from a fresh checkout.
>
> Here are a couple of other comments. That's not the right way to declare
> autoloads-force to be phony.

I thought it was an usual usage, but I copied it from autoloads. I
guess:

autoloads-force .PHONY:
	rm loaddefs.el
	$(MAKE) autoloads

should be

autoloads-force:
        rm loaddefs.el
        $(MAKE) autoloads

.PHONY: autoloads-force



> Also, there shouldn't need to be an autoloads-force at all: if some
> other files need to be built they should be listed as dependencies of
> autoload.


Indeed. It's always been this way, though -- from a non-bootstrap on
emacs-25 try

rm lisp/eshell/esh-groups.el
make
./src/emacs -Q

M-x eshell

It will crash. What you actually have to do is

cd lisp
rm loaddefs.el
make autoloads

The problem is that files are not dependencies, they are targets which
are build at the same time as loaddefs.el. AFAICT, there is no good way
of dealing with this in make (and perhaps there should be).

It is fixable, but it requires significant modifications to
autoloads.el, and will substantially complicate lisp/Makefile.in. I
didn't want to make that change, for fear of disappointing Glenn
further.

This is a minimal change, and was quite useful was I was producing my
(ill-tested) set of commits.

Phil

Re: reftex.el autoloads

[Eli Zaretskii]

> From: phillip.lord@russet.org.uk (Phillip Lord)
> Date: Fri, 18 Dec 2015 20:55:24 +0000
> Cc: Glenn Morris <rgm@gnu.org>, Eli Zaretskii <eliz@gnu.org>,
> 	emacs-devel@gnu.org
> 
> > Here are a couple of other comments. That's not the right way to declare
> > autoloads-force to be phony.
> 
> I thought it was an usual usage, but I copied it from autoloads. I
> guess:
> 
> autoloads-force .PHONY:
> 	rm loaddefs.el
> 	$(MAKE) autoloads
> 
> should be
> 
> autoloads-force:
>         rm loaddefs.el
>         $(MAKE) autoloads
> 
> .PHONY: autoloads-force
> 
> 
> 
> > Also, there shouldn't need to be an autoloads-force at all: if some
> > other files need to be built they should be listed as dependencies of
> > autoload.
> 
> 
> Indeed. It's always been this way, though -- from a non-bootstrap on
> emacs-25 try

Any reason why you need explicit Makefile targets for the new files?
Why not model them on calc/calc-loaddefs.el, which has not target in
the Makefile at all?

Re: reftex.el autoloads

[Phillip Lord]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: phillip.lord@russet.org.uk (Phillip Lord)
>> Date: Fri, 18 Dec 2015 20:55:24 +0000
>> Cc: Glenn Morris <rgm@gnu.org>, Eli Zaretskii <eliz@gnu.org>,
>> 	emacs-devel@gnu.org
>> 
>> > Here are a couple of other comments. That's not the right way to declare
>> > autoloads-force to be phony.
>> 
>> I thought it was an usual usage,

"I thought it was an unusual usage"

Hmmm. Too late in the year, I need a holiday.


>> > Also, there shouldn't need to be an autoloads-force at all: if some
>> > other files need to be built they should be listed as dependencies of
>> > autoload.
>> 
>> 
>> Indeed. It's always been this way, though -- from a non-bootstrap on
>> emacs-25 try
>
> Any reason why you need explicit Makefile targets for the new files?

To continue with the same functionality as my changes, no there is no
reason, and they don't have them.

> Why not model them on calc/calc-loaddefs.el, which has not target in
> the Makefile at all?

I thought that this is what I had done.

Phil

Re: reftex.el autoloads

[Eli Zaretskii]

> From: phillip.lord@russet.org.uk (Phillip Lord)
> Cc: <rgm@gnu.org>,  <eggert@cs.ucla.edu>,  <emacs-devel@gnu.org>
> Date: Fri, 18 Dec 2015 21:51:05 +0000
> 
> > Why not model them on calc/calc-loaddefs.el, which has not target in
> > the Makefile at all?
> 
> I thought that this is what I had done.

Sorry, you are right.  But then I don't understand what was that
discussion about Makefile targets about, as you didn't add any,
AFAICS.

Re: reftex.el autoloads

[Phillip Lord]

Eli Zaretskii <eliz@gnu.org> writes:
>> > Also, there shouldn't need to be an autoloads-force at all: if some
>> > other files need to be built they should be listed as dependencies of
>> > autoload.
>> 
>> 
>> Indeed. It's always been this way, though -- from a non-bootstrap on
>> emacs-25 try
>
> Any reason why you need explicit Makefile targets for the new files?
> Why not model them on calc/calc-loaddefs.el, which has not target in
> the Makefile at all?

Ah, just realised what you were asking.

The problem is this rule.

$(lisp)/loaddefs.el: $(LOADDEFS)
	@echo Directories for loaddefs: ${SUBDIRS_ALMOST}
	$(AM_V_GEN)$(emacs) -l autoload \
	    --eval '(setq autoload-ensure-writable t)' \
	    --eval '(setq autoload-builtin-package-versions t)' \
	    --eval '(setq generated-autoload-file (expand-file-name (unmsys--file-name "$@")))' \
	    -f batch-update-autoloads ${SUBDIRS_ALMOST}

So batch-update-autoloads only gets call if loaddefs.el is out-of-date.
If reftex-loaddefs is out of date, then it does not, and so
reftex-loaddefs will not get build.

The intuitive solution is do

$(lisp)/loaddefs.el $(lisp)/textmodes/reftex-loadefs.el $(AND_ALL_THE_OTHERS): $(LOADDEFS)
	@echo Directories for loaddefs: ${SUBDIRS_ALMOST}
	$(AM_V_GEN)$(emacs) -l autoload \
	    --eval '(setq autoload-ensure-writable t)' \
	    --eval '(setq autoload-builtin-package-versions t)' \
	    --eval '(setq generated-autoload-file (expand-file-name (unmsys--file-name "$@")))' \
	    -f batch-update-autoloads ${SUBDIRS_ALMOST}

but this fails because make actually runs batch-update-autoloads
multiple times which is both wasteful and causes a race condition with
-j.

As this doesn't work, I didn't do it (I tried it!). Instead, I added
autoloads-force. It's ugly, but it's explicit and ugly rather than
implicit and ugly.

Phil

Re: reftex.el autoloads

[Eli Zaretskii]

> From: phillip.lord@russet.org.uk (Phillip Lord)
> Cc: <rgm@gnu.org>,  <eggert@cs.ucla.edu>,  <emacs-devel@gnu.org>
> Date: Fri, 18 Dec 2015 22:05:30 +0000
> 
> The problem is this rule.
> 
> $(lisp)/loaddefs.el: $(LOADDEFS)
> 	@echo Directories for loaddefs: ${SUBDIRS_ALMOST}
> 	$(AM_V_GEN)$(emacs) -l autoload \
> 	    --eval '(setq autoload-ensure-writable t)' \
> 	    --eval '(setq autoload-builtin-package-versions t)' \
> 	    --eval '(setq generated-autoload-file (expand-file-name (unmsys--file-name "$@")))' \
> 	    -f batch-update-autoloads ${SUBDIRS_ALMOST}
> 
> So batch-update-autoloads only gets call if loaddefs.el is out-of-date.
> If reftex-loaddefs is out of date, then it does not, and so
> reftex-loaddefs will not get build.
> 
> The intuitive solution is do
> 
> $(lisp)/loaddefs.el $(lisp)/textmodes/reftex-loadefs.el $(AND_ALL_THE_OTHERS): $(LOADDEFS)
> 	@echo Directories for loaddefs: ${SUBDIRS_ALMOST}
> 	$(AM_V_GEN)$(emacs) -l autoload \
> 	    --eval '(setq autoload-ensure-writable t)' \
> 	    --eval '(setq autoload-builtin-package-versions t)' \
> 	    --eval '(setq generated-autoload-file (expand-file-name (unmsys--file-name "$@")))' \
> 	    -f batch-update-autoloads ${SUBDIRS_ALMOST}
> 
> but this fails because make actually runs batch-update-autoloads
> multiple times which is both wasteful and causes a race condition with
> -j.

Several alternative solutions for this were tried, and all of them
were found out as worse than the problem.

> As this doesn't work, I didn't do it (I tried it!). Instead, I added
> autoloads-force. It's ugly, but it's explicit and ugly rather than
> implicit and ugly.

This has never been a problem in practice, so I don't think that
additional rule is needed.

Re: reftex.el autoloads

[Phillip Lord]

Eli Zaretskii <eliz@gnu.org> writes:

>> The intuitive solution is do
>> 
>> $(lisp)/loaddefs.el $(lisp)/textmodes/reftex-loadefs.el $(AND_ALL_THE_OTHERS): $(LOADDEFS)
>> 	@echo Directories for loaddefs: ${SUBDIRS_ALMOST}
>> 	$(AM_V_GEN)$(emacs) -l autoload \
>> 	    --eval '(setq autoload-ensure-writable t)' \
>> 	    --eval '(setq autoload-builtin-package-versions t)' \
>> 	    --eval '(setq generated-autoload-file (expand-file-name (unmsys--file-name "$@")))' \
>> 	    -f batch-update-autoloads ${SUBDIRS_ALMOST}
>> 
>> but this fails because make actually runs batch-update-autoloads
>> multiple times which is both wasteful and causes a race condition with
>> -j.
>
> Several alternative solutions for this were tried, and all of them
> were found out as worse than the problem.


Indeed. It's something make just does not support well. The only
solution I have is to add makefile rules for each -loaddefs.el file
listing all dependencies. They don't change that often, so it's
possible, but I don't think it is worth it.

As I said at the beginning, personally, for the small files, I would
have placed the autoloads into loaddefs.el, and for the bigger files
(dired.el, reftex.el and ibuffer) I would remove this mechanism and
replace it with a simple require.


>> As this doesn't work, I didn't do it (I tried it!). Instead, I added
>> autoloads-force. It's ugly, but it's explicit and ugly rather than
>> implicit and ugly.
>
> This has never been a problem in practice, so I don't think that
> additional rule is needed.

I used it repeatedly to make the commits, and anyone adding new
autoloads to any of the autoloaded files would find it useful. And the
comment is more useful -- it too me a while to work out why the loaddefs
files where NOT being produced by make autoloads

But, feel free to remove it, if you wish. It's not a big deal.

Phil

Re: reftex.el autoloads

[Phillip Lord]

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 12/18/2015 08:39 AM, Glenn Morris wrote:
>> Somewhat disappointed, but obviously I'm in a minority.
>> Anyway, it doesn't build. Did you test it?
>
> Yes, I suggest reverting the change from master until it's tested and known to
> work from a fresh checkout.
>
> Here are a couple of other comments. That's not the right way to declare
> autoloads-force to be phony. Also, there shouldn't need to be an
> autoloads-force at all: if some other files need to be built they should be
> listed as dependencies of autoload.


Has anyone got any idea what might be causing this error on hydra?

building calendar/cal-loaddefs.el
  GEN      calendar/cal-loaddefs.el
building calendar/diary-loaddefs.el
  GEN      calendar/diary-loaddefs.el
Loading macroexp.elc...
cal-julian.el:0:0: error: end-of-file: nil
make[3]: *** [calendar/diary-loaddefs.el] Error 255

I can't reproduce it locally from a clean checkout. At the moment, the
only way I can test this is by leaving the broken code on master and
waiting 24 hours for a hydra build. Is there a better mechanism?

Phil

Re: reftex.el autoloads

[Eli Zaretskii]

> From: phillip.lord@russet.org.uk (Phillip Lord)
> Cc: Glenn Morris <rgm@gnu.org>,  Eli Zaretskii <eliz@gnu.org>,  <emacs-devel@gnu.org>
> Date: Sun, 20 Dec 2015 12:41:35 +0000
> 
> Has anyone got any idea what might be causing this error on hydra?
> 
> building calendar/cal-loaddefs.el
>   GEN      calendar/cal-loaddefs.el
> building calendar/diary-loaddefs.el
>   GEN      calendar/diary-loaddefs.el
> Loading macroexp.elc...
> cal-julian.el:0:0: error: end-of-file: nil
> make[3]: *** [calendar/diary-loaddefs.el] Error 255

Is this a parallel build (i.e. with -j switch to Make)?  If so, I
guess it means some file that generating diary-loaddefs.el needs is
being written to at the same time by some other part of the build
process.

Re: reftex.el autoloads

[Phillip Lord]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: phillip.lord@russet.org.uk (Phillip Lord)
>> Cc: Glenn Morris <rgm@gnu.org>, Eli Zaretskii <eliz@gnu.org>,
>> <emacs-devel@gnu.org>
>> Date: Sun, 20 Dec 2015 12:41:35 +0000
>> 
>> Has anyone got any idea what might be causing this error on hydra?
>> 
>> building calendar/cal-loaddefs.el
>>   GEN      calendar/cal-loaddefs.el
>> building calendar/diary-loaddefs.el
>>   GEN      calendar/diary-loaddefs.el
>> Loading macroexp.elc...
>> cal-julian.el:0:0: error: end-of-file: nil
>> make[3]: *** [calendar/diary-loaddefs.el] Error 255
>
> Is this a parallel build (i.e. with -j switch to Make)?  If so, I
> guess it means some file that generating diary-loaddefs.el needs is
> being written to at the same time by some other part of the build
> process.

Well, this is the hydra build and I don't know how that is configured,
but it looks like a simple "make" to me.

In my hands both "make", "make -j" and "make -j 10" (which is what I
normally use) works just fine.

I'm at a bit of a loss as to know how to fix this, since the only way I
can affect the hydra build is by commiting to master.

Phil

Re: reftex.el autoloads

[Glenn Morris]

Phillip Lord wrote:

> Has anyone got any idea what might be causing this error on hydra?
>
> building calendar/cal-loaddefs.el
>   GEN      calendar/cal-loaddefs.el
> building calendar/diary-loaddefs.el
>   GEN      calendar/diary-loaddefs.el
> Loading macroexp.elc...
> cal-julian.el:0:0: error: end-of-file: nil
> make[3]: *** [calendar/diary-loaddefs.el] Error 255

I've seen that issue twice in my own (non-parallel) daily builds (on Dec
4 and 5). I don't know what causes it. It was occurring before your
change too. But perhaps something has made it happen more frequently,
since it just happened twice in a row on hydra. There must be some bug
somewhere in autoload-generate-file-autoloads. Maybe add some debugging
code so we can see what happens.

> only way I can test this is by leaving the broken code on master and
> waiting 24 hours for a hydra build.

hydra builds aren't daily, they are whenever the system has capacity.
It checks for new commits every 5 mins (IIUC) and if there are changes
it does a build. But it is often overloaded.

Re: reftex.el autoloads

[Phillip Lord]

Glenn Morris <rgm@gnu.org> writes:

> Phillip Lord wrote:
>
>> Has anyone got any idea what might be causing this error on hydra?
>>
>> building calendar/cal-loaddefs.el
>>   GEN      calendar/cal-loaddefs.el
>> building calendar/diary-loaddefs.el
>>   GEN      calendar/diary-loaddefs.el
>> Loading macroexp.elc...
>> cal-julian.el:0:0: error: end-of-file: nil
>> make[3]: *** [calendar/diary-loaddefs.el] Error 255
>
> I've seen that issue twice in my own (non-parallel) daily builds (on Dec
> 4 and 5). I don't know what causes it. It was occurring before your
> change too. But perhaps something has made it happen more frequently,
> since it just happened twice in a row on hydra. There must be some bug
> somewhere in autoload-generate-file-autoloads. Maybe add some debugging
> code so we can see what happens.

Interesting.

>
>> only way I can test this is by leaving the broken code on master and
>> waiting 24 hours for a hydra build.
>
> hydra builds aren't daily, they are whenever the system has capacity.
> It checks for new commits every 5 mins (IIUC) and if there are changes
> it does a build. But it is often overloaded.

Is there anyway to replicate the build environment on hydra locally?

Phil

Re: reftex.el autoloads

[Glenn Morris]

Phillip Lord wrote:

> Is there anyway to replicate the build environment on hydra locally?

Sure, click the "Actions", "Reproduce locally" on
http://hydra.nixos.org/build/29216857

But there's nothing special about hydra.

IMO this is a Heisenbug that just happened to appear on hydra twice in a
row. We'll see at next evaluation! :)

Re: reftex.el autoloads

[Phillip Lord]

Glenn Morris <rgm@gnu.org> writes:

> Phillip Lord wrote:
>
>> Is there anyway to replicate the build environment on hydra locally?
>
> Sure, click the "Actions", "Reproduce locally" on
> http://hydra.nixos.org/build/29216857
>
> But there's nothing special about hydra.

It's the only place I can reproduce the problem at the moment!

> IMO this is a Heisenbug that just happened to appear on hydra twice in a
> row. We'll see at next evaluation! :)

Indeed.

Phil

Re: reftex.el autoloads

[Phillip Lord]

Glenn Morris <rgm@gnu.org> writes:

> Phillip Lord wrote:
>
>> Is there anyway to replicate the build environment on hydra locally?
>
> Sure, click the "Actions", "Reproduce locally" on
> http://hydra.nixos.org/build/29216857

I've tried this, but get...

Note: checking out 'f2bdf674b79297888f949130cb829e249c2a33f4'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b new_branch_name

HEAD is now at f2bdf67... audiofile: fix CVE-2015-7747
/dev/fd/63: input ‘tarball’ has unsupported type ‘build’



> But there's nothing special about hydra.
>
> IMO this is a Heisenbug that just happened to appear on hydra twice in a
> row. We'll see at next evaluation! :)


If as you say, hydra only builds on changes, this isn't going to happen
unless we change master, and almost no one is at the moment.

I tried cherry-picking my undo fixes across to a local copy of master
the other day, and this seemed to prevent a build crash on windows.
Unfortunately, I can't check this now as I am separated from that
machine till New Year.

Short on good ideas. All I can think of is:

1) Add a new job to hydra building a "test" branch forked from master.
2) Save the modifications I made to another branch, revert them on
master, and merge them again later (I guess this would work). 3) Add
debugging code to autoloads and commit to master. 4) Do nothing, leaving
master broken on hydra, and wait till emacs-25 fixes get merged, and see
if that fixes it (which it will if Martin is correct).

Phil

Re: reftex.el autoloads

[martin rudalics]

 > Has anyone got any idea what might be causing this error on hydra?
 >
 > building calendar/cal-loaddefs.el
 >    GEN      calendar/cal-loaddefs.el
 > building calendar/diary-loaddefs.el
 >    GEN      calendar/diary-loaddefs.el
 > Loading macroexp.elc...
 > cal-julian.el:0:0: error: end-of-file: nil
 > make[3]: *** [calendar/diary-loaddefs.el] Error 255

IMHO this is the issue first discussed in

http://lists.gnu.org/archive/html/emacs-devel/2015-11/msg01610.html

also causing bug#22012.  Master/trunk is unreliable to build and work
with until the undo-related issues get fixed as for the emacs-25 branch.

martin

Re: reftex.el autoloads

[Phillip Lord]

martin rudalics <rudalics@gmx.at> writes:

>> Has anyone got any idea what might be causing this error on hydra?
>>
>> building calendar/cal-loaddefs.el
>>    GEN      calendar/cal-loaddefs.el
>> building calendar/diary-loaddefs.el
>>    GEN      calendar/diary-loaddefs.el
>> Loading macroexp.elc...
>> cal-julian.el:0:0: error: end-of-file: nil
>> make[3]: *** [calendar/diary-loaddefs.el] Error 255
>
> IMHO this is the issue first discussed in
>
> http://lists.gnu.org/archive/html/emacs-devel/2015-11/msg01610.html
>
> also causing bug#22012.  Master/trunk is unreliable to build and work
> with until the undo-related issues get fixed as for the emacs-25 branch.


Ah, which idiot did that. Alas! I fear it was me!

Phil

Re: reftex.el autoloads

[Phillip Lord]

Glenn Morris <rgm@gnu.org> writes:

> Somewhat disappointed, but obviously I'm in a minority.
> Anyway, it doesn't build. Did you test it?
>
> http://hydra.nixos.org/build/29118180

Evidentally not well enough, apologies. I tested reftex from bootstrap,
but obviously forgot to do the last one.

Phil

Re: reftex.el autoloads

[Phillip Lord]

Glenn Morris <rgm@gnu.org> writes:

> Somewhat disappointed, but obviously I'm in a minority.
> Anyway, it doesn't build. Did you test it?
>
> http://hydra.nixos.org/build/29118180


I believe it should be functional now, at least it builds on my machine
from clean, both with make and make -j.

Apologies for the breakage.

Phil

Re: reftex.el autoloads

[Glenn Morris]

What's special about dired here?
Ie, could the same thing happen with other files?

Re: reftex.el autoloads

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  <eggert@cs.ucla.edu>,  <emacs-devel@gnu.org>
> Date: Sun, 20 Dec 2015 13:41:11 -0500
> 
> 
> What's special about dired here?

You mean, diary, right?

I don't know.  One idea is to instrument the place that reports the
error to display more info, ideally a full backtrace, so that we could
figure out what exactly goes on there.

Re: reftex.el autoloads

[Glenn Morris]

Eli Zaretskii wrote:

>> What's special about dired here?
>
> You mean, diary, right?

No, I'm asking why dired needs special treatment:

http://lists.gnu.org/archive/html/emacs-diffs/2015-12/msg00185.html

Re: reftex.el autoloads

[Phillip Lord]

Glenn Morris <rgm@gnu.org> writes:

> What's special about dired here?

I'm working on the assumption that the problems I had with dired.el have
been fixed and where a straight-forward bootstrap problem. This could be
a mistaken assumption.


> Ie, could the same thing happen with other files?

What is special is that the diary (and all the calendar) files have four
independent autoload cookies. I don't know enough about calendar to know
why it needs such a complex set of autoloads, although I do wonder
whether it still does.

I guess the best I can do at the moment is change

EMACSOPT = -batch --no-site-file --no-site-lisp

to

EMACSOPT = -batch --no-site-file --no-site-lisp --debug

Is it possible to get hydra to build another branch (called test or
something)? In the ideal world, I'd rather not do this sort of mucking
around on master.

And, in an entirely unrelated question, why is that "-batch" and not
"--batch"?

Phil

Re: reftex.el autoloads

[Glenn Morris]

Phillip Lord wrote:

>> What's special about dired here?
>
> I'm working on the assumption that the problems I had with dired.el have
> been fixed and where a straight-forward bootstrap problem.

I don't know what that means.

>> Ie, could the same thing happen with other files?
>
> What is special is that the diary (and all the calendar) files have four

My question was about dired. Why does dired need special handling?

> I don't know enough about calendar to know why it needs such a complex
> set of autoloads, although I do wonder whether it still does.

Nothing has changed, so yes it does. It's worked fine for years.
Though when I tried to say this earlier in this discussion, I didn't succeed.

> I guess the best I can do at the moment is change
>
> EMACSOPT = -batch --no-site-file --no-site-lisp
>
> to
>
> EMACSOPT = -batch --no-site-file --no-site-lisp --debug

No, just make the autoload-generate-file-autoloads condition-case handler
print some debug info when it encounters end-of-file error.

> Is it possible to get hydra to build another branch (called test or
> something)? In the ideal world, I'd rather not do this sort of mucking
> around on master.

hydra isn't a good fit for this kind of testing.

> And, in an entirely unrelated question, why is that "-batch" and not
> "--batch"?

They are equivalent.

Re: reftex.el autoloads

[Phillip Lord]

Glenn Morris <rgm@gnu.org> writes:

> Phillip Lord wrote:
>
>>> What's special about dired here?
>>
>> I'm working on the assumption that the problems I had with dired.el have
>> been fixed and where a straight-forward bootstrap problem.
>
> I don't know what that means.

Oh, that dired.el was used somewhere during the bootstrap build before
dired-loaddefs.el is generated.

cl-lib.el has the same problem. It deals with it like so:

(unless (load "cl-loaddefs" 'noerror 'quiet)
  ;; When bootstrapping, cl-loaddefs hasn't been built yet!
  (require 'cl-macs)
  (require 'cl-seq))

I took a punt and guessed that the dired autoloaded functions are not
needed during bootstrap so used the simpler approach.


>>> Ie, could the same thing happen with other files?
>>
>> What is special is that the diary (and all the calendar) files have four
>
> My question was about dired. Why does dired need special handling?

AFAICT, it's because of vc.el which requires dired.el. This is needed by
find-file-noselect to open loaddefs.el.


>> I don't know enough about calendar to know why it needs such a complex
>> set of autoloads, although I do wonder whether it still does.
>
> Nothing has changed, so yes it does. It's worked fine for years.
> Though when I tried to say this earlier in this discussion, I didn't succeed.

You did succeed. I did listen, and did take the issue seriously. At the
same time, I felt that there was a build irritation that was worth
removing. If no one else had agreed, I probably would not have touched
it, out of respect for your concerns.

>> I guess the best I can do at the moment is change
>>
>> EMACSOPT = -batch --no-site-file --no-site-lisp
>>
>> to
>>
>> EMACSOPT = -batch --no-site-file --no-site-lisp --debug
>
> No, just make the autoload-generate-file-autoloads condition-case handler
> print some debug info when it encounters end-of-file error.
>
>> Is it possible to get hydra to build another branch (called test or
>> something)? In the ideal world, I'd rather not do this sort of mucking
>> around on master.
>
> hydra isn't a good fit for this kind of testing.

I am stuck reproducing the bug elsewhere.

>> And, in an entirely unrelated question, why is that "-batch" and not
>> "--batch"?
>
> They are equivalent.

Ok.

Re: reftex.el autoloads

[Paul Eggert]

On 12/16/2015 06:15 PM, Glenn Morris wrote:
> So is everyone ok with this decision to ban versioned autoloads from
> Emacs? They've been there for years, causing no problem IMO.

I'd rather see them gone, yes. They're only a minor annoyance for me, 
but every little bit helps.

Re: reftex.el autoloads

[Phillip Lord]

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 12/16/2015 06:15 PM, Glenn Morris wrote:
>> So is everyone ok with this decision to ban versioned autoloads from
>> Emacs? They've been there for years, causing no problem IMO.
>
> I'd rather see them gone, yes. They're only a minor annoyance for me, but
> every little bit helps.


Okay. Am working through them now. I'll merge them all once I've done
the lost.

I'm leaving the one in obsolete -- it's autogenerated anymore (since the
make file excludes obsolete), and I see no particular reason to change
that.

Phil

Re: reftex.el autoloads

[David Engster]

Eli Zaretskii writes:
>> From: phillip.lord@russet.org.uk (Phillip Lord)
>> Cc: <emacs-devel@gnu.org>
>> Date: Thu, 10 Dec 2015 22:03:32 +0000
>> 
>> So, quite a lot of files seem to set generated-autoload-file to
>> loaddefs.el which seems a bit odd (since it's the default).
>
> Only in lisp/cedet/, I think.  I'm guessing this is the result of
> integrating CEDET.  If you want the details, maybe the CEDET people
> and/or Chong (which did most of the work) could tell you the reasons
> for that.  I suggest to look at Git logs and maybe ask them Or maybe
> Glenn (CC'ed) remembers.

If you don't set generated-autoload-file, your autoloads end up in the
top-level loaddefs.el. The CEDET packages have their own loaddefs.el
files underneath lisp/cedet:

lisp/cedet/srecode/loaddefs.el
lisp/cedet/semantic/loaddefs.el
lisp/cedet/ede/loaddefs.el

The reason for this is that those loaddefs.el autoload EIEIO classes
with eieio-defclass-autoload. If we'd have those in the toplevel
loaddefs.el, this would trigger loading EIEIO at startup.

-David

Re: reftex.el autoloads

[Glenn Morris]

Eli Zaretskii wrote:

> Only in lisp/cedet/, I think.  I'm guessing this is the result of
> integrating CEDET.  If you want the details, maybe the CEDET people
> and/or Chong (which did most of the work) could tell you the reasons
> for that.  I suggest to look at Git logs and maybe ask them Or maybe
> Glenn (CC'ed) remembers.

Because there are separate loaddefs files within the cedet directory.

./ede/loaddefs.el
./srecode/loaddefs.el
./semantic/loaddefs.el

The autoloads go there rather than in lisp/loaddefs.el