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

[Philip Kaludercic]

Leo Gaskin <leo.gaskin@le0.gs> writes:

>> Can you reproduce this using emacs -Q?
>
> Yes I can. When I run the command `M-x package-vc-install helm RET',
> then `M-x package-vc-rebuild helm RET' immediately after, in an Emacs
> instance launched with the -Q option, the helm-autoloads.el file does
> not contain the expected autoloads for the Helm package.

Ok, thanks.

>> I guess this could work, but it might also be an inconvenience for
>> people with a lot of packages?
>
> I think `package-generate-autoloads' already rarely or never reuses
> previously generated autoloads files.

Why shouldn't it, shouldn't the list of autoloads of a package stay
relatively constant over package releases?

> It seems to me that `package-vc-rebuild' is currently the only function
> that calls `package-generate-autoloads' on a package with autoloads
> files already present.  

True, the only other case is `package-unpack' (via the function
`package--make-autoloads-and-stuff'), which are respectively only
invoked by `package-install-from-archive' and
`package-install-from-buffer', both of which do not re-use anything.

>                         Other functions such as `package-update' first
> delete the old package, meaning an old autoloads file is never present.

Right.

> Also, for me at least, the amount of time it takes to generate these
> autoload files is not really noticeable, at least compared the time it
> takes to byte-compile the package.

I do not know if this is always the case, but I wouldn't be surprised if
that was so.

>> It seems to me that trying to better
>> understand the issue before proceeding to apply this quick fix would be
>> a better idea.  What do you think?
>
> Sure, it would be a good idea to understand the underlying problem.
> Unfortunately I do not understand the loaddefs generation code well
> enough to offer a better fix right now. Investigating the issue further
> would take more time, which I likely won't be able to spend on this
> this month.

This might also very well be an issue with package-vc.  Either way, I
will take a look at it, perhaps there might be a better understood
solution (even though your fix looks like it should be safe).  If I
don't find anything, we can always "fall back" on to the patch you
provided.

> On Sun, Apr 23, 2023 at 3:15 PM Philip Kaludercic <philipk@posteo.net> wrote:
>>
>> Leo Georg Gaskin <leo.gaskin@le0.gs> writes:
>>
>> > Tags: patch
>> >
>> > Hello!
>> >
>> > I've been using the new package-vc.el functionality to great effect,
>> > but I have also come across a somewhat annoying bug.  If I use any of
>> > the provided commands to rebuild my local package while a autoloads
>> > file is already present, the newly generated autoloads file is
>> > consistently either incomplete or empty.
>>
>> Can you reproduce this using emacs -Q?
>>
>> > The easiest way I've found to fix this is simply changing the
>> > `package-generate-autoloads' function to always rebuild the autoloads
>> > file by passing the relevant option to `loaddefs-generate'.  I think
>> > this change also makes sense on a larger scale, as package generation
>> > taking into account older build artifacts seems unintuitive.
>>
>> I guess this could work, but it might also be an inconvenience for
>> people with a lot of packages?  It seems to me that trying to better
>> understand the issue before proceeding to apply this quick fix would be
>> a better idea.  What do you think?
>>
>> > The attached patch implements this change.
>> >
>> > I've read that for small changes like this no copyright assignment
>> > is needed.  If I have misunderstood, please point me to the relevant
>> > documents so I can sign them.  Please also let me know if I have
>> > messed something up or you need any additional information.
>>
>> That should be the case.
>>
>> >
>> > Best wishes
>> >
>> > Leo Gaskin
>> >
>> >
>> > In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo
>> > version 1.16.0, Xaw3d scroll bars)
>> > Repository revision: 9848ae17161828190cc0ba31e89ae54a2f08a2ef
>> > Repository branch: master
>> > Windowing system distributor 'The X.Org Foundation', version 11.0.12101008
>> > System Description: NixOS 23.05 (Stoat)
>> >
>> > Configured using:
>> >  'configure
>> >  --prefix=/nix/store/y9bxk3kqk4isr28jcy1bclkdr5a4zd1v-emacs-git-20230407.0
>> >  --disable-build-details --with-modules --with-x-toolkit=lucid
>> >  --with-xft --with-cairo --with-native-compilation'
>> >
>> >>From dad173580c048cbe89d5287c4475e965c71a702e Mon Sep 17 00:00:00 2001
>> > From: Leo Gaskin <leo.gaskin@le0.gs>
>> > Date: Sat, 8 Apr 2023 23:13:59 +0200
>> > Subject: [PATCH] Always fully rebuild autoloads in 
>> > package-generate-autoloads
>> >
>> > ---
>> >  lisp/emacs-lisp/package.el | 3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
>> > index 685f983..09811f1 100644
>> > --- a/lisp/emacs-lisp/package.el
>> > +++ b/lisp/emacs-lisp/package.el
>> > @@ -1093,7 +1093,8 @@ untar into a directory named DIR; otherwise, signal 
>> > an error."
>> >          ;; the load path.  We don't hard-code `pkg-dir', to avoid
>> >          ;; issues if the package directory is moved around.
>> >          (or (and load-file-name (file-name-directory load-file-name))
>> > -            (car load-path)))))
>> > +            (car load-path))))
>> > +     nil t)
>> >      (let ((buf (find-buffer-visiting output-file)))
>> >        (when buf (kill-buffer buf)))
>> >      auto-name))