[bug#70303] [PATCH 0/2] Use guile-final for grafting.

[bug#70303] [PATCH 0/2] Use guile-final for grafting.

[Efraim Flashner]

Over the years the grafting code has changed bit by bit, with various
attempts to speed it up.  By switching the grafts to not use parallelism
in rewriting the leaves we can finally switch the guile-for-grafts to be
guile-final.  The segfault is still there if we perform the grafts in
parallel, but I believe it is fast enough that it should be fine to do
them sequentially.

There's probably room in replace-store-references in (guix build graft)
for changes if we're not going to use guile-2.0 here anymore.

There are also a number of default guiles in (guix grafts) which should
maybe be %guile-for-grafts instead of %guile-for-build.

Efraim Flashner (2):
  graft: Remove work-around for old guile.
  graft: Perform grafts with guile-final.

 guix/build/graft.scm | 55 +++++---------------------------------------
 guix/packages.scm    |  6 ++---
 2 files changed, 8 insertions(+), 53 deletions(-)


base-commit: 51de844a0ff6ea224367a384092896bce6848b9f
prerequisite-patch-id: ea387a4f9d860397a26c840c11c8742f0ac70fc3
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#70303] [PATCH 1/2] graft: Remove work-around for old guile.

[Efraim Flashner]

* guix/build/graft.scm (mkdir-p*): Remove function.
(rewrite-directory): Switch from mkdir-p* to mkdir-p.

Change-Id: Ib6a80648d271c19093c05af84acb967e069ccc19
---
 guix/build/graft.scm | 31 ++-----------------------------
 1 file changed, 2 insertions(+), 29 deletions(-)

diff --git a/guix/build/graft.scm b/guix/build/graft.scm
index 281dbaba6f..c8c7e33ab2 100644
--- a/guix/build/graft.scm
+++ b/guix/build/graft.scm
@@ -312,33 +312,6 @@ (define (exit-on-exception proc)
           (print-exception port #f key args)
           (primitive-exit 1))))))
 
-;; We need this as long as we support Guile < 2.0.13.
-(define* (mkdir-p* dir #:optional (mode #o755))
-  "This is a variant of 'mkdir-p' that works around
-<http://bugs.gnu.org/24659> by passing MODE explicitly in each 'mkdir' call."
-  (define absolute?
-    (string-prefix? "/" dir))
-
-  (define not-slash
-    (char-set-complement (char-set #\/)))
-
-  (let loop ((components (string-tokenize dir not-slash))
-             (root       (if absolute?
-                             ""
-                             ".")))
-    (match components
-      ((head tail ...)
-       (let ((path (string-append root "/" head)))
-         (catch 'system-error
-           (lambda ()
-             (mkdir path mode)
-             (loop tail path))
-           (lambda args
-             (if (= EEXIST (system-error-errno args))
-                 (loop tail path)
-                 (apply throw args))))))
-      (() #t))))
-
 (define* (rewrite-directory directory output mapping
                             #:optional (store (%store-directory)))
   "Copy DIRECTORY to OUTPUT, replacing strings according to MAPPING, a list of
@@ -387,7 +360,7 @@ (define* (rewrite-directory directory output mapping
   (define (rewrite-leaf file)
     (let ((stat (lstat file))
           (dest (destination file)))
-      (mkdir-p* (dirname dest))
+      (mkdir-p (dirname dest))
       (case (stat:type stat)
         ((symlink)
          (let ((target (readlink file)))
@@ -406,7 +379,7 @@ (define* (rewrite-directory directory output mapping
                                            store)
                  (chmod output (stat:perms stat)))))))
         ((directory)
-         (mkdir-p* dest))
+         (mkdir-p dest))
         (else
          (error "unsupported file type" stat)))))
 
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#70303] [PATCH 2/2] graft: Perform grafts with guile-final.

[Efraim Flashner]

* guix/build/graft.scm (rewrite-directory): Rewrite store directories in
individual files sequentially.
(exit-on-exception): Remove procedure.
* guix/packages.scm (guile-for-grafts): Switch to guile-final.

Change-Id: I50f7b23a3ceff8bb1495dc1f4bc772746147d924
---
 guix/build/graft.scm | 24 ++++--------------------
 guix/packages.scm    |  6 ++----
 2 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/guix/build/graft.scm b/guix/build/graft.scm
index c8c7e33ab2..7fc5ecba99 100644
--- a/guix/build/graft.scm
+++ b/guix/build/graft.scm
@@ -299,19 +299,6 @@ (define (rename-matching-files directory mapping)
                                (string-append (dirname file) "/" target))))
               matches)))
 
-(define (exit-on-exception proc)
-  "Return a procedure that wraps PROC so that 'primitive-exit' is called when
-an exception is caught."
-  (lambda (arg)
-    (catch #t
-      (lambda ()
-        (proc arg))
-      (lambda (key . args)
-        ;; Since ports are not thread-safe as of Guile 2.0, reopen stderr.
-        (let ((port (fdopen 2 "w0")))
-          (print-exception port #f key args)
-          (primitive-exit 1))))))
-
 (define* (rewrite-directory directory output mapping
                             #:optional (store (%store-directory)))
   "Copy DIRECTORY to OUTPUT, replacing strings according to MAPPING, a list of
@@ -383,13 +370,10 @@ (define* (rewrite-directory directory output mapping
         (else
          (error "unsupported file type" stat)))))
 
-  ;; Use 'exit-on-exception' to force an exit upon I/O errors, given that
-  ;; 'n-par-for-each' silently swallows exceptions.
-  ;; See <http://bugs.gnu.org/23581>.
-  (n-par-for-each (parallel-job-count)
-                  (exit-on-exception rewrite-leaf)
-                  (find-files directory (const #t)
-                              #:directories? #t))
+  ;; n-par-for-each can lead to segfaults in the grafting code.
+  (for-each rewrite-leaf
+            (find-files directory (const #t)
+                        #:directories? #t))
   (rename-matching-files output mapping))
 
 (define %graft-hooks
diff --git a/guix/packages.scm b/guix/packages.scm
index 930b1a3b0e..80642eb049 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -882,10 +882,8 @@ (define (default-guile)
 
 (define (guile-for-grafts)
   "Return the Guile package used to build grafting derivations."
-  ;; Guile 2.2 would not work due to <https://bugs.gnu.org/28211> when
-  ;; grafting packages.
-  (let ((distro (resolve-interface '(gnu packages guile))))
-    (module-ref distro 'guile-2.0)))
+  (let ((distro (resolve-interface '(gnu packages commencement))))
+    (module-ref distro 'guile-final)))
 
 (define* (default-guile-derivation #:optional (system (%current-system)))
   "Return the derivation for SYSTEM of the default Guile package used to run
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[bug#70303] [PATCH 0/2] Use guile-final for grafting.

[Ludovic Courtès]

Howdy!

Efraim Flashner <efraim@flashner.co.il> skribis:

> Over the years the grafting code has changed bit by bit, with various
> attempts to speed it up.  By switching the grafts to not use parallelism
> in rewriting the leaves we can finally switch the guile-for-grafts to be
> guile-final.  The segfault is still there if we perform the grafts in
> parallel, but I believe it is fast enough that it should be fine to do
> them sequentially.

Could you time the grafting derivation of, say, libreoffice or
ungoogled-chromium?

Typically I’d do it along these lines:

--8<---------------cut here---------------start------------->8---
$ guix build libreoffice
/gnu/store/24is7ypdx6sm56mkclxdx4hyj7yg4smb-libreoffice-7.6.3.1
$ sudo sh -c 'echo 3 > /proc/sys/vm/drop_caches'
$ time guix build libreoffice --check
The following graft will be made:
   /gnu/store/wdjqpxm2kbdvq7qlrzyjxb244zn3s3bv-libreoffice-7.6.3.1.drv
applying 137 grafts for libreoffice-7.6.3.1 ...
grafting '/gnu/store/5flppg3h8y235di2ilr3sx878gfl82db-libreoffice-7.6.3.1' -> '/gnu/store/24is7ypdx6sm56mkclxdx4hyj7yg4smb-libreoffice-7.6.3.1'...
successfully built /gnu/store/wdjqpxm2kbdvq7qlrzyjxb244zn3s3bv-libreoffice-7.6.3.1.drv
successfully built /gnu/store/wdjqpxm2kbdvq7qlrzyjxb244zn3s3bv-libreoffice-7.6.3.1.drv
/gnu/store/24is7ypdx6sm56mkclxdx4hyj7yg4smb-libreoffice-7.6.3.1

real    0m14.921s
user    0m7.588s
sys     0m0.389s
--8<---------------cut here---------------end--------------->8---

(That’s on my 4-core i7.)

It’s a bummer that the segfault is still there.  I remember week-long
‘rr’ debugging sessions in the past, where I did find a few issues; I
should try again but uh…

> There's probably room in replace-store-references in (guix build graft)
> for changes if we're not going to use guile-2.0 here anymore.

I believe ‘tests/grafts.scm’ may run some of this code under Guile 2.0
(using the ‘guile-bootstrap’ tarball provided for the tests).

Thanks,
Ludo’.

[bug#70303] [PATCH 0/2] Use guile-final for grafting.

[Efraim Flashner]

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

On Mon, Apr 15, 2024 at 10:28:17PM +0200, Ludovic Courtès wrote:
> Howdy!
> 
> Efraim Flashner <efraim@flashner.co.il> skribis:
> 
> > Over the years the grafting code has changed bit by bit, with various
> > attempts to speed it up.  By switching the grafts to not use parallelism
> > in rewriting the leaves we can finally switch the guile-for-grafts to be
> > guile-final.  The segfault is still there if we perform the grafts in
> > parallel, but I believe it is fast enough that it should be fine to do
> > them sequentially.
> 
> Could you time the grafting derivation of, say, libreoffice or
> ungoogled-chromium?
> 
> Typically I’d do it along these lines:
> 
> --8<---------------cut here---------------start------------->8---
> $ guix build libreoffice
> /gnu/store/24is7ypdx6sm56mkclxdx4hyj7yg4smb-libreoffice-7.6.3.1
> $ sudo sh -c 'echo 3 > /proc/sys/vm/drop_caches'
> $ time guix build libreoffice --check
> The following graft will be made:
>    /gnu/store/wdjqpxm2kbdvq7qlrzyjxb244zn3s3bv-libreoffice-7.6.3.1.drv
> applying 137 grafts for libreoffice-7.6.3.1 ...
> grafting '/gnu/store/5flppg3h8y235di2ilr3sx878gfl82db-libreoffice-7.6.3.1' -> '/gnu/store/24is7ypdx6sm56mkclxdx4hyj7yg4smb-libreoffice-7.6.3.1'...
> successfully built /gnu/store/wdjqpxm2kbdvq7qlrzyjxb244zn3s3bv-libreoffice-7.6.3.1.drv
> successfully built /gnu/store/wdjqpxm2kbdvq7qlrzyjxb244zn3s3bv-libreoffice-7.6.3.1.drv
> /gnu/store/24is7ypdx6sm56mkclxdx4hyj7yg4smb-libreoffice-7.6.3.1
> 
> real    0m14.921s
> user    0m7.588s
> sys     0m0.389s
> --8<---------------cut here---------------end--------------->8---
> 
> (That’s on my 4-core i7.)

I'm currently vising my parents for a few weeks during the Passover
break, and I've already lost connection to my desktop at home, so I'm
currently running the tests on my pinebook pro.

The first one is with the patches applied, the second one is without the
patches. I have the guix-daemon on my pinebook pro to use 3 cores, but
I'm not sure how much that would be honored by the grafting code.

(ins)efraim@pbp ~/workspace/guix$ sudo sh -c 'echo 3 > /proc/sys/vm/drop_caches'
Password:
(ins)efraim@pbp ~/workspace/guix$ time ./pre-inst-env guix build libreoffice --check
The following graft will be made:
   /gnu/store/i2aml4p5yg7h090bdzjpaqmds47g26d8-libreoffice-7.6.3.1.drv
applying 137 grafts for libreoffice-7.6.3.1 ...
grafting '/gnu/store/yd9slkfhdl8lzhhmhm40airimwb6yhj5-libreoffice-7.6.3.1' -> '/gnu/store/1cfjx934czp641v4fmwsz2js7158ivgm-libreoffice-7.6.3.1'...
successfully built /gnu/store/i2aml4p5yg7h090bdzjpaqmds47g26d8-libreoffice-7.6.3.1.drv
successfully built /gnu/store/i2aml4p5yg7h090bdzjpaqmds47g26d8-libreoffice-7.6.3.1.drv
/gnu/store/1cfjx934czp641v4fmwsz2js7158ivgm-libreoffice-7.6.3.1

real    1m35.537s
user    0m32.328s
sys     0m2.521s
(ins)efraim@pbp ~/workspace/guix$ sudo sh -c 'echo 3 > /proc/sys/vm/drop_caches'
(ins)efraim@pbp ~/workspace/guix$ time guix build libreoffice --check
The following graft will be made:
   /gnu/store/2f6i7r77z8msbjlspsp0aq5vlpjqnifp-libreoffice-7.6.3.1.drv
applying 137 grafts for libreoffice-7.6.3.1 ...
grafting '/gnu/store/yd9slkfhdl8lzhhmhm40airimwb6yhj5-libreoffice-7.6.3.1' -> '/gnu/store/xm3q8qsns8qqybq47zvv70n0y0qs4r65-libreoffice-7.6.3.1'...
successfully built /gnu/store/2f6i7r77z8msbjlspsp0aq5vlpjqnifp-libreoffice-7.6.3.1.drv
successfully built /gnu/store/2f6i7r77z8msbjlspsp0aq5vlpjqnifp-libreoffice-7.6.3.1.drv
/gnu/store/xm3q8qsns8qqybq47zvv70n0y0qs4r65-libreoffice-7.6.3.1

real    1m20.573s
user    0m29.688s
sys     0m2.346s

-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#70303] [PATCH 0/2] Use guile-final for grafting.

[Ludovic Courtès]

Hi!

Efraim Flashner <efraim@flashner.co.il> skribis:

> The first one is with the patches applied, the second one is without the
> patches. I have the guix-daemon on my pinebook pro to use 3 cores, but
> I'm not sure how much that would be honored by the grafting code.
>
> (ins)efraim@pbp ~/workspace/guix$ sudo sh -c 'echo 3 > /proc/sys/vm/drop_caches'
> Password:
> (ins)efraim@pbp ~/workspace/guix$ time ./pre-inst-env guix build libreoffice --check
> The following graft will be made:
>    /gnu/store/i2aml4p5yg7h090bdzjpaqmds47g26d8-libreoffice-7.6.3.1.drv
> applying 137 grafts for libreoffice-7.6.3.1 ...
> grafting '/gnu/store/yd9slkfhdl8lzhhmhm40airimwb6yhj5-libreoffice-7.6.3.1' -> '/gnu/store/1cfjx934czp641v4fmwsz2js7158ivgm-libreoffice-7.6.3.1'...
> successfully built /gnu/store/i2aml4p5yg7h090bdzjpaqmds47g26d8-libreoffice-7.6.3.1.drv
> successfully built /gnu/store/i2aml4p5yg7h090bdzjpaqmds47g26d8-libreoffice-7.6.3.1.drv
> /gnu/store/1cfjx934czp641v4fmwsz2js7158ivgm-libreoffice-7.6.3.1
>
> real    1m35.537s
> user    0m32.328s
> sys     0m2.521s
> (ins)efraim@pbp ~/workspace/guix$ sudo sh -c 'echo 3 > /proc/sys/vm/drop_caches'
> (ins)efraim@pbp ~/workspace/guix$ time guix build libreoffice --check
> The following graft will be made:
>    /gnu/store/2f6i7r77z8msbjlspsp0aq5vlpjqnifp-libreoffice-7.6.3.1.drv
> applying 137 grafts for libreoffice-7.6.3.1 ...
> grafting '/gnu/store/yd9slkfhdl8lzhhmhm40airimwb6yhj5-libreoffice-7.6.3.1' -> '/gnu/store/xm3q8qsns8qqybq47zvv70n0y0qs4r65-libreoffice-7.6.3.1'...
> successfully built /gnu/store/2f6i7r77z8msbjlspsp0aq5vlpjqnifp-libreoffice-7.6.3.1.drv
> successfully built /gnu/store/2f6i7r77z8msbjlspsp0aq5vlpjqnifp-libreoffice-7.6.3.1.drv
> /gnu/store/xm3q8qsns8qqybq47zvv70n0y0qs4r65-libreoffice-7.6.3.1
>
> real    1m20.573s
> user    0m29.688s
> sys     0m2.346s

That’s an 18% slowdown.  Could you make several runs to see how stable
that is?

Thanks,
Ludo’.