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 12:29:01 -0500 Message-ID: References: <164484721900.31751.1453162457552427931@vcs2.savannah.gnu.org> <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> <87mt8qnbaa.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="28927"; 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 18:29:34 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 1ovME9-0007Kr-VK for ged-emacs-devel@m.gmane-mx.org; Wed, 16 Nov 2022 18:29:34 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovMDs-000709-9W; Wed, 16 Nov 2022 12:29:16 -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 1ovMDq-0006y6-Cx for emacs-devel@gnu.org; Wed, 16 Nov 2022 12:29:14 -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 1ovMDo-0005u7-5n; Wed, 16 Nov 2022 12:29:14 -0500 Original-Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id A909E100142; Wed, 16 Nov 2022 12:29:09 -0500 (EST) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 519D01000E6; Wed, 16 Nov 2022 12:29:03 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1668619743; bh=BnwaHxGcwq1hFNmZGoxsSG1/BMQ7xbdv4DoDQNerEro=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=O47hXKH6irw0fg1UlGNsSXdTlvKROQEEtL0CMfZfWk8mUSf+Mf+7OP92x9Jjwa/4g Nc7ZNKyDpGQzhrHVSSbx2tw3SMTuuOuFXb9QxzYJxx0JA15TzjGb4j3BNg7/iNeRbt 6Tf9ZfRJPeSDOvvvG7pS/uyQ624r3C31bpn4wevfseQ2hG4vqokjoGa7Tbi+9CEK5f YXYTbcZMKWfwUFzMp9VLoO90Zi+xL9O5c+SKO0I+nb9c7mgSpBBtFNJodRzhhzCJRY 6T8y8kEKV0ZiZdIyz/MKwbBZE9s1pxGsAt/weWhCm0WOlJgNiknci+Y1Hqcbo9cek/ 7bMRSrRooF6zQ== Original-Received: from lechazo (lechon.iro.umontreal.ca [132.204.27.242]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 370CE1209CD; Wed, 16 Nov 2022 12:29:03 -0500 (EST) In-Reply-To: <87mt8qnbaa.fsf@posteo.net> (Philip Kaludercic's message of "Wed, 16 Nov 2022 15:56:13 +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:299958 Archived-At: >>> @@ -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? > > Because we are only interested in the sub-directory containing the lisp > files. But is there any harm in considering the whole parent directory instead? >>> @@ -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 ma= ybe 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). > > I see what you mean. But this would be change that is unrelated to > package-vc, so it could just be removed directly on master. Removing it would is indeed a decision unrelated to `package-vc`. But in the mean time you can simply not change that code. >>> @@ -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. > > I might be missing something, but the previous signature was missing a > package description object that the change required. No, I mean that the change should not be needed (and hence the change in signature shouldn't be needed either). >> Why do we need this? AFAIK this recurses into subdirectories so using >> `package-desc-dir` still compiles all the files just fine, no? > Same as above, if we know all the lisp code is located in one > sub-directory, there is no need to compile everything in the > directory -- which might contain test files or other scripts that were > not meant to be compiled. In elpa-admin.el I compile all those ancillary files as well. I'm not sure we should refrain from compiling them, actually. [ But yes: their compilation will fail more often. I consider it a problem in the packages. ] >> IIUC this part of the change is because `package-delete` does not delete >> package-vc packages, right? If so, I support that 100%: > > This change reverts a previous feature, back when package-vc didn't load > sub-directories, but cloned the repository into some XDG directory and > created a symbolic link from ~/.emacs.d/elpa to the right sub-directory. > To do that, knowledge of what package was been deleted was required: Good, thanks. > This is currently not done, but could be added. I am imagining checking > of the package directory is the root directory of a repository (does > that work for all VCS?), and if so double-prompting the user. But I > would be opposed to preventing users from deleting packages installed > using `package-vc', if only it would go against my workflow of fetching > the sources for a package, preparing and sending a patch, and then > reverting to the release package. I'm not saying we should prevent it, but just that we should be careful to only delete with the full, express, and informed consent of the users. Stefan