GNU bug report logs - #43890
‘package-input-rewriting/spec’ can introduce unnecessary variants

Previous Next

Full log

Message #10 received at 43890 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 43890 <at> debbugs.gnu.org
Subject: Re: bug#43890: ‘package-input-rewriting/spec’ can introduce unnecessary variants
Date: Sun, 11 Oct 2020 15:09:37 +0200

[Message part 1 (text/plain, inline)]

Ludovic Courtès <ludovic.courtes <at> inria.fr> skribis:

> $ guix describe
> Generacio 162	Oct 01 2020 00:23:38	(nuna)
>   guix 7607ace
>     repository URL: https://git.savannah.gnu.org/git/guix.git
>     branch: master
>     commit: 7607ace5091aea0157ba5c8a508129cc5fc4f931
> $ guix build inkscape --no-grafts -d
> /gnu/store/arjs5hb4wmy6dh5d3y8bbs808ki9abf8-inkscape-1.0.1.drv
> $ guix build inkscape --no-grafts -d --with-graft=glib=glib-networking
> /gnu/store/zd8mm3w6x9c97anfaly77fz28s5y3i5h-inkscape-1.0.1.drv
> $ guix build inkscape --no-grafts -d --with-graft=libreoffice=abiword
> /gnu/store/arjs5hb4wmy6dh5d3y8bbs808ki9abf8-inkscape-1.0.1.drv
>
> The last one is fine: it has no effect.
>
> The second one is problematic: since we’re using ‘--no-grafts’, the
> ‘--with-graft’ option should have absolutely no effect; yet, it yields a
> different derivation.
>
> On closer inspection, we see that the core issue is that
> ‘gobject-introspection’ in the second case ends up with ‘libffi’ twice
> in its ‘*-guile-builder’ script, a problem similar to
> <https://issues.guix.gnu.org/38100>.  (‘libffi’ is propagated by both
> ‘glib’ and ‘gobject-introspection’.)

Here are test cases for this:

[Message part 2 (text/x-patch, inline)]

diff --git a/tests/guix-build.sh b/tests/guix-build.sh
index 6dbb53206e..1cfff329f1 100644
--- a/tests/guix-build.sh
+++ b/tests/guix-build.sh
@@ -262,6 +262,12 @@ drv1=`guix build glib -d`
 drv2=`guix build glib -d --with-input=libreoffice=inkscape`
 test "$drv1" = "$drv2"
 
+# '--with-graft' should have no effect when using '--no-grafts'.
+# See <https://bugs.gnu.org/43890>.
+drv1=`guix build inkscape -d --no-grafts`
+drv2=`guix build inkscape -d --no-grafts --with-graft=glib=glib-networking`
+test "$drv1" = "$drv2"
+
 # Rewriting implicit inputs.
 drv1=`guix build hello -d`
 drv2=`guix build hello -d --with-input=gcc=gcc-toolchain`
diff --git a/tests/packages.scm b/tests/packages.scm
index 5d5abcbd76..e7c43b8939 100644
--- a/tests/packages.scm
+++ b/tests/packages.scm
@@ -1419,7 +1419,8 @@
                  (build-system trivial-build-system)
                  (inputs `(("dep" ,dep1)))))
          (rewrite (package-input-rewriting/spec
-                   `(("coreutils" . ,(const sed)))))
+                   `(("coreutils" . ,(const sed)))
+                   #:deep? #f))                  ;avoid creating circular deps
          (p1      (rewrite p0)))
     (match (package-inputs p1)
       ((("dep" dep))
@@ -1430,6 +1431,49 @@
           (derivation-file-name
            (package-derivation %store coreutils))))))))
 
+(test-assert "package-input-rewriting/spec, identity"
+  ;; Make sure that 'package-input-rewriting/spec' doesn't gratuitously
+  ;; introduce variants.  In this case, the LIBFFI propagated input should not
+  ;; be duplicated when passing GOBJECT through REWRITE.
+  ;; See <https://issues.guix.gnu.org/43890>.
+  (let* ((libffi  (dummy-package "libffi"
+                    (build-system trivial-build-system)))
+         (glib    (dummy-package "glib"
+                    (build-system trivial-build-system)
+                    (propagated-inputs `(("libffi" ,libffi)))))
+         (gobject (dummy-package "gobject-introspection"
+                    (build-system trivial-build-system)
+                    (inputs `(("glib" ,glib)))
+                    (propagated-inputs `(("libffi" ,libffi)))))
+         (rewrite (package-input-rewriting/spec
+                   `(("glib" . ,identity)))))
+    (and (= (length (package-transitive-inputs gobject))
+            (length (package-transitive-inputs (rewrite gobject))))
+         (string=? (derivation-file-name
+                    (package-derivation %store (rewrite gobject)))
+                   (derivation-file-name
+                    (package-derivation %store gobject))))))
+
+(test-assert "package-input-rewriting, identity"
+  ;; Similar to the test above, but with 'package-input-rewriting'.
+  ;; See <https://issues.guix.gnu.org/43890>.
+  (let* ((libffi  (dummy-package "libffi"
+                    (build-system trivial-build-system)))
+         (glib    (dummy-package "glib"
+                    (build-system trivial-build-system)
+                    (propagated-inputs `(("libffi" ,libffi)))))
+         (gobject (dummy-package "gobject-introspection"
+                    (build-system trivial-build-system)
+                    (inputs `(("glib" ,glib)))
+                    (propagated-inputs `(("libffi" ,libffi)))))
+         (rewrite (package-input-rewriting `((,glib . ,glib)))))
+    (and (= (length (package-transitive-inputs gobject))
+            (length (package-transitive-inputs (rewrite gobject))))
+         (string=? (derivation-file-name
+                    (package-derivation %store (rewrite gobject)))
+                   (derivation-file-name
+                    (package-derivation %store gobject))))))
+
 (test-equal "package-patched-vulnerabilities"
   '(("CVE-2015-1234")
     ("CVE-2016-1234" "CVE-2018-4567")

[Message part 3 (text/plain, inline)]

Unfortunately it’s again pretty hard to fix.

We should rely less on pointer equality (and not break “equational
reasoning”), but OTOH (1) we need it for performance reasons, and (2)
packages are parameterized in arbitrary ways (its thunked fields can
depend on (%current-system), etc.) which makes it impossible to define a
faithful ‘package=?’ predicate.

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.