unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
* bug#43890: ‘package-input-rewriting/spec’ can introduce unnecessary variants
@ 2020-10-09 20:14 Ludovic Courtès
  2020-10-11 13:09 ` Ludovic Courtès
  2020-10-20 14:35 ` Ludovic Courtès
  0 siblings, 2 replies; 3+ messages in thread
From: Ludovic Courtès @ 2020-10-09 20:14 UTC (permalink / raw)
  To: 43890

Consider this example:

--8<---------------cut here---------------start------------->8---
$ 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
--8<---------------cut here---------------end--------------->8---

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’.)

Ludo’.




^ permalink raw reply	[flat|nested] 3+ messages in thread

* bug#43890: ‘package-input-rewriting/spec’ can introduce unnecessary variants
  2020-10-09 20:14 bug#43890: ‘package-input-rewriting/spec’ can introduce unnecessary variants Ludovic Courtès
@ 2020-10-11 13:09 ` Ludovic Courtès
  2020-10-20 14:35 ` Ludovic Courtès
  1 sibling, 0 replies; 3+ messages in thread
From: Ludovic Courtès @ 2020-10-11 13:09 UTC (permalink / raw)
  To: 43890

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

Ludovic Courtès <ludovic.courtes@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:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 3764 bytes --]

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")

[-- Attachment #3: Type: text/plain, Size: 388 bytes --]


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’.

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* bug#43890: ‘package-input-rewriting/spec’ can introduce unnecessary variants
  2020-10-09 20:14 bug#43890: ‘package-input-rewriting/spec’ can introduce unnecessary variants Ludovic Courtès
  2020-10-11 13:09 ` Ludovic Courtès
@ 2020-10-20 14:35 ` Ludovic Courtès
  1 sibling, 0 replies; 3+ messages in thread
From: Ludovic Courtès @ 2020-10-20 14:35 UTC (permalink / raw)
  To: 43890-done

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

> Consider this example:
>
> $ 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.

Fixed in 8db4ebb0cd9bfdcf1aea63eb8d20eb6af0c87c93.  \o/

It makes ‘--with-debug-info’ more practical.

The difficulty is to find out where the difference is and what piece of
code introduced a non-eq?-but-equal package.  Likewise, the test suite
catches corner cases that can take a while to address.

Related to that, commit 6b4663363c061071c10209f71aed1017a241af6c deletes
duplicates in ‘bag->derivation’, which should make the whole thing less
sensitive to the introduction of non-eq?-but-equal packages in the
graph.

Ludo’.




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-10-20 14:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-09 20:14 bug#43890: ‘package-input-rewriting/spec’ can introduce unnecessary variants Ludovic Courtès
2020-10-11 13:09 ` Ludovic Courtès
2020-10-20 14:35 ` Ludovic Courtès

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).