‘package-input-rewriting/spec’ can introduce unnecessary variants

DoneSubmitted by Ludovic Courtès.
Details
2 participants
  • Ludovic Courtès
  • Ludovic Courtès
Owner
unassigned
Severity
important
L
L
Ludovic Courtès wrote on 9 Oct 22:14 +0200
(address . bug-guix@gnu.org)
87v9fji2cv.fsf@inria.fr
Consider this example:
Toggle snippet (14 lines)$ guix describeGeneracio 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 adifferent derivation.
On closer inspection, we see that the core issue is that‘gobject-introspection’ in the second case ends up with ‘libffi’ twicein its ‘*-guile-builder’ script, a problem similar tohttps://issues.guix.gnu.org/38100. (‘libffi’ is propagated by both‘glib’ and ‘gobject-introspection’.)
Ludo’.
L
L
Ludovic Courtès wrote on 9 Oct 22:22 +0200
control message for bug #43890
(address . control@debbugs.gnu.org)
87tuv3i1yz.fsf@gnu.org
severity 43890 importantquit
L
L
Ludovic Courtès wrote on 11 Oct 15:09 +0200
Re: bug#43890: ‘package-input-rewriting/spec ’ can introduce unnecessary variants
(address . 43890@debbugs.gnu.org)
87lfgcgb8u.fsf@gnu.org
Ludovic Courtès <ludovic.courtes@inria.fr> skribis:
Toggle quote (25 lines)> $ 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:
Toggle diff (81 lines)diff --git a/tests/guix-build.sh b/tests/guix-build.shindex 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.scmindex 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")
Unfortunately it’s again pretty hard to fix.
We should rely less on pointer equality (and not break “equationalreasoning”), but OTOH (1) we need it for performance reasons, and (2)packages are parameterized in arbitrary ways (its thunked fields candepend on (%current-system), etc.) which makes it impossible to define afaithful ‘package=?’ predicate.
Ludo’.
L
L
Ludovic Courtès wrote 29 hours ago
(address . 43890-done@debbugs.gnu.org)
878sc1dky2.fsf@gnu.org
Ludovic Courtès <ludovic.courtes@inria.fr> skribis:
Toggle quote (21 lines)> 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 ofcode introduced a non-eq?-but-equal package. Likewise, the test suitecatches corner cases that can take a while to address.
Related to that, commit 6b4663363c061071c10209f71aed1017a241af6c deletesduplicates in ‘bag->derivation’, which should make the whole thing lesssensitive to the introduction of non-eq?-but-equal packages in thegraph.
Ludo’.
Closed
?