From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Tegar Syahputra Newsgroups: gmane.emacs.devel Subject: Re: Suggestion for package.el: atomic package-upgrade Date: Mon, 31 Jul 2023 20:13:07 +0700 Message-ID: <287fc0a8-f877-a598-8436-dcd57f1fe888@gmail.com> References: <87o7js37gf.fsf@posteo.net> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="3034"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.1 Cc: emacs-devel@gnu.org To: Philip Kaludercic Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Mon Jul 31 15:14:16 2023 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 1qQSj2-0000dg-Fp for ged-emacs-devel@m.gmane-mx.org; Mon, 31 Jul 2023 15:14:16 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qQSi7-00087j-As; Mon, 31 Jul 2023 09:13:19 -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 1qQSi5-00087X-St for emacs-devel@gnu.org; Mon, 31 Jul 2023 09:13:17 -0400 Original-Received: from mail-pl1-x630.google.com ([2607:f8b0:4864:20::630]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qQSi3-0002XA-T4 for emacs-devel@gnu.org; Mon, 31 Jul 2023 09:13:17 -0400 Original-Received: by mail-pl1-x630.google.com with SMTP id d9443c01a7336-1bbf0f36ce4so16699435ad.0 for ; Mon, 31 Jul 2023 06:13:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1690809194; x=1691413994; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=Q4MiIV1FUx3JJD1oMHPS0KgYC7bM/Uq+6JGgMOmylMQ=; b=dHQVUw0xFNZV7BFnMIaqiPRWVSe+cdf4wCriqrY563R1MhQBZtoASk5Euk/mO6J/0m kC2Zp+FWKoWiZcy2RcfyJDWLznXadkxw6aa/pHpNyolSXi4X93Q0cvMHKfDR9CcLSVIq BBPcTvoBEVqM40yIljQDXrrIFotJBGczAnUZ8OrVFRyUDjkfUkYbt2/rNF3BOHwEbrMQ zd5zZU6TjzrOy/U2hNs40iNk6SkLpkI+9hp7R+6/MAezVeCh90ge2KPes7ZtA+9f+tcJ wtzVY8ktusvj06vL+0PGUVDOyTklRd4oYHh3JjYbhlg2A4CF+024wgMWl4yOFZ0dBPwh i8DQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690809194; x=1691413994; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Q4MiIV1FUx3JJD1oMHPS0KgYC7bM/Uq+6JGgMOmylMQ=; b=FVAs0f3IIHUVD+rFWOOxwhZQd/TQfKATZBAMgg0LInQdt/YFklkfZ1FcueEqDdQDvP V952m9pWa9Xh+RTayoAcSe/WACaNM6DkYK+bhWLW2Kjj/XymQYGkmKcZ+zvtz1PCBs/C rm04f5XkyTeqx3s7V0VtGsUwBaYnuDHI2whwM+hWGy7rq+cCPUtfkUIu9NT8QVQInjer VRNPg8BgTzZizoin7LmZrdFz2Fz6uu+JaeQqwarMDGsx+/ZOK2O7DZKCTBlRCbxcuGXZ XXhXjbL4PGAQa2pTAUFUpl5GMT3TB1e6bQo4SBwXR6Xdxnmmpqh9nKoXTPAvqv5GpQ9w HjFA== X-Gm-Message-State: ABy/qLYPdH1d/Tqzqi89Fq9AXcIPbg+R+SKmDxea0jyLnSslcMVGOvUT cTky9/ZQcgXWCeE0yOroPEE= X-Google-Smtp-Source: APBJJlHFvKrd4S1KGVf79UPzuAGMWAECw9mZjEKX5kUn5Km12qSQqgVuH8yMiWXJViehRIfib/X3QQ== X-Received: by 2002:a17:902:ab05:b0:1bb:2093:efb1 with SMTP id ik5-20020a170902ab0500b001bb2093efb1mr9170183plb.27.1690809193658; Mon, 31 Jul 2023 06:13:13 -0700 (PDT) Original-Received: from [192.168.0.10] ([101.255.148.162]) by smtp.gmail.com with ESMTPSA id az10-20020a170902a58a00b001bc038b92e1sm3458645plb.49.2023.07.31.06.13.11 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 31 Jul 2023 06:13:13 -0700 (PDT) Content-Language: en-US In-Reply-To: <87o7js37gf.fsf@posteo.net> Received-SPF: pass client-ip=2607:f8b0:4864:20::630; envelope-from=dqs7cp2e@gmail.com; helo=mail-pl1-x630.google.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 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, FREEMAIL_FROM=0.001, NICE_REPLY_A=-0.101, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 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:308219 Archived-At: >> The current `package-upgrade' from package.el delete old package >> before installing the new one. This can be problematic if the user >> interrupt the process or if there is some network problems. >> >> `package-install' allow the same package to be installed if the >> argument is `package-desc' instead symbol representing package name. >> This allow package to be upgraded atomically. Is this a bad idea? > No, I think this is a good idea, but it would be best if you could write > a Git patch and send it to "bug-gnu-emacs@gnu.org" (you can use M-x > submit-emacs-patch). I'm not part of FSF contributor, and wasn't really thinking of contributing directly. Just giving a heads up that's all. >> diff -u --label \#\ --label \#\ /tmp/buffer-content-4azQGZ /tmp/buffer-content-x8FLpt >> --- # >> +++ # >> @@ -2275,16 +2275,20 @@ >> package using this command, first upgrade the package to a >> newer version from ELPA by using `\\\\[package-menu-mark-install]' after `\\[list-packages]'." >> (interactive >> - (list (completing-read >> - "Upgrade package: " (package--upgradeable-packages) nil t))) >> - (let* ((package (if (symbolp name) >> - name >> - (intern name))) >> - (pkg-desc (cadr (assq package package-alist)))) >> - (if (package-vc-p pkg-desc) >> - (package-vc-upgrade pkg-desc) >> - (package-delete pkg-desc 'force 'dont-unselect) >> - (package-install package 'dont-select)))) >> + (list (intern (completing-read >> + "Upgrade package: " (package--upgradeable-packages) nil t)))) >> + (let* ((name (if (symbolp name) >> + name >> + (intern name))) > If you always intern the package name, you don't need this check > anymore. On the other hand, you don't need to intern the name, because > of this check, and I think it is better to keep it because that gives > the user more flexibility when invoking the function. I intern the interactive input because the actual type needed is symbol type. The check was from original, it accept both string and symbol. >> + (old-pkg-desc (cadr (assq name package-alist))) >> + (new-pkg-desc (cadr (assq name package-archive-contents)))) >> + (if (package-vc-p old-pkg-desc) >> + (package-vc-upgrade old-pkg-desc) >> + (unwind-protect > I am wondering if unwind-protect is the best approach here, or even > necessary. Wouldn't something like > > --8<---------------cut here---------------start------------->8--- > (when (progn (package-install new-pkg-desc 'dont-select) t) > (package-delete old-pkg-desc 'force 'dont-unselect)) > --8<---------------cut here---------------end--------------->8--- No, you must check if the package is installed. That will always delete `old-pkg-desc' even if installation error occurs. > have the same effect? My reasoning is that we are not actually cleaning > anything up in the UNWIND-FORMS, but just checking if the > `package-install' was not interrupted, right? Yes, it's to check if the installations was interrupted or if error occur. I don't think `package-install' gives meaningful return that indicates package was properly installed. The output it gives could be used but, I just thought checking the output string may not be reliable or elegant. >> + (package-install new-pkg-desc 'dont-select) >> + (if (package-installed-p (package-desc-name new-pkg-desc) >> + (package-desc-version new-pkg-desc)) > If you check the definition of `package-installed-p', you will find it > does not use MIN-VERSION if the first argument satisfied > `package-desc-p', which makes sense because with a concrete descriptor, > we can unambiguously check if a specific package version has been > installed (the implementation just checks if the expected directory > exists). Yeah, and new-pkg-desc which is a package-desc from package-archive-contents is implied to be remote thus doesn't include directory. I'm OP, sorry I didn't configure my email name earlier.