duplicates in manifests are “installed” more than once

DoneSubmitted by Ricardo Wurmus.
Details
5 participants
  • Leo Prikler
  • Ludovic Courtès
  • Mark H Weaver
  • Ricardo Wurmus
  • zimoun
Owner
unassigned
Severity
normal
R
R
Ricardo Wurmus wrote on 30 Jun 2016 15:59
(address . bug-guix@gnu.org)
idjy45mvkuq.fsf@bimsb-sys02.mdc-berlin.net
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
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?
L
L
Ludovic Courtès wrote on 30 Jun 2016 23:14
Re: bug#23874: duplicates in manifests are “instal led” more than once
(name . Ricardo Wurmus)(address . ricardo.wurmus@mdc-berlin.de)(address . 23874@debbugs.gnu.org)
87bn2inzvr.fsf@gnu.org
Hi!

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

Toggle quote (8 lines)
> 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’.
Z
Z
zimoun wrote on 14 Sep 2020 20:15
(name . Ludovic Courtès)(address . ludo@gnu.org)
87a6xsql5w.fsf@gmail.com
Dear,

Reminder: the manifest containing duplicates packages, such that:

Toggle snippet (7 lines)
(use-package-modules admin)

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

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

Toggle snippet (10 lines)
The following packages will be installed:
stress 1.0.4
stress 1.0.4
stress 1.0.4

[...]

building profile with 3 packages...

And, another UI issue:

Toggle snippet (7 lines)
$ 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


Note that the same kind of issue appears with:

Toggle snippet (13 lines)
$ 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

but list the installed packages is correct:

Toggle snippet (6 lines)
$ 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


On Thu, 30 Jun 2016 at 23:14, ludo@gnu.org (Ludovic Courtès) wrote:
Toggle quote (14 lines)
> 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’

Toggle snippet (8 lines)
(manifest (match files
(() (profile-manifest profile))
(_ (map-manifest-entries
manifest-entry-with-provenance
(concatenate-manifests
(map load-manifest files))))))

However, why should propagated inputs take into account?

Toggle quote (8 lines)
> 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

L
L
Leo Prikler wrote on 2 Dec 2020 14:22
[PATCH 1/2] profiles: Remove duplicates in manifest transactions.
(address . 23874@debbugs.gnu.org)
20201202132244.30694-1-leo.prikler@student.tugraz.at
* 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(-)

Toggle diff (33 lines)
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
L
L
Leo Prikler wrote on 2 Dec 2020 14:22
[PATCH 2/2] profiles: Delete duplicate manifest entries in packages->manifest.
(address . 23874@debbugs.gnu.org)
20201202132244.30694-2-leo.prikler@student.tugraz.at
* gnu/profiles.scm (packages->manifest): Delete duplicate entries.
---
guix/profiles.scm | 34 ++++++++++++++++++----------------
1 file changed, 18 insertions(+), 16 deletions(-)

Toggle diff (47 lines)
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
M
M
Mark H Weaver wrote on 2 Dec 2020 21:26
Re: bug#23874: [PATCH 1/2] profiles: Remove duplicates in manifest transactions.
87ft4oeyyl.fsf@netris.org
Hi,

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

Toggle quote (11 lines)
> * 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
[...]
Toggle quote (12 lines)
> @@ -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
L
L
Leo Prikler wrote on 2 Dec 2020 23:25
b9e9701b14ef7759c2f0518f2ab31323f7073bc7.camel@student.tugraz.at
Hi Mark,

Am Mittwoch, den 02.12.2020, 15:26 -0500 schrieb Mark H Weaver:
Toggle quote (32 lines)
> 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
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(-)

Toggle diff (38 lines)
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
Z
Z
zimoun wrote on 3 Dec 2020 00:24
864kl323mj.fsf@gmail.com
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:

Toggle quote (3 lines)
> 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
L
L
Leo Prikler wrote on 3 Dec 2020 00:39
[PATCH v2 1/2] profiles: Remove duplicates in manifest transactions.
(address . 23874@debbugs.gnu.org)
20201202233901.12902-1-leo.prikler@student.tugraz.at
* guix/profiles.scm (manifest-transaction-effects): Delete duplicate effects.
---
guix/profiles.scm | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)

Toggle diff (36 lines)
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
L
L
Leo Prikler wrote on 3 Dec 2020 00:39
[PATCH v2 2/2] profiles: Delete duplicate manifest entries in packages->manifest.
(address . 23874@debbugs.gnu.org)
20201202233901.12902-2-leo.prikler@student.tugraz.at
* gnu/profiles.scm (packages->manifest): Delete duplicate entries.
---
guix/profiles.scm | 34 ++++++++++++++++++----------------
1 file changed, 18 insertions(+), 16 deletions(-)

Toggle diff (47 lines)
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
L
L
Ludovic Courtès wrote on 5 Dec 2020 16:38
Re: bug#23874: [PATCH 1/2] profiles: Remove duplicates in manifest transactions.
(name . Leo Prikler)(address . leo.prikler@student.tugraz.at)
87360k5kku.fsf@gnu.org
Hi Leo,

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

Thanks,
Ludo’.
L
L
Leo Prikler wrote on 5 Dec 2020 17:20
[PATCH v3 1/2] profiles: Remove duplicates in manifest transactions.
(address . 23874@debbugs.gnu.org)
20201205162010.26652-1-leo.prikler@student.tugraz.at
* 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(-)

Toggle diff (91 lines)
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
L
L
Leo Prikler wrote on 5 Dec 2020 17:20
[PATCH v3 2/2] profiles: Delete duplicate manifest entries in packages->manifest.
(address . 23874@debbugs.gnu.org)
20201205162010.26652-2-leo.prikler@student.tugraz.at
* 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(-)

Toggle diff (68 lines)
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
L
L
Leo Prikler wrote on 5 Dec 2020 17:29
Re: bug#23874: [PATCH 1/2] profiles: Remove duplicates in manifest transactions.
(name . Ludovic Courtès)(address . ludo@gnu.org)
99b77520efe23cd4ed7188c6dfcbcc4788644689.camel@student.tugraz.at
Hi Ludo

Am Samstag, den 05.12.2020, 16:38 +0100 schrieb Ludovic Courtès:
Toggle quote (8 lines)
> 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
L
L
Ludovic Courtès wrote on 7 Dec 2020 10:16
(name . Leo Prikler)(address . leo.prikler@student.tugraz.at)
87eek20yde.fsf@gnu.org
Hi,

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

Toggle quote (8 lines)
> 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’.
Closed
Z
Z
zimoun wrote on 7 Dec 2020 16:28
(address . 23874-done@debbugs.gnu.org)
861rg17i0c.fsf@gmail.com
Hi,

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

Toggle quote (2 lines)
> 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
Closed
?
Your comment

This issue is archived.

To comment on this conversation send email to 23874@debbugs.gnu.org