From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#62720: 29.0.60; Not easy at all to upgrade :core packages like Eglot Date: Tue, 11 Apr 2023 20:55:58 +0300 Message-ID: <83a5ze9uc1.fsf@gnu.org> References: <87a5zj2vfo.fsf@gmail.com> <87wn2modrm.fsf@posteo.net> <87ile6o2ov.fsf@posteo.net> <87y1mz38rl.fsf@posteo.net> <87ile2n0kn.fsf@gmail.com> <83v8i2abqi.fsf@gnu.org> <87wn2ilgx7.fsf@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="6249"; mail-complaints-to="usenet@ciao.gmane.io" Cc: larsi@gnus.org, 62720@debbugs.gnu.org, philipk@posteo.net, monnier@iro.umontreal.ca To: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Apr 11 19:56:24 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1pmIEB-0001PM-Rd for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 11 Apr 2023 19:56:24 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pmIDr-0003a6-PV; Tue, 11 Apr 2023 13:56:03 -0400 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 1pmIDq-0003Zv-LH for bug-gnu-emacs@gnu.org; Tue, 11 Apr 2023 13:56:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pmIDq-0007nn-7l for bug-gnu-emacs@gnu.org; Tue, 11 Apr 2023 13:56:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pmIDp-0004gP-UA for bug-gnu-emacs@gnu.org; Tue, 11 Apr 2023 13:56:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 11 Apr 2023 17:56:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 62720 X-GNU-PR-Package: emacs Original-Received: via spool by 62720-submit@debbugs.gnu.org id=B62720.168123572817959 (code B ref 62720); Tue, 11 Apr 2023 17:56:01 +0000 Original-Received: (at 62720) by debbugs.gnu.org; 11 Apr 2023 17:55:28 +0000 Original-Received: from localhost ([127.0.0.1]:38138 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pmIDI-0004fa-1u for submit@debbugs.gnu.org; Tue, 11 Apr 2023 13:55:28 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:54806) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pmIDE-0004fJ-9B for 62720@debbugs.gnu.org; Tue, 11 Apr 2023 13:55:26 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pmID7-0007by-0M; Tue, 11 Apr 2023 13:55:17 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-version:References:Subject:In-Reply-To:To:From: Date; bh=GpyvUjG7W0YUfyPtMr/E03Fvcxa4TGE/NJtgTFCZocQ=; b=ZQpTUgFl0ZQDWfTwtTE4 bAd1VxEZsR7/UZUEz3a34xDQhaS4GUbPqlY5vHtU2+K6iMqfVzuqveY3phPM0empAyJ7NC0PSb5IT YpNt8k8hpse/fPur35TDM8BckkE8rbj2qth5Sk/0TzjVaBTPocdlfCHPr4L2SH6Xl/b3nmC3e2ti1 1kIstE12EBWMjyj9mje49frXyE2lDWpB+uwMhjZRUP/Z7sE+cTne7a4SiSkCbx0IQq702deVXof5e 5PTtklIkCelgDOKNzCRAsY3ZXpc7sEK86IXnnx4f8bG2JHi7ko5ZaK58WVj1s2m4GpMFCTyYCOKua HSpfI7/DaybmRw==; Original-Received: from [87.69.77.57] (helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pmID6-0008Tz-GF; Tue, 11 Apr 2023 13:55:16 -0400 In-Reply-To: <87wn2ilgx7.fsf@gmail.com> (message from =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= on Tue, 11 Apr 2023 13:52:36 +0100) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:259658 Archived-At: > From: João Távora > Cc: philipk@posteo.net, monnier@iro.umontreal.ca, 62720@debbugs.gnu.org, > larsi@gnus.org > Date: Tue, 11 Apr 2023 13:52:36 +0100 > > Eli Zaretskii writes: > > >> Eli, what do you think? > > > > I'd prefer it to go to master, not to emacs-29. The problem is not > > grave enough and OTOH the workaround is simple enough. So changing > > package.el in such non-trivial ways is not something I'd like to risk > > now. > > Please reconsider. If we do this, than Emacs 29 users will be almost > locked out of upgrading Eglot and a lot of other built-in packages. > I'll have to teach people that workaround in the manual, where such > workarounds don't really belong. OK, I looked closer at the patch and the code involved in this, and also re-read this discussion. I cannot agree with installing your patch, as submitted, on the emacs-29 branch, sorry. It modifies code that affects "normal" invocations of package-update, and also numerous other functions in package.el (via the change in package--updateable-packages), in ways that are very hard for me to audit. It is hard to audit because there are parts of it that read like some kind of "black magic": > + (nonbuiltin (assq package package-alist))) Why is the return value of assq the sign that the package is "nonbuiltin"? > + (cond (nonbuiltin > + (let ((desc (cadr nonbuiltin))) > + (if (package-vc-p desc) > + (package-vc-update desc) > + (package-delete desc 'force) > + (package-install package 'dont-select)))) > + (t > + (package-install > + (cadr (assq package package-archive-contents))))))) Why the different way of calling package-install for "built-in" packages? > - (package-desc-version (cadr elt)) > + (if (vectorp (cdr elt)) > + (aref (cdr elt) 0) > + (package-desc-version (cadr elt))) What is the significance of the (vectorp (cdr elt)) test? > - (package-vc-p (cadr (assq (car elt) package-alist))))) > - package-alist))) > + (and (consp (cdr elt)) > + (package-desc-p (cadr elt)) > + (package-vc-p (cadr elt))))) > + (seq-union package-alist package--builtins > + (lambda (a b) (eq (car a) (car b))))))) What is the significance of the (consp (cdr elt)) test? And why do we need to add package--builtins to the list? How am I supposed to assess the safety of this patch, given all this semi-obfuscated code, and given that I'm not the every-day maintainer of package.el and am not familiar with all the quirks of its code? (It is quite possible that this obfuscated nature of the code is not your fault, but is caused by how package.el is implemented. In which case I hope that we could clean up the code of package.el on master to allow updating :core packages more seamlessly and with simpler code.) OTOH, the workaround you described in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=62720#5 doesn't sound too awful to me, given that this problem exists for a while and is not specific to Eglot. However, if we still want to have a better solution that will be safe enough to be installed on emacs-29, I can suggest two alternatives: . add a prefix argument to package-update, which would mean to update a package unconditionally, even if it is a built-in or of an older version, and make this special case be handled by code that is completely independent of the code we have in package-update now, so that the "normal" case is unaffected; or . add a new command, say, package-update-core-package, which will then be used only for :core packages OK? > M-x package-update and M-x package-update-all are new in Emacs 29. They might be new, but package-update was virtually unmodified since a year ago, during which time it was used by many people, and so its current code can be considered to be well tested. Your modifications are by contrast completely new. > The change I'm proposing it not really "non-trivial". I can walk > you or anybody through the code, or write tests if that would > improve the outlook. See above. Given the problems I mentioned, I'm allowed to doubt that you yourself understand the changes well enough to vouch for them. And even if you did vouch, my gray hair won't believe you. So I prefer to go for much safer, if slightly less clean, changes. I hope one of the two alternatives I suggested will be acceptable.