From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Philip Kaludercic Newsgroups: gmane.emacs.devel Subject: Re: feature/package-vc has been merged Date: Wed, 09 Nov 2022 21:33:41 +0000 Message-ID: <87zgcz7qyy.fsf@posteo.net> References: <164484721900.31751.1453162457552427931@vcs2.savannah.gnu.org> <87y1song5x.fsf@posteo.net> <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> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="32814"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Eli Zaretskii , rms@gnu.org, emacs-devel@gnu.org, Lars Ingebrigtsen To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Wed Nov 09 22:34:11 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 1ossi2-0008HP-8i for ged-emacs-devel@m.gmane-mx.org; Wed, 09 Nov 2022 22:34:10 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1osshi-00012o-6B; Wed, 09 Nov 2022 16:33:50 -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 1osshg-00012H-7e for emacs-devel@gnu.org; Wed, 09 Nov 2022 16:33:48 -0500 Original-Received: from mout02.posteo.de ([185.67.36.66]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1osshd-0005KN-JW for emacs-devel@gnu.org; Wed, 09 Nov 2022 16:33:48 -0500 Original-Received: from submission (posteo.de [185.67.36.169]) by mout02.posteo.de (Postfix) with ESMTPS id EE5B7240105 for ; Wed, 9 Nov 2022 22:33:43 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1668029624; bh=FHAP4qpUuawCH6KFojy7zkpKZkCuwfkG712ZPk+7H2c=; h=From:To:Cc:Subject:Date:From; b=br+s09yp1pU2ZWWrXrB0+nfQQHeeEKEcqBKYl/T6njB2JRxRMPBjbIFOEWt4g0nff /7ySybWgUQuaLniwT7GezcFKguwVSNnMFxPLlXAIPp8pMcxOJkZOm4HKpVlymf214M FBoM1g8p43lTw/9ddLS3QiCNlnrxYe51Z6PRkY04rMPmqscOhUXwvlsdr+Atj0DAwm Fi5eKtvYfTz1rXYSC46wsJ4o5QhdSQ4ixgEnOQmnkic1Cque24fmyQkUUplUP3rb05 CBz0rfd49JNcfiSKZrWbRGkNrUTYpfDv+QIXDT9SYUh/K6UE1u+cOfTFOzOSj8ycig r+m+plZLzw3mQ== Original-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4N6yrj6Sp8z9rxS; Wed, 9 Nov 2022 22:33:41 +0100 (CET) In-Reply-To: (Stefan Monnier's message of "Wed, 09 Nov 2022 16:21:22 -0500") Received-SPF: pass client-ip=185.67.36.66; envelope-from=philipk@posteo.net; helo=mout02.posteo.de X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.001, 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:299440 Archived-At: --=-=-= Content-Type: text/plain Stefan Monnier writes: >>> But for `package-vc` that's already the case since it has complete control >>> over how the `-pkg.el` and the `-autoloads.el` are generated. >> >> ..., this is handled by `package-vc--unpack-1' which up until now did >> not depend on a package specification, just a package description. This >> is so, so that it can be invoked by `package-vc-refresh' or >> `package-vc-install-from-checkout' which all don't necessarily have >> specifications. It would be imaginable to store the specification in a >> file like -spec.eld, but there are too many files already so this >> is just getting more and more messy. >> >> If you believe that this is not worth it or shortsighted on my part, >> I'll implement the code necessary for what you suggest to work. > > Ah... so this is extra info stored in the `-pkg.el` file but only > for packages installed by `package-vc`. So it's not expected to appear > in `-pkg.el` coming from ELPA repositories. Right. > But then it should not > be used in `package.el`, only in `package-vc-...` functions, right? I have attached the entire patch below. There are a few points where we want to have regular package.el functions also respect :lisp-dir, but these changes mostly consist of just replacing package-desc-dir with an auxiliary function that returns the lisp directory. > BTW, I'm beginning to sense that maybe instead of storing `:lisp-dir` in > the package description (of package-vc-installed-packages), we > should/could store there the whole package-spec. This way > `package-vc-refresh` can use the package-spec and throw away the rest of > the package description. That would also be possible, I'd have to think about it and try it out. > For `package-vc-install-from-checkout`, we need to "invent" a package > spec anyway, right? Well no, because that function uses `package-vc--unpack-1' that is invoked during the regular installation after all the information from the specification has been made use of. But even if that weren't the case, we'd just use the "empty" specification in this case, because the argument to `package-vc-install-from-checkout' is a lisp directory to begin with. --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Allow-specifying-a-lisp-dir-for-package-descriptions.patch >From 1eeca3607efcc08b3534e9bbdaa57d2bd62839ec Mon Sep 17 00:00:00 2001 From: Philip Kaludercic Date: Tue, 8 Nov 2022 23:45:35 +0100 Subject: [PATCH] Allow specifying a :lisp-dir for package descriptions * lisp/emacs-lisp/package-vc.el (package-vc-repository-store): Remove obsolete variable. (package-vc--unpack-1): Respect :lisp-dir. (package-vc--unpack): Add :lisp-dir to the package description if necessary. * lisp/emacs-lisp/package.el (package-lisp-dir): Add new inline function. (package--reload-previously-loaded): Use 'package-lisp-dir'. (package-activate-1): Use 'package-lisp-dir'. (package-generate-autoloads): Change first parameter from NAME to PKG-DESC. (package--make-autoloads-and-stuff): Use 'package-lisp-dir'. (package--compile): Use 'package-lisp-dir'. (package--native-compile-async): Use 'package-lisp-dir'. (package--delete-directory): Remove 'package-vc-p' check and drop second parameter. (package-delete): Remove second argument when invoking 'package--delete-directory'. (package-recompile): Use 'package-lisp-dir'. --- lisp/emacs-lisp/package-vc.el | 34 +++++------------- lisp/emacs-lisp/package.el | 65 ++++++++++++++++++++--------------- 2 files changed, 46 insertions(+), 53 deletions(-) diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el index f8948905ea..93a96abb68 100644 --- a/lisp/emacs-lisp/package-vc.el +++ b/lisp/emacs-lisp/package-vc.el @@ -100,12 +100,6 @@ package-vc-heuristic-alist vc-handled-backends))) :version "29.1") -(defcustom package-vc-repository-store - (expand-file-name "emacs/vc-packages" (xdg-data-home)) - "Directory used by to store repositories." - :type 'directory - :version "29.1") - (defcustom package-vc-default-backend 'Git "Default VC backend used when cloning a package repository. If no repository type was specified or could be guessed by @@ -386,7 +380,7 @@ package-vc--unpack-1 ;; dependency list wasn't know beforehand, and they might have ;; to be installed explicitly. (let (deps) - (dolist (file (directory-files pkg-dir t "\\.el\\'" t)) + (dolist (file (directory-files (package-lisp-dir pkg-desc) t "\\.el\\'" t)) (with-temp-buffer (insert-file-contents file) (when-let* ((require-lines (lm-header-multiline "package-requires"))) @@ -402,10 +396,9 @@ package-vc--unpack-1 (package-compute-transaction nil (delete-dups deps)))) (let ((default-directory (file-name-as-directory pkg-dir)) - (name (package-desc-name pkg-desc)) (pkg-file (expand-file-name (package--description-file pkg-dir) pkg-dir))) ;; Generate autoloads - (package-generate-autoloads name pkg-dir) + (package-generate-autoloads pkg-desc pkg-dir) ;; Generate package file (package-vc--generate-description-file pkg-desc pkg-file) @@ -492,28 +485,17 @@ package-vc--unpack (pcase-let* (((map :url :lisp-dir) pkg-spec) (name (package-desc-name pkg-desc)) (dirname (package-desc-full-name pkg-desc)) - (pkg-dir (expand-file-name dirname package-user-dir)) - (real-dir (if (null lisp-dir) - pkg-dir - (unless (file-exists-p package-vc-repository-store) - (make-directory package-vc-repository-store t)) - (file-name-concat - package-vc-repository-store - ;; FIXME: We aren't sure this directory - ;; will be unique, but we can try other - ;; names to avoid an unnecessary error. - (file-name-base url))))) + (pkg-dir (expand-file-name dirname package-user-dir))) (setf (package-desc-dir pkg-desc) pkg-dir) (when (file-exists-p pkg-dir) (if (yes-or-no-p "Overwrite previous checkout?") - (package--delete-directory pkg-dir pkg-desc) + (package--delete-directory pkg-dir) (error "There already exists a checkout for %s" name))) - (package-vc--clone pkg-desc pkg-spec real-dir rev) - (unless (eq pkg-dir real-dir) - ;; Link from the right position in `repo-dir' to the package - ;; directory in the ELPA store. - (make-symbolic-link (file-name-concat real-dir lisp-dir) pkg-dir)) + (package-vc--clone pkg-desc pkg-spec pkg-dir rev) + (when lisp-dir + (push (cons :lisp-dir lisp-dir) + (package-desc-extras pkg-desc))) (package-vc--unpack-1 pkg-desc pkg-dir))) (defun package-vc--read-package-name (prompt &optional allow-url installed) 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)))) +(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)))))) + (cl-defstruct (package-desc ;; Rename the default constructor from `make-package-desc'. (:constructor package-desc-create) @@ -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 @@ -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. @@ -1080,9 +1089,10 @@ package-autoload-ensure-default-file (defvar autoload-timestamps) (defvar version-control) -(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. @@ -1090,17 +1100,29 @@ package-generate-autoloads (backup-inhibited t) (version-control 'never)) (loaddefs-generate - pkg-dir output-file - nil - "(add-to-list 'load-path (directory-file-name - (or (file-name-directory #$) (car load-path))))") + (package-lisp-dir pkg-desc) + output-file nil + (prin1-to-string + `(add-to-list + 'load-path + ;; Add the directory that will contain the autoload file to + ;; the load path. We don't hard-code `pkg-dir', to avoid + ;; issues if the package directory is moved around. + ,(if-let ((base '(or (and load-file-name (file-name-directory load-file-name)) + (car load-path))) + (extras (package-desc-extras pkg-desc)) + (lisp-dir (alist-get :lisp-dir extras))) + ;; In case the package specification indicates that the lisp + ;; files are found in a subdirectory, append that directory. + `(expand-file-name ,lisp-dir ,base) + base)))) (let ((buf (find-buffer-visiting output-file))) (when buf (kill-buffer buf))) auto-name)) (defun package--make-autoloads-and-stuff (pkg-desc pkg-dir) "Generate autoloads, description file, etc., for PKG-DESC installed at PKG-DIR." - (package-generate-autoloads (package-desc-name pkg-desc) pkg-dir) + (package-generate-autoloads pkg-desc pkg-dir) (let ((desc-file (expand-file-name (package--description-file pkg-dir) pkg-dir))) (unless (file-exists-p desc-file) @@ -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))) (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)))) ;;;; Inferring package from current buffer (defun package-read-from-string (str) @@ -2419,7 +2441,7 @@ package--newest-p (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." @@ -2427,18 +2449,7 @@ package--delete-directory (cl-loop for file in (directory-files-recursively dir "\\.el\\'") do (comp-clean-up-stale-eln (comp-el-to-eln-filename file)))) - (if (and (package-vc-p pkg-desc) - (require 'package-vc) ;load `package-vc-repository-store' - (file-in-directory-p dir package-vc-repository-store)) - (progn - (delete-directory - (expand-file-name - (car (file-name-split - (file-relative-name dir package-vc-repository-store))) - package-vc-repository-store) - t) - (delete-file (directory-file-name dir))) - (delete-directory dir t))) + (delete-directory dir t)) (defun package-delete (pkg-desc &optional force nosave) @@ -2493,7 +2504,7 @@ package-delete (package-desc-name pkg-used-elsewhere-by))) (t (add-hook 'post-command-hook #'package-menu--post-refresh) - (package--delete-directory dir pkg-desc) + (package--delete-directory dir) ;; Remove NAME-VERSION.signed and NAME-readme.txt files. ;; ;; NAME-readme.txt files are no longer created, but they @@ -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))) -- 2.35.1 --=-=-=--