Subject: Re: bug#22139: Indirect dependencies are not grafted

From: [email-masked] (Ludovic Courtès)
Date: Tue, 01 Mar 2016 00:35:59 +0100

[email-masked] (Ludovic Courtès) skribis:

> The grafting mechanism has a shortcoming: it is not recursive.
>
> Suppose we use ‘replace’ to provide a patch libpng.  If a package has a
> direct dependency on libpng, it is appropriately grafted to refer to the
> new libpng.  However, if a package depends on libfoo, which in turn
> depends on libpng, then that package will keep referring to the old
> libfoo, which refers to the old libpng.

The ‘wip-recursive-grafts’ branch fixes that.  It also changes
‘graft-derivation’ to choose whether to graft something based on its
*run-time* dependencies (as reported by ‘guix gc -R’) instead of its
compile-time dependencies.

The advantage is that fewer things will be grafted; the disadvantage is
that things like --dry-run will seem to have no effect since sometimes,
the thing will start by building/downloading stuff.  I think the
advantage outweighs the disadvantage, but we’ll see how it goes in
practice.

There’s room for optimization in a few places, but overall it performs
well and there’s no performance regression in the absence of grafts
AFAICS.  So I think I may merge it real soon, possibly so we can use it
for the OpenSSL fix tomorrow and crash-test it.  Thoughts?

Here’s a patch I used to test grafting (it artificially adds a
‘replacement’ for OpenSSL that is slightly different and yields a
different derivation):

--- a/gnu/packages/tls.scm
+++ b/gnu/packages/tls.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012, 2013, 2014, 2015 Ludovic Courtès <[email-masked]>
+;;; Copyright © 2012, 2013, 2014, 2015, 2016 Ludovic Courtès <[email-masked]>
 ;;; Copyright © 2014, 2015, 2016 Mark H Weaver <[email-masked]>
 ;;; Copyright © 2014 Ian Denhardt <[email-masked]>
 ;;; Copyright © 2013, 2015 Andreas Enge <[email-masked]>
@@ -177,7 +177,7 @@ protocols, as well as to parse and write X.5009, PKCS 12, OpenPGP and other
 required structures.")
     (license license:lgpl2.1+)))
 
-(define-public openssl
+(define openssl/fixed
   (package
    (name "openssl")
    (version "1.0.2f")
@@ -191,9 +191,7 @@ required structures.")
             (sha256
              (base32
               "171fkdg9v6j29d962nh6kb79kfm8kkhy7n9makw39d7jvvj4wawk"))
-            (patches (map search-patch
-                          '("openssl-runpath.patch"
-                            "openssl-c-rehash.patch")))))
+            (patches (map search-patch '("openssl-runpath.patch")))))
    (build-system gnu-build-system)
    (native-inputs `(("perl" ,perl)))
    (arguments
@@ -282,6 +280,26 @@ required structures.")
    (license license:openssl)
    (home-page "http://www.openssl.org/")))
 
+(define-public openssl
+  (package
+    (inherit openssl/fixed)
+    (name "openssl")
+    (version "1.0.2f")
+    (source (origin
+              (method url-fetch)
+              (uri (list (string-append "ftp://ftp.openssl.org/source/"
+                                        name "-" version ".tar.gz")
+                         (string-append "ftp://ftp.openssl.org/source/old/"
+                                        (string-trim-right version char-set:letter)
+                                        "/" name "-" version ".tar.gz")))
+              (sha256
+               (base32
+                "171fkdg9v6j29d962nh6kb79kfm8kkhy7n9makw39d7jvvj4wawk"))
+              (patches (map search-patch
+                            '("openssl-runpath.patch"
+                              "openssl-c-rehash.patch")))))
+    (replacement openssl/fixed)))
+
 (define-public libressl
   (package
     (name "libressl")

Then you can run things like:

  guix gc -R $(guix build git | head -1) | grep openssl

and compare with:

  guix gc -R $(guix build git --no-grafts | head -1) | grep openssl

There should be exactly one ‘openssl’ reference in both cases; in the
first case it should be the replacement, and in the second case the
original.

Feedback very much welcome!

Ludo’.