bug#32126: call-with-temporary-directory rarely cleans up after itself

[ludo@gnu.org (Ludovic Courtès)]

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

Hello,

Leo Famulari <leo@famulari.name> skribis:

> However, with the patch there is an error in the test 'gexp->script
> #:module-path':
>
> ------
> actual-value: #f
> actual-error:
> + (system-error
> +   "lstat"
> +   "~A: ~S"
> +   ("No such file or directory"
> +    "/tmp/guix-directory.6CrC8B/guix/base32.scm")
> +   (2))
> result: FAIL
> ------

Funny.  That test turned out to work thanks to the brokenness of
‘call-with-temporary-directory’: since it’s a monadic return, the actual
code was executed after we’d left the ‘call-with-temporary-directory’
extent, yet it expected to be able to access files from that temporary
directory.

This change fixes it:


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

diff --git a/tests/gexp.scm b/tests/gexp.scm
index 83fe81154..31c7ce22f 100644
--- a/tests/gexp.scm
+++ b/tests/gexp.scm
@@ -948,7 +948,7 @@
       (return (and (zero? (close-pipe pipe))
                    (= (expt n 2) (string->number str)))))))
 
-(test-assertm "gexp->script #:module-path"
+(test-assert "gexp->script #:module-path"
   (call-with-temporary-directory
    (lambda (directory)
      (define str
@@ -961,23 +961,24 @@
                         (define-public %fake! ,str))
                 port)))
 
-     (mlet* %store-monad ((exp -> (with-imported-modules '((guix base32))
-                                    (gexp (begin
-                                            (use-modules (guix base32))
-                                            (write (list %load-path
-                                                         %fake!))))))
-                          (drv    (gexp->script "guile-thing" exp
-                                                #:guile %bootstrap-guile
-                                                #:module-path (list directory)))
-                          (out -> (derivation->output-path drv))
-                          (done   (built-derivations (list drv))))
-       (let* ((pipe  (open-input-pipe out))
-              (data  (read pipe)))
-         (return (and (zero? (close-pipe pipe))
-                      (match data
-                        ((load-path str*)
-                         (and (string=? str* str)
-                              (not (member directory load-path))))))))))))
+     (run-with-store %store
+       (mlet* %store-monad ((exp -> (with-imported-modules '((guix base32))
+                                      (gexp (begin
+                                              (use-modules (guix base32))
+                                              (write (list %load-path
+                                                           %fake!))))))
+                            (drv    (gexp->script "guile-thing" exp
+                                                  #:guile %bootstrap-guile
+                                                  #:module-path (list directory)))
+                            (out -> (derivation->output-path drv))
+                            (done   (built-derivations (list drv))))
+         (let* ((pipe  (open-input-pipe out))
+                (data  (read pipe)))
+           (return (and (zero? (close-pipe pipe))
+                        (match data
+                          ((load-path str*)
+                           (and (string=? str* str)
+                                (not (member directory load-path)))))))))))))
 
 (test-assertm "program-file"
   (let* ((n      (random (expt 2 50)))

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


> From e3181f30ca0711e79aab9d71d798344dfb4636b5 Mon Sep 17 00:00:00 2001
> From: Leo Famulari <leo@famulari.name>
> Date: Wed, 11 Jul 2018 20:24:29 -0400
> Subject: [PATCH] utils: Really clean up temporary directories.
>
> * guix/utils.scm (delete-file-recursively): New variable.
> (call-with-temporary-directory): Use DELETE-FILE-RECURSIVELY instead of
> RMDIR.

Instead of duplicating ‘delete-file-recursively’, you can take it
directly from (guix build utils).  There’s already a #:use-module clause
at the top.

Thanks,
Ludo’.