Subject: Re: bug#62734: Always fully rebuild autoloads in package-generate-autoloads

From: Philip Kaludercic <[email-masked]>
Date: Fri, 28 Apr 2023 18:22:43 +0000

Eli Zaretskii <[email-masked]> writes:

>> From: Philip Kaludercic <[email-masked]>
>> Cc: [email-masked],  [email-masked]
>> Date: Fri, 28 Apr 2023 18:00:14 +0000
>> 
>> > I don't understand the original problem (what does package-vc.el have
>> > to do with rebuilding local packages? 
>> 
>> When package updates a package, it deletes the old code and downloads
>> the new stuff.  package-vc keeps the same code, but pulls the new
>> revisions, so it is necessary to re-generate the loaddef files for the
>> same files.
>
> What is meant by "building the package"?  Is it just compiling the
> Lisp files?

>From `package-vc-rebuild':

  Rebuilding an installation means scraping for new autoload
  cookies, re-compiling Emacs Lisp files, building and installing
  any documentation, downloading any missing dependencies.

>> >                                       and why is a newly generated
>> > loaddefs file incomplete or empty?), and I certainly don't think I
>> > understand the effects of this change on the other usage scenarios.
>> 
>> >From what I get, this is an issue in `loaddefs-generate'.  If we do not
>> force updating the file, and
>> 
>> --8<---------------cut here---------------start------------->8---
>> (time-less-p output-time
>>   (file-attribute-modification-time
>>   (file-attributes file)))
>> --8<---------------cut here---------------end--------------->8---
>> 
>> does not hold
>
> Why would it not hold?  Updating from VCS should update the timestamp
> of the updated files.

I don't think this necessarily holds if there were no changes affecting
a file.

>> Another idea is just to get rid of this faulty optimisation.  From my
>> tests this would also resolve the bug.
>
> I don't yet understand what optimization is that, but getting rid of
> it should not alter what the code does for the loaddefs files inside
> the Emacs tree, because there it does work, and I don't want to touch
> that.

Are you sure it does work?  I am currently having difficulties
understanding how the `with-temp-buffer' code snippet I quoted above
/can/ do the right thing.  I am under the impression that wherever
`loaddefs-generate' is invoked, it avoids this execution path, either by
passing no EXTRA-DATA or by making sure that at least some autoload is
collected.

>> > Why would we want to unconditionally rebuild all the loaddefs files
>> > every time package-generate-autoloads is invoked?  OTOH, that function
>> > is not really documented, so maybe I don't understand what is it
>> > supposed to do and in which conditions.
>> 
>> The matter was that for regular packages, it was already rebuilt every
>> time `loaddefs-generate' was invoked, since there were never any old
>> loaddefs to update.
>
> This is only true as long as updating a package removes the previous
> version entirely, including the generated files.  This is not
> something I'd like us to assume in every corner of package.el, since
> some day it can become false.

Ok, fair point considering that package-vc currently does this.