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

[Leo Gaskin]

> 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.

> 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.
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.  Other functions such as `package-update' first
delete the old package, meaning an old autoloads file is never present.

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.

> 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.

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))