bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Parts of 59732a83c8 (removing DOC file scanning for .el files) were
reverted because of loaddefs.el handling (see bug#52969).  Stefan M
suggested the possibility of re-reverting that, but that would mean that
loaddefs.el would have to be byte-compiled, and it's unclear whether
that would lead to problems or not.

So somebody should try that out, and then if that works, revert
460f35e96df.


In GNU Emacs 29.0.50 (build 13, x86_64-pc-linux-gnu, GTK+ Version 3.24.31, cairo version 1.16.0)
 of 2022-01-04 built on xo
Repository revision: c899d9742a3dee2069eb3a4ee9380833b5574a95
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12013000
System Description: Debian GNU/Linux bookworm/sid


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Parts of 59732a83c8 (removing DOC file scanning for .el files) were
> reverted because of loaddefs.el handling (see bug#52969).  Stefan M
> suggested the possibility of re-reverting that, but that would mean that
> loaddefs.el would have to be byte-compiled, and it's unclear whether
> that would lead to problems or not.

Hm...  the commment in autoload.el is:

            ";; no-byte-compile: t\n" ;; #$ is byte-compiled into nil.

commit 1f7b602f843d82d2da46528c6cc2a16c794ca668
Author:     Noam Postavsky <npostavs@gmail.com>
AuthorDate: Fri Jul 5 22:26:13 2019 -0400

    Add commentary about #$ in autoload files
    
    * lisp/emacs-lisp/autoload.el (autoload-rubric): Add commentary about
    `#$' trick.

Oh, it refers to this:

+                ;; Use the `#$' to indicate current file, from which
+                ;; we extract the directory name.  Note that
+                ;; `package-quickstart-refresh' specifically replaces
+                ;; `#$', so any other tricks (e.g., `load-file-name')
+                ;; will not work for that case.
 		"(add-to-list 'load-path (directory-file-name
                          (or (file-name-directory #$) (car load-path))))\n\n")

Hm.  Anyway, when if I remove the no-byte-compile, then the build does
indeed break:

In toplevel form:
net/tramp-rclone.el:38:2: Error: Symbol’s value as variable is void: tramp-methods
make[3]: *** [Makefile:327: net/tramp-rclone.elc] Error 1
make[3]: *** Waiting for unfinished jobs....

In toplevel form:
net/tramp-sh.el:35:2: Error: Symbol’s value as variable is void: tramp-methods
make[3]: *** [Makefile:327: net/tramp-sh.elc] Error 1

In toplevel form:
net/tramp-smb.el:31:2: Error: Symbol’s value as variable is void: tramp-methods
make[3]: *** [Makefile:327: net/tramp-smb.elc] Error 1

In toplevel form:
net/tramp-sshfs.el:36:2: Error: Symbol’s value as variable is void: tramp-methods
make[3]: *** [Makefile:327: net/tramp-sshfs.elc] Error 1

In toplevel form:
net/tramp-sudoedit.el:37:2: Error: Symbol’s value as variable is void: tramp-methods

This is the simple patch I applied to test.

diff --git a/lisp/emacs-lisp/autoload.el b/lisp/emacs-lisp/autoload.el
index d0bf342b84..58df936dd9 100644
--- a/lisp/emacs-lisp/autoload.el
+++ b/lisp/emacs-lisp/autoload.el
@@ -408,7 +408,6 @@ autoload-rubric
 			  (file-name-sans-extension basename))))
 	    ";; Local Variables:\n"
 	    ";; version-control: never\n"
-            ";; no-byte-compile: t\n" ;; #$ is byte-compiled into nil.
 	    ";; no-update-autoloads: t\n"
 	    ";; coding: utf-8-emacs-unix\n"
 	    ";; End:\n"

I haven't tried debugging further, but it looks like it'll take some
work to get this er working.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> In toplevel form:
> net/tramp-sshfs.el:36:2: Error: Symbol’s value as variable is void: tramp-methods
> make[3]: *** [Makefile:327: net/tramp-sshfs.elc] Error 1
>
> In toplevel form:
> net/tramp-sudoedit.el:37:2: Error: Symbol’s value as variable is void: tramp-methods

(etc)

I've now fixed all the compilation warnings when byte-compiling the
in-tree loaddefs files.

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I've now fixed all the compilation warnings when byte-compiling the
> in-tree loaddefs files.

So now we just need a way to direct `documentation' to look into the
correct loaddefs.elc file for the doc string.  But... do we know which
loaddefs file the `autoload' form comes from?

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> So now we just need a way to direct `documentation' to look into the
> correct loaddefs.elc file for the doc string.  But... do we know which
> loaddefs file the `autoload' form comes from?

Never mind -- it was just because I'd forgotten to adjust loadup.el to
actually load the .elc file.

So this seems to work well now, as far as I can see, so we can indeed
byte-compile the loaddefs files in the Emacs tree.  But not the ones
from packages because of the #$ problem.

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

With the following, Emacs builds successfully.  The problem is that we
need to generate the loaddefs files before compile-main (so that it
picks up the .el files for compilation), but we do this after we've
built Emacs, so a change that updates loaddefs.el will first compile the
.elc file, and then the next "make" will rebuild src/emacs, which is
backwards.

But I'm unable to follow the logic completely in the relevant Makefiles.
I've tried a number of things here, but I seem to be unable to make the
src/Makefile depend on the lisp/loaddefs.elc file in any sensible way --
I either end up with generating the .elc file twice or none at all,
which is frustrating.

diff --git a/lisp/Makefile.in b/lisp/Makefile.in
index 9516f2fc36..8f08195df8 100644
--- a/lisp/Makefile.in
+++ b/lisp/Makefile.in
@@ -196,13 +196,16 @@ org-manuals:
 # from ../src rules, but that doesn't seem possible due to the various
 # non-trivial dependencies.
 
-# We make $(lisp)/loaddefs.el a dependency of .PHONY to cause Make to
+# We make autoloads a dependency of .PHONY to cause Make to
 # ignore its time stamp.  That's because the real dependencies of
 # loaddefs.el aren't known to Make, they are implemented in
 # loaddefs-generate--emacs-batch.
+autoloads .PHONY:
+	$(AM_V_GEN)$(emacs) \
+            -l $(lisp)/emacs-lisp/loaddefs-gen.elc \
+	    -f loaddefs-generate--emacs-batch ${SUBDIRS_ALMOST}
 
-autoloads .PHONY: $(lisp)/loaddefs.el
-$(lisp)/loaddefs.el: gen-lisp $(LOADDEFS) $(lisp)/emacs-lisp/loaddefs-gen.elc
+$(lisp)/loaddefs.el: $(LOADDEFS) $(lisp)/emacs-lisp/loaddefs-gen.elc
 	$(AM_V_GEN)$(emacs) \
             -l $(lisp)/emacs-lisp/loaddefs-gen.elc \
 	    -f loaddefs-generate--emacs-batch ${SUBDIRS_ALMOST}
diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
index 261e44aece..830799ec36 100644
--- a/lisp/emacs-lisp/loaddefs-gen.el
+++ b/lisp/emacs-lisp/loaddefs-gen.el
@@ -459,7 +459,7 @@ loaddefs-generate--compute-prefixes
             (push name prefs)))))
     (loaddefs-generate--make-prefixes prefs load-name)))
 
-(defun loaddefs-generate--rubric (file &optional type feature)
+(defun loaddefs-generate--rubric (file &optional type feature compile)
   "Return a string giving the appropriate autoload rubric for FILE.
 TYPE (default \"autoloads\") is a string stating the type of
 information contained in FILE.  TYPE \"package\" acts like the default,
@@ -467,7 +467,9 @@ loaddefs-generate--rubric
 
 If FEATURE is non-nil, FILE will provide a feature.  FEATURE may
 be a string naming the feature, otherwise it will be based on
-FILE's name."
+FILE's name.
+
+If COMPILE, don't include a \"don't compile\" cookie."
   (let ((lp (and (equal type "package") (setq type "autoloads"))))
     (with-temp-buffer
       (generate-lisp-file-heading
@@ -481,6 +483,7 @@ loaddefs-generate--rubric
       (insert "\f\n;;; End of scraped data\n\n")
       (generate-lisp-file-trailer
        file :provide (and (stringp feature) feature)
+       :compile compile
        :inhibit-provide (not feature))
       (buffer-string))))
 
@@ -585,7 +588,8 @@ loaddefs-generate
           (with-temp-buffer
             (if (and updating (file-exists-p loaddefs-file))
                 (insert-file-contents loaddefs-file)
-              (insert (loaddefs-generate--rubric loaddefs-file nil t))
+              (insert (loaddefs-generate--rubric
+                       loaddefs-file nil t include-package-version))
               (search-backward "\f")
               (when extra-data
                 (insert extra-data)
diff --git a/lisp/loadup.el b/lisp/loadup.el
index 21a87dbd77..a65c1724ae 100644
--- a/lisp/loadup.el
+++ b/lisp/loadup.el
@@ -185,9 +185,10 @@
 ;; should be updated by overwriting it with an up-to-date copy of
 ;; loaddefs.el that is not corrupted by local changes.
 ;; admin/update_autogen can be used to update ldefs-boot.el periodically.
-(condition-case nil (load "loaddefs.el")
-  ;; In case loaddefs hasn't been generated yet.
-  (file-error (load "ldefs-boot.el")))
+(condition-case nil
+    (load "loaddefs")
+  (file-error
+   (load "ldefs-boot.el")))
 
 (let ((new (make-hash-table :test #'equal)))
   ;; Now that loaddefs has populated definition-prefixes, purify its contents.
diff --git a/src/Makefile.in b/src/Makefile.in
index 7d15b7afd5..feb809ea10 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -652,13 +652,11 @@ $(pdmp):
 ## for the first time, this prevents any variation between configurations
 ## in the contents of the DOC file.
 ##
-$(etc)/DOC: $(libsrc)/make-docfile$(EXEEXT) $(doc_obj) $(lispsource)/loaddefs.el
+$(etc)/DOC: $(libsrc)/make-docfile$(EXEEXT) $(doc_obj)
 	$(AM_V_GEN)$(MKDIR_P) $(etc)
 	$(AM_V_at)rm -f $(etc)/DOC
 	$(AM_V_at)$(libsrc)/make-docfile -d $(srcdir) \
 	  $(SOME_MACHINE_OBJECTS) $(doc_obj) > $(etc)/DOC
-	$(AM_V_at)$(libsrc)/make-docfile -a $(etc)/DOC -d $(lispsource) \
-	  loaddefs.el
 
 $(libsrc)/make-docfile$(EXEEXT) $(libsrc)/make-fingerprint$(EXEEXT): \
   $(lib)/libgnu.a

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Andreas Schwab]

On Jul 31 2022, Lars Ingebrigtsen wrote:

> +autoloads .PHONY:

If you want to create a phony target you need to make that a dependency
of .PHONY.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Andreas Schwab <schwab@linux-m68k.org> writes:

> If you want to create a phony target you need to make that a dependency
> of .PHONY.

The problem turned out to be that pinyin.el was generated after Emacs
had been built, which triggered a change in loaddefs.el, which then
triggered a rebuilt of the emacs binary.  *phew*

This is now fixed, and it seems like I've now got it to not do any
excessive rebuilds, while still triggering rebuilds consistently.

Anybody got any comments before pushing this?  (We can then remove the
"\\n" hack in the reader, I think.)

diff --git a/lisp/Makefile.in b/lisp/Makefile.in
index 9516f2fc36..315b1fcf7b 100644
--- a/lisp/Makefile.in
+++ b/lisp/Makefile.in
@@ -123,10 +123,10 @@ SUBDIRS_FINDER =
 ## All subdirectories in which we might want to create subdirs.el.
 SUBDIRS_SUBDIRS = $(filter-out ${srcdir}/cedet% ${srcdir}/leim%,${SUBDIRS})
 
-# cus-load and finder-inf are not explicitly requested by anything, so
-# we add them here to make sure they get built.
+# cus-load, finder-inf and autoloads are not explicitly requested by
+# anything, so we add them here to make sure they get built.
 all: compile-main $(lisp)/cus-load.el $(lisp)/finder-inf.el generate-ja-dic \
-	org-manuals
+	org-manuals autoloads
 
 PHONY_EXTRAS =
 .PHONY: all custom-deps finder-data autoloads update-subdirs $(PHONY_EXTRAS) \
@@ -196,13 +196,10 @@ org-manuals:
 # from ../src rules, but that doesn't seem possible due to the various
 # non-trivial dependencies.
 
-# We make $(lisp)/loaddefs.el a dependency of .PHONY to cause Make to
-# ignore its time stamp.  That's because the real dependencies of
-# loaddefs.el aren't known to Make, they are implemented in
-# loaddefs-generate--emacs-batch.
-
-autoloads .PHONY: $(lisp)/loaddefs.el
-$(lisp)/loaddefs.el: gen-lisp $(LOADDEFS) $(lisp)/emacs-lisp/loaddefs-gen.elc
+# That's because the real dependencies of loaddefs.el aren't known to
+# Make, they are implemented in loaddefs-generate--emacs-batch, so
+# autoloads is an "all" dependency.
+autoloads:
 	$(AM_V_GEN)$(emacs) \
             -l $(lisp)/emacs-lisp/loaddefs-gen.elc \
 	    -f loaddefs-generate--emacs-batch ${SUBDIRS_ALMOST}
diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
index 36b0b1e9cd..830799ec36 100644
--- a/lisp/emacs-lisp/loaddefs-gen.el
+++ b/lisp/emacs-lisp/loaddefs-gen.el
@@ -588,7 +588,8 @@ loaddefs-generate
           (with-temp-buffer
             (if (and updating (file-exists-p loaddefs-file))
                 (insert-file-contents loaddefs-file)
-              (insert (loaddefs-generate--rubric loaddefs-file nil t))
+              (insert (loaddefs-generate--rubric
+                       loaddefs-file nil t include-package-version))
               (search-backward "\f")
               (when extra-data
                 (insert extra-data)
diff --git a/lisp/loadup.el b/lisp/loadup.el
index 21a87dbd77..a65c1724ae 100644
--- a/lisp/loadup.el
+++ b/lisp/loadup.el
@@ -185,9 +185,10 @@
 ;; should be updated by overwriting it with an up-to-date copy of
 ;; loaddefs.el that is not corrupted by local changes.
 ;; admin/update_autogen can be used to update ldefs-boot.el periodically.
-(condition-case nil (load "loaddefs.el")
-  ;; In case loaddefs hasn't been generated yet.
-  (file-error (load "ldefs-boot.el")))
+(condition-case nil
+    (load "loaddefs")
+  (file-error
+   (load "ldefs-boot.el")))
 
 (let ((new (make-hash-table :test #'equal)))
   ;; Now that loaddefs has populated definition-prefixes, purify its contents.
diff --git a/src/Makefile.in b/src/Makefile.in
index 7d15b7afd5..4a20176bdc 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -635,7 +635,8 @@ Emacs.pdmp:
 endif
 
 ifeq ($(DUMPING),pdumper)
-$(pdmp): emacs$(EXEEXT)
+$(pdmp): emacs$(EXEEXT) $(lispsource)/loaddefs.elc
+	echo "DUMPING"
 	LC_ALL=C $(RUN_TEMACS) -batch $(BUILD_DETAILS) -l loadup --temacs=pdump \
 		--bin-dest $(BIN_DESTDIR) --eln-dest $(ELN_DESTDIR)
 	cp -f $@ $(bootstrap_pdmp)
@@ -652,13 +653,11 @@ $(pdmp):
 ## for the first time, this prevents any variation between configurations
 ## in the contents of the DOC file.
 ##
-$(etc)/DOC: $(libsrc)/make-docfile$(EXEEXT) $(doc_obj) $(lispsource)/loaddefs.el
+$(etc)/DOC: $(libsrc)/make-docfile$(EXEEXT) $(doc_obj)
 	$(AM_V_GEN)$(MKDIR_P) $(etc)
 	$(AM_V_at)rm -f $(etc)/DOC
 	$(AM_V_at)$(libsrc)/make-docfile -d $(srcdir) \
 	  $(SOME_MACHINE_OBJECTS) $(doc_obj) > $(etc)/DOC
-	$(AM_V_at)$(libsrc)/make-docfile -a $(etc)/DOC -d $(lispsource) \
-	  loaddefs.el
 
 $(libsrc)/make-docfile$(EXEEXT) $(libsrc)/make-fingerprint$(EXEEXT): \
   $(lib)/libgnu.a

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Eli Zaretskii]

> Cc: 53024@debbugs.gnu.org, monnier@iro.umontreal.ca
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Mon, 01 Aug 2022 14:33:39 +0200
> 
> Andreas Schwab <schwab@linux-m68k.org> writes:
> 
> > If you want to create a phony target you need to make that a dependency
> > of .PHONY.
> 
> The problem turned out to be that pinyin.el was generated after Emacs
> had been built, which triggered a change in loaddefs.el, which then
> triggered a rebuilt of the emacs binary.  *phew*
> 
> This is now fixed, and it seems like I've now got it to not do any
> excessive rebuilds, while still triggering rebuilds consistently.

Thanks, but where are those subtle points documented?  We need them
documented so that we don't break this again in the future.

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, but where are those subtle points documented?  We need them
> documented so that we don't break this again in the future.

It was just a bug, so I'm not sure where you'd document something like
that.

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: schwab@linux-m68k.org,  53024@debbugs.gnu.org,  monnier@iro.umontreal.ca
> Date: Tue, 02 Aug 2022 11:52:51 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Thanks, but where are those subtle points documented?  We need them
> > documented so that we don't break this again in the future.
> 
> It was just a bug, so I'm not sure where you'd document something like
> that.

Your fix for that problem, AFAIU, was to replace a simple literal
'insert' with something else, and I thought the considerations for
doing one and not the other should be spelled out somewhere.

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Your fix for that problem, AFAIU, was to replace a simple literal
> 'insert' with something else, and I thought the considerations for
> doing one and not the other should be spelled out somewhere.

It was an auto-generated file that wasn't supposed to be scanned by
loaddefs, but it was, because it lacked the no-autoloads cookie.

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

I've now pushed this to master.  I've tested this with and without
native-comp, and with and without bootstrapping, and it seems to work
without triggering excessive re-compilation, but those makefile
dependencies are pretty complicated, and I may well have messed
something up.

So please do feel free to apply further fixes if you see something
wonky.

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I've now pushed this to master.

By the way, there's bits and pieces (like this in bytecomp.el) that's no
longer relevant:

  ;; Write the given form to the output buffer, being careful of docstrings
  ;; in defvar, defvaralias, defconst, autoload and
  ;; custom-declare-variable because make-docfile is so amazingly stupid.

[...]

               ;; Make the doc string start at beginning of line
               ;; for make-docfile's sake.
               (insert "\n")

So I guess I'll be going over this and adjusting to the current state of
affairs.

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> By the way, there's bits and pieces (like this in bytecomp.el) that's no
> longer relevant:
>
>   ;; Write the given form to the output buffer, being careful of docstrings
>   ;; in defvar, defvaralias, defconst, autoload and
>   ;; custom-declare-variable because make-docfile is so amazingly stupid.
>
> [...]
>
>                ;; Make the doc string start at beginning of line
>                ;; for make-docfile's sake.
>                (insert "\n")

Actually, I have no idea what it's talking about here -- isn't this
about generating .elc files?  make-docfile didn't scan .elc files, did
it?

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>                ;; Make the doc string start at beginning of line
>>                ;; for make-docfile's sake.
>>                (insert "\n")
>
> Actually, I have no idea what it's talking about here -- isn't this
> about generating .elc files?  make-docfile didn't scan .elc files, did
> it?

Yes, it did (in Emacs<29).


        Stefan

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>>                ;; Make the doc string start at beginning of line
>>>                ;; for make-docfile's sake.
>>>                (insert "\n")
>>
>> Actually, I have no idea what it's talking about here -- isn't this
>> about generating .elc files?  make-docfile didn't scan .elc files, did
>> it?
>
> Yes, it did (in Emacs<29).

Ah, OK.  Is there anything in bytecomp.el that should be adjusted now
that that's no longer the case?  I don't quite grok the code there, so
I'm not the person to adjust this.  If not, the commentary should be
changed.

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen [2022-08-05 13:46:11] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>>>                ;; Make the doc string start at beginning of line
>>>>                ;; for make-docfile's sake.
>>>>                (insert "\n")
>>>
>>> Actually, I have no idea what it's talking about here -- isn't this
>>> about generating .elc files?  make-docfile didn't scan .elc files, did
>>> it?
>>
>> Yes, it did (in Emacs<29).
> Ah, OK.  Is there anything in bytecomp.el that should be adjusted now
> that that's no longer the case?

I just removed the (insert "\n") since it's not needed any more, thanks.


        Stefan

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I just removed the (insert "\n") since it's not needed any more, thanks.

Thanks.  What about this bit?

;; Functions and variables with doc strings must be output separately,
;; so make-docfile can recognize them.  Most other things can be output
;; as byte-code.

(put 'autoload 'byte-hunk-handler 'byte-compile-file-form-autoload)
(defun byte-compile-file-form-autoload (form)

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> ;; Functions and variables with doc strings must be output separately,
> ;; so make-docfile can recognize them.  Most other things can be output
> ;; as byte-code.
>
> (put 'autoload 'byte-hunk-handler 'byte-compile-file-form-autoload)
> (defun byte-compile-file-form-autoload (form)

The comment is a bit off, indeed, but the code can't really be
simplified because the docstrings are still treated specially (to
implement `byte-compile-dynamic-docstrings`).

IIUC the comment predates `byte-compile-dynamic-docstrings`, which is
why they only mention `make-docfile`.  I've just updated the comments.

Admittedly, I think there are a few simplifications/improvements we
could now make in `bytecomp.el` (as mentioned in one comment), but the
benefit would be fairly minor, and it requires more significant changes
than just removing some unneeded code.


        Stefan

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> IIUC the comment predates `byte-compile-dynamic-docstrings`, which is
> why they only mention `make-docfile`.  I've just updated the comments.

Thanks.

> Admittedly, I think there are a few simplifications/improvements we
> could now make in `bytecomp.el` (as mentioned in one comment), but the
> benefit would be fairly minor, and it requires more significant changes
> than just removing some unneeded code.

Right.

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Andreas Schwab]

This appears to break bootstrap for me.

https://build.opensuse.org/package/live_build_log/home:AndreasSchwab:emacs:master/emacs/f/x86_64

./temacs --batch  -l loadup --temacs=pbootstrap \
        --bin-dest /usr/bin/ --eln-dest /usr/lib/emacs/29.0.50/
Loading loadup.el (source)...
Dump mode: pbootstrap
Using load-path (/home/abuild/rpmbuild/BUILD/emacs-29.0.50/lisp /home/abuild/rpm
build/BUILD/emacs-29.0.50/lisp/emacs-lisp /home/abuild/rpmbuild/BUILD/emacs-29.0
.50/lisp/progmodes /home/abuild/rpmbuild/BUILD/emacs-29.0.50/lisp/language /home
/abuild/rpmbuild/BUILD/emacs-29.0.50/lisp/international /home/abuild/rpmbuild/BUILD/emacs-29.0.50/lisp/textmodes /home/abuild/rpmbuild/BUILD/emacs-29.0.50/lisp/vc)
Loading emacs-lisp/debug-early...
Loading emacs-lisp/byte-run...
Loading emacs-lisp/backquote...
Loading subr...
Loading keymap...
Loading version...
Loading widget...
Loading custom...
Loading emacs-lisp/map-ynp...
Loading international/mule...
Loading international/mule-conf...
Loading env...
Loading format...
Loading bindings...
Loading window...
Loading files...
Loading emacs-lisp/macroexp...
Loading cus-face...
Loading faces...
Symbol's function definition is void: display-warning

Error: void-function (display-warning)
  mapbacktrace(#[1028 "\1\4\203\24\0\301\302!\210\300\4!\210\301\303!\210\202\35\0\301\304!\210\3\3B\262\1\211\2035\0\300\1@!\210\211A\211\262\2\2035\0\301\305!\210\202!\0\301\306!\207" [prin1 princ "  " "(" "  (" " " ")\n"] 7 "\n\n(fn EVALD FUNC ARGS FLAGS)"])
  debug-early-backtrace()
  debug-early(error (void-function display-warning))
  display-warning(comp "Cannot look up eln file as no source file was found for /home/abuild/rpmbuild/BUILD/emacs-29.0.50/lisp/loaddefs.elc")
  load("loaddefs")
  (condition-case nil (load "loaddefs") (file-error (load "ldefs-boot.el")))
  load("loadup.el")
make[1]: *** [Makefile:922: bootstrap-emacs.pdmp] Error 255


-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Andreas Schwab <schwab@linux-m68k.org> writes:

> Error: void-function (display-warning)
>   mapbacktrace(#[1028 "\1\4\203\24\0\301\302!\210\300\4!\210\301\303!\210\202\35\0\301\304!\210\3\3B\262\1\211\2035\0\300\1@!\210\211A\211\262\2\2035\0\301\305!\210\202!\0\301\306!\207" [prin1 princ "  " "(" "  (" " " ")\n"] 7 "\n\n(fn EVALD FUNC ARGS FLAGS)"])
>   debug-early-backtrace()
>   debug-early(error (void-function display-warning))
>   display-warning(comp "Cannot look up eln file as no source file was found for /home/abuild/rpmbuild/BUILD/emacs-29.0.50/lisp/loaddefs.elc")
>   load("loaddefs")
>   (condition-case nil (load "loaddefs") (file-error (load "ldefs-boot.el")))
>   load("loadup.el")

I'm unable to reproduce the problem.

With

./configure --with-native-compilation

and

make -j32 bootstrap

on the current trunk, I get no build failures.  Are there some
additional configuration settings necessary to trigger the problem?

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: 53024@debbugs.gnu.org,  'Eli Zaretskii' <eliz@gnu.org>,
>   monnier@iro.umontreal.ca
> Date: Thu, 04 Aug 2022 18:31:16 +0200
> 
> Error: void-function (display-warning)
>   mapbacktrace(#[1028 "\1\4\203\24\0\301\302!\210\300\4!\210\301\303!\210\202\35\0\301\304!\210\3\3B\262\1\211\2035\0\300\1@!\210\211A\211\262\2\2035\0\301\305!\210\202!\0\301\306!\207" [prin1 princ "  " "(" "  (" " " ")\n"] 7 "\n\n(fn EVALD FUNC ARGS FLAGS)"])
>   debug-early-backtrace()
>   debug-early(error (void-function display-warning))
>   display-warning(comp "Cannot look up eln file as no source file was found for /home/abuild/rpmbuild/BUILD/emacs-29.0.50/lisp/loaddefs.elc")
>   load("loaddefs")

Is this consistent?  That is, if you repeat the "make" command after
the failure, does it succeed to continue past this point?  If it does
not, is it true that lisp/loaddefs.el is not there?

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>>   debug-early-backtrace()
>>   debug-early(error (void-function display-warning))
>>   display-warning(comp "Cannot look up eln file as no source file
>> was found for
>> /home/abuild/rpmbuild/BUILD/emacs-29.0.50/lisp/loaddefs.elc")
>>   load("loaddefs")
>
> Is this consistent?  That is, if you repeat the "make" command after
> the failure, does it succeed to continue past this point?  If it does
> not, is it true that lisp/loaddefs.el is not there?

Perhaps this is a bug in the RPM build script.  That is, now that
loaddefs files are compiled, it only copies over the .elc files to
rpmbuild/BUILD, and not the .el files, or something along those lines?

In any case, perhaps we should blacklist the loaddefs files from being
native-compiled?  It doesn't make much sense to native-compile them, I
think.  But I don't know whether we have any machinery to do that...

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Andreas Schwab <schwab@linux-m68k.org>,  53024@debbugs.gnu.org,
>   monnier@iro.umontreal.ca
> Date: Thu, 04 Aug 2022 19:14:57 +0200
> 
> In any case, perhaps we should blacklist the loaddefs files from being
> native-compiled?  It doesn't make much sense to native-compile them, I
> think.  But I don't know whether we have any machinery to do that...

We do: no-native-compile.

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> We do: no-native-compile.

Right.

The issue here was apparently that distclean didn't remove the .elc
file (and that Andreas was running distclean instead of bootstrap?), but
just removed the .el file, and that made the nativecomp machinery
complain.

I'm not sure whether we should do anything about this -- not
introducing a new rule here, but keeping things simple seems like the
best solution in general.

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: schwab@linux-m68k.org,  53024@debbugs.gnu.org,  monnier@iro.umontreal.ca
> Date: Fri, 05 Aug 2022 13:44:14 +0200
> 
> The issue here was apparently that distclean didn't remove the .elc
> file (and that Andreas was running distclean instead of bootstrap?), but
> just removed the .el file, and that made the nativecomp machinery
> complain.

Doesn't distclean remove the *.eln files as well?  It should, IMO:
these files will never exist in a release tarball.

If the *.eln files were removed by distclean, then how come nativecomp
machinery at all kicked in?

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Doesn't distclean remove the *.eln files as well?  It should, IMO:
> these files will never exist in a release tarball.
>
> If the *.eln files were removed by distclean, then how come nativecomp
> machinery at all kicked in?

I don't quite follow you -- it's when the *.eln files are missing that
the nativecomp machinery does kick in.

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: schwab@linux-m68k.org,  53024@debbugs.gnu.org,  monnier@iro.umontreal.ca
> Date: Sat, 06 Aug 2022 14:09:25 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Doesn't distclean remove the *.eln files as well?  It should, IMO:
> > these files will never exist in a release tarball.
> >
> > If the *.eln files were removed by distclean, then how come nativecomp
> > machinery at all kicked in?
> 
> I don't quite follow you -- it's when the *.eln files are missing that
> the nativecomp machinery does kick in.

Is this still relevant, after all the recent changes?

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Is this still relevant, after all the recent changes?

Nope.  Just answering the question, in case anybody wondered.

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Andreas Schwab]

On Aug 04 2022, Lars Ingebrigtsen wrote:

> Perhaps this is a bug in the RPM build script.  That is, now that
> loaddefs files are compiled, it only copies over the .elc files to
> rpmbuild/BUILD, and not the .el files, or something along those lines?

This happens during build, the install directory hasn't even been
created yet.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Andreas Schwab <schwab@linux-m68k.org> writes:

>> Perhaps this is a bug in the RPM build script.  That is, now that
>> loaddefs files are compiled, it only copies over the .elc files to
>> rpmbuild/BUILD, and not the .el files, or something along those lines?
>
> This happens during build, the install directory hasn't even been
> created yet.

So does loaddefs.elc and/or loaddefs.el exist when you get this error?

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Andreas Schwab]

On Aug 04 2022, Lars Ingebrigtsen wrote:

> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>>> Perhaps this is a bug in the RPM build script.  That is, now that
>>> loaddefs files are compiled, it only copies over the .elc files to
>>> rpmbuild/BUILD, and not the .el files, or something along those lines?
>>
>> This happens during build, the install directory hasn't even been
>> created yet.
>
> So does loaddefs.elc and/or loaddefs.el exist when you get this error?

No loaddefs.el, it has been removed by make distclean.

make distclean
CFLAGS='-O2 -Wall -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type -flto=auto -g -D_GNU_SOURCE -DGDK_DISABLE_DEPRECATION_WARNINGS -DGLIB_DISABLE_DEPRECATION_WARNINGS -pipe -Wno-pointer-sign -Wno-unused-variable -Wno-unused-label -Wno-unprototyped-calls '
./configure --with-pop --without-hesiod --with-kerberos --with-kerberos5 --with-xim --with-wide-int --with-gpm --with-file-notification=inotify --with-modules --enable-autodepend --with-file-notification=gfile --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --datadir=/usr/share --localstatedir=/var --sharedstatedir=/var/lib --libexecdir=/usr/lib --enable-locallisppath=/usr/share/emacs/29.0.50/site-lisp:/usr/share/emacs/site-lisp --with-x --with-sound --with-xpm --with-jpeg --with-tiff --with-gif --with-png --with-rsvg --with-dbus --with-xft --with-x-toolkit=gtk3 --with-xwidgets --with-toolkit-scroll-bars --x-includes=/usr/include --x-libraries=/usr/lib64 --with-libotf --with-m17n-flt --with-cairo --with-native-compilation --build=x86_64-suse-linux
make -j4

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Andreas Schwab <schwab@linux-m68k.org> writes:

> No loaddefs.el, it has been removed by make distclean.

Then it should fall back to loading ldefs-boot.el, but apparently in
your case, it's trying to display a warning triggered by the eln
machinery (somehow) instead, and then failing.

I've tried reproducing the problem on both Ubuntu and Debian, but I'm
unable to.

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Andreas Schwab]

On Aug 04 2022, Lars Ingebrigtsen wrote:

> I've tried reproducing the problem on both Ubuntu and Debian, but I'm
> unable to.

Did you use a bootstrapped build?

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Andreas Schwab <schwab@linux-m68k.org> writes:

> Did you use a bootstrapped build?

Yes.

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

I see that Andreas has fixed this issue.

But trying to debug this, I see that another issue has cropped up again
(unrelated to Andreas' fix) -- if I

touch lisp/emacs-lisp/loaddefs-gen.el
make

it'll rebuild the loaddef.el(c) files as it should -- but then next time
you say "make", it'll do a new pdump.  And it really should do that on
the first make.  I think I had that working yesterday, but something
I've done today in the ordering must have brought it back.

Anybody see anything obvious?  If not, I'll poke at this some more
tomorrow.

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 53024@debbugs.gnu.org,  Eli Zaretskii <eliz@gnu.org>,
>   monnier@iro.umontreal.ca
> Date: Thu, 04 Aug 2022 21:20:35 +0200
> 
> Andreas Schwab <schwab@linux-m68k.org> writes:
> 
> > No loaddefs.el, it has been removed by make distclean.
> 
> Then it should fall back to loading ldefs-boot.el

Shouldn't the build re-generate loaddefs.el early on?  I thought you
recently made it an order-only prerequisite?  So why doesn't it get
re-generated before running the built binary for the first time?

bug#53024: 29.0.50; Wishlist: Byte-compile loaddefs.el?

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Shouldn't the build re-generate loaddefs.el early on?  I thought you
> recently made it an order-only prerequisite?  So why doesn't it get
> re-generated before running the built binary for the first time?

Hm...  $(pdmp) depends on loaddefs.elc, and that existed in this case,
but not loaddefs.el.  But perhaps it should depend on both?  That should
have fixed this issue...

I've now done so.