07/08: store-copy: Canonicalize the mtime and permissions of the store copy.

[Ludovic Courtès]

civodul pushed a commit to branch master
in repository guix.

commit 72dc64f8f720268930eed448abfc15d2a0eca3cf
Author: Ludovic Courtès <address@hidden>
Date:   Sun Nov 4 22:05:32 2018 +0100

    store-copy: Canonicalize the mtime and permissions of the store copy.
    
    Fixes a bug whereby directories in the output of 'guix pack -f tarball'
    would not be read-only.
    
    * guix/build/store-copy.scm (reset-permissions): New procedure.
    (populate-store): Pass #:keep-mtime? #t to 'copy-recursively'.  Call
    'reset-permissions'.
    * tests/pack.scm ("self-contained-tarball"): In CHECK, define
    'canonical?' and use it to check that every file has an mtime of 1 and
    is read-only.
    * tests/guix-pack.sh: Invoke "chmod -Rf +w" before "rm -rf" in trap.
---
 guix/build/store-copy.scm | 28 +++++++++++++++++++++++++++
 tests/guix-pack.sh        |  2 +-
 tests/pack.scm            | 48 +++++++++++++++++++++++++++++++++++------------
 3 files changed, 65 insertions(+), 13 deletions(-)

diff --git a/guix/build/store-copy.scm b/guix/build/store-copy.scm
index 64ade78..549aa4f 100644
--- a/guix/build/store-copy.scm
+++ b/guix/build/store-copy.scm
@@ -168,6 +168,28 @@ REFERENCE-GRAPHS, a list of reference-graph files."
 
   (reduce + 0 (map file-size items)))
 
+(define (reset-permissions file)
+  "Reset the permissions on FILE and its sub-directories so that they are all
+read-only."
+  ;; XXX: This procedure exists just to work around the inability of
+  ;; 'copy-recursively' to preserve permissions.
+  (file-system-fold (const #t)                    ;enter?
+                    (lambda (file stat _)         ;leaf
+                      (unless (eq? 'symlink (stat:type stat))
+                        (chmod file
+                               (if (zero? (logand (stat:mode stat)
+                                                  #o100))
+                                   #o444
+                                   #o555))))
+                    (const #t)                    ;down
+                    (lambda (directory stat _)    ;up
+                      (chmod directory #o555))
+                    (const #f)                    ;skip
+                    (const #f)                    ;error
+                    #t
+                    file
+                    lstat))
+
 (define* (populate-store reference-graphs target
                          #:key (log-port (current-error-port)))
   "Populate the store under directory TARGET with the items specified in
@@ -197,7 +219,13 @@ REFERENCE-GRAPHS, a list of reference-graph files."
         (for-each (lambda (thing)
                     (copy-recursively thing
                                       (string-append target thing)
+                                      #:keep-mtime? #t
                                       #:log (%make-void-port "w"))
+
+                    ;; XXX: Since 'copy-recursively' doesn't allow us to
+                    ;; preserve permissions, we have to traverse TARGET to
+                    ;; make sure everything is read-only.
+                    (reset-permissions (string-append target thing))
                     (report))
                   things)))))
 
diff --git a/tests/guix-pack.sh b/tests/guix-pack.sh
index 8c1f556..a43f4d1 100644
--- a/tests/guix-pack.sh
+++ b/tests/guix-pack.sh
@@ -49,7 +49,7 @@ the_pack="`guix pack --bootstrap -S /opt/gnu/bin=bin 
guile-bootstrap`"
 # exists because /opt/gnu/bin may be an absolute symlink to a store item that
 # has been GC'd.
 test_directory="`mktemp -d`"
-trap 'rm -rf "$test_directory"' EXIT
+trap 'chmod -Rf +w "$test_directory"; rm -rf "$test_directory"' EXIT
 cd "$test_directory"
 tar -xf "$the_pack"
 test -L opt/gnu/bin
diff --git a/tests/pack.scm b/tests/pack.scm
index a9bc894..40473a9 100644
--- a/tests/pack.scm
+++ b/tests/pack.scm
@@ -68,18 +68,42 @@
                                         #:archiver %tar-bootstrap))
        (check   (gexp->derivation
                  "check-tarball"
-                 #~(let ((bin (string-append "." #$profile "/bin")))
-                     (setenv "PATH"
-                             (string-append #$%tar-bootstrap "/bin"))
-                     (system* "tar" "xvf" #$tarball)
-                     (mkdir #$output)
-                     (exit
-                      (and (file-exists? (string-append bin "/guile"))
-                           (string=? (string-append #$%bootstrap-guile "/bin")
-                                     (readlink bin))
-                           (string=? (string-append ".." #$profile
-                                                    "/bin/guile")
-                                     (readlink "bin/Guile"))))))))
+                 (with-imported-modules '((guix build utils))
+                   #~(begin
+                       (use-modules (guix build utils)
+                                    (srfi srfi-1))
+
+                       (define store
+                         ;; The unpacked store.
+                         (string-append "." (%store-directory) "/"))
+
+                       (define (canonical? file)
+                         ;; Return #t if FILE is read-only and its mtime is 1.
+                         (let ((st (lstat file)))
+                           (or (not (string-prefix? store file))
+                               (eq? 'symlink (stat:type st))
+                               (and (= 1 (stat:mtime st))
+                                    (zero? (logand #o222
+                                                   (stat:mode st)))))))
+
+                       (define bin
+                         (string-append "." #$profile "/bin"))
+
+                       (setenv "PATH"
+                               (string-append #$%tar-bootstrap "/bin"))
+                       (system* "tar" "xvf" #$tarball)
+                       (mkdir #$output)
+                       (exit
+                        (and (file-exists? (string-append bin "/guile"))
+                             (file-exists? store)
+                             (every canonical?
+                                    (find-files "." (const #t)
+                                                #:directories? #t))
+                             (string=? (string-append #$%bootstrap-guile 
"/bin")
+                                       (readlink bin))
+                             (string=? (string-append ".." #$profile
+                                                      "/bin/guile")
+                                       (readlink "bin/Guile")))))))))
     (built-derivations (list check))))
 
 ;; The following test needs guile-sqlite3, libgcrypt, etc. as a consequence of