From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Jimmy Wong Newsgroups: gmane.emacs.bugs Subject: bug#63338: 29.0.90; package-vc-install'ing the same package multiple times results in duplication in package-selected-packages Date: Sun, 14 May 2023 13:51:38 +0100 Message-ID: References: <873547jeig.fsf@posteo.net> <83h6snc9ms.fsf@gnu.org> <5727a6ad-b2c6-4c8f-ac85-b6478c57a749@Spark> <87h6skfzeg.fsf@posteo.net> <87mt28880k.fsf@posteo.net> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="6460d95f_3c1fe6c6_b973" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="34160"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Eli Zaretskii , 63338@debbugs.gnu.org To: Philip Kaludercic Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun May 14 14:52:15 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 1pyBCx-0008i9-P2 for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 14 May 2023 14:52:15 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pyBCm-0000qM-8Z; Sun, 14 May 2023 08:52:04 -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 1pyBCk-0000pz-VO for bug-gnu-emacs@gnu.org; Sun, 14 May 2023 08:52:03 -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 1pyBCk-0000hx-Md for bug-gnu-emacs@gnu.org; Sun, 14 May 2023 08:52:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pyBCk-000224-5S for bug-gnu-emacs@gnu.org; Sun, 14 May 2023 08:52:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Jimmy Wong Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 14 May 2023 12:52:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 63338 X-GNU-PR-Package: emacs Original-Received: via spool by 63338-submit@debbugs.gnu.org id=B63338.16840687147797 (code B ref 63338); Sun, 14 May 2023 12:52:02 +0000 Original-Received: (at 63338) by debbugs.gnu.org; 14 May 2023 12:51:54 +0000 Original-Received: from localhost ([127.0.0.1]:40550 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pyBCb-00021f-GH for submit@debbugs.gnu.org; Sun, 14 May 2023 08:51:53 -0400 Original-Received: from mail-wr1-f54.google.com ([209.85.221.54]:47338) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pyBCZ-00021S-7w for 63338@debbugs.gnu.org; Sun, 14 May 2023 08:51:52 -0400 Original-Received: by mail-wr1-f54.google.com with SMTP id ffacd0b85a97d-30786c87cdaso8981028f8f.2 for <63338@debbugs.gnu.org>; Sun, 14 May 2023 05:51:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1684068705; x=1686660705; h=mime-version:subject:references:in-reply-to:message-id:cc:to:from :date:from:to:cc:subject:date:message-id:reply-to; bh=NjrX+YSmGjcz8gSmyVhyS4Kuf9DaPVO4YfHYtu3wYA0=; b=NWxirBCZguKKTlsPOrlWVsmyFv9CSiZwV18AE7RKJnaG4jFR2yZzQLsA0L4Xj/iFp4 41LqDZKrOA3LAnlzJ2FgTz4lRZrnudlQ5VFT0EVyeccjfI2rHrZEVaeW377dZ3ua6Sj7 Cgsbp2rBgFwJo15N89eDJZtx9jY+1f7tMBmYSrc2hQSKvuNzSq2ANWe1Tg4tyVZHnWTd cvP1oBo9EVOfIFYNawA7vfhB+IZtQYQ8kAJEnTE4rD2A8jAINNbfL9YvYVp4hXfASLwP Y4ISEGu15LOy3G2D5zea/OtNBo3YCctY7pLwBVbkXlooFH2r9iyOlJpXoykb2wHKM6s8 rzFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684068705; x=1686660705; h=mime-version:subject:references:in-reply-to:message-id:cc:to:from :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=NjrX+YSmGjcz8gSmyVhyS4Kuf9DaPVO4YfHYtu3wYA0=; b=D9IgQE/wcnUPLq2XtnDT4dh6Ky+x+lf0UosBtgisQRWzA4fgWAZR9L7s6cNFi6YiCR +y6HFileIIumXgijy0yOxmy1si3/vRrml5exI+Vk5YygznEWQtyLeVa6uQJzSR2Ial1W JhNjGeVm0V8efBjvW0KGicjLfxWCC0gGXxJtw7gGpn4QHbwKoUSjK5UNgEAt+jPhiHVN zI3tXFc+YA31c8+fp0v0z+CdB2cAhH6R+EvBuNmVu4333baeuQgxPDgegrmACvLoHArg 5pszk2kAAUMgyU5h/o97TBthhon5iwz+Vfmt3NYsKIR8ZmUp4vVR+hzWsbtbAc3MxL51 OXpw== X-Gm-Message-State: AC+VfDwzOx9U0+U6nBajnfRA/qRjFbQlLgL6HloD1d4zAfONesVqc0Qf cuGryy3+egeI6KyPCTUCmAs= X-Google-Smtp-Source: ACHHUZ7Ydj74LqFUzZIZy3PTWnILpYkuR63Z1DG1Gh0ndzXMjWfiLL0jEC56FDsFOavjIGxyoeK2Hg== X-Received: by 2002:adf:e787:0:b0:2f6:519c:6aa6 with SMTP id n7-20020adfe787000000b002f6519c6aa6mr22993936wrm.9.1684068705061; Sun, 14 May 2023 05:51:45 -0700 (PDT) Original-Received: from [192.168.86.97] ([152.37.102.187]) by smtp.gmail.com with ESMTPSA id h14-20020a5d6e0e000000b0030631dcbea6sm29239808wrz.77.2023.05.14.05.51.44 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 14 May 2023 05:51:44 -0700 (PDT) In-Reply-To: <87mt28880k.fsf@posteo.net> X-Readdle-Message-ID: baf82aa8-e9b6-47b0-90db-a382dce77210@Spark 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:261714 Archived-At: --6460d95f_3c1fe6c6_b973 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline This is acceptable to me On 13 May 2023 at 6:18 PM +0100, Philip Kaludercic = , wrote: > ping=3F > > Philip Kaludercic writes: > > > Jimmy Wong writes: > > > > > I don=E2=80=99t think you should dedup the variable that could have= been > > > modified by something else such as package-install. This may make > > > debugging harder should package.el itself introduce a bug that > > > duplicates pacakages in the variable. How about just using good old= > > > add-to-list=3F > > > > The issue is that we want to go through package--save-selected-packag= es, > > that is given a new value to assign to =60package-selected-packages'.= > > An otherwise, I my understanding is that add-to-list is not conventio= nal > > in executed code. > > > > The alternative is to check for duplicates before invoking the functi= on: > > > > diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-= vc.el > > index e9794eac783..b967aaa3d4d 100644 > > --- a/lisp/emacs-lisp/package-vc.el > > +++ b/lisp/emacs-lisp/package-vc.el > > =40=40 -507,9 +507,10 =40=40 package-vc--unpack-1 > > (package--reload-previously-loaded new-desc))) > > > > ;; Mark package as selected > > - (package--save-selected-packages > > - (cons (package-desc-name pkg-desc) > > - package-selected-packages)) > > + (let ((name (package-desc-name pkg-desc))) > > + (unless (memq name package-selected-packages) > > + (package--save-selected-packages > > + (cons name package-selected-packages)))) > > (package--quickstart-maybe-refresh) > > > > ;; Confirm that the installation was successful > > > > > > > > > On 8 May 2023 at 1:03 PM +0100, Eli Zaretskii , wro= te: > > > > > Cc: 63338=40debbugs.gnu.org > > > > > =46rom: Philip Kaludercic > > > > > Date: Mon, 08 May 2023 10:36:55 +0000 > > > > > > > > > > Jimmy Yuen Ho Wong writes: > > > > > > > > > > > Reproduction: > > > > > > > > > > > > 0. (setq custom-file (const user-emacs-directory =22custom.el= =22)) > > > > > > 1. M-x package-vc-install company > > > > > > 2. M-x package-vc-install company RET y > > > > > > 3. C-x C-f =7E/.emacs/custom.el > > > > > > 4. Observe that =60company=60 has been listed twice under > > > > > > =60package-selected-packages=60. > > > > > > > > > > > > Expectation: > > > > > > > > > > > > Installing the same package twice should not result in its du= plication > > > > > > in =60package-selected-packages=60. > > > > > > > > > > An easy fix would be just to ensure that package-selected-packa= ges is > > > > > always deduplicated before assigning the value: > > > > > > > > This is OK for the emacs-29 branch, thanks. --6460d95f_3c1fe6c6_b973 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline
This is acceptable to me
On 13 May 2023 at 6:18 PM +0100, Ph= ilip Kaludercic <philipk=40posteo.net>, wrote:
ping=3F

Philip Kaludercic <philipk=40posteo.net> writes:

Jimmy Wong <wyuenho=40gmail.com> writ= es:

I don=E2=80=99t think you should dedup the = variable that could have been
modified by something else such as package-install. This may make
debugging harder should package.el itself introduce a bug that
duplicates pacakages in the variable. How about just using good old
= add-to-list=3F

The issue is that we want to go through package--save-selected-packages,<= br /> that is given a new value to assign to =60package-selected-packages'.
An otherwise, I my understanding is that add-to-list is not conventional<= br /> in executed code.

The alternative is to check for duplicates before invoking the function:<= br />
diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.e= l
index e9794eac783..b967aaa3d4d 100644
--- a/lisp/emacs-lisp/package-vc.el
+++ b/lisp/emacs-lisp/package-vc.el
=40=40 -507,9 +507,10 =40=40 package-vc--unpack-1
(package--reload-previously-loaded new-desc)))

;; Mark package as selected
- (package--save-selected-packages
- (cons (package-desc-name pkg-desc)
- package-selected-packages))
+ (let ((name (package-desc-name pkg-desc)))
+ (unless (memq name package-selected-packages)
+ (package--save-selected-packages
+ (cons name package-selected-packages))))
(package--quickstart-maybe-refresh)

;; Confirm that the installation was successful



On 8 May 2023 at 1:03 PM +0100, Eli Zaretsk= ii <eliz=40gnu.org>, wrote:
Cc: 63338=40debbugs.gnu.org
=46rom: Philip Kaludercic <philipk=40posteo.net>
Date: Mon, 08 May 2023 10:36:55 +0000

Jimmy Yuen Ho Wong <wyuenho=40gmail.com> writes:

Reproduction:

0. (setq custom-file (const user-emacs-directory =22custom.el=22))
1. M-x package-vc-install company
2. M-x package-vc-install company RET y
3. C-x C-f =7E/.emacs/custom.el
4. Observe that =60company=60 has been listed twice under
=60package-selected-packages=60.

Expectation:

Installing the same package twice should not result in its duplication in =60package-selected-packages=60.

An easy fix would be just to ensure that package-selected-packages is
always deduplicated before assigning the value:

This is OK for the emacs-29 branch, thanks.
--6460d95f_3c1fe6c6_b973--