From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: feature/package-vc has been merged Date: Wed, 16 Nov 2022 10:23:55 -0500 Message-ID: References: <164484721900.31751.1453162457552427931@vcs2.savannah.gnu.org> <83v8nsyof7.fsf@gnu.org> <87leoond7l.fsf@posteo.net> <83r0yfzz01.fsf@gnu.org> <87bkpjyx3p.fsf@posteo.net> <83bkpjynmj.fsf@gnu.org> <87iljqya44.fsf@posteo.net> <8335auzo9s.fsf@gnu.org> <87zgd2ws8z.fsf@posteo.net> <831qqezkxj.fsf@gnu.org> <87y1slgq3m.fsf@posteo.net> <87bkpgfsqv.fsf@posteo.net> <87educ9fei.fsf@posteo.net> <8735as9cfb.fsf@posteo.net> <87pmdv98du.fsf@posteo.net> <87zgcz7qyy.fsf@posteo.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="3855"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: Eli Zaretskii , rms@gnu.org, emacs-devel@gnu.org, Lars Ingebrigtsen To: Philip Kaludercic Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Wed Nov 16 16:24:57 2022 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1ovKHY-0000kN-4y for ged-emacs-devel@m.gmane-mx.org; Wed, 16 Nov 2022 16:24:56 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovKGi-0001uF-Qu; Wed, 16 Nov 2022 10:24:04 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovKGh-0001u5-8F for emacs-devel@gnu.org; Wed, 16 Nov 2022 10:24:03 -0500 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovKGe-0003KK-A5; Wed, 16 Nov 2022 10:24:03 -0500 Original-Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 0ABE04409A3; Wed, 16 Nov 2022 10:23:58 -0500 (EST) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 08742441183; Wed, 16 Nov 2022 10:23:56 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1668612236; bh=/X/UM20ezi9/aK85r7KweSFzWkkeMwdEJ9jwAs+K8pA=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=Jvf/16xvmiT6jxRfZhmymETNNVSYESzgu7GCSVaTFZtQSbLfTjov2Nmn53xlF835+ oXrH9DO2qa+PT9AkoNomI0TQS5f7rRlU2muQeOqndi3Qy1F098Q6O4qMs16Pi9+ena otp/NplEK80lhSm3tPZgNDRo0swdMPbmHe5QSznNiptyAgvsQ7b877E2g6VklfokKM ov/YtP4tB4TESpKOxqDhYAFNiFt/B71/ZHo3xTfSbj/onmEOcG09AC41TrWZesR03N rdG+6f2h/2TIAWBUthomR6y6XVJ03t/ykIpa8/CbCs4Y9Xg6hYc/tC4yYhjum529+M Eo29CYlXHPBKQ== Original-Received: from lechazo (lechon.iro.umontreal.ca [132.204.27.242]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id C67E0120F20; Wed, 16 Nov 2022 10:23:55 -0500 (EST) In-Reply-To: <87zgcz7qyy.fsf@posteo.net> (Philip Kaludercic's message of "Wed, 09 Nov 2022 21:33:41 +0000") Received-SPF: pass client-ip=132.204.25.50; envelope-from=monnier@iro.umontreal.ca; helo=mailscanner.iro.umontreal.ca X-Spam_score_int: -42 X-Spam_score: -4.3 X-Spam_bar: ---- X-Spam_report: (-4.3 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:299938 Archived-At: > diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el > index a7bcdd214c..bf6849af65 100644 > --- a/lisp/emacs-lisp/package.el > +++ b/lisp/emacs-lisp/package.el > @@ -462,6 +462,15 @@ package-vc-p > (inline-letevals (pkg-desc) > (inline-quote (eq (package-desc-kind ,pkg-desc) 'vc)))) >=20=20 > +(define-inline package-lisp-dir (pkg-desc) > + "Return the directory with Lisp files for PKG-DESC." > + (inline-letevals (pkg-desc) > + (inline-quote > + (let* ((extras (package-desc-extras ,pkg-desc)) > + (lisp-dir (alist-get :lisp-dir extras)) > + (dir (package-desc-dir ,pkg-desc))) > + (file-name-directory (file-name-concat dir lisp-dir)))))) Any reason why this needs to be inlined? I'd expect the inlining to have a negligible effect since the body contains its fair share of further function calls, none of which are conditional and AFAICT all the calls to this pass a variable as argument, so the inlining will not expose further optimization opportunities. > @@ -827,7 +836,7 @@ package--reload-previously-loaded > byte-compilation of the new package to fail." > (with-demoted-errors "Error in package--load-files-for-activation: %s" > (let* (result > - (dir (package-desc-dir pkg-desc)) > + (dir (package-lisp-dir pkg-desc)) > ;; A previous implementation would skip `dir' itself. > ;; However, in normal use reloading from the same directory > ;; never happens anyway, while in certain cases external to Why do we need this change? > @@ -891,7 +900,7 @@ package-activate-1 > (package--reload-previously-loaded pkg-desc)) > (with-demoted-errors "Error loading autoloads: %s" > (load (package--autoloads-file-name pkg-desc) nil t)) > - (add-to-list 'load-path (directory-file-name pkg-dir))) > + (add-to-list 'load-path (package-lisp-dir pkg-desc))) > ;; Add info node. > (when (file-exists-p (expand-file-name "dir" pkg-dir)) > ;; FIXME: not the friendliest, but simple. This should really not be needed (actually, this `add-to-list` is not needed at all for any package (re)installed with Emacs=E2=89=A526 (or maybe= even 25, can't remember)). The "normal" behavior is that it's the autoloads file which adds to `load-path` (which makes it possible for that autoloads file to add the `:lisp-dir` instead of the root dir, indeed). > @@ -1080,9 +1089,10 @@ package-autoload-ensure-default-file > (defvar autoload-timestamps) > (defvar version-control) >=20=20 > -(defun package-generate-autoloads (name pkg-dir) > - "Generate autoloads in PKG-DIR for package named NAME." > - (let* ((auto-name (format "%s-autoloads.el" name)) > +(defun package-generate-autoloads (pkg-desc pkg-dir) > + "Generate autoloads for PKG-DESC in PKG-DIR." > + (let* ((name (package-desc-name pkg-desc)) > + (auto-name (format "%s-autoloads.el" name)) > ;;(ignore-name (concat name "-pkg.el")) > (output-file (expand-file-name auto-name pkg-dir)) > ;; We don't need 'em, and this makes the output reproducible. I thought an alternative was for `package-vc.el` to call this function with the `:lisp-dir` as `pkg-dir`, so we don't need to change this part of the code. > @@ -1118,7 +1140,7 @@ package--compile > (let ((byte-compile-ignore-files (package--parse-elpaignore pkg-desc)) > (warning-minimum-level :error) > (load-path load-path)) > - (byte-recompile-directory (package-desc-dir pkg-desc) 0 t))) > + (byte-recompile-directory (package-lisp-dir pkg-desc) 0 t))) Why do we need this? AFAIK this recurses into subdirectories so using `package-desc-dir` still compiles all the files just fine, no? > (defun package--native-compile-async (pkg-desc) > "Native compile installed package PKG-DESC asynchronously. > @@ -1126,7 +1148,7 @@ package--native-compile-async > `package-activate-1'." > (when (native-comp-available-p) > (let ((warning-minimum-level :error)) > - (native-compile-async (package-desc-dir pkg-desc) t)))) > + (native-compile-async (package-lisp-dir pkg-desc) t)))) Same here. > @@ -2419,7 +2441,7 @@ package--newest-p >=20=20 > (declare-function comp-el-to-eln-filename "comp.c") > (defvar package-vc-repository-store) > -(defun package--delete-directory (dir pkg-desc) > +(defun package--delete-directory (dir) > "Delete PKG-DESC directory DIR recursively. > Clean-up the corresponding .eln files if Emacs is native > compiled." IIUC this part of the change is because `package-delete` does not delete package-vc packages, right? If so, I support that 100%: Packages installed with `package-install` can be deleted without too much fuss because they usually don't contain any important info, but for `package-vc` this is completely different and we should either never delete them or at least not without a minimum of sanity checks (uncommited changes, stashes, additional branches) and very explicit prompt. > @@ -2549,7 +2560,7 @@ package-recompile > ;; load them (in case they contain byte code/macros that are now > ;; invalid). > (dolist (elc (directory-files-recursively > - (package-desc-dir pkg-desc) "\\.elc\\'")) > + (package-lisp-dir pkg-desc) "\\.elc\\'")) > (delete-file elc)) > (package--compile pkg-desc))) Same as above. Stefan