GNU bug report logs -
#62734
Always fully rebuild autoloads in package-generate-autoloads
Previous Next
Reported by: Leo Georg Gaskin <leo.gaskin <at> le0.gs>
Date: Sun, 9 Apr 2023 04:12:02 UTC
Severity: normal
Tags: patch
Done: Philip Kaludercic <philipk <at> posteo.net>
Bug is archived. No further changes may be made.
Full log
Message #41 received at 62734 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: leo.gaskin <at> le0.gs, 62734 <at> debbugs.gnu.org
>> Date: Sat, 29 Apr 2023 08:19:35 +0000
>>
>> >> Rebuilding an installation means scraping for new autoload
>> >> cookies, re-compiling Emacs Lisp files, building and installing
>> >> any documentation, downloading any missing dependencies.
>> >
>> > Thanks. As a tangent: this is confusing terminology, so it is
>> > unfortunate that it was selected for this operation.
>>
>> Is there a different way you would have put this? What we do here is
>> sort of combining the steps that the ELPA server and a local
>> package-install do into one, and in my mind this constitutes building
>> software.
>
> Compilation? Setup?
Scraping for auto-loads doesn't have anything to do with "compiling"
(and might be confused with byte compilation). "Setup" might be
imaginable, I will think about it.
> "Building" is a strange term when we are talking about a Lisp package.
How come?
>> > I don't follow: a file that didn't change doesn't need its autoloads
>> > updated, right?
>>
>> Right, but if we want to add some additional code to the autoloads (as
>> we are with package.el, when injecting load-path modification), then
>> loaddefs-generate does not re-use the old data, and instead just throws
>> it away and creates a new file with contents of EXTRA-DATA, but without
>> any autoload information.
>
> That is definitely a bug. But a package without autoloads is still a
> valid use case, and we should support it.
>
>> I think the central issue here is the
>>
>> (and (not defs) extra-data)
>>
>> check. Just because we did not find any new definitions to autoload
>> /and/ EXTRA-DATA is non-nil, does not mean the old contents should be
>> discarded. The else-case already does the right thing, so I really do
>> think that getting rid of the `if' could resolve the issue:
>
> What happens if a package has no autoloads? The doc string says in
> that case passing EXTRA-DATA will produce OUTPUT-FILE regardless.
> Does your patch handle that? (It's hard to tell, given all the
> whitespace changes in the patch.)
It would, as the else-case of the if branch I am proposing to eliminate
would still insert the EXTRA-DATA.
Here is a patch generated without any whitespace changes:
[Message part 2 (text/plain, inline)]
diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
index 1007be62dd9..8da0795870c 100644
--- a/lisp/emacs-lisp/loaddefs-gen.el
+++ b/lisp/emacs-lisp/loaddefs-gen.el
@@ -597,15 +597,6 @@ loaddefs-generate
defs))))))
(progress-reporter-done progress))
- ;; If we have no autoloads data, but we have EXTRA-DATA, then
- ;; generate the (almost) empty file anyway.
- (if (and (not defs) extra-data)
- (with-temp-buffer
- (insert (loaddefs-generate--rubric output-file nil t))
- (search-backward "\f")
- (insert extra-data)
- (ensure-empty-lines 1)
- (write-region (point-min) (point-max) output-file nil 'silent))
;; We have some data, so generate the loaddef files. First
;; group per output file.
(dolist (fdefs (seq-group-by (lambda (x) (expand-file-name (car x)))
@@ -663,7 +654,7 @@ loaddefs-generate
(write-region (point-min) (point-max) loaddefs-file nil 'silent)
(byte-compile-info
(file-relative-name loaddefs-file (car (ensure-list dir)))
- t "GEN"))))))))
+ t "GEN")))))))
(defun loaddefs-generate--print-form (def)
"Print DEF in a format that makes sense for version control."
This bug report was last modified 2 years and 25 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.