Package: emacs;
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.
Message #14 received at 62734 <at> debbugs.gnu.org (full text, mbox):
From: Philip Kaludercic <philipk <at> posteo.net> To: Leo Gaskin <leo.gaskin <at> le0.gs> Cc: 62734 <at> debbugs.gnu.org Subject: Re: bug#62734: Always fully rebuild autoloads in package-generate-autoloads Date: Tue, 25 Apr 2023 12:35:30 +0000
Leo Gaskin <leo.gaskin <at> 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 <at> posteo.net> wrote: >> >> Leo Georg Gaskin <leo.gaskin <at> 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 <at> 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))
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.