bug#23874: duplicates in manifests are “installed” more than once

bug#23874: duplicates in manifests are “installed” more than once

[Ricardo Wurmus]

When there are duplicate references to package variables in a manifest,
the same package will appear to be installed into the same profile
multiple times.

Here’s a manitest:

~~~~~~~~~~~~~~~~~~~~~~
(use-package-modules admin)

;; so stressed!
(packages->manifest
 (list stress stress stress))
~~~~~~~~~~~~~~~~~~~~~~

And here I’m instantiating it:

~~~~~~~~~~~~~~~~~~~~~~
guix package -p /tmp/test --manifest=manitest
installing new manifest from 'manitest' with 3 entries
substitute: updating list of substitutes from 'https://mirror.hydra.gnu.org'... 100.0%
substitute: updating list of substitutes from 'https://mirror.hydra.gnu.org'... 100.0%
substitute: updating list of substitutes from 'https://hydra.gnu.org'... 100.0%
The following derivations will be built:
   /gnu/store/1w51615has971qjwb9xxxvms8q99zr1n-profile.drv
   /gnu/store/jv7a1bm41gjgakb70nym65gp370dd4xs-ca-certificate-bundle.drv
   /gnu/store/1rgv811cqd4qk45y28lbzf8199m4zasv-info-dir.drv
The following file will be downloaded:
   /gnu/store/m31bvg97q7zmd9bvbss81ilyka5gq2hf-stress-1.0.1

Found valid signature for /gnu/store/m31bvg97q7zmd9bvbss81ilyka5gq2hf-stress-1.0.1
From https://mirror.hydra.gnu.org/nar/m31bvg97q7zmd9bvbss81ilyka5gq2hf-stress-1.0.1
Downloading m31bvg…-stress-1.0.1 (29KiB installed)...
 stress-1.0.1                                                 7.2MiB/s 00:00 | 14KiB transferred
3 packages in profile
The following environment variable definitions may be needed:
   export PATH="/tmp/test/bin"
rwurmus in guix: guix package -p /tmp/test -I
stress	1.0.1	out	/gnu/store/m31bvg97q7zmd9bvbss81ilyka5gq2hf-stress-1.0.1
stress	1.0.1	out	/gnu/store/m31bvg97q7zmd9bvbss81ilyka5gq2hf-stress-1.0.1
stress	1.0.1	out	/gnu/store/m31bvg97q7zmd9bvbss81ilyka5gq2hf-stress-1.0.1
rwurmus in guix:
~~~~~~~~~~~~~~~~~~~~~~

No conflicts are reported, so no harm is done, but seemingly having the
very same package more than once in a profile might be confusing.

Should Guix issue a warning when the same variable is referenced more
than once (I don’t like this because there really is no problem), or
should Guix delete duplicates from the list before creating a profile
generation?

bug#23874: duplicates in manifests are “installed” more than once

[Ludovic Courtès]

Hi!

Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> skribis:

> No conflicts are reported, so no harm is done, but seemingly having the
> very same package more than once in a profile might be confusing.
>
> Should Guix issue a warning when the same variable is referenced more
> than once (I don’t like this because there really is no problem), or
> should Guix delete duplicates from the list before creating a profile
> generation?

I think it should both delete duplicates, and then error out when two or
more packages with the same name remain.  Further, this should take into
account propagated inputs.

I think this is a pretty radical change, though, and I wonder about the
amount of breakage it would create.  For instance, it means that one
could create a profile containing both magit-referring-to-git-2.8 and
git-2.9, or emms-referring-to-vorbis-tools-1.0 and vorbis-tools-2.0.
Concretely, that means one will no longer be able to upgrade magit
without also upgrading git, for instance (assuming they live in the same
profile.)

WDYT?

Thanks,
Ludo’.

bug#23874: duplicates in manifests are “installed” more than once

[zimoun]

Dear,

Reminder: the manifest containing duplicates packages, such that:

--8<---------------cut here---------------start------------->8---
(use-package-modules admin)

;; so stressed!
(packages->manifest
 (list stress stress stress))
--8<---------------cut here---------------end--------------->8---

using “guix package -m manif.scm” leads to:

--8<---------------cut here---------------start------------->8---
The following packages will be installed:
   stress 1.0.4
   stress 1.0.4
   stress 1.0.4

[...]

building profile with 3 packages...
--8<---------------cut here---------------end--------------->8---

And, another UI issue:

--8<---------------cut here---------------start------------->8---
$ guix package -p /tmp/test -I
stress  1.0.4   out     /gnu/store/cm2fg1h2ad6v6zqwiiv1avg1mv2jzn66-stress-1.0.4
stress  1.0.4   out     /gnu/store/cm2fg1h2ad6v6zqwiiv1avg1mv2jzn66-stress-1.0.4
stress  1.0.4   out     /gnu/store/cm2fg1h2ad6v6zqwiiv1avg1mv2jzn66-stress-1.0.4
--8<---------------cut here---------------end--------------->8---


Note that the same kind of issue appears with:

--8<---------------cut here---------------start------------->8---
$ guix package -p /tmp/stress -i stress -i htop -i stress -i htop -i stress
The following packages will be installed:
   htop   3.0.1
   htop   3.0.1
   stress 1.0.4
   stress 1.0.4
   stress 1.0.4

[...]

building profile with 2 packages
--8<---------------cut here---------------end--------------->8---

but list the installed packages is correct:

--8<---------------cut here---------------start------------->8---
$ guix package -p /tmp/stress -I
htop    3.0.1   out     /gnu/store/pb0q52cfy7vld42xbnys26179wcm4k89-htop-3.0.1
stress  1.0.4   out     /gnu/store/cm2fg1h2ad6v6zqwiiv1avg1mv2jzn66-stress-1.0.4
--8<---------------cut here---------------end--------------->8---


On Thu, 30 Jun 2016 at 23:14, ludo@gnu.org (Ludovic Courtès) wrote:
> Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> skribis:
>
>> No conflicts are reported, so no harm is done, but seemingly having the
>> very same package more than once in a profile might be confusing.
>>
>> Should Guix issue a warning when the same variable is referenced more
>> than once (I don’t like this because there really is no problem), or
>> should Guix delete duplicates from the list before creating a profile
>> generation?
>
> I think it should both delete duplicates, and then error out when two or
> more packages with the same name remain.  Further, this should take into
> account propagated inputs.

The duplicates in the list of ’packages->manifest’ could be deleted.  I
mean in guix/scripts/packages.scm:process-action tweak the
’concatenate-manifests’

--8<---------------cut here---------------start------------->8---
           (manifest (match files
                       (() (profile-manifest profile))
                       (_  (map-manifest-entries
                            manifest-entry-with-provenance
                            (concatenate-manifests
                             (map load-manifest files))))))
--8<---------------cut here---------------end--------------->8---

However, why should propagated inputs take into account?

> I think this is a pretty radical change, though, and I wonder about the
> amount of breakage it would create.  For instance, it means that one
> could create a profile containing both magit-referring-to-git-2.8 and
> git-2.9, or emms-referring-to-vorbis-tools-1.0 and vorbis-tools-2.0.
> Concretely, that means one will no longer be able to upgrade magit
> without also upgrading git, for instance (assuming they live in the same
> profile.)

Well, it seems going further that only detects the same package asked to
be installed several times with the same transaction.

All the best,
simon

For reference: <http://issues.guix.gnu.org/issue/23874>.

bug#23874: [PATCH 1/2] profiles: Remove duplicates in manifest transactions.

[Leo Prikler]

* guix/profiles.scm (manifest-transaction-effects): Delete duplicates in
install and remove.  Let multiple upgrades and downgrades shadow previous
transactions of the same kind.
---
 guix/profiles.scm | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 1b15257210..99b7dbf299 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -724,8 +724,11 @@ replace it."
     (match input
       (()
        (let ((remove (manifest-transaction-remove transaction)))
-         (values (manifest-matching-entries manifest remove)
-                 (reverse install) (reverse upgrade) (reverse downgrade))))
+         (values (delete-duplicates
+                  (manifest-matching-entries manifest remove)
+                  manifest-entry=?)
+                 (delete-duplicates (reverse install) manifest-entry=?)
+                 (reverse upgrade) (reverse downgrade))))
       ((entry rest ...)
        ;; Check whether installing ENTRY corresponds to the installation of a
        ;; new package or to an upgrade.
@@ -740,10 +743,10 @@ replace it."
          (loop rest
                (if previous install (cons entry install))
                (if (and previous newer?)
-                   (alist-cons previous entry upgrade)
+                   (assoc-set! upgrade previous entry)
                    upgrade)
                (if (and previous (not newer?))
-                   (alist-cons previous entry downgrade)
+                   (assoc-set! downgrade previous entry)
                    downgrade)))))))
 
 (define (manifest-perform-transaction manifest transaction)
-- 
2.29.2

bug#23874: [PATCH 2/2] profiles: Delete duplicate manifest entries in packages->manifest.

[Leo Prikler]

* gnu/profiles.scm (packages->manifest): Delete duplicate entries.
---
 guix/profiles.scm | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 99b7dbf299..14b98852bd 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -399,22 +399,24 @@ denoting a specific output of a package."
                 'inferior-package->manifest-entry))
 
   (manifest
-   (map (match-lambda
-          (((? package? package) output)
-           (package->manifest-entry package output))
-          ((? package? package)
-           (package->manifest-entry package))
-          ((thing output)
-           (if inferiors-loaded?
-               ((inferior->entry) thing output)
-               (throw 'wrong-type-arg 'packages->manifest
-                      "Wrong package object: ~S" (list thing) (list thing))))
-          (thing
-           (if inferiors-loaded?
-               ((inferior->entry) thing)
-               (throw 'wrong-type-arg 'packages->manifest
-                      "Wrong package object: ~S" (list thing) (list thing)))))
-        packages)))
+   (delete-duplicates
+    (map (match-lambda
+           (((? package? package) output)
+            (package->manifest-entry package output))
+           ((? package? package)
+            (package->manifest-entry package))
+           ((thing output)
+            (if inferiors-loaded?
+                ((inferior->entry) thing output)
+                (throw 'wrong-type-arg 'packages->manifest
+                       "Wrong package object: ~S" (list thing) (list thing))))
+           (thing
+            (if inferiors-loaded?
+                ((inferior->entry) thing)
+                (throw 'wrong-type-arg 'packages->manifest
+                       "Wrong package object: ~S" (list thing) (list thing)))))
+         packages)
+    manifest-entry=?)))
 
 (define (manifest->gexp manifest)
   "Return a representation of MANIFEST as a gexp."
-- 
2.29.2

bug#23874: [PATCH 1/2] profiles: Remove duplicates in manifest transactions.

[Mark H Weaver]

Hi,

Leo Prikler <leo.prikler@student.tugraz.at> writes:

> * guix/profiles.scm (manifest-transaction-effects): Delete duplicates in
> install and remove.  Let multiple upgrades and downgrades shadow previous
> transactions of the same kind.
> ---
>  guix/profiles.scm | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/guix/profiles.scm b/guix/profiles.scm
> index 1b15257210..99b7dbf299 100644
> --- a/guix/profiles.scm
> +++ b/guix/profiles.scm
[...]
> @@ -740,10 +743,10 @@ replace it."
>           (loop rest
>                 (if previous install (cons entry install))
>                 (if (and previous newer?)
> -                   (alist-cons previous entry upgrade)
> +                   (assoc-set! upgrade previous entry)
>                     upgrade)
>                 (if (and previous (not newer?))
> -                   (alist-cons previous entry downgrade)
> +                   (assoc-set! downgrade previous entry)
>                     downgrade)))))))

We should avoid mutating existing list structure, as done above with
'assoc-set!'.

     Thanks,
       Mark

bug#23874: [PATCH 1/2] profiles: Remove duplicates in manifest transactions.

[Leo Prikler]

[-- Attachment #1: Type: text/plain, Size: 1509 bytes --]

Hi Mark,

Am Mittwoch, den 02.12.2020, 15:26 -0500 schrieb Mark H Weaver:
> Hi,
> 
> Leo Prikler <leo.prikler@student.tugraz.at> writes:
> 
> > * guix/profiles.scm (manifest-transaction-effects): Delete
> > duplicates in
> > install and remove.  Let multiple upgrades and downgrades shadow
> > previous
> > transactions of the same kind.
> > ---
> >  guix/profiles.scm | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/guix/profiles.scm b/guix/profiles.scm
> > index 1b15257210..99b7dbf299 100644
> > --- a/guix/profiles.scm
> > +++ b/guix/profiles.scm
> [...]
> > @@ -740,10 +743,10 @@ replace it."
> >           (loop rest
> >                 (if previous install (cons entry install))
> >                 (if (and previous newer?)
> > -                   (alist-cons previous entry upgrade)
> > +                   (assoc-set! upgrade previous entry)
> >                     upgrade)
> >                 (if (and previous (not newer?))
> > -                   (alist-cons previous entry downgrade)
> > +                   (assoc-set! downgrade previous entry)
> >                     downgrade)))))))
> 
> We should avoid mutating existing list structure, as done above with
> 'assoc-set!'.
In this case the bug is not with mutating existing structures (as those
are allocated only for this loop), but in not checking, whether the
entries are the same.  Either way, I've now implemented a version, that
should fix both wrong-doings (see attachment).

Regards, Leo

[-- Attachment #2: 0001-profiles-Remove-duplicates-in-manifest-transactions.patch --]
[-- Type: text/x-patch, Size: 1952 bytes --]

From 67ae06895b0d6f2f31410cc32d733bfa098102d6 Mon Sep 17 00:00:00 2001
From: Leo Prikler <leo.prikler@student.tugraz.at>
Date: Wed, 2 Dec 2020 14:06:52 +0100
Subject: [PATCH 1/2] profiles: Remove duplicates in manifest transactions.

* guix/profiles.scm (manifest-transaction-effects): Delete duplicates in
install and remove.  Let multiple upgrades and downgrades shadow previous
transactions of the same kind.
---
 guix/profiles.scm | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 1b15257210..034591eb79 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -716,6 +716,12 @@ replace it."
     (manifest-pattern
       (name   (manifest-entry-name entry))
       (output (manifest-entry-output entry))))
+  (define manifest-entry-pair=?
+    (match-lambda*
+      (((m1a . m2a) (m1b . m2b))
+       (and (manifest-entry=? m1a m1b)
+            (manifest-entry=? m2a m2b)))
+      (_ #f)))
 
   (let loop ((input     (manifest-transaction-install transaction))
              (install   '())
@@ -724,8 +730,16 @@ replace it."
     (match input
       (()
        (let ((remove (manifest-transaction-remove transaction)))
-         (values (manifest-matching-entries manifest remove)
-                 (reverse install) (reverse upgrade) (reverse downgrade))))
+         (values (delete-duplicates
+                  (manifest-matching-entries manifest remove)
+                  manifest-entry=?)
+                 (delete-duplicates (reverse install) manifest-entry=?)
+                 (delete-duplicates
+                  (reverse upgrade)
+                  manifest-entry-pair=?)
+                 (delete-duplicates
+                  (reverse downgrade)
+                  manifest-entry-pair=?))))
       ((entry rest ...)
        ;; Check whether installing ENTRY corresponds to the installation of a
        ;; new package or to an upgrade.
-- 
2.29.2

bug#23874: [PATCH 1/2] profiles: Remove duplicates in manifest transactions.

[zimoun]

Hi Leo,

Thank you for working in this old bug.

On Wed, 02 Dec 2020 at 23:25, Leo Prikler <leo.prikler@student.tugraz.at> wrote:

> entries are the same.  Either way, I've now implemented a version, that
> should fix both wrong-doings (see attachment).

Is it a replacement of PATCH 1/2 only or both?  Well, could you instead
send a v2 (–reroll-count).  It eases for applying and reviewing.


Thank you again for trying to close this old bug.


Cheers,
simon

bug#23874: [PATCH v2 1/2] profiles: Remove duplicates in manifest transactions.

[Leo Prikler]

* guix/profiles.scm (manifest-transaction-effects): Delete duplicate effects.
---
 guix/profiles.scm | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 1b15257210..034591eb79 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -716,6 +716,12 @@ replace it."
     (manifest-pattern
       (name   (manifest-entry-name entry))
       (output (manifest-entry-output entry))))
+  (define manifest-entry-pair=?
+    (match-lambda*
+      (((m1a . m2a) (m1b . m2b))
+       (and (manifest-entry=? m1a m1b)
+            (manifest-entry=? m2a m2b)))
+      (_ #f)))

   (let loop ((input     (manifest-transaction-install transaction))
              (install   '())
@@ -724,8 +730,16 @@ replace it."
     (match input
       (()
        (let ((remove (manifest-transaction-remove transaction)))
-         (values (manifest-matching-entries manifest remove)
-                 (reverse install) (reverse upgrade) (reverse downgrade))))
+         (values (delete-duplicates
+                  (manifest-matching-entries manifest remove)
+                  manifest-entry=?)
+                 (delete-duplicates (reverse install) manifest-entry=?)
+                 (delete-duplicates
+                  (reverse upgrade)
+                  manifest-entry-pair=?)
+                 (delete-duplicates
+                  (reverse downgrade)
+                  manifest-entry-pair=?))))
       ((entry rest ...)
        ;; Check whether installing ENTRY corresponds to the installation of a
        ;; new package or to an upgrade.
--
2.29.2

bug#23874: [PATCH v2 2/2] profiles: Delete duplicate manifest entries in packages->manifest.

[Leo Prikler]

* gnu/profiles.scm (packages->manifest): Delete duplicate entries.
---
 guix/profiles.scm | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 034591eb79..59a313ea08 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -399,22 +399,24 @@ denoting a specific output of a package."
                 'inferior-package->manifest-entry))
 
   (manifest
-   (map (match-lambda
-          (((? package? package) output)
-           (package->manifest-entry package output))
-          ((? package? package)
-           (package->manifest-entry package))
-          ((thing output)
-           (if inferiors-loaded?
-               ((inferior->entry) thing output)
-               (throw 'wrong-type-arg 'packages->manifest
-                      "Wrong package object: ~S" (list thing) (list thing))))
-          (thing
-           (if inferiors-loaded?
-               ((inferior->entry) thing)
-               (throw 'wrong-type-arg 'packages->manifest
-                      "Wrong package object: ~S" (list thing) (list thing)))))
-        packages)))
+   (delete-duplicates
+    (map (match-lambda
+           (((? package? package) output)
+            (package->manifest-entry package output))
+           ((? package? package)
+            (package->manifest-entry package))
+           ((thing output)
+            (if inferiors-loaded?
+                ((inferior->entry) thing output)
+                (throw 'wrong-type-arg 'packages->manifest
+                       "Wrong package object: ~S" (list thing) (list thing))))
+           (thing
+            (if inferiors-loaded?
+                ((inferior->entry) thing)
+                (throw 'wrong-type-arg 'packages->manifest
+                       "Wrong package object: ~S" (list thing) (list thing)))))
+         packages)
+    manifest-entry=?)))
 
 (define (manifest->gexp manifest)
   "Return a representation of MANIFEST as a gexp."
-- 
2.29.2

bug#23874: [PATCH 1/2] profiles: Remove duplicates in manifest transactions.

[Ludovic Courtès]

Hi Leo,

Could you explain what each patch fixes, and perhaps add a test case for
each that illustrates that?

Thanks,
Ludo’.

bug#23874: [PATCH v3 1/2] profiles: Remove duplicates in manifest transactions.

[Leo Prikler]

* guix/profiles.scm (manifest-transaction-effects): Delete duplicates in
install and remove.  Let multiple upgrades and downgrades shadow previous
transactions of the same kind.
* tests/profiles.scm
("manifest-transaction-effects no double install or upgrades")
("manifest-transaction-effects no double downgrade")
("manifest-transaction-effects no double removal"): New tests.
---
 guix/profiles.scm  | 18 ++++++++++++++++--
 tests/profiles.scm | 28 ++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 1b15257210..034591eb79 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -716,6 +716,12 @@ replace it."
     (manifest-pattern
       (name   (manifest-entry-name entry))
       (output (manifest-entry-output entry))))
+  (define manifest-entry-pair=?
+    (match-lambda*
+      (((m1a . m2a) (m1b . m2b))
+       (and (manifest-entry=? m1a m1b)
+            (manifest-entry=? m2a m2b)))
+      (_ #f)))
 
   (let loop ((input     (manifest-transaction-install transaction))
              (install   '())
@@ -724,8 +730,16 @@ replace it."
     (match input
       (()
        (let ((remove (manifest-transaction-remove transaction)))
-         (values (manifest-matching-entries manifest remove)
-                 (reverse install) (reverse upgrade) (reverse downgrade))))
+         (values (delete-duplicates
+                  (manifest-matching-entries manifest remove)
+                  manifest-entry=?)
+                 (delete-duplicates (reverse install) manifest-entry=?)
+                 (delete-duplicates
+                  (reverse upgrade)
+                  manifest-entry-pair=?)
+                 (delete-duplicates
+                  (reverse downgrade)
+                  manifest-entry-pair=?))))
       ((entry rest ...)
        ;; Check whether installing ENTRY corresponds to the installation of a
        ;; new package or to an upgrade.
diff --git a/tests/profiles.scm b/tests/profiles.scm
index 055924ba3e..f0a1a1d11c 100644
--- a/tests/profiles.scm
+++ b/tests/profiles.scm
@@ -183,6 +183,16 @@
            (equal? (list glibc) install)
            (equal? (list (cons guile-1.8.8 guile-2.0.9)) upgrade)))))
 
+(test-assert "manifest-transaction-effects no double install or upgrades"
+  (let* ((m0 (manifest (list guile-1.8.8)))
+         (t  (manifest-transaction
+              (install (list guile-2.0.9 glibc glibc)))))
+    (let-values (((remove install upgrade downgrade)
+                  (manifest-transaction-effects m0 t)))
+      (and (null? remove) (null? downgrade)
+           (equal? (list glibc) install)
+           (equal? (list (cons guile-1.8.8 guile-2.0.9)) upgrade)))))
+
 (test-assert "manifest-transaction-effects and downgrades"
   (let* ((m0 (manifest (list guile-2.0.9)))
          (t  (manifest-transaction (install (list guile-1.8.8)))))
@@ -191,6 +201,14 @@
       (and (null? remove) (null? install) (null? upgrade)
            (equal? (list (cons guile-2.0.9 guile-1.8.8)) downgrade)))))
 
+(test-assert "manifest-transaction-effects no double downgrade"
+  (let* ((m0 (manifest (list guile-2.0.9)))
+         (t  (manifest-transaction (install (list guile-1.8.8 guile-1.8.8)))))
+    (let-values (((remove install upgrade downgrade)
+                  (manifest-transaction-effects m0 t)))
+      (and (null? remove) (null? install) (null? upgrade)
+           (equal? (list (cons guile-2.0.9 guile-1.8.8)) downgrade)))))
+
 (test-assert "manifest-transaction-effects and pseudo-upgrades"
   (let* ((m0 (manifest (list guile-2.0.9)))
          (t  (manifest-transaction (install (list guile-2.0.9)))))
@@ -209,6 +227,16 @@
     (and (manifest-transaction-removal-candidate? guile-2.0.9 t)
          (not (manifest-transaction-removal-candidate? glibc t)))))
 
+(test-assert "manifest-transaction-effects no double removal"
+  (let* ((m0 (manifest (list guile-2.0.9)))
+         (t  (manifest-transaction
+              (remove (list (manifest-pattern (name "guile")))))))
+    (let-values (((remove install upgrade downgrade)
+                  (manifest-transaction-effects m0 t)))
+      (and (= 1 (length remove))
+           (manifest-transaction-removal-candidate? guile-2.0.9 t)
+           (null? install) (null? downgrade) (null? upgrade)))))
+
 (test-assertm "profile-derivation"
   (mlet* %store-monad
       ((entry ->   (package->manifest-entry %bootstrap-guile))
-- 
2.29.2

bug#23874: [PATCH v3 2/2] profiles: Delete duplicate manifest entries in packages->manifest.

[Leo Prikler]

* gnu/profiles.scm (packages->manifest): Delete duplicate entries.
* tests/profiles.scm ("packages->manifest, no duplicates"): New test.
---
 guix/profiles.scm  | 34 ++++++++++++++++++----------------
 tests/profiles.scm | 10 ++++++++++
 2 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 034591eb79..59a313ea08 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -399,22 +399,24 @@ denoting a specific output of a package."
                 'inferior-package->manifest-entry))
 
   (manifest
-   (map (match-lambda
-          (((? package? package) output)
-           (package->manifest-entry package output))
-          ((? package? package)
-           (package->manifest-entry package))
-          ((thing output)
-           (if inferiors-loaded?
-               ((inferior->entry) thing output)
-               (throw 'wrong-type-arg 'packages->manifest
-                      "Wrong package object: ~S" (list thing) (list thing))))
-          (thing
-           (if inferiors-loaded?
-               ((inferior->entry) thing)
-               (throw 'wrong-type-arg 'packages->manifest
-                      "Wrong package object: ~S" (list thing) (list thing)))))
-        packages)))
+   (delete-duplicates
+    (map (match-lambda
+           (((? package? package) output)
+            (package->manifest-entry package output))
+           ((? package? package)
+            (package->manifest-entry package))
+           ((thing output)
+            (if inferiors-loaded?
+                ((inferior->entry) thing output)
+                (throw 'wrong-type-arg 'packages->manifest
+                       "Wrong package object: ~S" (list thing) (list thing))))
+           (thing
+            (if inferiors-loaded?
+                ((inferior->entry) thing)
+                (throw 'wrong-type-arg 'packages->manifest
+                       "Wrong package object: ~S" (list thing) (list thing)))))
+         packages)
+    manifest-entry=?)))
 
 (define (manifest->gexp manifest)
   "Return a representation of MANIFEST as a gexp."
diff --git a/tests/profiles.scm b/tests/profiles.scm
index f0a1a1d11c..2dec42bec1 100644
--- a/tests/profiles.scm
+++ b/tests/profiles.scm
@@ -384,6 +384,16 @@
            (manifest-entry-search-paths
             (package->manifest-entry mpl)))))
 
+(test-assert "packages->manifest, no duplicates"
+  (let ((expected
+         (manifest
+          (list
+           (package->manifest-entry packages:guile-2.2))))
+        (manifest (packages->manifest
+                   (list packages:guile-2.2 packages:guile-2.2))))
+    (every manifest-entry=? (manifest-entries expected)
+           (manifest-entries manifest))))
+
 (test-equal "packages->manifest, propagated inputs"
   (map (match-lambda
          ((label package)
-- 
2.29.2

bug#23874: [PATCH 1/2] profiles: Remove duplicates in manifest transactions.

[Leo Prikler]

Hi Ludo

Am Samstag, den 05.12.2020, 16:38 +0100 schrieb Ludovic Courtès:
> Hi Leo,
> 
> Could you explain what each patch fixes, and perhaps add a test case
> for
> each that illustrates that?
> 
> Thanks,
> Ludo’.
Tests sent along with v3.  These patches remove duplicates from
manifests constructed by packages->manifest and manifest-transaction-
effects, so that the UI reports them only once even if they're
specified multiple times (e.g. by "guix package -i stress stress
stress" or by more accidental copying of package names).  The first
patch does so for computing transactions (i.e. when using -i, -u and
-r), the second for manifests (-m).

Regards,
Leo

bug#23874: [PATCH 1/2] profiles: Remove duplicates in manifest transactions.

[Ludovic Courtès]

Hi,

Leo Prikler <leo.prikler@student.tugraz.at> skribis:

> Tests sent along with v3.  These patches remove duplicates from
> manifests constructed by packages->manifest and manifest-transaction-
> effects, so that the UI reports them only once even if they're
> specified multiple times (e.g. by "guix package -i stress stress
> stress" or by more accidental copying of package names).  The first
> patch does so for computing transactions (i.e. when using -i, -u and
> -r), the second for manifests (-m).

Oooh I see, sorry for overlooking the original bug report.

I added a “Fixes” line in the commit log and applied v3.

Thanks!

Ludo’.

bug#23874: [PATCH 1/2] profiles: Remove duplicates in manifest transactions.

[zimoun]

Hi,

On Mon, 07 Dec 2020 at 10:16, Ludovic Courtès <ludo@gnu.org> wrote:

> I added a “Fixes” line in the commit log and applied v3.

Cool!  Thank you Leo.  Cloing such old bug as #23874 is nice. :-)


Thanks,
simon